[roles/deploy-crc-cloud] Replace debug and fail tasks with assert#210
[roles/deploy-crc-cloud] Replace debug and fail tasks with assert#210elfiesmelfie wants to merge 1 commit intocrc-org:mainfrom
Conversation
WalkthroughReplaces two separate tasks (debug success message and conditional fail) with a single Ansible assert task in the cluster health-check play. The assertion uses the same condition ("'False' not in component_status.stdout_lines") and includes both Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ansible/roles/deploy-crc-cloud/tasks/wait_cluster_become_healthy.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
ansible/roles/deploy-crc-cloud/tasks/wait_cluster_become_healthy.yaml
[error] 16-16: syntax error: expected , but found ''
(syntax)
🔇 Additional comments (1)
ansible/roles/deploy-crc-cloud/tasks/wait_cluster_become_healthy.yaml (1)
15-18: Condition logic is sound.Once the syntax error is corrected, the conditional guard correctly ensures the success message only displays when all components are healthy (no 'False' values in the output), aligning with the PR objectives.
ansible/roles/deploy-crc-cloud/tasks/wait_cluster_become_healthy.yaml
Outdated
Show resolved
Hide resolved
ansible/roles/deploy-crc-cloud/tasks/wait_cluster_become_healthy.yaml
Outdated
Show resolved
Hide resolved
Simplifies the health check messages in wait_cluster_become_healthy.yaml by using a single assert task instead of separate conditional debug and fail tasks. This is more idiomatic Ansible and reduces duplication. This additionally addresses the unconditional debug message that was indiacting that the check were successful in all circumstances. Assisted-by: Claude
|
/lgtm |
|
recheck |
There was a problem hiding this comment.
In the end, it finish with an error.
EDIT:
checking again.
EDIT:
need to check, why now it stuck on place where many pods are in "Pending" state.
In the end, solution to make it running is:
oc adm certificate approve $(oc get csr --no-headers | awk '/Pending/ {print $1}')
|
@elfiesmelfie Let's wait for crc-cloud team to merge this change - #211 then I would test your change again. |
#211 has been merged! @danpawlik Can you review again? |
|
recheck |
Simplifies the health check messages in wait_cluster_become_healthy.yaml
by using a single assert task instead of separate conditional debug
and fail tasks. This is more idiomatic Ansible and reduces duplication.
This additionally addresses the unconditional debug message that was
indiacting that the check were successful in all circumstances.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.