code added to skip rhcl install on s390x#2985
Conversation
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds an early architecture check in a Robot Framework keyword for connectivity link installation. On ChangesArchitecture Guard for Operator Installation
Estimated code review effort: 1 (Trivial) | ~3 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: bdattoma, diwakarsharma2, kobihk 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 |
|
New changes are detected. LGTM label has been removed. |
![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 `@ods_ci/tasks/Resources/RHODS_OLM/install/oc_install.robot`:
- Around line 1081-1094: Remove the redundant s390x guard in the Connectivity
Link setup flow because the earlier ARCH_TYPE check already returns for s390x,
making the later IF/ELSE in oc_install.robot unreachable. Simplify the block
around Run And Return Rc And Output and Configure Authorino so the configuration
steps run unconditionally there, and delete the dead ELSE logging for s390x.
Keep the existing processLogger-style error handling intact for the command
failure path.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: c6f21d8b-dec8-4724-b954-355298fe9c81
📒 Files selected for processing (1)
ods_ci/tasks/Resources/RHODS_OLM/install/oc_install.robot
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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 `@ods_ci/tasks/Resources/RHODS_OLM/install/oc_install.robot`:
- Around line 1081-1094: Remove the redundant s390x guard in the Connectivity
Link setup flow because the earlier ARCH_TYPE check already returns for s390x,
making the later IF/ELSE in oc_install.robot unreachable. Simplify the block
around Run And Return Rc And Output and Configure Authorino so the configuration
steps run unconditionally there, and delete the dead ELSE logging for s390x.
Keep the existing processLogger-style error handling intact for the command
failure path.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: c6f21d8b-dec8-4724-b954-355298fe9c81
📒 Files selected for processing (1)
ods_ci/tasks/Resources/RHODS_OLM/install/oc_install.robot
🛑 Comments failed to post (1)
ods_ci/tasks/Resources/RHODS_OLM/install/oc_install.robot (1)
1081-1094: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Remove the now-dead s390x configuration check.
The early guard at lines 1048–1052 already
RETURNs when${ARCH_TYPE}iss390x, so the second check at line 1083 and itsELSEbranch (lines 1092–1094) are unreachable dead code.${ARCH_TYPE}is not modified between the two checks, so the condition at line 1083 will always be true. This should be simplified to unconditionally run the configuration steps.♻️ Proposed cleanup
# Wait for authorino-operator to be ready (installed by rhcl-operator as OLM dependency) Wait Until Csv Is Ready display_name=${AUTHORINO_CSV_NAME} ... operators_namespace=${CONNECTIVITY_LINK_NS} timeout=5m - # Skip configure_connectivity_link_operator.sh for s390x architecture - ${arch_type} = Get Variable Value ${ARCH_TYPE} amd64 - IF '${arch_type}' != 's390x' - ${rc} ${output} = Run And Return Rc And Output sh tasks/Resources/RHODS_OLM/install/configure_connectivity_link_operator.sh - Log ${output} console=yes - IF ${rc} != ${0} - Log Unable to configure Connectivity Link.\nCheck the cluster please console=yes - ... level=ERROR - FAIL Unable to configure Connectivity Link - END - Configure Authorino - ELSE - Log Skipping configure_connectivity_link_operator.sh for s390x architecture console=yes - Log Skipping Authorino configuration for s390x architecture console=yes - END + ${rc} ${output} = Run And Return Rc And Output sh tasks/Resources/RHODS_OLM/install/configure_connectivity_link_operator.sh + Log ${output} console=yes + IF ${rc} != ${0} + Log Unable to configure Connectivity Link.\nCheck the cluster please console=yes + ... level=ERROR + FAIL Unable to configure Connectivity Link + END + Configure Authorino END📝 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.# Wait for authorino-operator to be ready (installed by rhcl-operator as OLM dependency) Wait Until Csv Is Ready display_name=${AUTHORINO_CSV_NAME} ... operators_namespace=${CONNECTIVITY_LINK_NS} timeout=5m ${rc} ${output} = Run And Return Rc And Output sh tasks/Resources/RHODS_OLM/install/configure_connectivity_link_operator.sh Log ${output} console=yes IF ${rc} != ${0} Log Unable to configure Connectivity Link.\nCheck the cluster please console=yes ... level=ERROR FAIL Unable to configure Connectivity Link END Configure Authorino🤖 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 `@ods_ci/tasks/Resources/RHODS_OLM/install/oc_install.robot` around lines 1081 - 1094, Remove the redundant s390x guard in the Connectivity Link setup flow because the earlier ARCH_TYPE check already returns for s390x, making the later IF/ELSE in oc_install.robot unreachable. Simplify the block around Run And Return Rc And Output and Configure Authorino so the configuration steps run unconditionally there, and delete the dead ELSE logging for s390x. Keep the existing processLogger-style error handling intact for the command failure path.
|
5 participants
Uh oh!
There was an error while loading. Please reload this page.