search

carvel-dev / imgpkg

Store application configuration files in Docker/OCI registries
https://carvel.dev/imgpkg
Apache License 2.0
262 stars 61 forks source link

Annotations in lock file is not preserved during going from tar->registry. #217

Open mhoshi-vm opened 3 years ago

mhoshi-vm commented 3 years ago

What steps did you take: The goal is I want to install a helm chart to a offline env using kbld and imgpkg to reallocate the images. The procedure I thought in mind was...

helm template hoge hoge/foo > manifests/bar.yaml
kbld -f manifests/ --imgpkg-lock-output /tmp/image-lock.yaml
imgpkg copy --lock /tmp/image-lock.yaml --to-tar /tmp/images.tar
--- Go to offline env ----
imgpkg copy --tar /tmp/images.tar --to-repo <OFFLINEREPO> --lock-output /tmp/image-offline-lock.yaml
helm template hoge hoge/foo | kbld -f- -f /tmp/image-offline-lock.yaml

but the last commands errors with the following. 

ubuntu@ubuntu-449:~$ helm template argo argo/argo-cd | kbld -f- -f /tmp/image-offline-lock.yaml
kbld: Error: Validating imageslock/ (imgpkg.carvel.dev/v1alpha1) cluster:
  Validating Overrides[0]:
    Expected Image or ImageRepo to be non-empty

Annotations are missing in /tmp/image-offline-lock.yaml

buntu@ubuntu-449:~$ cat /tmp/image-offline-lock.yaml
---
apiVersion: imgpkg.carvel.dev/v1alpha1
images:
- image: harbor.lespaulstudioplus.info/library/imgpkg-demo@sha256:5e88f2205de172b60fd7af23ac92f34321688a83de9f7de7c9a6f394f6950877
- image: harbor.lespaulstudioplus.info/library/imgpkg-demo@sha256:2defd45426be3b5bc7415455ca529c910dab7473717eeece9c3cc26efa5ca037
- image: harbor.lespaulstudioplus.info/library/imgpkg-demo@sha256:6b10107669332f3f68b659f3f13ce33c869d2dcaacdd64fd3e6cf0a7d98b9cd7
- image: harbor.lespaulstudioplus.info/library/imgpkg-demo@sha256:044559541242562493957ab02c3ff7a9a55b8571aa2c8d98e0557b189c2d3968
- image: harbor.lespaulstudioplus.info/library/imgpkg-demo@sha256:e5399090dd46c83d0d5c6f8941f34d87a47ca54fda89221c1dabd2f5beb7ed15
- image: harbor.lespaulstudioplus.info/library/imgpkg-demo@sha256:1f22f3fb44446f752e1457d9b1612d08dc6a808c267fee3ef0488959be83c2a1
- image: harbor.lespaulstudioplus.info/library/imgpkg-demo@sha256:ca760f94b35eb78575b170e41d1e19e27359b29245dacfd1c42ae90452ecc08e
- image: harbor.lespaulstudioplus.info/library/imgpkg-demo@sha256:0f354ec1728d9ff32edcd7d1b8bbdfc798277ad36120dc3dc683be44524c8b60
- image: harbor.lespaulstudioplus.info/library/imgpkg-demo@sha256:0039796b887fd30e460353f14e46ba1004152aa97f5f59094cc21eac445fc89b
- image: harbor.lespaulstudioplus.info/library/imgpkg-demo@sha256:b53a2f76f949849cbe7e0b1e343a2076c7f205cfcfe8116e461e6861369fa81f
kind: ImagesLock

What happened: Annotations were lost during tar->registry imgpkg

What did you expect: Annotations should be preserved

Environment:

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.

DennisDenuto commented 3 years ago

Thanks for raising this issue and for the detailed steps.

I was able to reproduce this issue and have assigned a priority.

Impl Note: When copying from a lock file (with annotations) to a tarball, consider 'storing' the annotations information into the ImageRefDescriptors in the labels field. https://github.com/vmware-tanzu/carvel-imgpkg/blob/a114b01fb828548b594390c5739c3a98ecd66be5/pkg/imgpkg/imageset/unprocessed_image_refs.go#L18

This can store the annotations (per image) into the tarball's manifest.json file, and can be used later to inflate the lock-output file.

pivotaljohn commented 2 years ago

Let's make this a feature (does it need crisper acceptance criteria?) and we can point it.

iamhsk commented 2 years ago

Hi @mhoshi-vm, curious to learn if there is a reason why you are copying from the image lock file instead of copying from a bundle? In other words, what was your thought behind not creating a bundle with all the images?

mhoshi-vm commented 2 years ago

hi @iamhsk AFAIU if I go with the bundle path the procedure will look like this. (Which I know it works)

helm repo add gitea-charts https://dl.gitea.io/charts
mkdir -p bundle/.imgpkg
helm template gitea gitea-charts/gitea > bundle/config.yml
kbld -f bundle/config.yml --imgpkg-lock-output bundle/.imgpkg/images.yml
imgpkg push -b <intermediate repo>/gitea/gitea/bundle-public:4.0.1 -f bundle
imgpkg copy -b <intermediate repo>/gitea/gitea/bundle-public:4.0.1 --to-tar /tmp/gittea.tar
--- go to offline env ---
imgpkg copy --tar /tmp/gittea.tar --to-repo <offline repo>/gitea/gitea/bundle
imgpkg pull -b <offline repo>/gitea/gitea/bundle:4.0.1 -o /tmp/bundle
kbld -f /tmp/bundle/config.yml -f /tmp/bundle/.imgpkg/images.yml

