-
Notifications
You must be signed in to change notification settings - Fork 708
Add --all-tags/-a flag to image push #1335
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -283,6 +283,54 @@ extension ClientImage { | |
| return image | ||
| } | ||
|
|
||
| @discardableResult | ||
| public static func pushAllTags( | ||
| reference: String, platform: Platform? = nil, scheme: RequestScheme = .auto, maxConcurrentUploads: Int = 3, progressUpdate: ProgressUpdateHandler? = nil | ||
| ) async throws -> [ClientImage] { | ||
| guard maxConcurrentUploads > 0 else { | ||
| throw ContainerizationError(.invalidArgument, message: "maximum number of concurrent uploads must be greater than 0, got \(maxConcurrentUploads)") | ||
| } | ||
|
|
||
| // Normalize the reference, then extract the repository name without the tag. | ||
| let normalized = try Self.normalizeReference(reference) | ||
| let parsedRef = try Reference.parse(normalized) | ||
|
|
||
| let repositoryName: String | ||
| if let resolved = parsedRef.resolvedDomain { | ||
| repositoryName = "\(resolved)/\(parsedRef.path)" | ||
| } else { | ||
| repositoryName = parsedRef.name | ||
| } | ||
|
Comment on lines
+298
to
+303
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: Looks like this is being repeated at couple of places. I think moving this to a package level function inside Utility would make sense. |
||
|
|
||
| guard let host = parsedRef.domain else { | ||
| throw ContainerizationError(.invalidArgument, message: "could not extract host from reference \(normalized)") | ||
| } | ||
|
|
||
| let client = newXPCClient() | ||
| let request = newRequest(.imagePush) | ||
|
|
||
| request.set(key: .imageRepository, value: repositoryName) | ||
| request.set(key: .allTags, value: true) | ||
| try request.set(platform: platform) | ||
|
|
||
| let insecure = try scheme.schemeFor(host: host) == .http | ||
| request.set(key: .insecureFlag, value: insecure) | ||
| request.set(key: .maxConcurrentUploads, value: Int64(maxConcurrentUploads)) | ||
|
|
||
| var progressUpdateClient: ProgressUpdateClient? | ||
| if let progressUpdate { | ||
| progressUpdateClient = await ProgressUpdateClient(for: progressUpdate, request: request) | ||
| } | ||
|
|
||
| let response = try await client.send(request) | ||
| await progressUpdateClient?.finish() | ||
|
|
||
| let imageDescriptions = try response.imageDescriptions() | ||
| return imageDescriptions.map { desc in | ||
| ClientImage(description: desc) | ||
| } | ||
| } | ||
|
|
||
| public static func delete(reference: String, garbageCollect: Bool = false) async throws { | ||
| let client = newXPCClient() | ||
| let request = newRequest(.imageDelete) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -136,6 +136,96 @@ public actor ImagesService { | |
| } | ||
| } | ||
|
|
||
| public func pushAllTags(repositoryName: String, platform: Platform?, insecure: Bool, maxConcurrentUploads: Int, progressUpdate: ProgressUpdateHandler?) async throws | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering if we should instead move the logic to |
||
| -> [ImageDescription] | ||
| { | ||
| self.log.debug( | ||
| "ImagesService: enter", | ||
| metadata: [ | ||
| "func": "\(#function)", | ||
| "repositoryName": "\(repositoryName)", | ||
| "platform": "\(String(describing: platform))", | ||
| "insecure": "\(insecure)", | ||
| "maxConcurrentUploads": "\(maxConcurrentUploads)", | ||
| ] | ||
| ) | ||
| defer { | ||
| self.log.debug( | ||
| "ImagesService: exit", | ||
| metadata: [ | ||
| "func": "\(#function)", | ||
| "repositoryName": "\(repositoryName)", | ||
| "platform": "\(String(describing: platform))", | ||
| ] | ||
| ) | ||
| } | ||
|
|
||
| let allImages = try await imageStore.list() | ||
| let matchingImages = allImages.filter { image in | ||
| guard !Utility.isInfraImage(name: image.reference) else { return false } | ||
| guard let ref = try? Reference.parse(image.reference) else { return false } | ||
| let resolvedName: String | ||
| if let resolved = ref.resolvedDomain { | ||
| resolvedName = "\(resolved)/\(ref.path)" | ||
| } else { | ||
| resolvedName = ref.name | ||
| } | ||
| return resolvedName == repositoryName | ||
| } | ||
|
Comment on lines
+165
to
+174
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: This would be a good candidate which can be moved to Utility. |
||
|
|
||
| guard !matchingImages.isEmpty else { | ||
| throw ContainerizationError(.notFound, message: "no tags found for repository \(repositoryName)") | ||
| } | ||
|
|
||
| let maxConcurrent = maxConcurrentUploads > 0 ? maxConcurrentUploads : 3 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: This silent fallback behavior is different than what's in ClientImage (which has a |
||
|
|
||
| try await Self.withAuthentication(ref: repositoryName) { auth in | ||
| let progress = ContainerizationProgressAdapter.handler(from: progressUpdate) | ||
| var iterator = matchingImages.makeIterator() | ||
| var failures: [(reference: String, message: String)] = [] | ||
|
|
||
| await withTaskGroup(of: (String, String?).self) { group in | ||
| for _ in 0..<maxConcurrent { | ||
| guard let image = iterator.next() else { break } | ||
| let ref = image.reference | ||
| group.addTask { | ||
| do { | ||
| try await self.imageStore.push( | ||
| reference: ref, platform: platform, insecure: insecure, auth: auth, progress: progress) | ||
| return (ref, nil) | ||
| } catch { | ||
| return (ref, String(describing: error)) | ||
| } | ||
| } | ||
| } | ||
| for await (ref, error) in group { | ||
| if let error { | ||
| failures.append((ref, error)) | ||
| } | ||
| if let image = iterator.next() { | ||
| let nextRef = image.reference | ||
| group.addTask { | ||
| do { | ||
| try await self.imageStore.push( | ||
| reference: nextRef, platform: platform, insecure: insecure, auth: auth, progress: progress) | ||
| return (nextRef, nil) | ||
| } catch { | ||
| return (nextRef, String(describing: error)) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if !failures.isEmpty { | ||
| let details = failures.map { "\($0.reference): \($0.message)" }.joined(separator: "\n") | ||
| throw ContainerizationError(.internalError, message: "failed to push one or more tags:\n\(details)") | ||
| } | ||
| } | ||
|
|
||
| return matchingImages.map { $0.description.fromCZ } | ||
| } | ||
|
|
||
| public func tag(old: String, new: String) async throws -> ImageDescription { | ||
| self.log.debug( | ||
| "ImagesService: enter", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if this is super necessary since we get back a list of images we pushed