OCPBUGS-95614: fix(cpo): include unavailable components in KASLoadBalancerNotReachable message - #9232
Conversation
…le message When a KAS health check error occurs simultaneously with unavailable components, the component information was silently dropped due to switch-case priority. Append the component unavailability message to the health check error so operators see the full picture. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@hypershift-jira-solve-ci[bot]: This pull request references Jira Issue OCPBUGS-95614, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. 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. |
📝 WalkthroughWalkthroughThe controller now appends the control-plane component unavailability message to KAS load-balancer health-check failures. Tests cover message concatenation and precedence when both health-check and component-list errors occur. Suggested reviewers: 🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@hypershift-jira-solve-ci[bot]: This pull request references Jira Issue OCPBUGS-95614, which is valid. 3 validation(s) were run on this bug
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. |
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
`@control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go`:
- Around line 932-934: Update reconcileAvailabilityAndReadyStatus so
controlPlaneComponentsAvailable is queried when healthCheckErr is non-nil as
well as when the prior Available condition is false, ensuring
componentsNotAvailableMsg is populated and appended for KAS health-check
failures. Add a regression test covering a previously Available
HostedControlPlane transitioning to unavailable with both a KAS health-check
error and unavailable components.
🪄 Autofix
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: db9aa6e0-d15c-4963-aaed-2c4371d4fdf5
📒 Files selected for processing (2)
control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.gocontrol-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go
| if componentsNotAvailableMsg != "" { | ||
| message += "; " + componentsNotAvailableMsg | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Check component availability when the KAS health check fails.
reconcileAvailabilityAndReadyStatus only populates componentsNotAvailableMsg when the prior Available condition is false. If a previously available HostedControlPlane has a KAS health-check failure and unavailable components in the same reconcile, this branch receives an empty message and omits the component details.
Query controlPlaneComponentsAvailable when healthCheckErr != nil too. Add a regression test for the prior-Available transition.
Proposed fix
- if !alreadyAvailable {
+ if !alreadyAvailable || healthCheckErr != nil {
componentsNotAvailableMsg, componentsErr = r.controlPlaneComponentsAvailable(ctx, hostedControlPlane)
}🤖 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
`@control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go`
around lines 932 - 934, Update reconcileAvailabilityAndReadyStatus so
controlPlaneComponentsAvailable is queried when healthCheckErr is non-nil as
well as when the prior Available condition is false, ensuring
componentsNotAvailableMsg is populated and appended for KAS health-check
failures. Add a regression test covering a previously Available
HostedControlPlane transitioning to unavailable with both a KAS health-check
error and unavailable components.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
test_file=$(fd -a '^hostedcontrolplane_controller_test\.go$' control-plane-operator/controllers/hostedcontrolplane | head -n 1)
ast-grep outline "$test_file" --match 'TestReconcileAvailabilityStatus' --view expanded
rg -n -A100 -B10 'func TestReconcileAvailabilityStatus\b|func \(r \*HostedControlPlaneReconciler\) reconcileAvailabilityAndReadyStatus\b' \
control-plane-operator/controllers/hostedcontrolplaneLength of output: 30554
@muraee, the inspection confirms the finding.
reconcileAvailabilityAndReadyStatus still skips controlPlaneComponentsAvailable when the previous HostedControlPlaneAvailable condition is True.
TestReconcileAvailabilityStatus only tests message selection. It cannot verify the skipped component query. The regression test should call reconcileAvailabilityAndReadyStatus with a prior available condition, a KAS health-check error, and an unavailable ControlPlaneComponent.
You are interacting with an AI system.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9232 +/- ##
=======================================
Coverage 44.96% 44.96%
=======================================
Files 778 778
Lines 97452 97455 +3
=======================================
+ Hits 43820 43823 +3
Misses 50607 50607
Partials 3025 3025
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
@hypershift-jira-solve-ci[bot]: all tests passed! 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. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hypershift-jira-solve-ci[bot], muraee 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 |
What this PR does / why we need it:
When a KAS health check error occurs simultaneously with unavailable components, the component unavailability information was silently dropped due to switch-case priority in the control plane operator's status handling. This fix appends the component unavailability message to the health check error so operators see the full picture when diagnosing
KASLoadBalancerNotReachableconditions.Which issue(s) this PR fixes:
Fixes https://redhat.atlassian.net/browse/OCPBUGS-95614
Special notes for your reviewer:
The change ensures that when both a health check error and unavailable components exist, the unavailability details are included in the condition message rather than being silently dropped by the switch-case fallthrough.
Checklist:
Always review AI generated responses prior to use.
Generated with Claude Code via openshift-developer plugin
Summary by CodeRabbit