Merge pull request kubernetes#130539 from danwinship/endpointslice-cleanups

EndpointSlice cleanups

Author: k8s-ci-robot

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)

staging/src/k8s.io/endpointslice/metrics/cache_test.go

@@ -25,17 +25,14 @@ import (
 	discovery "k8s.io/api/discovery/v1"
 	"k8s.io/apimachinery/pkg/types"
 	endpointsliceutil "k8s.io/endpointslice/util"
-	"k8s.io/utils/pointer"
+	"k8s.io/utils/ptr"
 )
 
 func TestNumEndpointsAndSlices(t *testing.T) {
 	c := NewCache(int32(100))
 
-	p80 := int32(80)
-	p443 := int32(443)
-
-	pmKey80443 := endpointsliceutil.NewPortMapKey([]discovery.EndpointPort{{Port: &p80}, {Port: &p443}})
-	pmKey80 := endpointsliceutil.NewPortMapKey([]discovery.EndpointPort{{Port: &p80}})
+	pmKey80443 := endpointsliceutil.NewPortMapKey([]discovery.EndpointPort{{Port: ptr.To[int32](80)}, {Port: ptr.To[int32](443)}})
+	pmKey80 := endpointsliceutil.NewPortMapKey([]discovery.EndpointPort{{Port: ptr.To[int32](80)}})
 
 	spCacheEfficient := NewServicePortCache()
 	spCacheEfficient.Set(pmKey80, EfficiencyInfo{Endpoints: 45, Slices: 1})
@@ -64,11 +61,8 @@ func TestNumEndpointsAndSlices(t *testing.T) {
 func TestPlaceHolderSlice(t *testing.T) {
 	c := NewCache(int32(100))
 
-	p80 := int32(80)
-	p443 := int32(443)
-
-	pmKey80443 := endpointsliceutil.NewPortMapKey([]discovery.EndpointPort{{Port: &p80}, {Port: &p443}})
-	pmKey80 := endpointsliceutil.NewPortMapKey([]discovery.EndpointPort{{Port: &p80}})
+	pmKey80443 := endpointsliceutil.NewPortMapKey([]discovery.EndpointPort{{Port: ptr.To[int32](80)}, {Port: ptr.To[int32](443)}})
+	pmKey80 := endpointsliceutil.NewPortMapKey([]discovery.EndpointPort{{Port: ptr.To[int32](80)}})
 
 	sp := NewServicePortCache()
 	sp.Set(pmKey80, EfficiencyInfo{Endpoints: 0, Slices: 1})
@@ -113,79 +107,76 @@ func TestCache_ServicesByTrafficDistribution(t *testing.T) {
 	// Mutate and make assertions
 
 	desc := "service1 starts using trafficDistribution=PreferClose"
-	cache.UpdateTrafficDistributionForService(service1, ptrTo(corev1.ServiceTrafficDistributionPreferClose))
+	cache.UpdateTrafficDistributionForService(service1, ptr.To(corev1.ServiceTrafficDistributionPreferClose))
 	mustHaveServicesByTrafficDistribution(map[string]map[types.NamespacedName]bool{
 		corev1.ServiceTrafficDistributionPreferClose: {service1: true},
 	}, desc)
 
 	desc = "service1 starts using trafficDistribution=PreferClose, retries of similar mutation should be idempotent"
-	cache.UpdateTrafficDistributionForService(service1, ptrTo(corev1.ServiceTrafficDistributionPreferClose))
+	cache.UpdateTrafficDistributionForService(service1, ptr.To(corev1.ServiceTrafficDistributionPreferClose))
 	mustHaveServicesByTrafficDistribution(map[string]map[types.NamespacedName]bool{ // No delta
 		corev1.ServiceTrafficDistributionPreferClose: {service1: true},
 	}, desc)
 
 	desc = "service2 starts using trafficDistribution=PreferClose"
-	cache.UpdateTrafficDistributionForService(service2, ptrTo(corev1.ServiceTrafficDistributionPreferClose))
+	cache.UpdateTrafficDistributionForService(service2, ptr.To(corev1.ServiceTrafficDistributionPreferClose))
 	mustHaveServicesByTrafficDistribution(map[string]map[types.NamespacedName]bool{
 		corev1.ServiceTrafficDistributionPreferClose: {service1: true, service2: true}, // Delta
 	}, desc)
 
-	desc = "service3 starts using trafficDistribution=InvalidValue"
-	cache.UpdateTrafficDistributionForService(service3, ptrTo("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, ptrTo("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
-	}, desc)
-
-	desc = "service1 transitions trafficDistribution: PreferClose -> empty"
-	cache.UpdateTrafficDistributionForService(service1, ptrTo(""))
-	mustHaveServicesByTrafficDistribution(map[string]map[types.NamespacedName]bool{
-		corev1.ServiceTrafficDistributionPreferClose: {},                               // Delta
-		trafficDistributionImplementationSpecific:    {service1: true, service2: true}, // Delta
+		"FutureValue":        {}, // Delta
+		"AnotherFutureValue": {service2: true},
 	}, 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)
 
 }
 
 func benchmarkUpdateServicePortCache(b *testing.B, num int) {
 	c := NewCache(int32(100))
 	ns := "benchmark"
-	httpKey := endpointsliceutil.NewPortMapKey([]discovery.EndpointPort{{Port: pointer.Int32(80)}})
-	httpsKey := endpointsliceutil.NewPortMapKey([]discovery.EndpointPort{{Port: pointer.Int32(443)}})
+	httpKey := endpointsliceutil.NewPortMapKey([]discovery.EndpointPort{{Port: ptr.To[int32](80)}})
+	httpsKey := endpointsliceutil.NewPortMapKey([]discovery.EndpointPort{{Port: ptr.To[int32](443)}})
 	spCache := &ServicePortCache{items: map[endpointsliceutil.PortMapKey]EfficiencyInfo{
 		httpKey: {
 			Endpoints: 182,
@@ -224,7 +215,3 @@ func BenchmarkUpdateServicePortCache10000(b *testing.B) {
 func BenchmarkUpdateServicePortCache100000(b *testing.B) {
 	benchmarkUpdateServicePortCache(b, 100000)
 }
-
-func ptrTo[T any](obj T) *T {
-	return &obj
-}

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
 	)
 )