The problem is with this part. Is where I need to upload the bundle to an intermediate repo before going to the offline environment to generate the bundle.

imgpkg push -b <intermediate repo>/gitea/gitea/bundle-public:4.0.1 -f bundle
imgpkg copy -b <intermediate repo>/gitea/gitea/bundle-public:4.0.1 --to-tar /tmp/gittea.tar

kbld relocation does not require this intermediate repo and the procedure is much more simpler

joaopapereira commented 2 years ago

Hey @mhoshi-vm I was thinking about your request and trying to do some assessment of the need for imgpkg to allow the copy command to receive an ImagesLock as an input. The problem that you are describing looks like it is similar to #55. Do you think that if this issue would be solved you would use a Bundle instead?

About the ability to copy using an ImagesLock.

This last point seems like more of a burden than really a helpful feature on imgpkg. @cppforlife do you have any thoughts about this feature that might be escaping me?

mhoshi-vm commented 2 years ago

So the only concern that I have is that "it used to be much easier with kbld..." Comparing with kbld relocate (which is afaiu deprecated) even with #55 there seems to have much more steps with imgpkg. Very soon everyone will forget about kbld relocate so that might not be an issue though ...

I am not sure how much this feature is being used right now

At least this was a very powerful feature to download images and bring it to an offline environment.

joaopapereira commented 2 years ago

@mhoshi-vm I would like to dig a little bit more on

So the only concern that I have is that "it used to be much easier with kbld..."

What do you think makes it easier to use kbld instead of imgpkg?

making commented 2 years ago

There is a use case where you want to bring existing images (ex. Helm chart) into an air-gapped environment.

This can be achieved with the deprecated kbld pkg:

helm repo add gitea-charts https://dl.gitea.io/charts
mkdir -p bundle
helm template gitea gitea-charts/gitea > bundle/config.yml
kbld -f bundle/config.yml --lock-output images.lock
kbld pkg -f images.lock --output bundle.tar

# Copy gittea.tar to the air-gapped environment

kbld unpkg -f images.lock -i bundle.tar --repository internal-registry.example.com/gitea/gitea/images --lock-output relocated.lock
kbld -f relocated.lock -f bundle/config.yml

On the other hand, if you want to use imgpkg instead, you need to push to repository once like bellow: 

helm repo add gitea-charts https://dl.gitea.io/charts
mkdir -p bundle/.imgpkg
helm template gitea gitea-charts/gitea > bundle/config.yml
kbld -f bundle/config.yml --imgpkg-lock-output bundle/.imgpkg/images.yml
imgpkg push -b extra-intermediate-public-registry.example.com/gitea/gitea/bundle:4.1.1 -f bundle
imgpkg copy -b extra-intermediate-public-registry.example.com/gitea/gitea/bundle:4.1.1 --to-tar /tmp/bundle.tar

# Copy bundle.tar to the air-gapped environment

imgpkg copy --tar /tmp/bundle.tar --to-repo internal-registry.example.com/gitea/gitea/bundle
imgpkg pull -b internal-registry.example.com/gitea/gitea/bundle:4.1.1 -o /tmp/bundle
kbld -f /tmp/bundle/config.yml -f /tmp/bundle/.imgpkg/images.yml

This is painful, so it motivates me to continue using kbld pkg.

joaopapereira commented 2 years ago

Cool, thanks for the examples @making

I would like to dig a little bit into the pain, what do you think is causing the pain?

making commented 2 years ago

@joaopapereira In the use case, It is painful for restricted enterprise companies to prepare the registry to temporarily push the bundle in the online environment.

joaopapereira commented 2 years ago

Do you think that this https://github.com/vmware-tanzu/carvel-imgpkg/issues/55#issuecomment-962209740 would help?

So in your use case it would look like:

helm repo add gitea-charts https://dl.gitea.io/charts
mkdir -p bundle/.imgpkg
helm template gitea gitea-charts/gitea > bundle/config.yml
kbld -f bundle/config.yml --imgpkg-lock-output bundle/.imgpkg/images.yml

# The 2 new commands that would create a bundle tar directly and afterwards would copy all images to it without using an intermediate registry
imgpkg push -b extra-intermediate-public-registry.example.com/gitea/gitea/bundle:4.1.1  -f my-bundle/ --to-tar bundle.tar
imgpkg copy --tar /tmp/bundle.tar --inflate

# Copy bundle.tar to the air-gapped environment

imgpkg copy --tar /tmp/bundle.tar --to-repo internal-registry.example.com/gitea/gitea/bundle
imgpkg pull -b internal-registry.example.com/gitea/gitea/bundle:4.1.1 -o /tmp/bundle
kbld -f /tmp/bundle/config.yml -f /tmp/bundle/.imgpkg/images.yml
making commented 2 years ago

@joaopapereira Does it still require extra-intermediate-public-registry.example.com ?

joaopapereira commented 2 years ago

No, the registry information there is only to ensure we have a bundle name and to keep the interface of the push command

making commented 2 years ago

That sounds great although push sounds like associated with upload and might be a bit confusing.

mhoshi-vm commented 2 years ago

To be very honest, instead of imgpkg it will be great if we can keep maintaining kbld pkg, kbld unpkg, or kbld relocate as it was • much easier • much straight forward. But that just a nice to have. I don't have enough will to go against the decision of deprecating it.

In anyway if we consider #55 as a fix for this case, I am okay with that decision, just need to get use it once it is released.