From 56730d4ca97355fc4894c400d4bde376a271fa4d Mon Sep 17 00:00:00 2001 From: Juan Manuel Parrilla Madrid Date: Wed, 28 Oct 2026 17:47:08 +0100 Subject: [PATCH 1/6] feat(api): CNTRLPLANE-4577 | add DeterministicLBIPSupported condition Report whether the control plane operator image honors the deterministic load balancer configuration (loadBalancer.serviceAnnotations and loadBalancer.loadBalancerClass on the APIServer service) that the HostedCluster asks for. The condition is informational: a False value means the fields are being ignored by an older control plane operator, not that the HostedCluster is invalid, so reconciliation continues and the condition clears by itself once the image is upgraded. Co-Authored-By: Claude Opus 5 Signed-off-by: Juan Manuel Parrilla Madrid --- api/hypershift/v1beta1/hostedcluster_conditions.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/api/hypershift/v1beta1/hostedcluster_conditions.go b/api/hypershift/v1beta1/hostedcluster_conditions.go index eecdb18efb1b..e984311c244e 100644 --- a/api/hypershift/v1beta1/hostedcluster_conditions.go +++ b/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" From dd925aaac5276dc87f6b03ba3f70a5a013ace3f9 Mon Sep 17 00:00:00 2001 From: Juan Manuel Parrilla Madrid Date: Wed, 28 Oct 2026 17:47:10 +0100 Subject: [PATCH 2/6] chore(api): CNTRLPLANE-4577 | sync vendored API types Co-Authored-By: Claude Opus 5 Signed-off-by: Juan Manuel Parrilla Madrid --- .../hypershift/v1beta1/hostedcluster_conditions.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) 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" From a9eac89f29634e243de5580bf4aad090349c6001 Mon Sep 17 00:00:00 2001 From: Juan Manuel Parrilla Madrid Date: Wed, 28 Oct 2026 17:47:19 +0100 Subject: [PATCH 3/6] feat(hypershift-operator): CNTRLPLANE-4577 | detect control plane operator deterministic LB skew The control plane operator version is pinned to the HostedCluster release image, so a cluster on an older release runs an operator that silently ignores loadBalancer.serviceAnnotations and loadBalancer.loadBalancerClass indefinitely. Detect it from the image capability label and surface it as the DeterministicLBIPSupported condition. Only the APIServer service is checked, because that is the only service wired through to the KAS Services today. An image carrying no labels at all yields a nil map, which is indistinguishable from the lookup never having run, so the lookup result is tracked explicitly rather than inferred from the map being non-nil. The condition is written through the reconcile report as a non-critical operation and only when its value actually changes, so a status write conflict cannot abort the reconcile before the control plane chain runs and the steady state performs no status writes at all. Co-Authored-By: Claude Opus 5 Signed-off-by: Juan Manuel Parrilla Madrid --- .../hostedcluster/hostedcluster_controller.go | 67 ++++++ .../hostedcluster_controller_test.go | 223 ++++++++++++++++++ .../hostedcluster/reconcile_legacy.go | 12 + 3 files changed, 302 insertions(+) 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 { From d2e9b53af54938c8868f0a4da5cf48faf9b31756 Mon Sep 17 00:00:00 2001 From: Juan Manuel Parrilla Madrid Date: Wed, 28 Oct 2026 17:47:32 +0100 Subject: [PATCH 4/6] feat(control-plane-operator): CNTRLPLANE-4577 | pass serviceAnnotations and loadBalancerClass to the KAS Services Wire the LoadBalancerPublishingStrategy fields added in #9913, gated behind the DeterministicLBIP feature gate, through to the public and private KAS Services. Annotations are applied per key, never as a map overwrite, so annotations owned by other controllers survive. They are applied before the operator's own annotations so that operator-managed keys win a collision. The two fields are scoped differently because they mean different things: - serviceAnnotations carry the address, so only the Service exposing the primary endpoint gets them. Requesting one address on both KAS Services leaves whichever load balancer loses the race pending forever. Endpoint access is mutable, so the private Service also releases the address once a public endpoint appears and takes over as primary. - loadBalancerClass selects the provisioning controller, so every LoadBalancer Service needs it. Kubernetes only accepts it on create or when a Service becomes a LoadBalancer, and rejects it entirely on other Service types, so it is set accordingly and never cleared. Co-Authored-By: Claude Opus 5 Signed-off-by: Juan Manuel Parrilla Madrid --- .../hostedcontrolplane/infra/infra_test.go | 148 ++++++ .../hostedcontrolplane/kas/service.go | 47 ++ .../hostedcontrolplane/kas/service_test.go | 425 ++++++++++++++++++ 3 files changed, 620 insertions(+) diff --git a/control-plane-operator/controllers/hostedcontrolplane/infra/infra_test.go b/control-plane-operator/controllers/hostedcontrolplane/infra/infra_test.go index fd58f39fb73b..b309d927d853 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/infra/infra_test.go +++ b/control-plane-operator/controllers/hostedcontrolplane/infra/infra_test.go @@ -2219,3 +2219,151 @@ func TestReconcileOAuthService_AzureLoadBalancer(t *testing.T) { }) } } + +// kasLoadBalancerWithDeterministicIP returns KAS LoadBalancer publishing +// strategies carrying deterministic LB IP configuration. +func kasLoadBalancerWithDeterministicIP(annotations map[string]string, lbClass string) []hyperv1.ServicePublishingStrategyMapping { + services := kasServiceLoadBalancerOthersRoute() + for i := range services { + if services[i].Service != hyperv1.APIServer { + continue + } + services[i].ServicePublishingStrategy.LoadBalancer = &hyperv1.LoadBalancerPublishingStrategy{ + ServiceAnnotations: annotations, + LoadBalancerClass: lbClass, + } + } + return services +} + +// TestReconcileAPIServerService_DeterministicLBIPWiring verifies which KAS +// Service receives the deterministic LB IP configuration. serviceAnnotations +// carry the address and belong only to the Service exposing the primary +// endpoint, while loadBalancerClass selects the provisioning controller and +// belongs to every LoadBalancer Service. +func TestReconcileAPIServerService_DeterministicLBIPWiring(t *testing.T) { + t.Parallel() + + const addressAnnotation = "metallb.io/loadBalancerIPs" + const lbClass = "metallb.io/metallb" + + testCases := []struct { + name string + endpointAccess hyperv1.AWSEndpointAccessType + wantPublicServiceType corev1.ServiceType + wantAddressOnPublic bool + wantAddressOnPrivate bool + wantClassOnPublic bool + wantPrivateServiceExists bool + }{ + { + name: "When the cluster is public, it should configure only the public Service", + endpointAccess: hyperv1.Public, + wantPublicServiceType: corev1.ServiceTypeLoadBalancer, + wantAddressOnPublic: true, + wantClassOnPublic: true, + wantPrivateServiceExists: false, + }, + { + name: "When the cluster is private, it should configure only the private Service", + endpointAccess: hyperv1.Private, + wantPublicServiceType: corev1.ServiceTypeClusterIP, + wantAddressOnPublic: false, + wantClassOnPublic: false, + wantAddressOnPrivate: true, + wantPrivateServiceExists: true, + }, + { + name: "When the cluster is public and private, it should put the address only on the public Service", + endpointAccess: hyperv1.PublicAndPrivate, + wantPublicServiceType: corev1.ServiceTypeLoadBalancer, + wantAddressOnPublic: true, + wantClassOnPublic: true, + wantAddressOnPrivate: false, + wantPrivateServiceExists: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + g := NewGomegaWithT(t) + + hcp := withServices( + withAWSEndpointAccess(baseAWSHCP(), tc.endpointAccess), + kasLoadBalancerWithDeterministicIP(map[string]string{addressAnnotation: "10.1.1.100"}, lbClass), + ) + + fakeClient := fake.NewClientBuilder().WithScheme(api.Scheme).Build() + r := NewReconciler(fakeClient, testIngressDomain) + g.Expect(r.reconcileAPIServerService(context.Background(), hcp, upsert.New(false).CreateOrUpdate)).To(Succeed()) + + publicSvc := cpomanifests.KubeAPIServerService(testNamespace) + g.Expect(fakeClient.Get(context.Background(), client.ObjectKeyFromObject(publicSvc), publicSvc)).To(Succeed()) + g.Expect(publicSvc.Spec.Type).To(Equal(tc.wantPublicServiceType)) + + if tc.wantAddressOnPublic { + g.Expect(publicSvc.Annotations).To(HaveKeyWithValue(addressAnnotation, "10.1.1.100")) + } else { + g.Expect(publicSvc.Annotations).NotTo(HaveKey(addressAnnotation)) + } + if tc.wantClassOnPublic { + g.Expect(publicSvc.Spec.LoadBalancerClass).To(HaveValue(Equal(lbClass))) + } else { + // Kubernetes rejects loadBalancerClass on a non-LoadBalancer Service. + g.Expect(publicSvc.Spec.LoadBalancerClass).To(BeNil()) + } + + privateSvc := manifests.KubeAPIServerPrivateService(testNamespace) + err := fakeClient.Get(context.Background(), client.ObjectKeyFromObject(privateSvc), privateSvc) + if !tc.wantPrivateServiceExists { + g.Expect(apierrors.IsNotFound(err)).To(BeTrue(), "private Service should not exist") + return + } + g.Expect(err).NotTo(HaveOccurred()) + + // The class applies to every LoadBalancer Service, the address does not. + g.Expect(privateSvc.Spec.LoadBalancerClass).To(HaveValue(Equal(lbClass))) + if tc.wantAddressOnPrivate { + g.Expect(privateSvc.Annotations).To(HaveKeyWithValue(addressAnnotation, "10.1.1.100")) + } else { + g.Expect(privateSvc.Annotations).NotTo(HaveKey(addressAnnotation)) + } + }) + } +} + +// TestReconcileAPIServerService_WhenAPrivateClusterGainsAPublicEndpoint_ItShouldMoveTheAddressToThePublicService +// covers the endpoint access transition: the private Service must release the +// address so the public one can claim it, otherwise both load balancers request +// the same address and one stays pending. +func TestReconcileAPIServerService_WhenAPrivateClusterGainsAPublicEndpoint_ItShouldMoveTheAddressToThePublicService(t *testing.T) { + t.Parallel() + g := NewGomegaWithT(t) + + const addressAnnotation = "metallb.io/loadBalancerIPs" + services := kasLoadBalancerWithDeterministicIP(map[string]string{addressAnnotation: "10.1.1.100"}, "metallb.io/metallb") + + fakeClient := fake.NewClientBuilder().WithScheme(api.Scheme).Build() + r := NewReconciler(fakeClient, testIngressDomain) + createOrUpdate := upsert.New(false).CreateOrUpdate + ctx := context.Background() + + hcp := withServices(withAWSEndpointAccess(baseAWSHCP(), hyperv1.Private), services) + g.Expect(r.reconcileAPIServerService(ctx, hcp, createOrUpdate)).To(Succeed()) + + privateSvc := manifests.KubeAPIServerPrivateService(testNamespace) + g.Expect(fakeClient.Get(ctx, client.ObjectKeyFromObject(privateSvc), privateSvc)).To(Succeed()) + g.Expect(privateSvc.Annotations).To(HaveKeyWithValue(addressAnnotation, "10.1.1.100")) + + hcp = withServices(withAWSEndpointAccess(baseAWSHCP(), hyperv1.PublicAndPrivate), services) + g.Expect(r.reconcileAPIServerService(ctx, hcp, createOrUpdate)).To(Succeed()) + + privateSvc = manifests.KubeAPIServerPrivateService(testNamespace) + g.Expect(fakeClient.Get(ctx, client.ObjectKeyFromObject(privateSvc), privateSvc)).To(Succeed()) + g.Expect(privateSvc.Annotations).NotTo(HaveKey(addressAnnotation)) + + publicSvc := cpomanifests.KubeAPIServerService(testNamespace) + g.Expect(fakeClient.Get(ctx, client.ObjectKeyFromObject(publicSvc), publicSvc)).To(Succeed()) + g.Expect(publicSvc.Annotations).To(HaveKeyWithValue(addressAnnotation, "10.1.1.100")) +} diff --git a/control-plane-operator/controllers/hostedcontrolplane/kas/service.go b/control-plane-operator/controllers/hostedcontrolplane/kas/service.go index 5d8f2c25a90a..e2b21b909ec2 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/kas/service.go +++ b/control-plane-operator/controllers/hostedcontrolplane/kas/service.go @@ -20,6 +20,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/duration" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" ) const AWSNLBAnnotation = "service.beta.kubernetes.io/aws-load-balancer-type" @@ -71,6 +72,25 @@ func ReconcileService(svc *corev1.Service, strategy *hyperv1.ServicePublishingSt switch strategy.Type { case hyperv1.LoadBalancer: + // Deterministic LB fields only apply to a Service that ends up as type + // LoadBalancer: Kubernetes forbids spec.loadBalancerClass on any other + // type. Private-only clusters fall back to ClusterIP here and their LB + // is reconciled by ReconcilePrivateService. + // Apply user-provided service annotations per-key BEFORE CPO's own + // annotations, so CPO-managed keys take precedence on collision. + if isPublic && strategy.LoadBalancer != nil { + for k, v := range strategy.LoadBalancer.ServiceAnnotations { + svc.Annotations[k] = v + } + // loadBalancerClass is immutable on Kubernetes Services, but it may be + // set when creating a Service or when updating one to type LoadBalancer. + // The latter covers a private -> public endpoint access transition, where + // this Service already exists as ClusterIP. + if strategy.LoadBalancer.LoadBalancerClass != "" && + (isCreate || svc.Spec.Type != corev1.ServiceTypeLoadBalancer) { + svc.Spec.LoadBalancerClass = ptr.To(strategy.LoadBalancer.LoadBalancerClass) + } + } // AWS requires the load balancer type annotation to remain unchanged after // Service creation. The cluster-wide ValidatingAdmissionPolicy // "openshift-cloud-controller-manager-cloud-provider-aws" (OCPBUGS-16728) @@ -231,6 +251,33 @@ func ReconcilePrivateService(svc *corev1.Service, hcp *hyperv1.HostedControlPlan svc.Annotations = map[string]string{} } + apiServerStrategy := netutil.ServicePublishingStrategyByTypeForHCP(hcp, hyperv1.APIServer) + if apiServerStrategy != nil && apiServerStrategy.LoadBalancer != nil { + // serviceAnnotations carry the address, so only the Service exposing the + // primary endpoint gets them: requesting one address on both KAS Services + // leaves whichever load balancer loses the race pending forever. Endpoint + // access is mutable, so they are also removed again once a public endpoint + // appears and the public Service takes over as primary. + // They are applied per-key, BEFORE CPO's own platform annotations below, + // so CPO-managed keys take precedence on collision. + if netutil.IsPublicHCP(hcp) { + for k := range apiServerStrategy.LoadBalancer.ServiceAnnotations { + delete(svc.Annotations, k) + } + } else { + for k, v := range apiServerStrategy.LoadBalancer.ServiceAnnotations { + svc.Annotations[k] = v + } + } + // loadBalancerClass selects the provisioning controller rather than an + // address, so every LoadBalancer Service needs it regardless of which one + // is primary. This Service is always a LoadBalancer and Kubernetes makes + // the field immutable once set, hence create only. + if isCreate && apiServerStrategy.LoadBalancer.LoadBalancerClass != "" { + svc.Spec.LoadBalancerClass = ptr.To(apiServerStrategy.LoadBalancer.LoadBalancerClass) + } + } + switch hcp.Spec.Platform.Type { case hyperv1.AzurePlatform: svc.Annotations[azureutil.InternalLoadBalancerAnnotation] = azureutil.InternalLoadBalancerValue diff --git a/control-plane-operator/controllers/hostedcontrolplane/kas/service_test.go b/control-plane-operator/controllers/hostedcontrolplane/kas/service_test.go index a3cb78580026..7361245f5441 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/kas/service_test.go +++ b/control-plane-operator/controllers/hostedcontrolplane/kas/service_test.go @@ -13,6 +13,7 @@ import ( corev1 "k8s.io/api/core/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" ) func TestReconcileService(t *testing.T) { @@ -523,6 +524,430 @@ func TestReconcilePrivateServiceAWSNLBAnnotation(t *testing.T) { } } +func TestReconcileServiceAnnotationPassthrough(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + platform hyperv1.PlatformType + endpointAccess hyperv1.AWSEndpointAccessType + strategy hyperv1.ServicePublishingStrategy + resourceVersion string // non-empty = update, empty = create + existingServiceType corev1.ServiceType + existingAnnotations map[string]string + wantAnnotations map[string]string + wantLoadBalancerClass *string + wantServiceType corev1.ServiceType + }{ + { + name: "When creating a public LoadBalancer with serviceAnnotations it should apply them to the Service", + platform: hyperv1.AgentPlatform, + endpointAccess: hyperv1.Public, + strategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{ + ServiceAnnotations: map[string]string{ + "metallb.io/loadBalancerIPs": "10.1.1.100", + "metallb.io/address-pool": "production", + }, + }, + }, + wantAnnotations: map[string]string{ + "metallb.io/loadBalancerIPs": "10.1.1.100", + "metallb.io/address-pool": "production", + }, + }, + { + name: "When creating a public LoadBalancer with loadBalancerClass it should set it on the Service", + platform: hyperv1.AgentPlatform, + endpointAccess: hyperv1.Public, + strategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{ + LoadBalancerClass: "metallb.io/metallb", + }, + }, + wantLoadBalancerClass: ptr.To("metallb.io/metallb"), + }, + { + name: "When updating a LoadBalancer with serviceAnnotations it should apply them", + platform: hyperv1.AgentPlatform, + endpointAccess: hyperv1.Public, + resourceVersion: "1", + strategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{ + ServiceAnnotations: map[string]string{ + "metallb.io/loadBalancerIPs": "10.1.1.100", + }, + }, + }, + wantAnnotations: map[string]string{ + "metallb.io/loadBalancerIPs": "10.1.1.100", + }, + }, + { + name: "When updating a Service that is already a LoadBalancer without a class, it should not set loadBalancerClass", + platform: hyperv1.AgentPlatform, + endpointAccess: hyperv1.Public, + resourceVersion: "1", + existingServiceType: corev1.ServiceTypeLoadBalancer, + strategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{ + LoadBalancerClass: "metallb.io/metallb", + }, + }, + wantLoadBalancerClass: nil, + wantServiceType: corev1.ServiceTypeLoadBalancer, + }, + { + name: "When creating an AWS LoadBalancer with a serviceAnnotation that collides with CPO NLB annotation it should preserve CPO value", + platform: hyperv1.AWSPlatform, + endpointAccess: hyperv1.Public, + strategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{ + ServiceAnnotations: map[string]string{ + AWSNLBAnnotation: "clb", + }, + }, + }, + // CPO writes "nlb" AFTER user annotations, so CPO wins. + wantAnnotations: map[string]string{ + AWSNLBAnnotation: "nlb", + }, + }, + { + name: "When reconciling a LoadBalancer with serviceAnnotations it should preserve external annotations", + platform: hyperv1.AgentPlatform, + endpointAccess: hyperv1.Public, + resourceVersion: "1", + existingAnnotations: map[string]string{ + "metallb.io/ip-allocated-from-pool": "production", + }, + strategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{ + ServiceAnnotations: map[string]string{ + "metallb.io/loadBalancerIPs": "10.1.1.100", + }, + }, + }, + wantAnnotations: map[string]string{ + "metallb.io/loadBalancerIPs": "10.1.1.100", + "metallb.io/ip-allocated-from-pool": "production", + }, + }, + { + name: "When using a Route strategy with serviceAnnotations it should not apply them", + platform: hyperv1.AgentPlatform, + endpointAccess: hyperv1.Public, + strategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.Route, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{ + ServiceAnnotations: map[string]string{ + "metallb.io/loadBalancerIPs": "10.1.1.100", + }, + }, + }, + wantAnnotations: nil, + }, + { + name: "When a user manually changes a serviceAnnotation value on the Service it should be overwritten by HC spec value", + platform: hyperv1.AgentPlatform, + endpointAccess: hyperv1.Public, + resourceVersion: "1", + existingAnnotations: map[string]string{ + "metallb.io/loadBalancerIPs": "10.1.1.999", + }, + strategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{ + ServiceAnnotations: map[string]string{ + "metallb.io/loadBalancerIPs": "10.1.1.100", + }, + }, + }, + wantAnnotations: map[string]string{ + "metallb.io/loadBalancerIPs": "10.1.1.100", + }, + }, + { + name: "When the cluster is private-only, it should not apply loadBalancerClass to the ClusterIP Service", + platform: hyperv1.AWSPlatform, + endpointAccess: hyperv1.Private, + strategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{ + LoadBalancerClass: "metallb.io/metallb", + ServiceAnnotations: map[string]string{ + "metallb.io/loadBalancerIPs": "10.1.1.100", + }, + }, + }, + wantAnnotations: nil, + wantLoadBalancerClass: nil, + wantServiceType: corev1.ServiceTypeClusterIP, + }, + { + name: "When the cluster has public and private endpoints, it should apply the deterministic LB fields to the public Service", + platform: hyperv1.AWSPlatform, + endpointAccess: hyperv1.PublicAndPrivate, + strategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{ + LoadBalancerClass: "metallb.io/metallb", + ServiceAnnotations: map[string]string{ + "metallb.io/loadBalancerIPs": "10.1.1.100", + }, + }, + }, + wantAnnotations: map[string]string{ + "metallb.io/loadBalancerIPs": "10.1.1.100", + }, + wantLoadBalancerClass: ptr.To("metallb.io/metallb"), + wantServiceType: corev1.ServiceTypeLoadBalancer, + }, + { + name: "When a private cluster transitions to public, it should set loadBalancerClass on the existing Service", + platform: hyperv1.AWSPlatform, + endpointAccess: hyperv1.PublicAndPrivate, + resourceVersion: "1", + existingServiceType: corev1.ServiceTypeClusterIP, + strategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{ + LoadBalancerClass: "metallb.io/metallb", + }, + }, + wantLoadBalancerClass: ptr.To("metallb.io/metallb"), + wantServiceType: corev1.ServiceTypeLoadBalancer, + }, + { + name: "When the Service already exists as a LoadBalancer without a class, it should not set it", + platform: hyperv1.AWSPlatform, + endpointAccess: hyperv1.PublicAndPrivate, + resourceVersion: "1", + existingServiceType: corev1.ServiceTypeLoadBalancer, + strategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{ + LoadBalancerClass: "metallb.io/metallb", + }, + }, + wantLoadBalancerClass: nil, + wantServiceType: corev1.ServiceTypeLoadBalancer, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + svc := &corev1.Service{ + ObjectMeta: v1.ObjectMeta{ + ResourceVersion: tc.resourceVersion, + }, + Spec: corev1.ServiceSpec{ + Type: tc.existingServiceType, + }, + } + if tc.existingAnnotations != nil { + svc.Annotations = tc.existingAnnotations + } + + platform := hyperv1.PlatformSpec{Type: tc.platform} + if tc.endpointAccess != "" && tc.platform == hyperv1.AWSPlatform { + platform.AWS = &hyperv1.AWSPlatformSpec{EndpointAccess: tc.endpointAccess} + } + hcp := hyperv1.HostedControlPlane{Spec: hyperv1.HostedControlPlaneSpec{Platform: platform}} + + err := ReconcileService(svc, &tc.strategy, &v1.OwnerReference{}, 6443, []string{}, &hcp) + g.Expect(err).ToNot(HaveOccurred()) + + for k, v := range tc.wantAnnotations { + g.Expect(svc.Annotations).To(HaveKeyWithValue(k, v), "expected annotation %s=%s", k, v) + } + + if tc.wantLoadBalancerClass != nil { + g.Expect(svc.Spec.LoadBalancerClass).To(Equal(tc.wantLoadBalancerClass)) + } else { + g.Expect(svc.Spec.LoadBalancerClass).To(BeNil()) + } + + if tc.wantServiceType != "" { + g.Expect(svc.Spec.Type).To(Equal(tc.wantServiceType)) + } + + // Verify Route strategy didn't apply LB annotations + if tc.strategy.Type == hyperv1.Route && tc.wantAnnotations == nil { + g.Expect(svc.Annotations).ToNot(HaveKey("metallb.io/loadBalancerIPs")) + } + }) + } +} + +func TestReconcilePrivateServiceAnnotationPassthrough(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + platform hyperv1.PlatformType + endpointAccess hyperv1.AWSEndpointAccessType + serviceAnnotations map[string]string + loadBalancerClass string + resourceVersion string + existingAnnotations map[string]string + wantAnnotations map[string]string + notWantAnnotations []string + wantLoadBalancerClass *string + }{ + { + name: "When creating an AWS private service with serviceAnnotations it should apply them", + platform: hyperv1.AWSPlatform, + serviceAnnotations: map[string]string{ + "service.beta.kubernetes.io/aws-load-balancer-eip-allocations": "eipalloc-abc123", + }, + wantAnnotations: map[string]string{ + "service.beta.kubernetes.io/aws-load-balancer-eip-allocations": "eipalloc-abc123", + }, + }, + { + name: "When creating a private service with loadBalancerClass it should set it", + platform: hyperv1.AWSPlatform, + endpointAccess: hyperv1.Private, + loadBalancerClass: "metallb.io/metallb", + wantLoadBalancerClass: ptr.To("metallb.io/metallb"), + }, + { + name: "When updating a private service with loadBalancerClass it should not set it", + platform: hyperv1.AWSPlatform, + endpointAccess: hyperv1.Private, + resourceVersion: "1", + loadBalancerClass: "metallb.io/metallb", + wantLoadBalancerClass: nil, + }, + { + name: "When creating a private service with serviceAnnotations it should preserve CPO platform annotations", + platform: hyperv1.AWSPlatform, + serviceAnnotations: map[string]string{ + "metallb.io/loadBalancerIPs": "10.1.1.100", + }, + wantAnnotations: map[string]string{ + "metallb.io/loadBalancerIPs": "10.1.1.100", + // CPO's own AWS annotations must also be present + "service.beta.kubernetes.io/aws-load-balancer-cross-zone-load-balancing-enabled": "true", + "service.beta.kubernetes.io/aws-load-balancer-internal": "true", + }, + }, + { + name: "When reconciling a private service it should preserve external annotations not in serviceAnnotations", + platform: hyperv1.AWSPlatform, + endpointAccess: hyperv1.Private, + resourceVersion: "1", + existingAnnotations: map[string]string{ + "metallb.io/ip-allocated-from-pool": "production", + }, + serviceAnnotations: map[string]string{ + "metallb.io/loadBalancerIPs": "10.1.1.100", + }, + wantAnnotations: map[string]string{ + "metallb.io/loadBalancerIPs": "10.1.1.100", + "metallb.io/ip-allocated-from-pool": "production", + }, + }, + { + name: "When the cluster also has a public endpoint, it should apply loadBalancerClass but not the address annotations", + platform: hyperv1.AWSPlatform, + endpointAccess: hyperv1.PublicAndPrivate, + serviceAnnotations: map[string]string{ + "metallb.io/loadBalancerIPs": "10.1.1.100", + }, + loadBalancerClass: "metallb.io/metallb", + wantAnnotations: map[string]string{ + // CPO's own AWS private annotations are still applied. + "service.beta.kubernetes.io/aws-load-balancer-internal": "true", + }, + notWantAnnotations: []string{"metallb.io/loadBalancerIPs"}, + wantLoadBalancerClass: ptr.To("metallb.io/metallb"), + }, + { + name: "When a private cluster gains a public endpoint, it should remove the address annotations from the private Service", + platform: hyperv1.AWSPlatform, + endpointAccess: hyperv1.PublicAndPrivate, + resourceVersion: "1", + existingAnnotations: map[string]string{ + // Written while the cluster was private-only and this Service was + // the primary endpoint. + "metallb.io/loadBalancerIPs": "10.1.1.100", + "metallb.io/ip-allocated-from-pool": "production", + }, + serviceAnnotations: map[string]string{ + "metallb.io/loadBalancerIPs": "10.1.1.100", + }, + wantAnnotations: map[string]string{ + // Keys outside serviceAnnotations are still left alone. + "metallb.io/ip-allocated-from-pool": "production", + }, + notWantAnnotations: []string{"metallb.io/loadBalancerIPs"}, + wantLoadBalancerClass: nil, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + svc := &corev1.Service{ + ObjectMeta: v1.ObjectMeta{ + ResourceVersion: tc.resourceVersion, + }, + } + if tc.existingAnnotations != nil { + svc.Annotations = tc.existingAnnotations + } + + platform := hyperv1.PlatformSpec{Type: tc.platform} + if tc.endpointAccess != "" { + platform.AWS = &hyperv1.AWSPlatformSpec{EndpointAccess: tc.endpointAccess} + } + + hcp := &hyperv1.HostedControlPlane{ + Spec: hyperv1.HostedControlPlaneSpec{ + Platform: platform, + Services: []hyperv1.ServicePublishingStrategyMapping{ + { + Service: hyperv1.APIServer, + ServicePublishingStrategy: hyperv1.ServicePublishingStrategy{ + Type: hyperv1.LoadBalancer, + LoadBalancer: &hyperv1.LoadBalancerPublishingStrategy{ + ServiceAnnotations: tc.serviceAnnotations, + LoadBalancerClass: tc.loadBalancerClass, + }, + }, + }, + }, + }, + } + + err := ReconcilePrivateService(svc, hcp, &v1.OwnerReference{}) + g.Expect(err).ToNot(HaveOccurred()) + + for k, v := range tc.wantAnnotations { + g.Expect(svc.Annotations).To(HaveKeyWithValue(k, v), "expected annotation %s=%s", k, v) + } + for _, k := range tc.notWantAnnotations { + g.Expect(svc.Annotations).ToNot(HaveKey(k), "unexpected annotation %s", k) + } + + if tc.wantLoadBalancerClass != nil { + g.Expect(svc.Spec.LoadBalancerClass).To(Equal(tc.wantLoadBalancerClass)) + } else { + g.Expect(svc.Spec.LoadBalancerClass).To(BeNil()) + } + }) + } +} + func TestKonnectivityServiceReconcile(t *testing.T) { // Define common inputs From a868e5fc61d6c4ea5fccbca145263e8dba534505 Mon Sep 17 00:00:00 2001 From: Juan Manuel Parrilla Madrid Date: Wed, 28 Oct 2026 17:47:33 +0100 Subject: [PATCH 5/6] docs: CNTRLPLANE-4577 | regenerate API reference Co-Authored-By: Claude Opus 5 Signed-off-by: Juan Manuel Parrilla Madrid --- docs/content/reference/aggregated-docs.md | 10 ++++++++++ docs/content/reference/api.md | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/docs/content/reference/aggregated-docs.md b/docs/content/reference/aggregated-docs.md index 26646c2c6767..8c34ad009876 100644 --- a/docs/content/reference/aggregated-docs.md +++ b/docs/content/reference/aggregated-docs.md @@ -49564,6 +49564,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/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.

From a94986a7503d5ba7f745afc161b979cada0f8eb7 Mon Sep 17 00:00:00 2001 From: Juan Manuel Parrilla Madrid Date: Wed, 28 Oct 2026 17:47:41 +0100 Subject: [PATCH 6/6] build(images): CNTRLPLANE-4577 | advertise deterministic LB support Add the control-plane-operator-supports-deterministic-lb capability label so the hypershift-operator can tell whether the control plane operator image honors serviceAnnotations and loadBalancerClass. Co-Authored-By: Claude Opus 5 Signed-off-by: Juan Manuel Parrilla Madrid --- Containerfile.control-plane | 1 + Containerfile.operator | 1 + Dockerfile | 1 + Dockerfile.control-plane | 1 + Dockerfile.dev | 1 + 5 files changed, 5 insertions(+) diff --git a/Containerfile.control-plane b/Containerfile.control-plane index 04e8820426fa..5bd2e9e25790 100644 --- a/Containerfile.control-plane +++ b/Containerfile.control-plane @@ -29,3 +29,4 @@ LABEL io.openshift.hypershift.control-plane-pki-operator-signs-csrs=true LABEL io.openshift.hypershift.hosted-cluster-config-operator-reports-node-count=true LABEL io.openshift.hypershift.control-plane-operator-supports-kas-custom-kubeconfig=true LABEL io.openshift.hypershift.control-plane-operator.v2-isdefault=true +LABEL io.openshift.hypershift.control-plane-operator-supports-deterministic-lb=true diff --git a/Containerfile.operator b/Containerfile.operator index 6be0e9f69c53..aaae3ec51480 100644 --- a/Containerfile.operator +++ b/Containerfile.operator @@ -66,3 +66,4 @@ LABEL io.openshift.hypershift.restricted-psa=true LABEL io.openshift.hypershift.control-plane-pki-operator-signs-csrs=true LABEL io.openshift.hypershift.hosted-cluster-config-operator-reports-node-count=true LABEL io.openshift.hypershift.control-plane-operator.v2-isdefault=true +LABEL io.openshift.hypershift.control-plane-operator-supports-deterministic-lb=true diff --git a/Dockerfile b/Dockerfile index cfde0c7b28a0..71aa4255fdb6 100644 --- a/Dockerfile +++ b/Dockerfile @@ -34,3 +34,4 @@ LABEL io.openshift.hypershift.restricted-psa=true LABEL io.openshift.hypershift.control-plane-pki-operator-signs-csrs=true LABEL io.openshift.hypershift.hosted-cluster-config-operator-reports-node-count=true LABEL io.openshift.hypershift.control-plane-operator.v2-isdefault=true +LABEL io.openshift.hypershift.control-plane-operator-supports-deterministic-lb=true diff --git a/Dockerfile.control-plane b/Dockerfile.control-plane index fdea2e18c84a..5eacf4ebf900 100644 --- a/Dockerfile.control-plane +++ b/Dockerfile.control-plane @@ -29,3 +29,4 @@ LABEL io.openshift.hypershift.control-plane-pki-operator-signs-csrs=true LABEL io.openshift.hypershift.hosted-cluster-config-operator-reports-node-count=true LABEL io.openshift.hypershift.control-plane-operator-supports-kas-custom-kubeconfig=true LABEL io.openshift.hypershift.control-plane-operator.v2-isdefault=true +LABEL io.openshift.hypershift.control-plane-operator-supports-deterministic-lb=true diff --git a/Dockerfile.dev b/Dockerfile.dev index 3789aaaabe4d..18d364cda459 100644 --- a/Dockerfile.dev +++ b/Dockerfile.dev @@ -42,3 +42,4 @@ LABEL io.openshift.hypershift.control-plane-pki-operator-signs-csrs=true LABEL io.openshift.hypershift.hosted-cluster-config-operator-reports-node-count=true LABEL io.openshift.hypershift.control-plane-operator-supports-kas-custom-kubeconfig=true LABEL io.openshift.hypershift.control-plane-operator.v2-isdefault=true +LABEL io.openshift.hypershift.control-plane-operator-supports-deterministic-lb=true