"DeterministicLBIPSupported"
DeterministicLBIPSupported indicates whether the HostedCluster’s +deterministic load balancer configuration (loadBalancer.serviceAnnotations +and loadBalancer.loadBalancerClass on the APIServer service) is honored by +the control plane operator image in use. +A False condition means the HostedCluster asks for annotations or an LB +class that the control plane operator image predates, so the fields are +ignored. Upgrading the control plane release image resolves it; the +HostedCluster keeps reconciling in the meantime.
+"EtcdAvailable"
EtcdAvailable bubbles up the same condition from HCP. It signals if etcd is available. A failure here often means a software bug or a non-stable cluster.
diff --git a/docs/content/reference/api.md b/docs/content/reference/api.md index 8e9e9f34be69..6fcef7611079 100644 --- a/docs/content/reference/api.md +++ b/docs/content/reference/api.md @@ -6897,6 +6897,16 @@ firewall rules, missing data plane nodes, or problems with infrastructure components like the konnectivity-agent workload. Unknown means the status cannot be determined (e.g., no worker nodes available or unable to inspect)."DeterministicLBIPSupported"
DeterministicLBIPSupported indicates whether the HostedCluster’s +deterministic load balancer configuration (loadBalancer.serviceAnnotations +and loadBalancer.loadBalancerClass on the APIServer service) is honored by +the control plane operator image in use. +A False condition means the HostedCluster asks for annotations or an LB +class that the control plane operator image predates, so the fields are +ignored. Upgrading the control plane release image resolves it; the +HostedCluster keeps reconciling in the meantime.
+"EtcdAvailable"
EtcdAvailable bubbles up the same condition from HCP. It signals if etcd is available. A failure here often means a software bug or a non-stable cluster.
diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go index 0f01306a49f5..61dd9a55bb76 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go @@ -143,6 +143,7 @@ const ( controlPlanePKIOperatorSignsCSRsLabel = "io.openshift.hypershift.control-plane-pki-operator-signs-csrs" useRestrictedPodSecurityLabel = "io.openshift.hypershift.restricted-psa" defaultToControlPlaneV2Label = "io.openshift.hypershift.control-plane-operator.v2-isdefault" + controlPlaneOperatorSupportsDeterministicLBLabel = "io.openshift.hypershift.control-plane-operator-supports-deterministic-lb" apiOpenShiftComLabelPrefix = "api.openshift.com/" @@ -1467,6 +1468,9 @@ func (r *HostedClusterReconciler) reconcile(ctx context.Context, req ctrl.Reques var pullSecretBytes []byte controlPlaneOperatorImage := r.HypershiftOperatorImage var controlPlaneOperatorImageLabels map[string]string + // An image with no labels at all yields a nil map, which is indistinguishable + // from the lookup never having run, so track the lookup explicitly. + cpoImageLabelsResolved := false report.execute("CPOImageAndNamespace", critical, func() error { var err error @@ -1482,6 +1486,7 @@ func (r *HostedClusterReconciler) reconcile(ctx context.Context, req ctrl.Reques if err != nil { return fmt.Errorf("failed to get controlPlaneOperatorImageLabels: %w", err) } + cpoImageLabelsResolved = true return r.reconcileControlPlaneNamespace(ctx, createOrUpdate, hcluster, controlPlaneNamespace, controlPlaneOperatorImageLabels) }) @@ -1574,6 +1579,27 @@ func (r *HostedClusterReconciler) reconcile(ctx context.Context, req ctrl.Reques _, defaultToControlPlaneV2 = controlPlaneOperatorImageLabels[defaultToControlPlaneV2Label] } + // Version-skew detection: the HC may ask for deterministic LB fields the + // CPO image predates. This is informational and non-blocking: the fields are + // ignored until the image is upgraded, and the condition clears by itself + // once the label shows up. + if cpoImageLabelsResolved { + report.execute("DeterministicLBIPSupport", nonCritical, func() error { + // Not using statuspatching.PatchStatusCondition here on purpose: it + // re-fetches the HostedCluster, which would drop in-memory spec + // mutations made earlier in this reconcile (e.g. the KMS role ARN + // dereferenced from a Secret). Writing only on an actual change keeps + // the steady state free of status writes. + if !meta.SetStatusCondition(&hcluster.Status.Conditions, deterministicLBIPSupportedCondition(hcluster, controlPlaneOperatorImageLabels)) { + return nil + } + if err := r.Client.Status().Update(ctx, hcluster); err != nil { + return fmt.Errorf("failed to update %s condition: %w", hyperv1.DeterministicLBIPSupported, err) + } + return nil + }) + } + // Phase 7: Core HCP chain — sequential (HCP → InfraCR → CAPI Cluster). // Always runs regardless of Phase 6a failures (these are K8s resource operations). report.execute("CoreHCPChain", critical, func() error { @@ -3403,6 +3429,47 @@ func servicePublishingStrategyByType(hcp *hyperv1.HostedCluster, svcType hyperv1 return nil } +// hcUsesLBIPFields returns true if the APIServer service in the HC spec has +// serviceAnnotations or loadBalancerClass set. Used for version-skew detection: +// if the HC uses these fields but the CPO image does not support them, the +// fields are silently ignored. Only the APIServer service is wired through to +// the KAS Services, so the other service types are unaffected by the skew. +func hcUsesLBIPFields(hc *hyperv1.HostedCluster) bool { + for _, mapping := range hc.Spec.Services { + if mapping.Service != hyperv1.APIServer || mapping.ServicePublishingStrategy.LoadBalancer == nil { + continue + } + if len(mapping.ServicePublishingStrategy.LoadBalancer.ServiceAnnotations) > 0 || + mapping.ServicePublishingStrategy.LoadBalancer.LoadBalancerClass != "" { + return true + } + } + return false +} + +// deterministicLBIPSupportedCondition reports whether the control plane operator +// image honors the deterministic LB fields configured on the HostedCluster. It is +// True when the image advertises support or when the HC does not use the fields. +func deterministicLBIPSupportedCondition(hc *hyperv1.HostedCluster, controlPlaneOperatorImageLabels map[string]string) metav1.Condition { + condition := metav1.Condition{ + Type: string(hyperv1.DeterministicLBIPSupported), + ObservedGeneration: hc.Generation, + Status: metav1.ConditionTrue, + Reason: hyperv1.AsExpectedReason, + Message: "The control plane operator supports deterministic load balancer IP configuration", + } + + _, supportsDeterministicLB := controlPlaneOperatorImageLabels[controlPlaneOperatorSupportsDeterministicLBLabel] + if supportsDeterministicLB || !hcUsesLBIPFields(hc) { + return condition + } + + condition.Status = metav1.ConditionFalse + condition.Reason = hyperv1.UnsupportedControlPlaneOperatorReason + condition.Message = "HostedCluster specifies serviceAnnotations or loadBalancerClass but the control plane operator image does not support deterministic LB IP management; upgrade the control plane release image" + return condition +} + // reconcileCLISecrets makes sure the secrets that were created by the cli, and are safe to be deleted with the // hosted cluster, has an owner reference of the hosted cluster. func (r *HostedClusterReconciler) reconcileCLISecrets(ctx context.Context, createOrUpdate upsert.CreateOrUpdateFN, hcluster *hyperv1.HostedCluster) error { diff --git a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go index 3d39fbc5cd75..8d6032897c30 100644 --- a/hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go +++ b/hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go @@ -9737,3 +9737,226 @@ func TestReconcilePullSecretSync(t *testing.T) { }) } } + +func TestHcUsesLBIPFields(t *testing.T) { + tests := []struct { + name string + hc *hyperv1.HostedCluster + expected bool + }{ + { + name: "When HC has no serviceAnnotations or loadBalancerClass, it should return false", + hc: &hyperv1.HostedCluster{ + Spec: hyperv1.HostedClusterSpec{ + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{}, + }, + }, + }, + }, + }, + expected: false, + }, + { + name: "When HC has serviceAnnotations, it should return true", + hc: &hyperv1.HostedCluster{ + Spec: hyperv1.HostedClusterSpec{ + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{ + ServiceAnnotations: map[string]string{ + "metallb.universe.tf/address-pool": "production", + }, + }, + }, + }, + }, + }, + }, + expected: true, + }, + { + name: "When HC has loadBalancerClass, it should return true", + hc: &hyperv1.HostedCluster{ + Spec: hyperv1.HostedClusterSpec{ + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{ + LoadBalancerClass: "metallb.io/metallb", + }, + }, + }, + }, + }, + }, + expected: true, + }, + { + name: "When HC has nil LoadBalancer strategy, it should return false", + hc: &hyperv1.HostedCluster{ + Spec: hyperv1.HostedClusterSpec{ + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.Route, + }, + }, + }, + }, + }, + expected: false, + }, + { + name: "When only a non-APIServer service uses LB fields, it should return false", + hc: &hyperv1.HostedCluster{ + Spec: hyperv1.HostedClusterSpec{ + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{}, + }, + }, + { + Service: hyperv1.OAuthServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{ + ServiceAnnotations: map[string]string{"key": "val"}, + }, + }, + }, + }, + }, + }, + expected: false, + }, + { + name: "When HC has multiple services and the APIServer uses LB fields, it should return true", + hc: &hyperv1.HostedCluster{ + Spec: hyperv1.HostedClusterSpec{ + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.OAuthServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{}, + }, + }, + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{ + ServiceAnnotations: map[string]string{"key": "val"}, + }, + }, + }, + }, + }, + }, + expected: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + g.Expect(hcUsesLBIPFields(tt.hc)).To(Equal(tt.expected)) + }) + } +} + +func TestDeterministicLBIPSupportedCondition(t *testing.T) { + hcWithFields := &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{Generation: 7}, + Spec: hyperv1.HostedClusterSpec{ + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{ + ServiceAnnotations: map[string]string{ + "metallb.io/loadBalancerIPs": "10.1.1.100", + }, + }, + }, + }, + }, + }, + } + hcWithoutFields := &hyperv1.HostedCluster{ + ObjectMeta: metav1.ObjectMeta{Generation: 7}, + Spec: hyperv1.HostedClusterSpec{ + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{}, + }, + }, + }, + }, + } + supportedLabels := map[string]string{controlPlaneOperatorSupportsDeterministicLBLabel: "true"} + + tests := []struct { + name string + hc *hyperv1.HostedCluster + labels map[string]string + expectedStatus metav1.ConditionStatus + expectedReason string + }{ + { + name: "When the CPO image advertises support, it should report True", + hc: hcWithFields, + labels: supportedLabels, + expectedStatus: metav1.ConditionTrue, + expectedReason: hyperv1.AsExpectedReason, + }, + { + name: "When the CPO image lacks the label and the HC uses deterministic LB fields, it should report False", + hc: hcWithFields, + labels: map[string]string{"other-label": "true"}, + expectedStatus: metav1.ConditionFalse, + expectedReason: hyperv1.UnsupportedControlPlaneOperatorReason, + }, + { + name: "When the CPO image lacks the label but the HC does not use the fields, it should report True", + hc: hcWithoutFields, + labels: map[string]string{"other-label": "true"}, + expectedStatus: metav1.ConditionTrue, + expectedReason: hyperv1.AsExpectedReason, + }, + { + name: "When image labels are unavailable and the HC uses deterministic LB fields, it should report False", + hc: hcWithFields, + labels: nil, + expectedStatus: metav1.ConditionFalse, + expectedReason: hyperv1.UnsupportedControlPlaneOperatorReason, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + condition := deterministicLBIPSupportedCondition(tt.hc, tt.labels) + g.Expect(condition.Type).To(Equal(string(hyperv1.DeterministicLBIPSupported))) + g.Expect(condition.Status).To(Equal(tt.expectedStatus)) + g.Expect(condition.Reason).To(Equal(tt.expectedReason)) + g.Expect(condition.ObservedGeneration).To(Equal(tt.hc.Generation)) + }) + } +} diff --git a/hypershift-operator/controllers/hostedcluster/reconcile_legacy.go b/hypershift-operator/controllers/hostedcluster/reconcile_legacy.go index 6b7bc9be62ca..ba1439e83508 100644 --- a/hypershift-operator/controllers/hostedcluster/reconcile_legacy.go +++ b/hypershift-operator/controllers/hostedcluster/reconcile_legacy.go @@ -1058,6 +1058,18 @@ func (r *HostedClusterReconciler) reconcileLegacy(ctx context.Context, req ctrl. _, useRestrictedPSA := controlPlaneOperatorImageLabels[useRestrictedPodSecurityLabel] _, defaultToControlPlaneV2 := controlPlaneOperatorImageLabels[defaultToControlPlaneV2Label] + // The label lookup above already returned on error, so the labels are always + // resolved here. Writing only on an actual change keeps the steady state free + // of status writes. + if meta.SetStatusCondition(&hcluster.Status.Conditions, deterministicLBIPSupportedCondition(hcluster, controlPlaneOperatorImageLabels)) { + if err := r.Client.Status().Update(ctx, hcluster); err != nil { + if apierrors.IsConflict(err) { + return ctrl.Result{Requeue: true}, nil + } + return ctrl.Result{}, fmt.Errorf("failed to update %s condition: %w", hyperv1.DeterministicLBIPSupported, err) + } + } + // Reconcile the hosted cluster namespace _, err = createOrUpdate(ctx, r.Client, controlPlaneNamespace, func() error { if controlPlaneNamespace.Labels == nil { diff --git a/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_conditions.go b/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_conditions.go index eecdb18efb1b..e984311c244e 100644 --- a/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_conditions.go +++ b/vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hostedcluster_conditions.go @@ -329,6 +329,16 @@ const ( // False: re-encryption is in progress or has failed. // Absent: encryption is not configured. EtcdDataEncryptionUpToDate ConditionType = "EtcdDataEncryptionUpToDate" + + // DeterministicLBIPSupported indicates whether the HostedCluster's + // deterministic load balancer configuration (loadBalancer.serviceAnnotations + // and loadBalancer.loadBalancerClass on the APIServer service) is honored by + // the control plane operator image in use. + // A False condition means the HostedCluster asks for annotations or an LB + // class that the control plane operator image predates, so the fields are + // ignored. Upgrading the control plane release image resolves it; the + // HostedCluster keeps reconciling in the meantime. + DeterministicLBIPSupported ConditionType = "DeterministicLBIPSupported" ) // Reasons. @@ -346,6 +356,10 @@ const ( SecretNotFoundReason = "SecretNotFound" WaitingForGracePeriodReason = "WaitingForGracePeriod" BlockedReason = "Blocked" + // UnsupportedControlPlaneOperatorReason is used with the + // DeterministicLBIPSupported condition when the control plane operator image + // does not implement a feature the HostedCluster has configured. + UnsupportedControlPlaneOperatorReason = "UnsupportedControlPlaneOperator" InfraStatusFailureReason = "InfraStatusFailure" WaitingOnInfrastructureReadyReason = "WaitingOnInfrastructureReady"