Copy fails when we try to copy to the root of the destination registry

[meghanajangi]

What steps did you take: When copying an image to a registry which has no repository or no folder beneath it, it fails. imgpkg copy -i nginx --to-repo ip-10-0-0-146.ap-south-1.compute.internal/ or imgpkg copy -i nginx --to-repo ip-10-0-0-146.ap-south-1.compute.internal

What happened: The image copy fails with the following error message:

root@ip-172-32-0-87:~# imgpkg copy -i nginx --to-repo ip-10-0-0-146.ap-south-1.compute.internal/
0 B / ? [---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------=]   0.00% 2562047h47m16s
copy | done importing images
Error: Building import repository ref: repository must be between 2 and 255 runes in length:

root@ip-172-32-0-87:~# imgpkg copy -i nginx --to-repo ip-10-0-0-146.ap-south-1.compute.internal
copy | exporting 1 images...
copy | will export index.docker.io/library/nginx@sha256:1c70a669bbf07f9862f269162d776c35144b116938d1becb4e4676270cff8f75
copy | exported 1 images
copy | importing 1 images...
panic: send on closed channel
	panic: close of closed channel

goroutine 1 [running]:
github.com/google/go-containerregistry/pkg/v1/remote.sendError(...)
	github.com/google/[email protected]/pkg/v1/remote/write.go:185
github.com/google/go-containerregistry/pkg/v1/remote.MultiWrite.func1(0xc000274380, 0xc0000e1360)
	github.com/google/[email protected]/pkg/v1/remote/multi_write.go:99 +0x85
github.com/google/go-containerregistry/pkg/v1/remote.MultiWrite(0xc00008e270, 0xc0002367c0, 0x4, 0x4, 0x98e400, 0xc0003030c0)
	github.com/google/[email protected]/pkg/v1/remote/multi_write.go:160 +0x1220
github.com/k14s/imgpkg/pkg/imgpkg/registry.Registry.MultiWrite.func1(0x3b9aca00, 0xc00009e1c0)
	github.com/k14s/imgpkg/pkg/imgpkg/registry/registry.go:112 +0x89
github.com/k14s/imgpkg/pkg/imgpkg/util.Retry(0xc0000e1438, 0x60, 0xc025e0)
	github.com/k14s/imgpkg/pkg/imgpkg/util/retry.go:25 +0xa3
github.com/k14s/imgpkg/pkg/imgpkg/registry.Registry.MultiWrite(0xc0002367c0, 0x3, 0x4, 0x0, 0x0, 0x0, 0xc00008e270, 0x5, 0x44132e, 0xc0000e14e0)
	github.com/k14s/imgpkg/pkg/imgpkg/registry/registry.go:111 +0x99
github.com/k14s/imgpkg/pkg/imgpkg/imageset.(*ImageSet).Import(0xc0004c53e0, 0xc000030540, 0x1, 0x1, 0x0, 0x90079e, 0xf, 0x7ffdb9e077ee, 0x29, 0x99a458, ...)
	github.com/k14s/imgpkg/pkg/imgpkg/imageset/image_set.go:114 +0x342
github.com/k14s/imgpkg/pkg/imgpkg/imageset.ImageSet.Relocate(0x5, 0x98e580, 0xc00026c1e0, 0xc00000e0a8, 0x0, 0x90079e, 0xf, 0x7ffdb9e077ee, 0x29, 0x99a458, ...)
	github.com/k14s/imgpkg/pkg/imgpkg/imageset/image_set.go:48 +0x1d6
github.com/k14s/imgpkg/pkg/imgpkg/cmd.CopyRepoSrc.CopyToRepo(0x7ffdb9e077de, 0x5, 0x0, 0x0, 0x0, 0x0, 0x0, 0x5, 0x992360, 0xc00026c1e0, ...)
	github.com/k14s/imgpkg/pkg/imgpkg/cmd/copy_repo_src.go:57 +0x257
