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

Pass labels to v1.Config during build #323

Closed
zhouhaibing089 opened this issue Feb 28, 2021 · 4 comments · Fixed by #324
Closed

Pass labels to v1.Config during build #323

zhouhaibing089 opened this issue Feb 28, 2021 · 4 comments · Fixed by #324

Comments

@zhouhaibing089
Copy link
Contributor

zhouhaibing089 commented Feb 28, 2021

Hi, thanks for working on this awesome project. This is very useful in our Go-centered projects.

One thing which can be nice to have is to add Labels to the image config. Today, some container registries have API(s) around image labels. For e.g, it is possible to add a specified label in Quay to tell the registry when this image will expire so will be cleaned up automatically. This is an awesome feature for CI images, by the way.

LABEL quay.expires-after=20h

kaniko allows us to do this with a flag --label as describe here. I also looked at the ko code base and if I understand correctly, here is the place where we can tweak a bit to support labels as well?

https://github.com/google/ko/blob/c14c08e982592bcea40a3bd95de94836bfbe40f9/pkg/build/gobuild.go#L603-L607

Do you think this is something reasonable to be added? Thanks. :)

@imjasonh
Copy link
Member

imjasonh commented Mar 1, 2021

We can do this, and I agree we should. I'd also like to support image manifest annotations. I've hacked up a PoC here: main...ImJasonH:image-label

Some design questions:

  1. Unfortunately, "labels" is an overloaded term 😅 . ko apply already supports only applying objects that match a k8s label selector, so we'll have to be careful to document that "labels" in this context means image labels. In the PoC above I've gone with --image-label.
  2. ko can produce multiple images, some of which might have already been built and published before, in which case ko purposefully isn't going to produce a new image. If you're adding --image-label quay.expires-after=24h that'll be 24h after the first time the image was built and published, not the latest time. Is that the expected behavior?
  3. What should the flag be called? I've gone with --image-label with no short form for now.

@zhouhaibing089
Copy link
Contributor Author

Wow, Thanks Jason!

It's a nice choice to use --image-label, both explicit and accurate.

Regarding your second point, the way how Quay works is:

https://github.com/quay/quay/blob/bd7252c536bcf8c49cca167d7c4b433489d36044/data/model/oci/tag.py#L459-L470

It adds the expiration seconds on top of lifetime_start_ts. In the case you described, even the push is tried, the lifetime_start_ts won't change anyway. That means the behavior is expected in my opinion.

@jonjohnsonjr
Copy link
Collaborator

with no short form for now

You're using StringSliceVarP which specifically has a short form:

// StringSliceVarP is like StringSliceVar, but accepts a shorthand letter that can be used after a single dash.

You want StringSliceVar.

FWIW that change looks good to me, if we get a test :)

@imjasonh
Copy link
Member

imjasonh commented Mar 3, 2021

FWIW that change looks good to me, if we get a test :)

Adding a test caught a bug, the system works! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants