diff --git a/pkg/apis/core/annotation_key_constants.go b/pkg/apis/core/annotation_key_constants.go index 3053200a204..60cff22b9d4 100644 --- a/pkg/apis/core/annotation_key_constants.go +++ b/pkg/apis/core/annotation_key_constants.go @@ -122,8 +122,24 @@ const ( // This annotation is beta-level and is only honored when PodDeletionCost feature is enabled. PodDeletionCost = "controller.kubernetes.io/pod-deletion-cost" - // AnnotationTopologyAwareHints can be used to enable or disable Topology - // Aware Hints for a Service. This may be set to "Auto" or "Disabled". Any - // other value is treated as "Disabled". - AnnotationTopologyAwareHints = "service.kubernetes.io/topology-aware-hints" + // DeprecatedAnnotationTopologyAwareHints can be used to enable or disable + // Topology Aware Hints for a Service. This may be set to "Auto" or + // "Disabled". Any other value is treated as "Disabled". This annotation has + // been deprecated in favor of the `service.kubernetes.io/topology-mode` + // annotation which also allows "Auto" and "Disabled", but is not limited to + // those (it's open ended to provide room for experimentation while we + // pursue configuration for topology via specification). When both + // `service.kubernetes.io/topology-aware-hints` and + // `service.kubernetes.io/topology-mode` annotations are set, the value of + // `service.kubernetes.io/topology-aware-hints` has precedence. + DeprecatedAnnotationTopologyAwareHints = "service.kubernetes.io/topology-aware-hints" + + // AnnotationTopologyMode can be used to enable or disable Topology Aware + // Routing for a Service. Well known values are "Auto" and "Disabled". + // Implementations may choose to develop new topology approaches, exposing + // them with domain-prefixed values. For example, "example.com/lowest-rtt" + // could be a valid implementation-specific value for this annotation. These + // heuristics will often populate topology hints on EndpointSlices, but that + // is not a requirement. + AnnotationTopologyMode = "service.kubernetes.io/topology-mode" ) diff --git a/pkg/controller/endpointslice/reconciler_test.go b/pkg/controller/endpointslice/reconciler_test.go index 18a33c49277..429379955ef 100644 --- a/pkg/controller/endpointslice/reconciler_test.go +++ b/pkg/controller/endpointslice/reconciler_test.go @@ -1843,7 +1843,7 @@ func TestReconcileTopology(t *testing.T) { service := svc.DeepCopy() service.Annotations = map[string]string{ - corev1.AnnotationTopologyAwareHints: tc.hintsAnnotation, + corev1.DeprecatedAnnotationTopologyAwareHints: tc.hintsAnnotation, } r.reconcile(service, tc.pods, tc.existingSlices, time.Now()) diff --git a/pkg/controller/endpointslice/utils.go b/pkg/controller/endpointslice/utils.go index 448ae9fb347..dc8e2a05d16 100644 --- a/pkg/controller/endpointslice/utils.go +++ b/pkg/controller/endpointslice/utils.go @@ -378,12 +378,17 @@ func unchangedSlices(existingSlices, slicesToUpdate, slicesToDelete []*discovery return unchangedSlices } -// hintsEnabled returns true if the provided annotations include a -// v1.AnnotationTopologyAwareHints key with a value set to "Auto" or "auto". +// hintsEnabled returns true if the provided annotations include either +// v1.AnnotationTopologyMode or v1.DeprecatedAnnotationTopologyAwareHints key +// with a value set to "Auto" or "auto". When both are set, +// v1.DeprecatedAnnotationTopologyAwareHints has precedence. func hintsEnabled(annotations map[string]string) bool { - val, ok := annotations[v1.AnnotationTopologyAwareHints] + val, ok := annotations[v1.DeprecatedAnnotationTopologyAwareHints] if !ok { - return false + val, ok = annotations[v1.AnnotationTopologyMode] + if !ok { + return false + } } return val == "Auto" || val == "auto" } diff --git a/pkg/controller/endpointslice/utils_test.go b/pkg/controller/endpointslice/utils_test.go index e4cdda5b972..265422141d9 100644 --- a/pkg/controller/endpointslice/utils_test.go +++ b/pkg/controller/endpointslice/utils_test.go @@ -1149,24 +1149,72 @@ func Test_hintsEnabled(t *testing.T) { annotations: map[string]string{"topology-hints": "enabled"}, expectEnabled: false, }, { - name: "annotation == enabled", - annotations: map[string]string{v1.AnnotationTopologyAwareHints: "enabled"}, + name: "hints annotation == enabled", + annotations: map[string]string{v1.DeprecatedAnnotationTopologyAwareHints: "enabled"}, expectEnabled: false, }, { - name: "annotation == aUto", - annotations: map[string]string{v1.AnnotationTopologyAwareHints: "aUto"}, + name: "hints annotation == aUto", + annotations: map[string]string{v1.DeprecatedAnnotationTopologyAwareHints: "aUto"}, expectEnabled: false, }, { - name: "annotation == auto", - annotations: map[string]string{v1.AnnotationTopologyAwareHints: "auto"}, + name: "hints annotation == auto", + annotations: map[string]string{v1.DeprecatedAnnotationTopologyAwareHints: "auto"}, expectEnabled: true, }, { - name: "annotation == Auto", - annotations: map[string]string{v1.AnnotationTopologyAwareHints: "Auto"}, + name: "hints annotation == Auto", + annotations: map[string]string{v1.DeprecatedAnnotationTopologyAwareHints: "Auto"}, expectEnabled: true, }, { - name: "annotation == disabled", - annotations: map[string]string{v1.AnnotationTopologyAwareHints: "disabled"}, + name: "hints annotation == disabled", + annotations: map[string]string{v1.DeprecatedAnnotationTopologyAwareHints: "disabled"}, + expectEnabled: false, + }, { + name: "mode annotation == enabled", + annotations: map[string]string{v1.AnnotationTopologyMode: "enabled"}, + expectEnabled: false, + }, { + name: "mode annotation == aUto", + annotations: map[string]string{v1.AnnotationTopologyMode: "aUto"}, + expectEnabled: false, + }, { + name: "mode annotation == auto", + annotations: map[string]string{v1.AnnotationTopologyMode: "auto"}, + expectEnabled: true, + }, { + name: "mode annotation == Auto", + annotations: map[string]string{v1.AnnotationTopologyMode: "Auto"}, + expectEnabled: true, + }, { + name: "mode annotation == disabled", + annotations: map[string]string{v1.AnnotationTopologyMode: "disabled"}, + expectEnabled: false, + }, { + name: "mode annotation == enabled", + annotations: map[string]string{v1.AnnotationTopologyMode: "enabled"}, + expectEnabled: false, + }, { + name: "mode annotation == aUto", + annotations: map[string]string{v1.AnnotationTopologyMode: "aUto"}, + expectEnabled: false, + }, { + name: "mode annotation == auto", + annotations: map[string]string{v1.AnnotationTopologyMode: "auto"}, + expectEnabled: true, + }, { + name: "mode annotation == Auto", + annotations: map[string]string{v1.AnnotationTopologyMode: "Auto"}, + expectEnabled: true, + }, { + name: "mode annotation == disabled", + annotations: map[string]string{v1.AnnotationTopologyMode: "disabled"}, + expectEnabled: false, + }, { + name: "mode annotation == disabled, hints annotation == auto", + annotations: map[string]string{v1.AnnotationTopologyMode: "disabled", v1.DeprecatedAnnotationTopologyAwareHints: "auto"}, + expectEnabled: true, + }, { + name: "mode annotation == auto, hints annotation == disabled", + annotations: map[string]string{v1.AnnotationTopologyMode: "auto", v1.DeprecatedAnnotationTopologyAwareHints: "disabled"}, expectEnabled: false, }} for _, tc := range testCases { diff --git a/pkg/proxy/service.go b/pkg/proxy/service.go index 5832f0ba4c6..ab8e6fa5d14 100644 --- a/pkg/proxy/service.go +++ b/pkg/proxy/service.go @@ -176,7 +176,13 @@ func (sct *ServiceChangeTracker) newBaseServiceInfo(port *v1.ServicePort, servic externalPolicyLocal: externalPolicyLocal, internalPolicyLocal: internalPolicyLocal, internalTrafficPolicy: service.Spec.InternalTrafficPolicy, - hintsAnnotation: service.Annotations[v1.AnnotationTopologyAwareHints], + } + + // v1.DeprecatedAnnotationTopologyAwareHints has precedence over v1.AnnotationTopologyMode. + var ok bool + info.hintsAnnotation, ok = service.Annotations[v1.DeprecatedAnnotationTopologyAwareHints] + if !ok { + info.hintsAnnotation, _ = service.Annotations[v1.AnnotationTopologyMode] } loadBalancerSourceRanges := make([]string, len(service.Spec.LoadBalancerSourceRanges)) diff --git a/pkg/proxy/topology.go b/pkg/proxy/topology.go index b58f67af335..e8248a93a7e 100644 --- a/pkg/proxy/topology.go +++ b/pkg/proxy/topology.go @@ -148,11 +148,9 @@ func canUseTopology(endpoints []Endpoint, svcInfo ServicePort, nodeLabels map[st if !utilfeature.DefaultFeatureGate.Enabled(features.TopologyAwareHints) { return false } + // Any non-empty and non-disabled values for the hints annotation are acceptable. hintsAnnotation := svcInfo.HintsAnnotation() - if hintsAnnotation != "Auto" && hintsAnnotation != "auto" { - if hintsAnnotation != "" && hintsAnnotation != "Disabled" && hintsAnnotation != "disabled" { - klog.InfoS("Skipping topology aware endpoint filtering since Service has unexpected value", "annotationTopologyAwareHints", v1.AnnotationTopologyAwareHints, "hints", hintsAnnotation) - } + if hintsAnnotation == "" || hintsAnnotation == "disabled" || hintsAnnotation == "Disabled" { return false } diff --git a/pkg/proxy/topology_test.go b/pkg/proxy/topology_test.go index 3dcc3a38bac..ad375c28e15 100644 --- a/pkg/proxy/topology_test.go +++ b/pkg/proxy/topology_test.go @@ -105,7 +105,7 @@ func TestCategorizeEndpoints(t *testing.T) { localEndpoints: nil, }, { - name: "hints, hints annotation == aUto (wrong capitalization), hints ignored", + name: "hints, hints annotation == aUto (wrong capitalization), hints no longer ignored", hintsEnabled: true, nodeLabels: map[string]string{v1.LabelTopologyZone: "zone-a"}, serviceInfo: &BaseServicePortInfo{hintsAnnotation: "aUto"}, @@ -115,7 +115,7 @@ func TestCategorizeEndpoints(t *testing.T) { &BaseEndpointInfo{Endpoint: "10.1.2.5:80", ZoneHints: sets.NewString("zone-c"), Ready: true}, &BaseEndpointInfo{Endpoint: "10.1.2.6:80", ZoneHints: sets.NewString("zone-a"), Ready: true}, }, - clusterEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.4:80", "10.1.2.5:80", "10.1.2.6:80"), + clusterEndpoints: sets.NewString("10.1.2.3:80", "10.1.2.6:80"), localEndpoints: nil, }, { name: "hints, hints annotation empty, hints ignored", diff --git a/pkg/proxy/types.go b/pkg/proxy/types.go index 30e035a6e63..1b6b843a8c1 100644 --- a/pkg/proxy/types.go +++ b/pkg/proxy/types.go @@ -89,7 +89,7 @@ type ServicePort interface { InternalPolicyLocal() bool // InternalTrafficPolicy returns service InternalTrafficPolicy InternalTrafficPolicy() *v1.ServiceInternalTrafficPolicy - // HintsAnnotation returns the value of the v1.AnnotationTopologyAwareHints annotation. + // HintsAnnotation returns the value of the v1.DeprecatedAnnotationTopologyAwareHints annotation. HintsAnnotation() string // ExternallyAccessible returns true if the service port is reachable via something // other than ClusterIP (NodePort/ExternalIP/LoadBalancer) diff --git a/staging/src/k8s.io/api/core/v1/annotation_key_constants.go b/staging/src/k8s.io/api/core/v1/annotation_key_constants.go index 8f99648587c..61f86f850a3 100644 --- a/staging/src/k8s.io/api/core/v1/annotation_key_constants.go +++ b/staging/src/k8s.io/api/core/v1/annotation_key_constants.go @@ -144,8 +144,19 @@ const ( // This annotation is beta-level and is only honored when PodDeletionCost feature is enabled. PodDeletionCost = "controller.kubernetes.io/pod-deletion-cost" - // AnnotationTopologyAwareHints can be used to enable or disable Topology - // Aware Hints for a Service. This may be set to "Auto" or "Disabled". Any - // other value is treated as "Disabled". - AnnotationTopologyAwareHints = "service.kubernetes.io/topology-aware-hints" + // DeprecatedAnnotationTopologyAwareHints can be used to enable or disable + // Topology Aware Hints for a Service. This may be set to "Auto" or + // "Disabled". Any other value is treated as "Disabled". This annotation has + // been deprecated in favor of the "service.kubernetes.io/topology-mode" + // annotation. + DeprecatedAnnotationTopologyAwareHints = "service.kubernetes.io/topology-aware-hints" + + // AnnotationTopologyMode can be used to enable or disable Topology Aware + // Routing for a Service. Well known values are "Auto" and "Disabled". + // Implementations may choose to develop new topology approaches, exposing + // them with domain-prefixed values. For example, "example.com/lowest-rtt" + // could be a valid implementation-specific value for this annotation. These + // heuristics will often populate topology hints on EndpointSlices, but that + // is not a requirement. + AnnotationTopologyMode = "service.kubernetes.io/topology-mode" ) diff --git a/test/e2e/network/topology_hints.go b/test/e2e/network/topology_hints.go index 0424aa46380..1eb3d6e1872 100644 --- a/test/e2e/network/topology_hints.go +++ b/test/e2e/network/topology_hints.go @@ -65,7 +65,7 @@ var _ = common.SIGDescribe("[Feature:Topology Hints]", func() { ObjectMeta: metav1.ObjectMeta{ Name: "topology-hints", Annotations: map[string]string{ - v1.AnnotationTopologyAwareHints: "Auto", + v1.AnnotationTopologyMode: "Auto", }, }, Spec: v1.ServiceSpec{