github.com/k14s/imgpkg/pkg/imgpkg/cmd.(*CopyOptions).Run(0xc000150000, 0x0, 0x0)
	github.com/k14s/imgpkg/pkg/imgpkg/cmd/copy.go:140 +0xd28
github.com/k14s/imgpkg/pkg/imgpkg/cmd.NewCopyCmd.func1(0xc000142f00, 0xc00009f080, 0x0, 0x4, 0x0, 0x0)
	github.com/k14s/imgpkg/pkg/imgpkg/cmd/copy.go:44 +0x2a
github.com/cppforlife/cobrautil.WrapRunEForCmd.func1.1(0xc000142f00, 0xc00009f080, 0x0, 0x4, 0x0, 0x0)
	github.com/cppforlife/[email protected]/misc.go:45 +0xb2
github.com/cppforlife/cobrautil.WrapRunEForCmd.func1.1(0xc000142f00, 0xc00009f080, 0x0, 0x4, 0x0, 0x0)
	github.com/cppforlife/[email protected]/misc.go:45 +0xb2
github.com/cppforlife/cobrautil.WrapRunEForCmd.func1.1(0xc000142f00, 0xc00009f080, 0x0, 0x4, 0x0, 0x0)
	github.com/cppforlife/[email protected]/misc.go:45 +0xb2
github.com/spf13/cobra.(*Command).execute(0xc000142f00, 0xc00009f040, 0x4, 0x4, 0xc000142f00, 0xc00009f040)
	github.com/spf13/[email protected]/command.go:852 +0x472
github.com/spf13/cobra.(*Command).ExecuteC(0xc000142500, 0xc000142500, 0xc00009ecc0, 0xc000040748)
	github.com/spf13/[email protected]/command.go:960 +0x375
github.com/spf13/cobra.(*Command).Execute(...)
	github.com/spf13/[email protected]/command.go:897
main.main()
	github.com/k14s/imgpkg/cmd/imgpkg/imgpkg.go:30 +0x188

What did you expect: It should succeed or display a clear error message stating the reason for failure.

Anything else you would like to add: NA

Environment:

• imgpkg version (use imgpkg --version): imgpkg version 0.8.0
• Docker registry used (e.g. Docker HUB): source is Docker Hub and destination registry is private docker registry
• OS (e.g. from /etc/os-release): Ubuntu 20.04.2 LTS (Focal Fossa)

---

Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible" 👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.

[perithompson]

In skopeo or docker, you do not need to append the image name to the end of the destination repo, it would be good if we could do the same with imgpkg so the destination name automatically gets used if the user does not specify it

imgpkg copy -i nginx --to-repo ip-10-0-0-146.ap-south-1.compute.internal/ would result in imgpkg copy -i nginx --to-repo ip-10-0-0-146.ap-south-1.compute.internal/nginx

This makes it much easier to automate and loop over

[jayunit100]

Yeah agree on this one

[SUPRRITHA]

Yeah agree!

[DennisDenuto]

Thanks for submitting this issue :-)

This makes it much easier to automate and loop over

Fair enough, I can see how this change might simplify scripts. I'm curious if you can expand on your usecase, and if you have considered using imgpkg bundles? There is currently an open proposal to introduce naming strategies when copying a bundle (and their referenced images) https://github.com/vmware-tanzu/carvel-community/pull/22 and https://github.com/vmware-tanzu/carvel-community/tree/003-copy-bundles-with-rename/proposals/imgpkg/003-copy-bundles-with-rename

Ok back to this request for copying images (not bundles)...

There might be a small trade-off here, where this might be simpler for scripts, but maybe more error prone / confusing for human interactions.

using your 2nd example...

imgpkg copy -i nginx --to-repo ip-10-0-0-146.ap-south-1.compute.internal/repo1

could have users expecting different results. i.e. does it copy to registry-url/repo1 or registry-url/repo1/nginx

