Skip to content
This repository has been archived by the owner on Mar 16, 2024. It is now read-only.

Add acorn copy command (#1809) #1883

Merged
merged 5 commits into from
Jul 10, 2023
Merged

Conversation

g-linville
Copy link
Contributor

@g-linville g-linville commented Jul 5, 2023

for #1809

This implements the acorn copy (alias acorn image copy) command, which behaves similarly to crane copy. It can be used to:

  • copy an image from the internal registry to a remote registry (i.e. acorn push)
  • copy images between remote registries
  • copy entire repositories between remote registries (with the --all-tags option)

This command cannot do the following:

  • copy an image from a remote registry to the internal registry (i.e. acorn pull)
  • copy images between projects in the internal registry
  • re-tag images in the internal registry (i.e. acorn tag)
  • copy entire "repositories" from the internal registry to somewhere else
    • since our "tags" on local images really only exist in the API-level and not in the registry itself, there is not a clean way to be able to copy a whole "repo" of images sourced from the internal registry

By default, this command will avoid overwriting remote tags that already exist unless --force is specified.

I left comments on some areas of the code which should hopefully clarify things.

Checklist

  • The title of this PR would make a good line in Acorn's Release Note's Changelog
  • The title of this PR ends with a link to the main issue being address in parentheses, like: This is a title (#1216). Here's an example
  • All relevant issues are referenced in the PR description. NOTE: don't use GitHub keywords that auto-close issues
  • Commits follow contributing guidance
  • Automated tests added to cover the changes. If tests couldn't be added, an explanation is provided in the Verification and Testing section
  • Changes to user-facing functionality, API, CLI, and upgrade impacts are clearly called out in PR description
  • PR has at least two approvals before merging (or a reasonable exception, like it's just a docs change)

@g-linville g-linville changed the title Draft: Add acorn copy command Add acorn copy command Jul 5, 2023
@g-linville g-linville changed the title Add acorn copy command Add acorn copy command (#1809) Jul 5, 2023
Total int64 `json:"total,omitempty"`
Complete int64 `json:"complete,omitempty"`
Error string `json:"error,omitempty"`
CurrentTask string `json:"currentTask,omitempty"`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this field so that we can tell the user what is currently happening. This is very useful in particular for --all-tags.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I do acorn copy --all-tags <source> <dest> and then hit cancel out (say through ctrl+c), does the copy continue happening in the background?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just tested this to see. The copy terminates properly and does not continue in the background

@@ -189,7 +189,7 @@ func isNotInternalRepo(prefix, image string) error {
}

if prefix == "" {
return nil
prefix = fmt.Sprintf("%s.%s", system.RegistryName, system.ImagesNamespace)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated this check so that it works with the default internal registry (registry.acorn-image-system) if nothing else is specified.

Comment on lines +51 to +58
if update.CurrentTask != "" && update.CurrentTask != currentTask {
if bar != nil {
bar.Add(bar.Total - bar.Current)
_, _ = bar.Stop()
bar = nil
}
currentTask = update.CurrentTask
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, we reset the progress bar if the current task has changed, since we need to start a new bar for the new task.

url := c.RESTClient.Get().
Namespace(c.Namespace).
Resource("images").
Name(strings.ReplaceAll(fmt.Sprintf("%s-%s", srcImage, dstImage), "/", "+")).
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name parameter here is not used server-side at all.

@g-linville g-linville marked this pull request as ready for review July 5, 2023 20:46
Signed-off-by: Grant Linville <grant@acorn.io>
Signed-off-by: Grant Linville <grant@acorn.io>
Signed-off-by: Grant Linville <grant@acorn.io>
Signed-off-by: Grant Linville <grant@acorn.io>
Copy link
Contributor

@StrongMonkey StrongMonkey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't have too much context but PR looks good to me.

Great job on the work to show progress on copy!

Copy link
Contributor

@thedadams thedadams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job with this!

pkg/server/registry/apigroups/acorn/images/copy.go Outdated Show resolved Hide resolved
Signed-off-by: Grant Linville <grant@acorn.io>
@g-linville g-linville merged commit 4ed75e7 into acorn-io:main Jul 10, 2023
3 checks passed
@g-linville g-linville deleted the acorn-copy branch July 10, 2023 21:22
cloudnautique pushed a commit to cloudnautique/runtime that referenced this pull request Sep 28, 2023
Signed-off-by: Grant Linville <grant@acorn.io>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants