Skip to content
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

Take my imgpkg BundleLock and do something magical with it because it's relevant for kbld #158

Open
voor opened this issue Sep 10, 2021 · 15 comments
Labels
discussion This issue is not a bug or feature and a conversation is needed to find an appropriate resolution enhancement This issue is a feature request

Comments

@voor
Copy link

voor commented Sep 10, 2021

Describe the problem/challenge you have
I currently push imgpkg bundles to repositories and always use --lock-output current-version.yml to make sure I have a file that lets me know what tag I used and where my image is located. I am an experienced Carvel user, so I know that I then need to copy bundle.image out of that resulting file and paste it into my App CR (or Package CR) for use in kapp-controller. Future users might not understand that flow, or get confused at how next to consume that bundle programmatically.

Example:

imgpkg copy -b ${IMGPKG_BUNDLE}:${CHART_VERSION} --to-repo ${IMGPKG_BUNDLE} --lock-output current-version.yml

I now have a file that looks like this at current-version.yml:

---
apiVersion: imgpkg.carvel.dev/v1alpha1
bundle:
  image: harbor.example.com/imgpkg/charts/gitlab-runner@sha256:8112e7828ee08c39712a3c598210def3655f5a9778e15fe18277b9455f91af3f
  tag: 0.32.0
kind: BundleLock

I want to update my Package CR intelligently to replace this-thing with the new bundle image:

spec:
  template:
    spec:
      fetch:
        - imgpkgBundle:
            image: this-thing

So that I can maintain a "templated" Package CR that gets stamped out once it has the necessary image reference.

Describe the solution you'd like
I don't have a strong opinion about this, so happy to be moved to a different flow, but I mentally picture something like this:

imgpkg copy -b ${IMGPKG_BUNDLE}:${CHART_VERSION} --to-repo ${IMGPKG_BUNDLE} --lock-output current-version.yml
kbld -f package-cr.yml -f current-version.yml -f kbld-config.yml > versioned-package-cr.yml

The kbld config has something to basically tell this-thing gets preresolved to the BundleLock, so that instead of attempting to resolve this-thing (which would obviously fail) it'll rewrite it to the value passed in.

Anything else you would like to add:
I will not know what the tag I am pushing to is, so requiring that to assist with image resolution does not help my use case, although it would probably solve a very similar use case. Also, I do not always push with tags, the example I grabbed just happened to use them.


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.

@voor voor added carvel triage This issue has not yet been reviewed for validity enhancement This issue is a feature request labels Sep 10, 2021
@Jeremy-Boyle
Copy link

I would also like to see this @voor good suggestion !

@voor
Copy link
Author

voor commented Sep 14, 2021

So I thought a little more about this, and realized with the power of ytt I can basically achieve my goal and maintain pretty good flexibliity.
https://carvel.dev/ytt/#gist:https://gist.github.com/voor/fc6724f87ab0cd0c029f76184ee5290c

I just use the BundleLock as a data values input into ytt and have my App CR (or Package CR) templatized accordingly.

This does not work very well when the App CR or Package CR already includes ytt directives, of which 100% of them do, so while this was a great thought exercise I'm not sure how applicable it will become. Sharing it here for others that might be able to use this, though.

Alternatively, I could keep these data values in my templated App CR and find some way to insert the BundleLock at runtime, that'll probably be my next thought experiment.

@pivotaljohn
Copy link
Contributor

So I thought a little more about this, and realized with the power of ytt I can basically achieve my goal and maintain pretty good flexibliity.
https://carvel.dev/ytt/#gist:https://gist.github.com/voor/fc6724f87ab0cd0c029f76184ee5290c

Yes: for those reading along, when kbld is only replacing preresolved image references (and not building or resolving it), then it's very similar to templating or patching the file. 👍🏻

This does not work very well when the App CR or Package CR already includes ytt directives, of which 100% of them do, so while this was a great thought exercise I'm not sure how applicable it will become. Sharing it here for others that might be able to use this, though.

I'm curious, what about that approach "does not work very well"?

