NO-JIRA: Improve test/e2e-aws-ovn-upgrade-paused for 4.22#78284
NO-JIRA: Improve test/e2e-aws-ovn-upgrade-paused for 4.22#78284hongkailiu wants to merge 1 commit intoopenshift:mainfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@hongkailiu: 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. |
|
/pj-rehearse periodic-ci-openshift-release-main-nightly-4.22-upgrade-from-stable-4.20-e2e-aws-ovn-upgrade-paused |
|
@hongkailiu: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
WalkthroughThe changes update CI operator configuration for OpenShift upgrade testing by introducing a new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 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 |
![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 the current code and only fix it if needed.
Inline comments:
In `@ci-operator/step-registry/openshift/e2e/test/openshift-e2e-test-ref.yaml`:
- Around line 12-15: The second openshift-tests run-upgrade invocation uses only
${TEST_ARGS_2:-} which contradicts the documented “additional” semantics of
TEST_ARGS_2; update the call in
ci-operator/step-registry/openshift/e2e/test/openshift-e2e-test-commands.sh so
the second run-upgrade passes the combined arguments (e.g., expand to use both
${TEST_ARGS} and ${TEST_ARGS_2:-} together) or alternatively rename/document
TEST_ARGS_2 as a replacement; specifically modify the second openshift-tests
run-upgrade invocation to include ${TEST_ARGS} ${TEST_ARGS_2:-} (or merge them
into a single variable) so long-lived disable lists aren’t dropped on the second
hop.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: be4a4478-8920-4210-abdb-26d31de8d3f9
📒 Files selected for processing (3)
ci-operator/config/openshift/release/openshift-release-main__nightly-4.22-upgrade-from-stable-4.20.yamlci-operator/step-registry/openshift/e2e/test/openshift-e2e-test-commands.shci-operator/step-registry/openshift/e2e/test/openshift-e2e-test-ref.yaml
| - name: TEST_ARGS_2 | ||
| default: "" | ||
| documentation: |- | ||
| Additional arguments to be passed to the 2nd call of 'openshift-test', e.g., when TEST_TYPE=upgrade-paused. |
There was a problem hiding this comment.
TEST_ARGS_2 is documented as additive, but upgrade-paused currently uses it as a replacement.
ci-operator/step-registry/openshift/e2e/test/openshift-e2e-test-commands.sh passes only ${TEST_ARGS_2:-} to the second openshift-tests run-upgrade call, not ${TEST_ARGS} plus ${TEST_ARGS_2}. With the new contract here, jobs like ci-operator/config/openshift/release/openshift-release-main__nightly-4.22-upgrade-from-stable-4.20.yaml will apply the long paused-upgrade disable list only to the first control-plane hop and re-enable those monitors for the second hop. Either the second invocation should combine both vars, or this variable should be renamed/documented as a replacement instead of “additional”.
Suggested fix in ci-operator/step-registry/openshift/e2e/test/openshift-e2e-test-commands.sh
- openshift-tests run-upgrade "${TEST_UPGRADE_SUITE}" \
- --to-image "${OPENSHIFT_UPGRADE1_RELEASE_IMAGE_OVERRIDE}" "${TEST_ARGS_2:-}" \
+ openshift-tests run-upgrade "${TEST_UPGRADE_SUITE}" "${TEST_ARGS:-}" \
+ --to-image "${OPENSHIFT_UPGRADE1_RELEASE_IMAGE_OVERRIDE}" "${TEST_ARGS_2:-}" \
--options "${TEST_UPGRADE_OPTIONS-}" \
--provider "${TEST_PROVIDER}" \
-o "${ARTIFACT_DIR}/e2e.log" \
--junit-dir "${ARTIFACT_DIR}/junit" &🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ci-operator/step-registry/openshift/e2e/test/openshift-e2e-test-ref.yaml`
around lines 12 - 15, The second openshift-tests run-upgrade invocation uses
only ${TEST_ARGS_2:-} which contradicts the documented “additional” semantics of
TEST_ARGS_2; update the call in
ci-operator/step-registry/openshift/e2e/test/openshift-e2e-test-commands.sh so
the second run-upgrade passes the combined arguments (e.g., expand to use both
${TEST_ARGS} and ${TEST_ARGS_2:-} together) or alternatively rename/document
TEST_ARGS_2 as a replacement; specifically modify the second openshift-tests
run-upgrade invocation to include ${TEST_ARGS} ${TEST_ARGS_2:-} (or merge them
into a single variable) so long-lived disable lists aren’t dropped on the second
hop.
There was a problem hiding this comment.
"Additional" is inherited from documentation of the existing env. var. TEST_ARGS in Line 10-11.
It means the additional to the existing args such as --to-image etc. when invoking openshift-tests.
TEST_ARGS_2: The _2 suffix indicates it is applied only to the 2nd upgrade hop.
There was a problem hiding this comment.
@hongkailiu, thank you for the clarification! That makes the design clear:
- "Additional" in both
TEST_ARGSandTEST_ARGS_2means extra arguments on top of the hardcoded flags (like--to-image,--options,--provider, etc.) passed toopenshift-tests— not cumulative between the two variables. TEST_ARGS_2is scoped exclusively to the 2nd upgrade hop invocation, whileTEST_ARGSapplies to the primary invocation. They are independent, hop-specific argument sets.
My original concern was a misreading of the semantics. The current implementation and documentation are correct. Apologies for the noise!
📝 Learnings were identified and not saved (knowledge base disabled). Enable
|
[REHEARSALNOTIFIER]
A total of 17302 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: |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: hongkailiu 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 |
|
Install failure in the previous one. /pj-rehearse periodic-ci-openshift-release-main-nightly-4.22-upgrade-from-stable-4.20-e2e-aws-ovn-upgrade-paused |
|
@hongkailiu: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
The rehearsal still hit and this is the reason. $ git branch -a --contains 2d60dffe5a572f3078f9ee8018a9d31e61eadf17 | grep origin
remotes/origin/HEAD -> origin/main
remotes/origin/main
remotes/origin/release-4.23
remotes/origin/release-5.0
remotes/origin/release-5.1The paused test is available only for even minor releases. I have to backport it to 4.22. And I need a bug for it. I will convert https://redhat.atlassian.net/browse/OTA-1977 to a bug. What a bumpy road. Update Here we go openshift/origin#31065 |
|
openshift/origin#31065 got in. Waiting for https://prow.ci.openshift.org/view/gs/test-platform-results/logs/branch-ci-openshift-origin-release-4.22-images/2047684665384374272 to rebuild the image. |
|
/pj-rehearse periodic-ci-openshift-release-main-nightly-4.22-upgrade-from-stable-4.20-e2e-aws-ovn-upgrade-paused |
|
@hongkailiu: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
I need to wait for the new nightly. |
|
This one has the backport /pj-rehearse periodic-ci-openshift-release-main-nightly-4.22-upgrade-from-stable-4.20-e2e-aws-ovn-upgrade-paused |
|
@hongkailiu: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@hongkailiu: 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. |
|
infrastructure failure in the previous run. /pj-rehearse periodic-ci-openshift-release-main-nightly-4.22-upgrade-from-stable-4.20-e2e-aws-ovn-upgrade-paused |
|
@hongkailiu: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
2 participants
This is to follow up
Summary by CodeRabbit