Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@ func ReconcileService(svc *corev1.Service, strategy *hyperv1.ServicePublishingSt
}
} else {
svc.Spec.Type = corev1.ServiceTypeClusterIP
// Enable topology aware routing so that callers (KCM, scheduler, operators, etc.)
// are routed to a KAS pod in their own AZ, avoiding cross-AZ data transfer charges.
// KAS pods are spread across zones via requiredDuringScheduling anti-affinity,
// satisfying the >=1 endpoint per zone precondition for TAR to activate.
svc.Annotations["service.kubernetes.io/topology-mode"] = "Auto"
}
case hyperv1.NodePort:
svc.Spec.Type = corev1.ServiceTypeNodePort
Expand All @@ -104,6 +109,7 @@ func ReconcileService(svc *corev1.Service, strategy *hyperv1.ServicePublishingSt
case hyperv1.Route:
if hcp.Spec.Platform.Type != hyperv1.IBMCloudPlatform || svc.Spec.Type != corev1.ServiceTypeNodePort {
svc.Spec.Type = corev1.ServiceTypeClusterIP
svc.Annotations["service.kubernetes.io/topology-mode"] = "Auto"
}
default:
return fmt.Errorf("invalid publishing strategy for Kube API server service: %s", strategy.Type)
Expand Down Expand Up @@ -140,6 +146,9 @@ func ReconcileServiceClusterIP(svc *corev1.Service, owner *metav1.OwnerReference
if svc.Annotations == nil {
svc.Annotations = map[string]string{}
}
// Enable topology aware routing so that callers are routed to a KAS pod in their
// own AZ, avoiding cross-AZ data transfer charges.
svc.Annotations["service.kubernetes.io/topology-mode"] = "Auto"
svc.Spec.Type = corev1.ServiceTypeClusterIP
svc.Spec.Ports[0] = portSpec
return nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,81 @@ func TestReconcileService(t *testing.T) {
}
}

func TestReconcileServiceTopologyAwareRouting(t *testing.T) {
const topologyModeAnnotation = "service.kubernetes.io/topology-mode"

testCases := []struct {
name string
hcp *hyperv1.HostedControlPlane
strategy hyperv1.ServicePublishingStrategy
expectTARAnnotation bool
}{
{
name: "When Route strategy it should set topology-mode annotation on ClusterIP service",
hcp: &hyperv1.HostedControlPlane{
Spec: hyperv1.HostedControlPlaneSpec{
Platform: hyperv1.PlatformSpec{Type: hyperv1.AWSPlatform},
},
},
strategy: hyperv1.ServicePublishingStrategy{Type: hyperv1.Route},
expectTARAnnotation: true,
},
Comment on lines +138 to +146

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

This test case will fail due to incomplete implementation.

The test expects ReconcileService to set the topology-mode annotation for Route strategy, but the implementation in service.go does not add the annotation for the Route case (only for LoadBalancer+private). This test case will fail until the Route case is updated to add the annotation.

See the comment on service.go lines 109-112 for the required fix.

🤖 Prompt for 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.

In `@control-plane-operator/controllers/hostedcontrolplane/kas/service_test.go`
around lines 138 - 146, The test fails because ReconcileService in service.go
does not set the "service.alpha.openshift.io/topology-mode" annotation for the
Route publishing strategy; update the Route branch in ReconcileService (the
logic handling hyperv1.ServicePublishingStrategy{Type: hyperv1.Route}) to add
the same topology-mode annotation used for the private LoadBalancer case (set to
"internal" or the value used elsewhere) on the ClusterIP service object so the
expectTARAnnotation check in the test passes.

{
name: "When private-only AWS LoadBalancer strategy it should set topology-mode annotation on ClusterIP service",
hcp: &hyperv1.HostedControlPlane{
Spec: hyperv1.HostedControlPlaneSpec{
Platform: hyperv1.PlatformSpec{
Type: hyperv1.AWSPlatform,
AWS: &hyperv1.AWSPlatformSpec{
EndpointAccess: hyperv1.Private,
},
},
},
},
strategy: hyperv1.ServicePublishingStrategy{Type: hyperv1.LoadBalancer},
expectTARAnnotation: true,
},
{
name: "When public AWS LoadBalancer strategy it should not set topology-mode annotation on LB service",
hcp: &hyperv1.HostedControlPlane{
Spec: hyperv1.HostedControlPlaneSpec{
Platform: hyperv1.PlatformSpec{
Type: hyperv1.AWSPlatform,
AWS: &hyperv1.AWSPlatformSpec{
EndpointAccess: hyperv1.Public,
},
},
},
},
strategy: hyperv1.ServicePublishingStrategy{Type: hyperv1.LoadBalancer},
expectTARAnnotation: false,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
g := NewWithT(t)
svc := &corev1.Service{}
err := ReconcileService(svc, &tc.strategy, &v1.OwnerReference{}, 6443, []string{}, tc.hcp)
g.Expect(err).To(BeNil())
if tc.expectTARAnnotation {
g.Expect(svc.Annotations).To(HaveKeyWithValue(topologyModeAnnotation, "Auto"))
} else {
g.Expect(svc.Annotations).ToNot(HaveKey(topologyModeAnnotation))
}
})
}
}

func TestReconcileServiceClusterIPTopologyAwareRouting(t *testing.T) {
g := NewWithT(t)
svc := &corev1.Service{}
err := ReconcileServiceClusterIP(svc, &v1.OwnerReference{})
g.Expect(err).To(BeNil())
g.Expect(svc.Annotations).To(HaveKeyWithValue("service.kubernetes.io/topology-mode", "Auto"))
g.Expect(svc.Spec.Type).To(Equal(corev1.ServiceTypeClusterIP))
}

func TestReconcileServiceAzureInternalLB(t *testing.T) {
// The main KAS service (kube-apiserver-azure-lb) should never have the internal LB
// annotation. For Azure Private, this service becomes ClusterIP because isPublic=false.
Expand Down