Repository navigation
CNTRLPLANE-4577: CPO passthrough of serviceAnnotations and loadBalancerClass to KAS Service #10133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
56730d4
dd925aa
a9eac89
d2e9b53
a868e5f
a94986a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
| } | ||
| } | ||
|
Comment on lines
+263
to
+271
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| // 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 | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Script executed:
sed -n 55,130p control-plane-operator/controllers/hostedcontrolplane/kas/service.go rg -n 'isCreate' control-plane-operator/controllers/hostedcontrolplane/kas/service.goRepository: openshift/hypershift
Length of output: 4319
Script executed:
Repository: openshift/hypershift
Length of output: 34372
Web query:
Kubernetes v0.37.1 Service dropTypeDependentFields loadBalancerClass type changeResult:
Script executed:
Repository: openshift/hypershift
Length of output: 34598
Script executed:
Repository: openshift/hypershift
Length of output: 41457
Script executed:
Repository: openshift/hypershift
Length of output: 30076
Handle the private-to-public endpoint-access transition when
LoadBalancerClassis configured.When endpoint access changes from private to public or public-and-private,
ReconcileServicereuses the existing KAS Service and changes it fromClusterIPtoLoadBalancer. BecauseLoadBalancerClassis assigned only whenisCreateis true, the configured class is not applied during this transition.The reverse transition does not retain an immutable class. Kubernetes clears
loadBalancerClasswhen a Service changes to a non-LoadBalancertype. Narrow the correction to the private-to-public transition: recreate the Service, or reject that transition whileLoadBalancerClassis configured.