ROX-30858: do not use container in build#19734
Merged
Conversation
Replace pull-tag-push workflow with docker buildx imagetools create for scanner, collector, and fact image retagging. This eliminates unnecessary image pulls and pushes by copying manifests directly in the registry. Changes: - Extract image retagging to separate job that runs once per branding instead of per-architecture - Use docker buildx imagetools create for registry-to-registry manifest copying without local pulls - Remove pull-retag-push.sh script (inlined into lib.sh) - Copy full multi-arch manifests instead of amd64-only - Remove scanner/collector/fact from manifest creation step (buildx creates multi-arch manifests directly) Benefits: - Eliminates GBs of network transfer (6 images × pull + push) - Near-instantaneous manifest copy vs pull+push - Supports all architectures from source images - Runs in parallel with build-and-push-main job ROX-30858 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
Signed-off-by: Tomasz Janiszewski <tomek@redhat.com>
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThe PR refactors the CI/CD build workflow by removing container-based job definitions and replacing them with explicit toolchain setup actions. It introduces a dedicated Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- There are multiple jobs now repeating the same
actions/setup-go@v6(and similarlysetup-node) invocations; consider a composite/local action or reusable workflow to avoid duplication and keep versions/configuration centralized. - The
pre-build-docsjob installsprotobuf-compilerviaapt-geton the runner each time; if this becomes a bottleneck, consider using a dedicated setup-protoc action or caching mechanism rather than a per-runapt-getinstall. - You moved several QUAY_* secrets from the container env into the job
env; double-check that all downstream steps that previously relied on those env vars inside the container still resolve them correctly in the new runner-based environment.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There are multiple jobs now repeating the same `actions/setup-go@v6` (and similarly `setup-node`) invocations; consider a composite/local action or reusable workflow to avoid duplication and keep versions/configuration centralized.
- The `pre-build-docs` job installs `protobuf-compiler` via `apt-get` on the runner each time; if this becomes a bottleneck, consider using a dedicated setup-protoc action or caching mechanism rather than a per-run `apt-get` install.
- You moved several QUAY_* secrets from the container env into the job `env`; double-check that all downstream steps that previously relied on those env vars inside the container still resolve them correctly in the new runner-based environment.
## Individual Comments
### Comment 1
<location path=".github/workflows/build.yaml" line_range="314-315" />
<code_context>
+ go-version-file: go.mod
+ cache: false
+
+ - name: Install protobuf compiler
+ run: sudo apt-get update && sudo apt-get install -y protobuf-compiler
+
- name: Cache Go dependencies
</code_context>
<issue_to_address>
**suggestion (performance):** Harden the apt-get install step to be fully non-interactive and avoid pulling unnecessary packages.
You can do this by setting `DEBIAN_FRONTEND=noninteractive` and using `--no-install-recommends`, for example:
```yaml
- name: Install protobuf compiler
run: |
sudo DEBIAN_FRONTEND=noninteractive apt-get update
sudo DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends protobuf-compiler
```
This avoids interactive prompts and skips unnecessary recommended packages.
```suggestion
- name: Install protobuf compiler
run: |
sudo DEBIAN_FRONTEND=noninteractive apt-get update
sudo DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends protobuf-compiler
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Following the established pattern for Go tools (mockgen, stringer, easyjson), ossls is now: - Declared in tools/tools-import.go for go.mod tracking - Installed via Makefile's go-tool macro ($(OSSLS_BIN)) - Used in ossls-audit and ossls-notice targets via the variable This approach: - Ensures version is tracked in go.mod/go.sum - Leverages Makefile's caching via go-tool infrastructure - Removes need for explicit `go install` in CI workflow - Follows project's tool management conventions The ossls tool will be automatically built when running `make deps` or when the ossls-* targets are invoked. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
ef60167 to
8823517
Compare
The Makefile automatically downloads protoc from GitHub releases via make/protogen.mk (PROTOC_VERSION 32.1), so the manual apt-get installation is unnecessary and redundant. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
operator/tools/kubebuilder requires Go 1.25.7, but root go.mod has 1.25.0. Using kubebuilder's go.mod ensures the job has the required Go version. Fixes error: go.mod requires go >= 1.25.7 (running go 1.25.0) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19734 +/- ##
==========================================
- Coverage 49.59% 49.59% -0.01%
==========================================
Files 2756 2761 +5
Lines 208036 208143 +107
==========================================
+ Hits 103183 103226 +43
- Misses 97192 97252 +60
- Partials 7661 7665 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
hub-comment was a CircleCI-specific tool that's not available on native GitHub runners. Replace it with mshick/add-pr-comment@v3 action which: - Creates/updates PR comments (no duplicates via message-id) - Uses outputs from define-job-matrix job (no git checkout needed) - Reuses already-computed build-tag and short-commit - Already proven in codebase (performance-tests.yml) - Native GitHub Actions integration This eliminates the dependency on the apollo-ci container's hub-comment tool and follows the pattern already established in performance-tests.yml. Fixes: ./scripts/ci/lib.sh: line 2470: hub-comment: command not found Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Contributor
🚀 Build Images ReadyImages are ready for commit a73bc3a. To use with deploy scripts: export MAIN_IMAGE_TAG=4.11.x-542-ga73bc3a484 |
Reverts Go version from 1.25.7 to 1.25.0 to fix Bundle Helper Tests failures. The tests run 'go mod tidy' and different Go versions produce different outputs, breaking byte-for-byte comparison between Python and Go implementations. Also restores container usage for pre-build-cli and pre-build-go-binaries jobs for consistency with previous behavior. Keeps all other changes: ossls dependency, container removal from other jobs, push-matching-collector-scanner job, and PR comment improvements. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Move ossls build tool from main go.mod to tools/build/go.mod to avoid polluting production dependencies. Build/CI tools should be isolated from runtime dependencies. Changes: - Create tools/build/ module for build-time tools - Move ossls dependency to tools/build/go.mod - Update Makefile to reference tools/build module - Remove ossls from main go.mod and tools/tools-import.go Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add dependabot monitoring for the new tools/build/ Go module to ensure build tool dependencies are kept up to date automatically. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Remove "Set up Go" steps from jobs that only perform Docker operations: - build-and-push-main: downloads pre-built binaries, builds Docker images - push-main-manifests: pushes Docker manifest lists - build-and-push-operator: downloads pre-built binaries, builds Docker images These jobs only use "make tag" which just echoes a version string and don't require Go to be installed. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build.yaml:
- Around line 446-451: The job-level environment currently exports registry
secrets (QUAY_RHACS_ENG_RO_USERNAME, QUAY_RHACS_ENG_RO_PASSWORD,
QUAY_RHACS_ENG_RW_USERNAME, QUAY_RHACS_ENG_RW_PASSWORD,
QUAY_STACKROX_IO_RW_USERNAME, QUAY_STACKROX_IO_RW_PASSWORD) making them
available to every step; remove these keys from the top-level env and instead
add them only to the specific login/push steps (the steps that perform
quay/registry login and image push), by setting those secrets as env on those
step entries so only the registry login/push steps receive QUAY_* secrets; apply
the same change for the other occurrences mentioned (the other jobs that contain
the same QUAY_* variables).
- Around line 736-743: The workflow step "Comment on the PR" uses the action
mshick/add-pr-comment@v3 but the job lacks the required permissions, so add
explicit permissions: include pull-requests: write under the job (or top-level
workflow) permissions block to grant the GITHUB_TOKEN write access for PR
comments; ensure the permissions scope is applied to the job that contains the
"Comment on the PR" step so mshick/add-pr-comment@v3 can post the comment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 344e65b8-b20c-4597-be5f-de727196b73d
⛔ Files ignored due to path filters (1)
tools/build/go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
.github/dependabot.yaml.github/workflows/build.yamlMakefiletools/build/go.modtools/build/tools-import.go
✅ Files skipped from review due to trivial changes (3)
- .github/dependabot.yaml
- tools/build/tools-import.go
- tools/build/go.mod
Contributor
|
/retest |
davdhacs
added a commit
that referenced
this pull request
Apr 2, 2026
Use ubuntu-24.04-arm for arm64 jobs across the build workflow: - pre-build-go-binaries: native arm64 compilation with CGO_ENABLED=1 (was cross-compiled from amd64 with CGO_ENABLED=0). Warm build step: 48s native vs 61s cross-compile. Also enables CGO for arm64 binaries matching amd64 production builds. - build-and-push-main: native docker buildx eliminates QEMU overhead (240s → ~110s for image assembly). - build-and-push-operator: same QEMU elimination. s390x/ppc64le continue to cross-compile from amd64 (CGO_ENABLED=0) as no native runners are available. Follows the pattern from the collector repo. Based on Tomecz's container removal branch (#19734). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Using custom container is slow as setup takes ~60s while using ubuntu-latest with setup actions is almost instant.
Keep container for building go binaries as we need to handle that holistically with https://docs.google.com/document/d/1bEwLCHQEcEQDaIHD6_zXjNUeDgRfv3KDx4HKVK0Y-gc/edit?tab=t.0
User-facing documentation
Testing and quality
Automated testing
How I validated my change
change me!