Add Cluster Profile Sets allowlist#79093
Conversation
openshift-ci
Bot
added
the
do-not-merge/hold
WalkthroughThis PR makes two independent config updates: it changes one test job's ChangesTest Job Cluster Profile Update
Boskos Config Generation Enhancement
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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
🤖 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 `@core-services/prow/02_config/generate-boskos.py`:
- Around line 922-923: Replace the membership test that uses the banned form "if
not cps_name in ignored_cps" with the preferred Python operator order by
changing it to "if cps_name not in ignored_cps" where the code assigns
cps['cluster_profile_sets'][cps_name] = list(cps_data.keys()); update the
condition in the loop that references cps_name and ignored_cps to use the "not
in" form to satisfy the linter.
🪄 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: 09478930-5f87-4c88-b472-127c8a71a2bc
📒 Files selected for processing (2)
ci-operator/config/openshift/openshift-tests-private/openshift-openshift-tests-private-release-4.22__multi-nightly.yamlcore-services/prow/02_config/generate-boskos.py
| if not cps_name in ignored_cps: | ||
| cps['cluster_profile_sets'][cps_name] = list(cps_data.keys()) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify no E713-style membership checks remain in this file
rg -nP '\bif\s+not\s+\S+\s+in\s+\S+' core-services/prow/02_config/generate-boskos.pyRepository: openshift/release
Length of output: 197
Use not in membership form to satisfy lint.
Line 922 uses if not cps_name in ignored_cps, which triggers Ruff E713; switch to if cps_name not in ignored_cps.
🔧 Proposed fix
- if not cps_name in ignored_cps:
+ if cps_name not in ignored_cps:
cps['cluster_profile_sets'][cps_name] = list(cps_data.keys())📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if not cps_name in ignored_cps: | |
| cps['cluster_profile_sets'][cps_name] = list(cps_data.keys()) | |
| if cps_name not in ignored_cps: | |
| cps['cluster_profile_sets'][cps_name] = list(cps_data.keys()) |
🧰 Tools
🪛 Ruff (0.15.12)
[error] 922-922: Test for membership should be not in
Convert to not in
(E713)
🤖 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 `@core-services/prow/02_config/generate-boskos.py` around lines 922 - 923,
Replace the membership test that uses the banned form "if not cps_name in
ignored_cps" with the preferred Python operator order by changing it to "if
cps_name not in ignored_cps" where the code assigns
cps['cluster_profile_sets'][cps_name] = list(cps_data.keys()); update the
condition in the loop that references cps_name and ignored_cps to use the "not
in" form to satisfy the linter.
danilo-gemoli
force-pushed
the
feat/boskos/cluster-profile-sets-whitelist
branch
from
f2855b8 to
309c9de
Compare
|
New changes are detected. LGTM label has been removed. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
core-services/prow/02_config/generate-boskos.py (1)
923-924:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix membership check style to unblock Ruff (
E713).Line 923 still uses
if not cps_name in ignored_cps, which Ruff flags. Usenot inform.🔧 Proposed fix
- if not cps_name in ignored_cps: + if cps_name not in ignored_cps: cps['cluster_profile_sets'][cps_name] = list(cps_data.keys())#!/bin/bash # Verify no E713-form membership checks remain in this file. rg -nP '\bif\s+not\s+\S+\s+in\s+\S+' core-services/prow/02_config/generate-boskos.pyExpected result: no matches.
🤖 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 `@core-services/prow/02_config/generate-boskos.py` around lines 923 - 924, Change the membership check in the block that assigns cps['cluster_profile_sets'][cps_name] to use Python's preferred "not in" syntax: replace the condition "if not cps_name in ignored_cps" with "if cps_name not in ignored_cps" so the check involving cps_name and ignored_cps conforms to Ruff/E713; the rest of the assignment to cps['cluster_profile_sets'][cps_name] = list(cps_data.keys()) should remain unchanged.
🤖 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.
Duplicate comments:
In `@core-services/prow/02_config/generate-boskos.py`:
- Around line 923-924: Change the membership check in the block that assigns
cps['cluster_profile_sets'][cps_name] to use Python's preferred "not in" syntax:
replace the condition "if not cps_name in ignored_cps" with "if cps_name not in
ignored_cps" so the check involving cps_name and ignored_cps conforms to
Ruff/E713; the rest of the assignment to cps['cluster_profile_sets'][cps_name] =
list(cps_data.keys()) should remain unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 6c97f70f-9f82-442e-8eb8-aa34130bd6bd
📒 Files selected for processing (2)
ci-operator/config/openshift/openshift-tests-private/openshift-openshift-tests-private-release-4.22__multi-nightly.yamlcore-services/prow/02_config/generate-boskos.py
✅ Files skipped from review due to trivial changes (1)
- ci-operator/config/openshift/openshift-tests-private/openshift-openshift-tests-private-release-4.22__multi-nightly.yaml
danilo-gemoli
changed the title
| # } | ||
| 'allowlist': { | ||
| 'openshift/openshift-tests-private': { | ||
| 'release-4.22': { |
There was a problem hiding this comment.
is it updated automatically per new release as this jobs will exists on any new OCP release the may be created
There was a problem hiding this comment.
No but some of these stanza accept regexps as input (see openshift/ci-tools#5171), therefore we can easily set something like this release-(4\.2\d|5\.\d+). Feel free to suggest a better pattern.
There was a problem hiding this comment.
agree, may be better with a regex, but we may also need to consider the presubmits that runs against the main on installer, mentioned here.
| 'openshift/openshift-tests-private': { | ||
| 'release-4.22': { | ||
| 'multi-nightly': [ | ||
| 'aws-ipi-public-ipv4-pool-byo-subnet-mini-perm-arm-f14' |
There was a problem hiding this comment.
|
/retest-required |
danilo-gemoli
force-pushed
the
feat/boskos/cluster-profile-sets-whitelist
branch
from
309c9de to
fb677ca
Compare
|
/unhold |
openshift-ci
Bot
removed
the
do-not-merge/hold
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.22-multi-nightly-aws-ipi-public-ipv4-pool-mini-perm-arm-f14 |
|
@tthvo: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
danilo-gemoli
force-pushed
the
feat/boskos/cluster-profile-sets-whitelist
branch
from
fb677ca to
6154673
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danilo-gemoli, Prucek 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 |
|
/pj-rehearse pull-ci-openshift-installer-release-4.21-e2e-aws-ovn-public-ipv4-pool |
|
@mtulio: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-openshift-installer-release-4.22-e2e-aws-ovn-public-ipv4-pool |
|
@tthvo: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-openshift-tests-private-release-4.22-multi-nightly-aws-ipi-public-ipv4-pool-mini-perm-arm-f14 |
|
@tthvo: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
Current configuration is still not routing consistently to the region we wanted. We need to figure out the correct job configuration to make lease controller to run only in Previously it was achieved through job config: Currently looks like the pool have more regions, balancing eventually to us-east-1. /hold |
openshift-ci
Bot
added
the
do-not-merge/hold
danilo-gemoli
force-pushed
the
feat/boskos/cluster-profile-sets-whitelist
branch
from
6154673 to
68a5f65
Compare
|
[REHEARSALNOTIFIER]
A total of 61 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse pull-ci-openshift-installer-release-4.21-e2e-aws-ovn-public-ipv4-pool pull-ci-openshift-installer-release-4.22-e2e-aws-ovn-public-ipv4-pool periodic-ci-openshift-openshift-tests-private-release-4.22-multi-nightly-aws-ipi-public-ipv4-pool-mini-perm-arm-f14 |
|
@danilo-gemoli: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@danilo-gemoli: The following tests failed, say
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. |
4 participants
Requires openshift/ci-tools#5171.
/hold
The test
aws-ipi-public-ipv4-pool-byo-subnet-mini-perm-arm-f14works withawsonly since it uses the BYOIP feature.Overview
Adds a Cluster Profile Sets whitelist/allowlist mechanism and updates a private test to use the AWS profile required for BYOIP. This supports enforcement of cluster-profile-set usage in a companion ci-tools PR (openshift/ci-tools#5171). The change is currently held (/hold).
Changes
core-services/prow/02_config/generate-boskos.py
ci-operator/config/openshift/openshift-tests-private/openshift-openshift-tests-private-release-4.22__multi-nightly.yaml
Impact and context