I immediately see a potential collision with other data values, given that they are all merged into a single struct. If that is part of the problem, it's worth mentioning that we've kicked around the idea of providing a way to place the parsed contents of a YAML file within the data.values struct. (thread on #carvel). 🤷🏻

@voor
Copy link
Author

voor commented Sep 23, 2021

We already have ytt directives in our App CR, so when we ran through ytt to add in the bundle lock information it would attempt to execute those other directives, and output pure yaml. This would be unblocked from carvel-dev/ytt#63

@cppforlife
Copy link
Contributor

few paths i see, add imgpkg builder (#56), and/or provide a way for imgpkg push to produce images lock. the problem with producing image lock is that it does not help kbld (running against PackageCR) know what to replace.

@pivotaljohn pivotaljohn added discussion This issue is not a bug or feature and a conversation is needed to find an appropriate resolution and removed carvel triage This issue has not yet been reviewed for validity labels Oct 5, 2021
@pivotaljohn
Copy link
Contributor

So, @voor, what do you think of the imgpkg builder angle?

It has the "feature" of where you'd produce the bundle through kbld... how well does that meld with your workflow?

Adding something like this:

---
apiVersion: kbld.k14s.io/v1alpha1
kind: Config
sources:
- image: this-thing
  path: path/to/bundle/root
  imgpkg:
    copy:
      toRepo: ${IMGPKG_BUNDLE}
      rawOptions: ["--lock-output", "current-version.yml"]

(where you replace ${IMGPKG_BUNDLE} yourself or turn that into a ytt template.)

which could either be added to kbld-config.yml or be it's own file bundle-kbld-config.yml

... and then ...

$ kbld -f package-cr.yml -f bundle-kbld-config.yml -f kbld-config.yml > versioned-package-cr.yml

@voor
Copy link
Author

voor commented Oct 5, 2021

Would it be fair to say the next step in this chain would be using the same builder to then create the Package Repository? Possibly used in conjunction with vendir to grab the various package CR's from all their little repositories that have the folders of current-version.yml inside them?

@voor
Copy link
Author

voor commented Oct 22, 2021

Conversation out of band but relevant to this issue: We are using this heavily with helm and so helm template is an important step to get the kbld image references properly, we do not store a sample or otherwise inside the bundle so therefore all we have is the raw chart folder. This creates a slippery slope for kbld.

@pivotaljohn
Copy link
Contributor

Would it be fair to say the next step in this chain would be using the same builder to then create the Package Repository? Possibly used in conjunction with vendir to grab the various package CR's from all their little repositories that have the folders of current-version.yml inside them?

This starts to get tricky, actually. Because we don't expect that the folks who are producing a Package would necessarily produce the whole Package Repository.... 🤔 .

I sense a cousin of the "prefer composition over inheritance" advise being relevant here....

@pivotaljohn
Copy link
Contributor

Conversation out of band but relevant to this issue: We are using this heavily with helm and so helm template is an important step to get the kbld image references properly, we do not store a sample or otherwise inside the bundle so therefore all we have is the raw chart folder. This creates a slippery slope for kbld.

Apologies for my thick-headedness... but I'm not sure what that slipper slope is for kbld... could you elaborate just a tad more on the risk you're pointing at?

@voor
Copy link
Author

voor commented Oct 25, 2021

Apologies for my thick-headedness... but I'm not sure what that slipper slope is for kbld... could you elaborate just a tad more on the risk you're pointing at?

Maybe it is I who is thick-headed, and maybe what I'm asking is that kbld needs a helm template builder that can take a given folder of chart/ and render it with helm template such that it can gather the necessary images. So, if you go back up a level, when the Package is built and needs to generate the imgpkg bundle (that contains the necessary image.yml) it is not missing the helm chart's containers.

@pivotaljohn
Copy link
Contributor

kbld needs a helm template builder that can take a given folder of chart/ and render it with helm template such that it can gather the necessary images

How are you imagining that kbld would get the values necessary to helm template successfully? Would kbld consume the Package? 🤔

From where I'm sitting, it's not likely that kbld would achieve this with a Builder, per se; kbld's builders are a "function" that takes as input an identified image reference and produces the corresponding image (optionally publishing/pushing)... YAML rendering, and scanning have already happened by the time Builders enter the picture.

@voor
Copy link
Author

voor commented Oct 26, 2021

Think I probably should have linked to this repo a lot earlier in this conversation to have a solid example, but if you take a look at: https://github.com/voor/gitlab-runner-helm-imgpkg/tree/main/bundle you can see the ultimate format of the bundle just before it gets pushed, the script to actually generate this looks like this: https://github.com/voor/gitlab-runner-helm-imgpkg/blob/main/build.sh#L61-L63

My ultimate goal is to make this a lot easier to adopt, and how you can effectively take everything in this build.sh and achieve it with just the Carvel tools.

@pivotaljohn
Copy link
Contributor

Oooooooookay, that example really helped clarify what you're going for here. 🙏🏻

That and a quick convo with Dmitriy about this... which restored in my mind the idea that kbld aims to address the general need of "hey, I got a bunch of software bits/blobs that go with my configuration; please identify them, ensure they are in their proper form (nominally an OCI image), and give me back an inventory of them with resolved references.".

So, I think I see two feature requests in here:

  1. the ability to ingest a BundleLock and convert that to a Config containing an override that (pre-)resolves that "image reference" to that named in the BundleLock
  2. the ability to configure a source of type imgpkg that, when invoked, produces the bundle.
    • @cppforlife pointed out that producing an imgpkg bundle requires preparing an .imgpkg/images.yml file (typically the output of a kbld invocation... which... itself depends on having the rendered manifests for the contents of the bundle)... so this could include the ability to point to some script (either in a file or inline) that would do whatever prep is required by the imgpkg push (or perhaps imgpkg build?).
    • as your case illustrates, doing this build step might require some local setup... so this might include an option to refer to a container image that's pre-configured with all that tooling and performs the prep. 🐢 🐢 🐢 🐢 🐢 🐢 🐢 ...

The delta in the example you pointed to is that most of the build.sh script you pointed to would be repurposed as configuration for an imgpkg kbld-builder.

How well am I tracking alongside you, now?

@voor
Copy link
Author

voor commented Nov 2, 2021

Yes, I think I'm tracking. By the way, here's an updated build.sh that shows how this is progressively getting "worse" as we start to implement hacks around Package Repsitories as well. So for those following along at home:

  • Very basic use case (yamls that we reliably know are going through ytt at a later date, but we're still going to run them through ytt prior to kbld in order to get our --imgpkg-lock-output for the imgpkg push
  • More advanced (but extremely common for us) use case of Helm+ytt (best of friends), this is overwhelmingly the most common use we are doing lately (over 80% of our packages are Helm+ytt)
  • Very rare but also mentioned use case of "repackaging" somebody elses PackageRepository, injecting a ytt overlay in the process.

@aaronshurley aaronshurley moved this to To Triage in Carvel Jul 25, 2022
@github-project-automation github-project-automation bot moved this to To Triage in Carvel Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion This issue is not a bug or feature and a conversation is needed to find an appropriate resolution enhancement This issue is a feature request
Projects
Status: To Triage
Development

No branches or pull requests

4 participants