fix: control-plane-operator: improve opaque conditions

[stevekuznetsov]

When we have the context on why something is not progressing or presenting the way we expected it to be, we should say so. Users will be thrilled to not need to learn about which code flows might have been triggered and think through the implicit reasoning behind why a condition is failing.

Summary by CodeRabbit

Release Notes

• Improvements
  • Enhanced health check diagnostics for API server load balancers, clearly distinguishing “not yet provisioned” from “missing required ingress details.”
  • Health check failures now report more context, including which specific checks are failing.
  • Route-based health readiness errors now include richer, condition-level diagnostic information when key routing attributes are missing.

[openshift-merge-bot]

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: acf42662-6c17-44e9-9238-11af23399573

📥 Commits

Reviewing files that changed from the base of the PR and between d4efb04 and f6150cd.

📒 Files selected for processing (4)

• control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go
• control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go
• control-plane-operator/controllers/hostedcontrolplane/kas/healthcheck.go
• control-plane-operator/controllers/hostedcontrolplane/kas/healthcheck_test.go

🚧 Files skipped from review as they are similar to previous changes (4)

• control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go
• control-plane-operator/controllers/hostedcontrolplane/kas/healthcheck.go
• control-plane-operator/controllers/hostedcontrolplane/kas/healthcheck_test.go
• control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go

---

📝 Walkthrough

Walkthrough

The pull request improves diagnostic output for two KAS health check paths. In kas/healthcheck.go, GetHealthcheckEndpointForRoute now returns errors that include the Route namespace, name, spec host, and a formatted summary of ingress admission conditions (via a new routeIngressDiagnostic helper) instead of a generic "not admitted" message. In hostedcontrolplane_controller.go, healthCheckKASLoadBalancers attaches detailed provisioning context when no ingress entries exist, healthCheckKASEndpoint switches to the /healthz?verbose endpoint, and a new parseFailingHealthChecks helper extracts [-]-prefixed failing check names from the response body to include in error messages. Tests are updated to use substring-based assertions and cover the new diagnostic content.

🚥 Pre-merge checks | ✅ 9 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Test Structure And Quality	⚠️ Warning	Tests lack meaningful assertion failure messages. Assertions like Expect(err).ToNot(HaveOccurred()) and Expect(err).To(HaveOccurred()) do not include diagnostic context strings as required by t...	Add meaningful failure messages to all assertions. Example: Expect(err).NotTo(HaveOccurred(), "failed to split host:port from listener address").
Title check	❓ Inconclusive	The title is partially related to the changeset. While it mentions improving opaque conditions, the actual changes are very specific: enhancing health check diagnostics (load balancer provisioning messages, verbose health endpoint parsing, failing health checks extraction) and improving error context in Route and KubeAPI checks. The title is accurate but overly broad and doesn't clearly convey the primary technical focus of health check enhancements.	Consider a more specific title like 'fix: improve health check diagnostics with detailed error messages' to better reflect the concrete changes in KubeAPI and Route health checking.

✅ Passed checks (9 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	All test names in modified test files are stable and deterministic static strings with no dynamic content, pod names, timestamps, UUIDs, node names, namespaces, or IP addresses.
Topology-Aware Scheduling Compatibility	✅ Passed	This PR modifies only health check diagnostics and error messages (healthcheck.go, hostedcontrolplane_controller.go) without introducing any scheduling constraints, deployment manifests, affinity r...
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	The PR adds standard Go unit tests (not Ginkgo e2e tests), so the IPv6/disconnected network check does not apply. The tests use local httptest servers and mock objects only.
No-Weak-Crypto	✅ Passed	PR contains no weak crypto algorithms, custom crypto implementations, or insecure secret comparisons. Uses standard crypto/rand appropriately for secure random password generation.
Container-Privileges	✅ Passed	The PR introduces no container privilege escalation settings (privileged: true, hostPID, hostNetwork, hostIPC, SYS_ADMIN, allowPrivilegeEscalation, runAsRoot). Changes are limited to Go health chec...
No-Sensitive-Data-In-Logs	✅ Passed	Code exposes only operational diagnostics (hostnames, health check names, route conditions) in error messages, not sensitive data like passwords, tokens, API keys, PII, or customer data.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: stevekuznetsov
Once this PR has been reviewed and has the lgtm label, please assign enxebre for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

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 1034-1049: The parseFailingHealthChecks function iterates through
a scanner but never checks for read or tokenization errors that may occur during
scanning. After the for loop that processes scanner.Scan() completes, add a
check for scanner.Err() to detect and handle any errors that occurred during the
body read process. This will ensure that read failures are not silently ignored
and proper error handling is applied.

🪄 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 YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 1de23565-6794-4bcc-8dee-11cecb2d61d2

📥 Commits

Reviewing files that changed from the base of the PR and between d3162ab and d4efb04.

📒 Files selected for processing (4)

• control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go
• control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go
• control-plane-operator/controllers/hostedcontrolplane/kas/healthcheck.go
• control-plane-operator/controllers/hostedcontrolplane/kas/healthcheck_test.go

[codecov]

Codecov Report

❌ Patch coverage is 78.18182% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 42.14%. Comparing base (d3162ab) to head (f6150cd).

Files with missing lines	Patch %	Lines
...ostedcontrolplane/hostedcontrolplane_controller.go	63.63%	11 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8804      +/-   ##
==========================================
+ Coverage   42.12%   42.14%   +0.01%     
==========================================
  Files         767      767              
  Lines       95067    95113      +46     
==========================================
+ Hits        40051    40088      +37     
- Misses      52202    52210       +8     
- Partials     2814     2815       +1     

Files with missing lines	Coverage Δ
.../controllers/hostedcontrolplane/kas/healthcheck.go	100.00% <100.00%> (ø)
...ostedcontrolplane/hostedcontrolplane_controller.go	45.93% <63.63%> (+0.20%)	⬆️

Flag	Coverage Δ
cmd-support	35.46% <ø> (ø)
cpo-hostedcontrolplane	44.63% <78.18%> (+0.09%)	⬆️
cpo-other	44.25% <ø> (ø)
hypershift-operator	51.92% <ø> (ø)
other	31.69% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[openshift-ci]

@stevekuznetsov: all tests passed!

Full PR test history. Your PR dashboard.

Details

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.