Repository navigation
ci: bypass layer cache on non-commit rebuilds #25
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,6 +51,8 @@ jobs: | |
| RHSM_ORG_ID=/tmp/RHSM_ORG_ID | ||
| cache-from: type=gha | ||
| cache-to: type=gha,mode=max | ||
| # Non-commit rebuilds exist to pick up errata; a cached dnf layer ships none. | ||
| no-cache: ${{ github.event_name != 'push' && github.event_name != 'pull_request' }} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. MEDIUM (Performance Check): Every non-push/non-pull-request build disables the build cache for the entire image. Scheduled or manually dispatched rebuilds therefore repeat work in unrelated layers, increasing build time and resource use even though the stated goal is to refresh the DNF layer. Suggestion: Invalidate or rebuild only the package-install portion needed to pick up errata, while retaining cached results for unaffected layers. Evidence:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not a bug: full rebuilds only on weekly cron and parent dispatch, about 3 minutes each; per-layer busting would need an ARG in every Containerfile for no real saving. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. HIGH (Constitution): For scheduled, repository-dispatch, and workflow-dispatch runs, this expression sets Suggestion: Remove the Evidence:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not a bug: constitution #68 (9b6fa10) and #69 now require no-cache on non-commit builds; the quoted rule is the old text this PR replaces. |
||
|
|
||
| - name: Save image as tarball | ||
| run: docker save ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:latest -o /tmp/image.tar | ||
|
|
||
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.
HIGH (Constitution): This changes the CI rule to permit
no-cacheon non-commit builds, contradicting the in-force requirement to never useno-cache: truein CI workflows. This file is not one of the specified constitution amendment files.Suggestion: Keep the prohibition on
no-cache: truein CI workflows, or amend the constitution through an authorized constitution file.Evidence:
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.
Not a bug: constitution #68 (9b6fa10) and #69 now require no-cache on non-commit builds; the quoted rule is the old text this PR replaces.