Skip to content

Tweak update-golang builder reuse - #3280

Draft
thegreyd wants to merge 2 commits into
openshift-eng:mainfrom
thegreyd:update-golang-tweaks
Draft

Tweak update-golang builder reuse#3280
thegreyd wants to merge 2 commits into
openshift-eng:mainfrom
thegreyd:update-golang-tweaks

Conversation

@thegreyd

@thegreyd thegreyd commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

What changed

  • Treat --build-system both --assembly test as a Konflux-only pipeline run, silently skipping Brew builder operations.
  • Check for existing Golang builders immediately after validating that assemblies are enabled.
  • Run GO_LATEST/GO_EXTRA/GO_PREVIOUS, --tag-builds, RPM availability, and plashet preparation only when at least one requested builder is missing.

Why

Test assemblies cannot safely use Brew floating tags, but a dual-build-system invocation should still be able to run its Konflux half. Existing builders also do not need build-preparation validation or repeated RPM/plashet work.

Impact

Existing builder images are reused earlier. Test-assembly runs requested with both build systems proceed through Konflux without reporting a Brew error. Brew-only test-assembly runs remain unsupported.

Validation

  • pytest -q -p no:cacheprovider pyartcd/tests/pipelines/test_update_golang.py (111 passed)
  • ruff check pyartcd/pyartcd/pipelines/update_golang.py pyartcd/tests/pipelines/test_update_golang.py
  • ruff format --check pyartcd/pyartcd/pipelines/update_golang.py pyartcd/tests/pipelines/test_update_golang.py
  • Global pre-commit and commit-message hooks

Summary by CodeRabbit

  • New Features

    • Test assemblies now validate against the Golang configuration before processing.
    • Test assemblies reject Brew-only builds and automatically use Konflux when both build systems are selected.
    • Builder image checks now avoid unnecessary RPM validation, tagging, and metadata generation.
    • RHEL 10 builds are excluded from builder-image operations while still supporting required RPM preparation.
    • External RPM and non-latest-version workflows skip unnecessary processing.
  • Bug Fixes

    • Updated command-line validation to enforce revised test-assembly and build-system rules.
    • Assembly workflows now use the appropriate builder and skip production-only processing when applicable.

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
@openshift-ci

openshift-ci Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 12, 2026
@openshift-ci

openshift-ci Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign thegreyd for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: openshift-eng/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 92301a2f-ff1c-419d-bb08-207e4ff0a00f

📥 Commits

Reviewing files that changed from the base of the PR and between 1c0f825 and 4948cc0.

📒 Files selected for processing (2)
  • pyartcd/pyartcd/pipelines/update_golang.py
  • pyartcd/tests/pipelines/test_update_golang.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • pyartcd/tests/pipelines/test_update_golang.py
  • pyartcd/pyartcd/pipelines/update_golang.py

Walkthrough

Changes

Golang assembly-aware update flow

Layer / File(s) Summary
Assembly validation and build-system normalization
pyartcd/pyartcd/pipelines/update_golang.py, pyartcd/tests/pipelines/test_update_golang.py
Test assemblies reject Brew-only builds. both normalizes to Konflux. Assembly support validation runs before version validation.
Conditional builder and RPM processing
pyartcd/pyartcd/pipelines/update_golang.py, pyartcd/tests/pipelines/test_update_golang.py
Existing builders are checked before RPM validation. RHEL 10 is excluded from image operations. RPM processing and plashet generation run only for required missing builders and versions.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant UpdateGolangPipeline
  participant group_yml
  participant Brew
  participant Konflux
  participant RPMProcessing

  CLI->>UpdateGolangPipeline: provide assembly and build_system
  UpdateGolangPipeline->>UpdateGolangPipeline: normalize both to Konflux
  UpdateGolangPipeline->>group_yml: validate enabled assembly
  UpdateGolangPipeline->>Brew: check existing builder
  UpdateGolangPipeline->>Konflux: check existing builder
  alt builder is missing
    UpdateGolangPipeline->>RPMProcessing: process required RPM versions
    RPMProcessing-->>UpdateGolangPipeline: return RPM and plashet results
  else builder exists
    UpdateGolangPipeline-->>UpdateGolangPipeline: reuse existing builder
  end
Loading

Possibly related PRs


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)

Check name Status Explanation Resolution
No-Injection-Vectors ❌ Error The PR adds yaml.load calls for repository-provided image/group content; its YAML(typ='rt') handler is not SafeLoader, matching the explicit unsafe YAML condition. Parse repository YAML with a SafeLoader or an equivalent explicitly safe parser before reading sign_golang_rpm.
✅ Passed checks (10 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: reusing existing builders in the update-golang pipeline.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
No-Weak-Crypto ✅ Passed The PR diff adds no MD5, SHA1, DES, RC4, Blowfish, ECB, custom crypto, or secret comparisons; changes only pipeline validation and builder reuse logic.
Container-Privileges ✅ Passed The parent-to-HEAD diff changes only two Python files, and added lines contain no privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, or allowPrivilegeEscalation settings.
No-Sensitive-Data-In-Logs ✅ Passed The PR adds only build-status logging with Golang versions and variable names; the diff adds no passwords, tokens, API keys, PII, session IDs, hostnames, or customer data.
No-Hardcoded-Secrets ✅ Passed The main-to-HEAD diff changes only update_golang.py and its tests; added lines contain no secret assignments, credentialed URLs, private keys, or long base64 strings.
Ai-Attribution ✅ Passed The two PR commits and contributor description contain no AI-tool mention or AI Co-Authored-By trailer; no attribution requirement is triggered.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pyartcd/pyartcd/pipelines/update_golang.py`:
- Around line 370-396: Ensure RHEL 10 is still included in the RPM preparation,
tagging, and buildroot-availability decision while remaining excluded from image
builder lookups and image builds. Update the gating logic around
el_nvr_map_for_images, brew_missing, konflux_missing, and process_rpm_builds so
an RHEL 10-only request or fully existing RHEL 8/9 builders still processes RPM
builds, and guard get_existing_builders_brew and get_existing_builders_konflux
when the image map is empty. Add a regression test covering an RHEL 10-only
request.
🪄 Autofix

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: Repository: openshift-eng/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 333f3a42-43a6-42d5-8e22-e781482e77fb

📥 Commits

Reviewing files that changed from the base of the PR and between ffedb54 and 1c0f825.

📒 Files selected for processing (2)
  • pyartcd/pyartcd/pipelines/update_golang.py
  • pyartcd/tests/pipelines/test_update_golang.py

Comment thread pyartcd/pyartcd/pipelines/update_golang.py
rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant