Merge pull request kubernetes#82289 from robscott/endpointslice-fixes

Fixing bugs related to Endpoint Slices

Author: k8s-ci-robot

cmd/kube-controller-manager/app/BUILD

@@ -112,6 +112,7 @@ go_library(
         "//pkg/volume/util:go_default_library",
         "//pkg/volume/vsphere_volume:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
+        "//staging/src/k8s.io/api/discovery/v1alpha1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",

cmd/kube-controller-manager/app/discovery.go

@@ -23,12 +23,14 @@ package app
 import (
 	"net/http"
 
-	"k8s.io/apimachinery/pkg/runtime/schema"
+	discoveryv1alpha1 "k8s.io/api/discovery/v1alpha1"
+	"k8s.io/klog"
 	endpointslicecontroller "k8s.io/kubernetes/pkg/controller/endpointslice"
 )
 
 func startEndpointSliceController(ctx ControllerContext) (http.Handler, bool, error) {
-	if !ctx.AvailableResources[schema.GroupVersionResource{Group: "discovery", Version: "v1alpha1", Resource: "endpointslices"}] {
+	if !ctx.AvailableResources[discoveryv1alpha1.SchemeGroupVersion.WithResource("endpointslices")] {
+		klog.Warningf("Not starting endpointslice-controller, discovery.k8s.io/v1alpha1 resources are not available")
 		return nil, false, nil
 	}
 

pkg/apis/discovery/validation/validation.go

@@ -36,10 +36,14 @@ var (
 	maxEndpoints           = 1000
 )
 
+// ValidateEndpointSliceName can be used to check whether the given endpoint
+// slice name is valid. Prefix indicates this name will be used as part of
+// generation, in which case trailing dashes are allowed.
+var ValidateEndpointSliceName = apimachineryvalidation.NameIsDNSSubdomain
+
 // ValidateEndpointSlice validates an EndpointSlice.
 func ValidateEndpointSlice(endpointSlice *discovery.EndpointSlice) field.ErrorList {
-	validateEndpointSliceName := apimachineryvalidation.NameIsDNSSubdomain
-	allErrs := apivalidation.ValidateObjectMeta(&endpointSlice.ObjectMeta, true, validateEndpointSliceName, field.NewPath("metadata"))
+	allErrs := apivalidation.ValidateObjectMeta(&endpointSlice.ObjectMeta, true, ValidateEndpointSliceName, field.NewPath("metadata"))
 
 	addrType := discovery.AddressType("")
 	if endpointSlice.AddressType == nil {

pkg/controller/.import-restrictions

@@ -188,6 +188,8 @@
         "k8s.io/kubernetes/pkg/apis/core/v1",
         "k8s.io/kubernetes/pkg/apis/core/v1/helper",
         "k8s.io/kubernetes/pkg/apis/core/validation",
+        "k8s.io/kubernetes/pkg/apis/discovery",
+        "k8s.io/kubernetes/pkg/apis/discovery/validation",
         "k8s.io/kubernetes/pkg/cloudprovider",
         "k8s.io/kubernetes/pkg/cloudprovider/providers/gce",
         "k8s.io/kubernetes/pkg/controller",

pkg/controller/endpointslice/BUILD

@@ -13,6 +13,7 @@ go_library(
     deps = [
         "//pkg/api/v1/pod:go_default_library",
         "//pkg/apis/core:go_default_library",
+        "//pkg/apis/discovery/validation:go_default_library",
         "//pkg/controller:go_default_library",
         "//pkg/controller/util/endpoint:go_default_library",
         "//pkg/util/hash:go_default_library",

pkg/controller/endpointslice/endpointslice_controller.go

@@ -21,6 +21,7 @@ import (
 	"time"
 
 	v1 "k8s.io/api/core/v1"
+	discovery "k8s.io/api/discovery/v1alpha1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/labels"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
@@ -42,10 +43,6 @@ import (
 )
 
 const (
-	// serviceNameLabel is used to indicate the name of a Kubernetes service
-	// associated with an EndpointSlice.
-	serviceNameLabel = "kubernetes.io/service-name"
-
 	// maxRetries is the number of times a service will be retried before it is
 	// dropped out of the queue. Any sync error, such as a failure to create or
 	// update an EndpointSlice could trigger a retry. With the current
@@ -276,7 +273,7 @@ func (c *Controller) syncService(key string) error {
 		return err
 	}
 
-	esLabelSelector := labels.Set(map[string]string{serviceNameLabel: service.Name}).AsSelectorPreValidated()
+	esLabelSelector := labels.Set(map[string]string{discovery.LabelServiceName: service.Name}).AsSelectorPreValidated()
 	endpointSlices, err := c.endpointSliceLister.EndpointSlices(service.Namespace).List(esLabelSelector)
 
 	if err != nil {

pkg/controller/endpointslice/endpointslice_controller_test.go

@@ -108,7 +108,7 @@ func TestSyncServiceWithSelector(t *testing.T) {
 	assert.Len(t, sliceList.Items, 1, "Expected 1 endpoint slices")
 	slice := sliceList.Items[0]
 	assert.Regexp(t, "^"+serviceName, slice.Name)
-	assert.Equal(t, serviceName, slice.Labels[serviceNameLabel])
+	assert.Equal(t, serviceName, slice.Labels[discovery.LabelServiceName])
 	assert.EqualValues(t, []discovery.EndpointPort{}, slice.Ports)
 	assert.EqualValues(t, []discovery.Endpoint{}, slice.Endpoints)
 	assert.NotEmpty(t, slice.Annotations["endpoints.kubernetes.io/last-change-trigger-time"])
@@ -189,11 +189,11 @@ func TestSyncServiceEndpointSliceSelection(t *testing.T) {
 
 	// 3 slices, 2 with matching labels for our service
 	endpointSlices := []*discovery.EndpointSlice{{
-		ObjectMeta: metav1.ObjectMeta{Name: "matching-1", Namespace: ns, Labels: map[string]string{serviceNameLabel: serviceName}},
+		ObjectMeta: metav1.ObjectMeta{Name: "matching-1", Namespace: ns, Labels: map[string]string{discovery.LabelServiceName: serviceName}},
 	}, {
-		ObjectMeta: metav1.ObjectMeta{Name: "matching-2", Namespace: ns, Labels: map[string]string{serviceNameLabel: serviceName}},
+		ObjectMeta: metav1.ObjectMeta{Name: "matching-2", Namespace: ns, Labels: map[string]string{discovery.LabelServiceName: serviceName}},
 	}, {
-		ObjectMeta: metav1.ObjectMeta{Name: "not-matching-1", Namespace: ns, Labels: map[string]string{serviceNameLabel: "something-else"}},
+		ObjectMeta: metav1.ObjectMeta{Name: "not-matching-1", Namespace: ns, Labels: map[string]string{discovery.LabelServiceName: "something-else"}},
 	}}
 
 	// need to add them to both store and fake clientset

pkg/controller/endpointslice/reconciler_test.go

@@ -23,7 +23,6 @@ import (
 	"github.com/stretchr/testify/assert"
 
 	corev1 "k8s.io/api/core/v1"
-	v1 "k8s.io/api/core/v1"
 	discovery "k8s.io/api/discovery/v1alpha1"
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -52,7 +51,7 @@ func TestReconcileEmpty(t *testing.T) {
 	assert.Len(t, slices, 1, "Expected 1 endpoint slices")
 
 	assert.Regexp(t, "^"+svc.Name, slices[0].Name)
-	assert.Equal(t, svc.Name, slices[0].Labels[serviceNameLabel])
+	assert.Equal(t, svc.Name, slices[0].Labels[discovery.LabelServiceName])
 	assert.EqualValues(t, []discovery.EndpointPort{}, slices[0].Ports)
 	assert.EqualValues(t, []discovery.Endpoint{}, slices[0].Endpoints)
 }
@@ -83,7 +82,7 @@ func TestReconcile1Pod(t *testing.T) {
 	slices := fetchEndpointSlices(t, client, namespace)
 	assert.Len(t, slices, 1, "Expected 1 endpoint slices")
 	assert.Regexp(t, "^"+svc.Name, slices[0].Name)
-	assert.Equal(t, svc.Name, slices[0].Labels[serviceNameLabel])
+	assert.Equal(t, svc.Name, slices[0].Labels[discovery.LabelServiceName])
 	assert.Equal(t, slices[0].Annotations, map[string]string{
 		"endpoints.kubernetes.io/last-change-trigger-time": triggerTime.Format(time.RFC3339Nano),
 	})
@@ -125,7 +124,7 @@ func TestReconcile1EndpointSlice(t *testing.T) {
 	assert.Len(t, slices, 1, "Expected 1 endpoint slices")
 
 	assert.Regexp(t, "^"+svc.Name, slices[0].Name)
-	assert.Equal(t, svc.Name, slices[0].Labels[serviceNameLabel])
+	assert.Equal(t, svc.Name, slices[0].Labels[discovery.LabelServiceName])
 	assert.EqualValues(t, []discovery.EndpointPort{}, slices[0].Ports)
 	assert.EqualValues(t, []discovery.Endpoint{}, slices[0].Endpoints)
 }
@@ -362,9 +361,9 @@ func TestReconcileEndpointSlicesUpdatePacking(t *testing.T) {
 	// ensure that endpoints in each slice will be marked for update.
 	for i, pod := range pods {
 		if i%10 == 0 {
-			pod.Status.Conditions = []v1.PodCondition{{
-				Type:   v1.PodReady,
-				Status: v1.ConditionFalse,
+			pod.Status.Conditions = []corev1.PodCondition{{
+				Type:   corev1.PodReady,
+				Status: corev1.ConditionFalse,
 			}}
 		}
 	}
@@ -397,10 +396,10 @@ func TestReconcileEndpointSlicesNamedPorts(t *testing.T) {
 
 	svc := corev1.Service{
 		ObjectMeta: metav1.ObjectMeta{Name: "named-port-example", Namespace: namespace},
-		Spec: v1.ServiceSpec{
-			Ports: []v1.ServicePort{{
+		Spec: corev1.ServiceSpec{
+			Ports: []corev1.ServicePort{{
 				TargetPort: portNameIntStr,
-				Protocol:   v1.ProtocolTCP,
+				Protocol:   corev1.ProtocolTCP,
 			}},
 			Selector: map[string]string{"foo": "bar"},
 		},
@@ -412,10 +411,10 @@ func TestReconcileEndpointSlicesNamedPorts(t *testing.T) {
 		ready := !(i%3 == 0)
 		portOffset := i % 5
 		pod := newPod(i, namespace, ready, 1)
-		pod.Spec.Containers[0].Ports = []v1.ContainerPort{{
+		pod.Spec.Containers[0].Ports = []corev1.ContainerPort{{
 			Name:          portNameIntStr.StrVal,
 			ContainerPort: int32(8080 + portOffset),
-			Protocol:      v1.ProtocolTCP,
+			Protocol:      corev1.ProtocolTCP,
 		}}
 		pods = append(pods, pod)
 	}
@@ -433,7 +432,7 @@ func TestReconcileEndpointSlicesNamedPorts(t *testing.T) {
 	expectUnorderedSlicesWithLengths(t, fetchedSlices, []int{60, 60, 60, 60, 60})
 
 	// generate data structures for expected slice ports and address types
-	protoTCP := v1.ProtocolTCP
+	protoTCP := corev1.ProtocolTCP
 	ipAddressType := discovery.AddressTypeIP
 	expectedSlices := []discovery.EndpointSlice{}
 	for i := range fetchedSlices {

pkg/controller/endpointslice/utils.go

@@ -32,6 +32,7 @@ import (
 	"k8s.io/klog"
 	podutil "k8s.io/kubernetes/pkg/api/v1/pod"
 	api "k8s.io/kubernetes/pkg/apis/core"
+	"k8s.io/kubernetes/pkg/apis/discovery/validation"
 	"k8s.io/kubernetes/pkg/util/hash"
 )
 
@@ -158,8 +159,8 @@ func newEndpointSlice(service *corev1.Service, endpointMeta *endpointMeta) *disc
 	ownerRef := metav1.NewControllerRef(service, gvk)
 	return &discovery.EndpointSlice{
 		ObjectMeta: metav1.ObjectMeta{
-			Labels:          map[string]string{serviceNameLabel: service.Name},
-			GenerateName:    fmt.Sprintf("%s.", service.Name),
+			Labels:          map[string]string{discovery.LabelServiceName: service.Name},
+			GenerateName:    getEndpointSlicePrefix(service.Name),
 			OwnerReferences: []metav1.OwnerReference{*ownerRef},
 			Namespace:       service.Namespace,
 		},
@@ -169,6 +170,16 @@ func newEndpointSlice(service *corev1.Service, endpointMeta *endpointMeta) *disc
 	}
 }
 
+// getEndpointSlicePrefix returns a suitable prefix for an EndpointSlice name.
+func getEndpointSlicePrefix(serviceName string) string {
+	// use the dash (if the name isn't too long) to make the pod name a bit prettier
+	prefix := fmt.Sprintf("%s-", serviceName)
+	if len(validation.ValidateEndpointSliceName(prefix, true)) != 0 {
+		prefix = serviceName
+	}
+	return prefix
+}
+
 // boolPtrChanged returns true if a set of bool pointers have different values.
 func boolPtrChanged(ptr1, ptr2 *bool) bool {
 	if (ptr1 == nil) != (ptr2 == nil) {

pkg/controller/endpointslice/utils_test.go

@@ -22,7 +22,6 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
-	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/api/core/v1"
 	discovery "k8s.io/api/discovery/v1alpha1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -58,8 +57,8 @@ func TestNewEndpointSlice(t *testing.T) {
 
 	expectedSlice := discovery.EndpointSlice{
 		ObjectMeta: metav1.ObjectMeta{
-			Labels:          map[string]string{serviceNameLabel: service.Name},
-			GenerateName:    fmt.Sprintf("%s.", service.Name),
+			Labels:          map[string]string{discovery.LabelServiceName: service.Name},
+			GenerateName:    fmt.Sprintf("%s-", service.Name),
 			OwnerReferences: []metav1.OwnerReference{*ownerRef},
 			Namespace:       service.Namespace,
 		},
@@ -81,7 +80,7 @@ func TestPodToEndpoint(t *testing.T) {
 
 	multiIPPod.Status.PodIPs = []v1.PodIP{{IP: "1.2.3.4"}, {IP: "1234::5678:0000:0000:9abc:def0"}}
 
-	node1 := &corev1.Node{
+	node1 := &v1.Node{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: readyPod.Spec.NodeName,
 			Labels: map[string]string{
@@ -288,14 +287,14 @@ func newClientset() *fake.Clientset {
 	return client
 }
 
-func newServiceAndendpointMeta(name, namespace string) (corev1.Service, endpointMeta) {
+func newServiceAndendpointMeta(name, namespace string) (v1.Service, endpointMeta) {
 	portNum := int32(80)
 	portNameIntStr := intstr.IntOrString{
 		Type:   intstr.Int,
 		IntVal: portNum,
 	}
 
-	svc := corev1.Service{
+	svc := v1.Service{
 		ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace},
 		Spec: v1.ServiceSpec{
 			Ports: []v1.ServicePort{{
@@ -317,7 +316,7 @@ func newServiceAndendpointMeta(name, namespace string) (corev1.Service, endpoint
 	return svc, endpointMeta
 }
 
-func newEmptyEndpointSlice(n int, namespace string, endpointMeta endpointMeta, svc corev1.Service) *discovery.EndpointSlice {
+func newEmptyEndpointSlice(n int, namespace string, endpointMeta endpointMeta, svc v1.Service) *discovery.EndpointSlice {
 	return &discovery.EndpointSlice{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      fmt.Sprintf("%s.%d", svc.Name, n),