no-jira: workaround: remove capi credentialsRequest in Default cluster#78710
no-jira: workaround: remove capi credentialsRequest in Default cluster#78710tthvo wants to merge 1 commit intoopenshift:mainfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@tthvo: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughScript cleanup logic is changed to conditionally remove TechPreviewNoUpgrade CredentialsRequest manifests only when feature-gate/feature-set conditions are unmet, and adds a temporary workaround that deletes the cluster-api credentials-request manifest when ChangesCredentialsRequest manifest handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: tthvo The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/pj-rehearse periodic-ci-openshift-release-main-nightly-4.22-e2e-aws-ovn-ha-cert-rotation-suspend-30d |
|
@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-disc-priv-arm-mixarch-f7 |
|
@tthvo: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
tthvo
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ci-operator/step-registry/aws/provision/cco-manual-users/static/aws-provision-cco-manual-users-static-commands.sh (1)
159-167:⚠️ Potential issue | 🟠 MajorUse a safe default for
FEATURE_SETin the guard condition to preventnounsetabort.Line 159 dereferences
${FEATURE_SET}directly. Withset -o nounsetenabled (line 3), an unsetFEATURE_SETterminates the script before the workaround block at line 167 executes.✅ Suggested fix
-if [[ "${EXTRACT_MANIFEST_INCLUDED}" != "true" ]] && [[ "${FEATURE_SET}" != "TechPreviewNoUpgrade" ]] && [[ ! -f ${SHARED_DIR}/manifest_feature_gate.yaml ]]; then +if [[ "${EXTRACT_MANIFEST_INCLUDED}" != "true" ]] && [[ "${FEATURE_SET:-}" != "TechPreviewNoUpgrade" ]] && [[ ! -f ${SHARED_DIR}/manifest_feature_gate.yaml ]]; then remove_tech_preview_feature_from_manifests "${cr_yaml_d}" "TechPreviewNoUpgrade" || exit 1 fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ci-operator/step-registry/aws/provision/cco-manual-users/static/aws-provision-cco-manual-users-static-commands.sh` around lines 159 - 167, The guard conditional that checks FEATURE_SET (the [[ "${FEATURE_SET}" != "TechPreviewNoUpgrade" ]] expression) can trigger nounset; change that check to use a safe default parameter expansion (e.g. ${FEATURE_SET:-}) or mirror the later pattern ([[ -z "${FEATURE_SET:-}" ]]) so the script won't abort under set -o nounset; update the conditional that contains EXTRACT_MANIFEST_INCLUDED and FEATURE_SET (and the call to remove_tech_preview_feature_from_manifests) to use ${FEATURE_SET:-} wherever FEATURE_SET is dereferenced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@ci-operator/step-registry/aws/provision/cco-manual-users/static/aws-provision-cco-manual-users-static-commands.sh`:
- Around line 159-167: The guard conditional that checks FEATURE_SET (the [[
"${FEATURE_SET}" != "TechPreviewNoUpgrade" ]] expression) can trigger nounset;
change that check to use a safe default parameter expansion (e.g.
${FEATURE_SET:-}) or mirror the later pattern ([[ -z "${FEATURE_SET:-}" ]]) so
the script won't abort under set -o nounset; update the conditional that
contains EXTRACT_MANIFEST_INCLUDED and FEATURE_SET (and the call to
remove_tech_preview_feature_from_manifests) to use ${FEATURE_SET:-} wherever
FEATURE_SET is dereferenced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 465e6792-d6d3-4a33-bc61-42ad8d0e0a73
📒 Files selected for processing (1)
ci-operator/step-registry/aws/provision/cco-manual-users/static/aws-provision-cco-manual-users-static-commands.sh
Credentials request filtering is temporarily broken due to https://issues.redhat.com/browse/OCPBUGS-77845. GA clusters in manual mode are failing on the missing namespace for the capi creds request at bootstrap phase.
|
[REHEARSALNOTIFIER]
A total of 256 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 periodic-ci-openshift-openshift-tests-private-release-4.22-multi-nightly-aws-ipi-disc-priv-arm-mixarch-f7 |
|
@tthvo: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-release-main-nightly-4.22-e2e-aws-ovn-ha-cert-rotation-suspend-30d |
|
@tthvo: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@tthvo: The following test 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. |
2 participants
Description
Credentials request filtering is temporarily broken due to https://issues.redhat.com/browse/OCPBUGS-77845. GA clusters in manual mode are failing on the missing namespace for the capi cred request at bootstrap phase.
Basically, I copied the approach from #76228. Currently, the step uses
upi-installerimage, which has a really oldocversion (v4.2.0-alpha.0-2466-gd76df14). Unless we can find another image to replace with a newer oc version, this is a workaround...Summary by CodeRabbit