ROSAENG-58795 | task: add on-demand to rosa-clusters-service#82042
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe Konflux master configuration adds five PR-check jobs for ROSA HCP, AWS, GCP, ROSA Classic, and core modes. Each sources secrets, uses the ChangesKonflux PR checks
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
openshift-ci
Bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ci-operator/config/openshift-online/rosa-clusters-service/openshift-online-rosa-clusters-service-konflux-master.yaml (1)
34-73: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSame ~40-line secret-export block duplicated identically across all 5 jobs.
Lines 34-73 (pr-check-rosa-hcp) are byte-for-byte repeated in lines 83-121 (pr-check-aws), 131-169 (pr-check-gcp), 179-217 (pr-check-rosa-classic), and 227-265 (pr-check-core), differing only in the trailing
./pr_check.sh --<mode>line. Any future secret addition, rename, or fix must be replicated in five places, which is error-prone.Consider extracting the shared credential-export logic into a step-registry
ref(e.g.rosa-clusters-service-load-sandbox-creds-ref) and chaining it before a mode-specificcommands:/ref, or moving the export logic into a shared script invoked via./hack/load-sandbox-env.shfrom the repo, so each job only needs the mode flag.🤖 Prompt for 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. In `@ci-operator/config/openshift-online/rosa-clusters-service/openshift-online-rosa-clusters-service-konflux-master.yaml` around lines 34 - 73, The sandbox credential-export block is duplicated across all five job definitions. Extract the shared exports into a reusable step-registry ref or repository script, then update each job to invoke that shared logic before its mode-specific pr_check.sh command, leaving only the differing mode flag in each job.
🤖 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
`@ci-operator/config/openshift-online/rosa-clusters-service/openshift-online-rosa-clusters-service-konflux-master.yaml`:
- Around line 33-81: Update all five pr-check-* job definitions, including
pr-check-rosa-hcp and the surrounding entries, to explicitly disable automatic
execution by adding always_run: false or the repository’s equivalent trigger
configuration. Preserve optional: true for non-blocking behavior and apply the
same on-demand trigger rule consistently to every pr-check-* entry.
---
Nitpick comments:
In
`@ci-operator/config/openshift-online/rosa-clusters-service/openshift-online-rosa-clusters-service-konflux-master.yaml`:
- Around line 34-73: The sandbox credential-export block is duplicated across
all five job definitions. Extract the shared exports into a reusable
step-registry ref or repository script, then update each job to invoke that
shared logic before its mode-specific pr_check.sh command, leaving only the
differing mode flag in each job.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 290ddb85-886b-4fb4-b9d2-3429a0e2eff7
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift-online/rosa-clusters-service/openshift-online-rosa-clusters-service-konflux-master-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (1)
ci-operator/config/openshift-online/rosa-clusters-service/openshift-online-rosa-clusters-service-konflux-master.yaml
|
/pj-rehearse |
|
@davidleerh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse |
|
@davidleerh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pr-check-rosa-hcp |
|
@davidleerh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pr-check-rosa-hcp |
|
@davidleerh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pr-check-rosa-hcp |
|
/pj-rehearse |
|
@davidleerh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
1 similar comment
|
@davidleerh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse |
|
@davidleerh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
2 similar comments
|
@davidleerh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@davidleerh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse |
|
@davidleerh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse |
|
@davidleerh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse |
|
@davidleerh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse |
1 similar comment
|
/pj-rehearse |
|
@davidleerh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@davidleerh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse ack |
|
@davidleerh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
openshift-merge-bot
Bot
added
the
rehearsals-ack
8aba4d5 to
10ffba8
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: davidleerh, jfrazierRedHat The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@davidleerh: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
Summary by CodeRabbit
Updated the OpenShift Online Konflux “konflux-master” CI configuration for
rosa-clusters-service(ci-operator/config/openshift-online/rosa-clusters-service/openshift-online-rosa-clusters-service-konflux-master.yaml) to add five additional PR-check jobs:pr-check-rosa-hcp,pr-check-aws,pr-check-gcp,pr-check-rosa-classic, andpr-check-core.Each job runs
./pr_check.shin thesrccontainer after sourcingprow/load_prow_secrets.sh(withset -x) and mounts therosa-clusters-service-sandboxsecret at/usr/local/rosa-clusters-service-sandbox. Thepr-check-aws,pr-check-gcp,pr-check-rosa-classic, andpr-check-corejobs are configured as on-demand (optional: trueandalways_run: false), whilepr-check-rosa-hcpis added with the same command/secret wiring but is not markedoptional/always_run: falsein this change.