diff --git a/staging/src/k8s.io/endpointslice/metrics/cache.go b/staging/src/k8s.io/endpointslice/metrics/cache.go index 6f102bb498e..30fd4de4436 100644 --- a/staging/src/k8s.io/endpointslice/metrics/cache.go +++ b/staging/src/k8s.io/endpointslice/metrics/cache.go @@ -20,7 +20,6 @@ import ( "math" "sync" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" endpointsliceutil "k8s.io/endpointslice/util" ) @@ -60,12 +59,6 @@ type Cache struct { servicesByTrafficDistribution map[string]map[types.NamespacedName]bool } -const ( - // Label value for cases when service.spec.trafficDistribution is set to an - // unknown value. - trafficDistributionImplementationSpecific = "ImplementationSpecific" -) - // ServicePortCache tracks values for total numbers of desired endpoints as well // as the efficiency of EndpointSlice endpoints distribution for each unique // Service Port combination. @@ -151,13 +144,6 @@ func (c *Cache) UpdateTrafficDistributionForService(serviceNN types.NamespacedNa } trafficDistribution := *trafficDistributionPtr - // If we don't explicitly recognize a value for trafficDistribution, it should - // be treated as an implementation specific value. All such implementation - // specific values should use the label value "ImplementationSpecific" to not - // explode the metric labels cardinality. - if trafficDistribution != corev1.ServiceTrafficDistributionPreferClose { - trafficDistribution = trafficDistributionImplementationSpecific - } serviceSet, ok := c.servicesByTrafficDistribution[trafficDistribution] if !ok { serviceSet = make(map[types.NamespacedName]bool) diff --git a/staging/src/k8s.io/endpointslice/metrics/cache_test.go b/staging/src/k8s.io/endpointslice/metrics/cache_test.go index 002917b2aae..9db2e2372a1 100644 --- a/staging/src/k8s.io/endpointslice/metrics/cache_test.go +++ b/staging/src/k8s.io/endpointslice/metrics/cache_test.go @@ -124,53 +124,50 @@ func TestCache_ServicesByTrafficDistribution(t *testing.T) { corev1.ServiceTrafficDistributionPreferClose: {service1: true, service2: true}, // Delta }, desc) - desc = "service3 starts using trafficDistribution=InvalidValue" - cache.UpdateTrafficDistributionForService(service3, ptr.To("InvalidValue")) + desc = "service3 starts using trafficDistribution=FutureValue" + cache.UpdateTrafficDistributionForService(service3, ptr.To("FutureValue")) mustHaveServicesByTrafficDistribution(map[string]map[types.NamespacedName]bool{ corev1.ServiceTrafficDistributionPreferClose: {service1: true, service2: true}, - trafficDistributionImplementationSpecific: {service3: true}, // Delta + "FutureValue": {service3: true}, // Delta }, desc) desc = "service4 starts using trafficDistribution=nil" cache.UpdateTrafficDistributionForService(service4, nil) mustHaveServicesByTrafficDistribution(map[string]map[types.NamespacedName]bool{ // No delta corev1.ServiceTrafficDistributionPreferClose: {service1: true, service2: true}, - trafficDistributionImplementationSpecific: {service3: true}, + "FutureValue": {service3: true}, }, desc) - desc = "service2 transitions trafficDistribution: PreferClose -> InvalidValue" - cache.UpdateTrafficDistributionForService(service2, ptr.To("InvalidValue")) + desc = "service2 transitions trafficDistribution: PreferClose -> AnotherFutureValue" + cache.UpdateTrafficDistributionForService(service2, ptr.To("AnotherFutureValue")) mustHaveServicesByTrafficDistribution(map[string]map[types.NamespacedName]bool{ - corev1.ServiceTrafficDistributionPreferClose: {service1: true}, // Delta - trafficDistributionImplementationSpecific: {service3: true, service2: true}, // Delta + corev1.ServiceTrafficDistributionPreferClose: {service1: true}, // Delta + "FutureValue": {service3: true}, + "AnotherFutureValue": {service2: true}, // Delta }, desc) desc = "service3 gets deleted" cache.DeleteService(service3) mustHaveServicesByTrafficDistribution(map[string]map[types.NamespacedName]bool{ corev1.ServiceTrafficDistributionPreferClose: {service1: true}, - trafficDistributionImplementationSpecific: {service2: true}, // Delta + "FutureValue": {}, // Delta + "AnotherFutureValue": {service2: true}, }, desc) - desc = "service1 transitions trafficDistribution: PreferClose -> empty" - cache.UpdateTrafficDistributionForService(service1, ptr.To("")) - mustHaveServicesByTrafficDistribution(map[string]map[types.NamespacedName]bool{ - corev1.ServiceTrafficDistributionPreferClose: {}, // Delta - trafficDistributionImplementationSpecific: {service1: true, service2: true}, // Delta - }, desc) - - desc = "service1 transitions trafficDistribution: InvalidValue -> nil" + desc = "service1 transitions trafficDistribution: PreferClose -> nil" cache.UpdateTrafficDistributionForService(service1, nil) mustHaveServicesByTrafficDistribution(map[string]map[types.NamespacedName]bool{ - corev1.ServiceTrafficDistributionPreferClose: {}, - trafficDistributionImplementationSpecific: {service2: true}, // Delta + corev1.ServiceTrafficDistributionPreferClose: {}, // Delta + "FutureValue": {}, + "AnotherFutureValue": {service2: true}, }, desc) - desc = "service2 transitions trafficDistribution: InvalidValue -> nil" + desc = "service2 transitions trafficDistribution: AnotherFutureValue -> nil" cache.UpdateTrafficDistributionForService(service2, nil) mustHaveServicesByTrafficDistribution(map[string]map[types.NamespacedName]bool{ corev1.ServiceTrafficDistributionPreferClose: {}, - trafficDistributionImplementationSpecific: {}, // Delta + "FutureValue": {}, + "AnotherFutureValue": {}, // Delta }, desc) } diff --git a/staging/src/k8s.io/endpointslice/metrics/metrics.go b/staging/src/k8s.io/endpointslice/metrics/metrics.go index 236a83c3218..b7228a1810a 100644 --- a/staging/src/k8s.io/endpointslice/metrics/metrics.go +++ b/staging/src/k8s.io/endpointslice/metrics/metrics.go @@ -129,7 +129,7 @@ var ( Help: "Number of Services using some specific trafficDistribution", StabilityLevel: metrics.ALPHA, }, - []string{"traffic_distribution"}, // One of ["PreferClose", "ImplementationSpecific"] + []string{"traffic_distribution"}, // A trafficDistribution value ) ) diff --git a/staging/src/k8s.io/endpointslice/reconciler_test.go b/staging/src/k8s.io/endpointslice/reconciler_test.go index 243bc807c20..f46fcfdabb5 100644 --- a/staging/src/k8s.io/endpointslice/reconciler_test.go +++ b/staging/src/k8s.io/endpointslice/reconciler_test.go @@ -2017,7 +2017,7 @@ func TestReconcile_TrafficDistribution(t *testing.T) { desc string trafficDistributionFeatureGateEnabled bool - trafficDistribution string + trafficDistribution *string topologyAnnotation string // Defines how many hints belong to a particular zone. @@ -2031,7 +2031,7 @@ func TestReconcile_TrafficDistribution(t *testing.T) { name: "trafficDistribution=PreferClose, topologyAnnotation=Disabled", desc: "When trafficDistribution is enabled and topologyAnnotation is disabled, hints should be distributed as per the trafficDistribution field", trafficDistributionFeatureGateEnabled: true, - trafficDistribution: corev1.ServiceTrafficDistributionPreferClose, + trafficDistribution: ptr.To(corev1.ServiceTrafficDistributionPreferClose), topologyAnnotation: "Disabled", wantHintsDistributionByZone: map[string]int{ "zone-a": 1, // {pod-0} @@ -2059,7 +2059,7 @@ func TestReconcile_TrafficDistribution(t *testing.T) { name: "feature gate disabled; trafficDistribution=PreferClose, topologyAnnotation=Disabled", desc: "When feature gate is disabled, trafficDistribution should be ignored", trafficDistributionFeatureGateEnabled: false, - trafficDistribution: corev1.ServiceTrafficDistributionPreferClose, + trafficDistribution: ptr.To(corev1.ServiceTrafficDistributionPreferClose), topologyAnnotation: "Disabled", wantHintsDistributionByZone: map[string]int{"": 6}, // Equivalent to no hints. wantMetrics: expectedMetrics{ @@ -2080,7 +2080,7 @@ func TestReconcile_TrafficDistribution(t *testing.T) { name: "trafficDistribution=PreferClose, topologyAnnotation=Auto", desc: "When trafficDistribution and topologyAnnotation are both enabled, precedence should be given to topologyAnnotation", trafficDistributionFeatureGateEnabled: true, - trafficDistribution: corev1.ServiceTrafficDistributionPreferClose, + trafficDistribution: ptr.To(corev1.ServiceTrafficDistributionPreferClose), topologyAnnotation: "Auto", wantHintsDistributionByZone: map[string]int{ "zone-a": 2, // {pod-0, pod-3} (pod-3 is just an example, it could have also been either of the other two) @@ -2103,10 +2103,10 @@ func TestReconcile_TrafficDistribution(t *testing.T) { }, }, { - name: "trafficDistribution=, topologyAnnotation=", - desc: "When trafficDistribution and topologyAnnotation are both disabled, no hints should be added, but the servicesCountByTrafficDistribution metric should reflect this", + name: "trafficDistribution=nil, topologyAnnotation=", + desc: "When trafficDistribution and topologyAnnotation are both disabled, no hints should be added", trafficDistributionFeatureGateEnabled: true, - trafficDistribution: "", + trafficDistribution: nil, topologyAnnotation: "", wantHintsDistributionByZone: map[string]int{"": 6}, // Equivalent to no hints. wantMetrics: expectedMetrics{ @@ -2121,9 +2121,6 @@ func TestReconcile_TrafficDistribution(t *testing.T) { slicesChangedPerSync: 1, // 1 means both topologyAnnotation and trafficDistribution were not used. slicesChangedPerSyncTopology: 0, // 0 means topologyAnnotation was not used. slicesChangedPerSyncTrafficDist: 0, // 0 means trafficDistribution was not used. - servicesCountByTrafficDistribution: map[string]int{ - "ImplementationSpecific": 1, - }, }, }, } @@ -2142,7 +2139,7 @@ func TestReconcile_TrafficDistribution(t *testing.T) { r.topologyCache.SetNodes(logger, nodes) service := svc.DeepCopy() - service.Spec.TrafficDistribution = &tc.trafficDistribution + service.Spec.TrafficDistribution = tc.trafficDistribution service.Annotations = map[string]string{ corev1.DeprecatedAnnotationTopologyAwareHints: tc.topologyAnnotation, }