jobs/release: tag containers images with version

[jbtrystram]

This tag the manifest with the full version number. First step for coreos/fedora-coreos-tracker#1367

Is it that simple ?
Obviously I need to add the GC code as well

[dustymabe]

haven't actually looked deeply at the code change, but one point to bring up:

I think we need to make this behavior conditional? i.e. we don't want it to run in all of our pipelines do we?

[dustymabe]

Obviously I need to add the GC code as well

maybe we can work with @gursewak1997 on this over in the code he is working on coreos/coreos-assembler#3798 - we don't need to update the same PR, but maybe let's let that land and then we'll figure out the right way to slot in GC for quay?

[jlebon]

This should be configured via the pipecfg instead. See docs/config.yaml.

[jbtrystram]

@jlebon thanks for the review ! Updated

[travier]

LGTM but I'm not super familiar with some bits here so let's wait for at least Jonathan

[jlebon]

My only concern with merging this right away is that as per coreos/fedora-coreos-tracker#1367 (comment), we have quite aggressive GC policies in mind for these tags. If we merge this but the GC side takes time to land, we'll have a tail of tags that will hang around and pollute our repo. I'm OK not blocking this on adding the GC side of it, but at least can we do this for production streams only for now?

[jbtrystram]

If we merge this but the GC side takes time to land, we'll have a tail of tags that will hang around and pollute our repo. I'm OK not blocking this on adding the GC side of it, but at least can we do this for production streams only for now?

I looked a bit at it but this config file seems to apply to all the streams, so I am not sure to implement that.
I'll start working on the GC side of it now anyway :)

[jbtrystram]

follow-up i am thinking about while working on the GC code for this: we should include the container images published to quay in the meta.json file pointing to the tags created

[jlebon]

follow-up i am thinking about while working on the GC code for this: we should include the container images published to quay in the meta.json file pointing to the tags created

That should happen automatically. See https://github.com/coreos/coreos-assembler/blob/38235218542c39a29255933d3cea64e91754e85f/src/cmd-push-container-manifest#L128.

[jbtrystram]

Good, thanks for the confirmation.
Can we merge this ? It will make working on the GC code easier

[jbtrystram]

I'm OK not blocking this on adding the GC side of it, but at least can we do this for production streams only for now?

@jlebon i don't see knobs to adjust that per-stream in pipecfg.
Or we merge as it is, as the GC code is actively worked on right now

[jlebon]

Re. the GC side of this, just cross-posting: coreos/coreos-assembler#3798 (comment)

I'm thinking the GC script should live in either cosa or https://github.com/coreos/fedora-coreos-releng-automation/... kinda leaning towards the latter to make it easier to share with other projects in the future? (Similarly to fedora-ostree-pruner there).

[jlebon]

I'm OK not blocking this on adding the GC side of it, but at least can we do this for production streams only for now?

@jlebon i don't see knobs to adjust that per-stream in pipecfg. Or we merge as it is, as the GC code is actively worked on right now

It's not documented (you're welcome to add it to docs/config.yaml as part of this PR :)), but we do support it at the stream-level:

fedora-coreos-pipeline/utils.groovy

Line 460 in dc58259

registry_repos += pipecfg.streams[stream].additional_registry_repos ?: [:]

.

It's a bit awkward because I think you'd need to redefine the whole oscontainer object instead of having a way to just specify the additional tags, but it should be fine for now. Once we have the GC side in place, we can remove it and make it global.

Can we merge this ? It will make working on the GC code easier

Definitely for working on the GC side, I would suggest using your own registry repo.

[jlebon]

GC pieces are in cosa. We will wire it up shortly. Let's do this.

[c4rt0]

/lgtm

[c4rt0]

Ouch, got here too late :)