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

jobs/release: tag containers images with version #995

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jbtrystram
Copy link
Contributor

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

@jbtrystram jbtrystram requested a review from dustymabe May 21, 2024 13:18
@dustymabe
Copy link
Member

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
Copy link
Member

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
Copy link
Member

jlebon commented May 21, 2024

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

This tag the manifest with the full version number.
First step for coreos/fedora-coreos-tracker#1367
@jbtrystram jbtrystram force-pushed the tag-containers-with-version branch from 22bae57 to 078963c Compare May 21, 2024 15:13
@jbtrystram
Copy link
Contributor Author

@jlebon thanks for the review ! Updated

@jbtrystram jbtrystram marked this pull request as ready for review May 27, 2024 13:14
@jbtrystram jbtrystram requested a review from travier May 28, 2024 07:03
Copy link
Member

@travier travier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@jlebon
Copy link
Member

jlebon commented May 29, 2024

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
Copy link
Contributor Author

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 :)

@jlebon jlebon changed the title Jobs/release: tag containers images with version jobs/release: tag containers images with version Jun 6, 2024
@jbtrystram
Copy link
Contributor Author

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
Copy link
Member

jlebon commented Jun 25, 2024

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
Copy link
Contributor Author

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

@jbtrystram
Copy link
Contributor Author

jbtrystram commented Jun 25, 2024

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
Copy link
Member

jlebon commented Jun 26, 2024

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
Copy link
Member

jlebon commented Jun 26, 2024

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:

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.

jbtrystram added a commit to jbtrystram/coreos-assembler that referenced this pull request Jul 2, 2024
This script calls skopeo delete to prune image from a remote
directory. Currently only supports the FCOS tag structure.

If no duration is specified, no image will be pruned.

See coreos/fedora-coreos-tracker#1367
See coreos/fedora-coreos-pipeline#995
jbtrystram added a commit to jbtrystram/coreos-assembler that referenced this pull request Jul 2, 2024
This script calls skopeo delete to prune image from a remote
directory. Currently only supports the FCOS tag structure.

If no duration is specified, no image will be pruned.

See coreos/fedora-coreos-tracker#1367
See coreos/fedora-coreos-pipeline#995
jbtrystram added a commit to jbtrystram/coreos-assembler that referenced this pull request Jul 3, 2024
This script calls skopeo delete to prune image from a remote
directory. Currently only supports the FCOS tag structure.

If no duration is specified, no image will be pruned.

See coreos/fedora-coreos-tracker#1367
See coreos/fedora-coreos-pipeline#995
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 this pull request may close these issues.

None yet

4 participants