CNTRLPLANE-3774: [release-4.22] fix: control-plane-operator: improve opaque conditions#9058
CNTRLPLANE-3774: [release-4.22] fix: control-plane-operator: improve opaque conditions#9058csrwng wants to merge 7 commits into
Conversation
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. Signed-off-by: Steve Kuznetsov <stekuznetsov@microsoft.com>
…ostname When the shared ingress LB service has no hostname or IP, the controller was writing an empty RouterCanonicalHostname to all HCP-labeled routes. This overwrites any previously-valid value and causes the CPO health check to report "route not admitted", flapping the HostedCluster Available condition every reconcile cycle. Skip route status reconciliation entirely when the canonical hostname is empty, preserving the last-known-good route status. Signed-off-by: Steve Kuznetsov <stekuznetsov@microsoft.com> Commit-Message-Assisted-by: Claude (via Claude Code)
…n logs Differentiate the two sub-cases when skipping route status reconciliation: LB has no ingress entries at all (not yet provisioned, includes creation timestamp) vs LB has ingress entries with no hostname or IP. This lets operators diagnose the root cause from shared ingress controller logs alone. Signed-off-by: Steve Kuznetsov <stekuznetsov@microsoft.com> Commit-Message-Assisted-by: Claude (via Claude Code)
Both the shared ingress controller and the CPO HCP router write route Status.Ingress with RouterName "router", making their writes indistinguishable when diagnosing route status flapping. Add a hypershift.openshift.io/route-status-writer annotation set to "shared-ingress" or "hcp-router" by the respective controllers, and include its value in the CPO health check error message. Signed-off-by: Steve Kuznetsov <stekuznetsov@microsoft.com> Commit-Message-Assisted-by: Claude (via Claude Code)
The "route has no ingress status" case previously gave no indication of whether the route was never admitted or had its admission cleared. By including the route-status-writer annotation value, operators can tell from the HostedCluster condition message alone which controller last touched the route, without needing to inspect the route object. Signed-off-by: Steve Kuznetsov <stekuznetsov@microsoft.com> Commit-Message-Assisted-by: Claude (via Claude Code) Signed-off-by: Steve Kuznetsov <stekuznetsov@microsoft.com>
Signed-off-by: Steve Kuznetsov <stekuznetsov@microsoft.com>
Replace netutil.RouteStatusWriterAnnotation with util.RouteStatusWriterAnnotation since the support/netutil package does not exist on the release-4.22 branch. Inline k8sutil.CollectLBMessageIfNotProvisioned since the support/k8sutil package does not exist on the release-4.22 branch. Signed-off-by: Cesar Wong <cewong@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@csrwng: This pull request references CNTRLPLANE-3774 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target either version "4.22." or "openshift-4.22.", but it targets "openshift-5.0" instead. 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. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
area/control-plane-operator
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: csrwng 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 |
openshift-ci
Bot
added
area/hypershift-operator
|
@csrwng: 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. |
3 participants
Backport of #8804 to release-4.22.
Summary
/healthz?verboseoutput, surfacing which specific health checks are failing and adding endpoint context on request failureshypershift.openshift.io/route-status-writerannotation to attribute route status writes to either "shared-ingress" or "hcp-router"Backport Adaptations
netutil.RouteStatusWriterAnnotationwithutil.RouteStatusWriterAnnotationsincesupport/netutildoesn't exist on release-4.22k8sutil.CollectLBMessageIfNotProvisionedsincesupport/k8sutildoesn't exist on release-4.22healthCheckKASEndpointcallers to passctxparameter (function signature change was part of the PR but callers needed manual fixup due to cherry-pick context differences)