-
Notifications
You must be signed in to change notification settings - Fork 64
builder-common.bu: Prune untagged rhel-coreos-base and node-staging images #1285
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
base: main
Are you sure you want to change the base?
Conversation
ravanelli
commented
Dec 22, 2025
- Explicitly target untagged images with repository names that bypass the default dangling filter. Applied 12h retention policy for these specific images.
Summary of ChangesHello @ravanelli, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the existing image pruning mechanism by adding a targeted cleanup for specific untagged container images. The change focuses on Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request aims to prune specific untagged images to manage disk space. I've identified a critical issue with shell quoting in the new command that will cause it to fail, and a potential issue where the image filter may not match all the intended targets mentioned in the comments and description. My review includes a code suggestion to fix the critical quoting issue.
| --filter "reference=registry.ci.openshift.org/coreos/*" \ | ||
| --filter "until=12h" \ | ||
| --format "{{.ID}} {{.Tag}}" \ | ||
| | awk "\$2 == \"<none>\" {print \$1}" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The quoting for the awk command is incorrect. The shell invoked by bash -c will expand $2 and $1 inside the double-quoted string. Since they are unset positional parameters, they will expand to empty strings, causing awk to receive an invalid script and fail with a syntax error.
To fix this, the awk script should be enclosed in single quotes to prevent shell expansion. Since the entire bash -c command is already in single quotes, you'll need to escape the inner single quotes for awk as shown in the suggestion.
| awk '\''$2 == "<none>" {print $1}'\'' \
| ExecStart=/usr/bin/bash -c 'podman images \ | ||
| --filter "reference=registry.ci.openshift.org/coreos/*" \ | ||
| --filter "until=12h" \ | ||
| --format "{{.ID}} {{.Tag}}" \ | ||
| | awk "\$2 == \"<none>\" {print \$1}" \ | ||
| | xargs -r podman rmi -f' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment on lines 102-103 and the PR description state that this change is for both rhel-coreos-base and node-staging images. However, the podman images filter reference=registry.ci.openshift.org/coreos/* may only match images under the coreos repository. If node-staging images are in a different repository, they will not be pruned, which could lead to disk space issues over time.
To ensure both types of images are pruned correctly, consider using a more explicit approach. For example, you could add a second ExecStart command for node-staging images, or adjust the filter to match both repository names if possible within a single command.
HuijingHei
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, not sure if we should sync the time with above 48h.
We need to increase the disk as well, for now let's keep 12h, the node images are new, and the disk is getting full due the number of builds running, 12h is enough for this images, even less, they are created only during the builds, I will look if we can also clean up it, during the build as well, maybe be worth |
hmm. I'm not sure I understand why the What is the "default dangling filter" and why isn't it sufficient here? |
Yeah, I was looking at that and the issue is because the dangling needs to have |
|
What command do you run to view the repo and tag values for an image? |
podman images https://docs.podman.io/en/stable/markdown/podman-images.1.html |
b948539 to
402846a
Compare
…mages - Explicitly target untagged images with repository names that bypass the default dangling filter. Applied 12h retention policy for these specific images. Signed-off-by: Renata Ravanelli <rravanel@redhat.com>
402846a to
c0b9a1c
Compare