ko-build / ko

Build and deploy Go applications
https://ko.build
Apache License 2.0
7.47k stars 389 forks source link

Pass labels to v1.Config during build #323

Closed zhouhaibing089 closed 3 years ago

zhouhaibing089 commented 3 years ago

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 commented 3 years ago

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: https://github.com/google/ko/compare/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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

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