Personally, I think it will be less confusing if we only make it that an image is appended to the target repository if the target repository ends with a slash. So the above example would copy to ip-10-0-0-146.ap-south-1.compute.internal/repo1 since it did not end with a slash

Here is a table i've quickly put together, where the general rule is that only the image is appended, if the target contains a slash.

source	destination registry	output
image	repo/	repo/image
repo1/image	repo2/	repo2/image
repo1/path1/image	repo2/	repo2/image
repo1/image	repo2	repo2
repo1/path1/image	repo2	repo2
image	repo/path/	repo/path/image
image	repo/path	repo/path

Note: Another thing to consider would be whether to have this behavior consistent when copying a bundle too (-b)

[meghanajangi]

Hello Dennis,

I am trying to understand the table you have given. Do you mean that when I execute imgpkg copy -i nginx --to-repo ip-10-0-0-146.ap-south-1.compute.internal/ it should copy the image to destination registry as ip-10-0-0-146.ap-south-1.compute.internal/nginx:latest and when I execute imgpkg copy -i nginx --to-repo ip-10-0-0-146.ap-south-1.compute.internal it should copy the image to destination registry as ip-10-0-0-146.ap-south-1.compute.internal/nginx/nginx:latest ?

[jorgemoralespou]

In some discussion with @cppforlife he made clear that imgpkg is not a generic image copy tool, although it supports somehow that workflow (but discouraged throughout the documentation). If what you want is to copy images, renaming, retagging, etc... I would recommend you to stay with skopeo which is a tool purposely built for that scenario. Imgpkg is build initially for Carvel Bundles and that's it's main use case/driver. I'm not from the Carvel team, so I might be wrong.

[DennisDenuto]

@mjangi9292

imgpkg copy -i nginx --to-repo ip-10-0-0-146.ap-south-1.compute.internal/ it should copy the image to destination registry as ip-10-0-0-146.ap-south-1.compute.internal/nginx:latest

Yup 👍

imgpkg copy -i nginx --to-repo ip-10-0-0-146.ap-south-1.compute.internal

interestingly I didn't cover that case in the table. although what it does surprised me! it tries to copy to index.docker.io/library/ip-10-0-0-146.ap-south-1.compute.internal

It is treating it as a shorthand reference to the library repo, and the default registry (when a registry isn't provided, because it thinks the argument is a repo not a registry...) of dockerhub.

I am not sure what to do in this case. since repos can contain dots in the name. And I think we would want to prevent a breaking change of requiring registries be set in the --to-repo part of the copy command.

@jorgemoralespou i'd be curious to know which discussions are you referring to? are they on a public channel you can link? or was this a zoom session?

but discouraged throughout the documentation

I am taking this to mean, it is discouraged through omission of having documented "workflows" use the -i flag. (the 2 use cases we have use the -b flag?). Or do i need to go re-read the docs (it has been a while), and theres a section actually discouraging using the -i flag?

imgpkg Bundles is definitely what we recommend (see my initial reply to this thread hehe). Although this issue seems like a minor enhancement to the UX of an existing supported feature.

This is def thought provoking for me (around purpose / fit of maintaining the -i) I'll add it as a discussion topic in today's community meeting.

I am from the Carvel team, but I might also be wrong too! :-)

[jorgemoralespou]

It's from internal discussions with @cppforlife as we initially were using imgpkg copy as a way to copy images and were finding many missing things that we requested. Since imgpkg is meant for a different use case (package files up in an oci image) he just mentioned that we should not expect all that skopeo do in relation to copy use cases in imgpkg.

Maybe saying that image usage is discouraged but definitely, the docs almost mostly talk about bundles (except one little paragraph page: https://carvel.dev/imgpkg/docs/latest/working-directly-with-images/)

[cppforlife]

In some discussion with @cppforlife he made clear that imgpkg is not a generic image copy tool

i believe what i was previously said is that "imgpkg copy" is not a command that would cover all copying cases that somebody may want (e.g. scopeo has various options for changing format, compression, encryption, etc.). for majority of cases "imgpkg copy" should be usable (including one mentioned in this issue), assuming we dont have a bug.

imgpkg copy is/should be capable of:

• copying single image/imageindex (we have an open bug for imageindex though)
• copying multiple images/imageindexes via ImagesLock file
• copying bundles

we do not "promote" -i flag in the documentation because we wanted to keep users focused on bundles, since it's a concept we believe is appropriate for most imgpkg users.

it tries to copy to index.docker.io/library/ip-10-0-0-146.ap-south-1.compute.internal

i dont believe that's whats happening. i think the error is saying that repository is not provided and should be (repository must be between 2 and 255 runes in length:). it would be worth improving error message in go-containerregistry if it's confusing.

...panic: send on closed channel panic: close of closed channel

this is unfortunate panic from older version of imgpkg. it's unrelated to original error. latest version of imgpkg does not have this problem.

In skopeo or docker, you do not need to append the image name to the end of the destination repo

this is too confusing imho. i would be in favor of adding something more explicit like --to-repo new-reg.com/{{.SrcRepo}} or something along those lines to indicate that source repo should be retained. this gets pretty confusing though in combination with other options when multiple images are in play. x-ref CopyWithRename proposal.

to add another problem to the queue, as of now we have https://github.com/vmware-tanzu/carvel-imgpkg/issues/142 which unfortunately causes only first image of imageindex to be copied (nginx is an imageindex).

[DennisDenuto]

@cppforlife

i dont believe that's whats happening.

imgpkg copy -i nginx --to-repo ip-10-0-0-146.ap-south-1.compute.internal

copy | will export index.docker.io/library/nginx@sha256:3f13b4376446cf92b0cb9a5c46ba75d57c41f627c4edb8b635fa47386ea29e20

copy | Error: Error uploading images: HEAD https://index.docker.io/v2/library/ip-10-0-0-146.ap-south-1.compute.internal/blobs/sha256:8a268f30c42a7c778c9c9497d043dfac6143281918cb9337f20335d4f11e1937: unexpected status code 401 Unauthorized (HEAD responses have no body, use GET for details)

[cppforlife]

but, with a slash (per original comment)

% ./imgpkg copy -i nginx --to-repo ip-10-0-0-146.ap-south-1.compute.internal/
Error: Building import repository ref: repository must be between 2 and 255 runes in length:

[DennisDenuto]

Thanks @cppforlife for the suggestion!

Curious to hear what folk (@perithompson @mjangi9292) think about instead of a trailing slash, using a template structure {{.SrcRepo}} to indicate that the source repo should be retained

[voor]

{{.SrcRepo}} would definitely work for my use case, where I just want to take a BundleLock file and move it from one place to the next, preserving the entirety of the path. So if my image was originalplace.com/someproject/somegroup/yadda/nginx:1.0.0 it would get moved to newplace.com/someproject/somegroup/yadda/nginx:1.0.0 and the contents of {{.SrcRepo}} would be someproject/somegroup/yadda/nginx and the BundleLock file would contain the tag reference 1.0.0.

[meghanajangi]

I agree with the {{.SrcRepo}} suggestion , this would be very helpful.

[voor]

You might also want to check the proposal here https://github.com/vmware-tanzu/carvel-community/pull/22 to see if it helps some of those needs.

[DennisDenuto]

Thanks for the replies! I'm going to carvel accept this issue. With the intent to provide some sort of templating functionality (like discussed above {{.SrcRepo}}) to allow specifying the destination repo.

[voor]

While not ideal, one workaround that we've been using is some sed magic. Being able to replace this with a native capability would be fantastic. For those that need a temporary workaround, here's how we're doing it: https://gist.github.com/voor/7b63796b891297f04de7ac1a6b7d601d