From 6adf60fdf4fd0428cc7f101fbbb608cd02d99cf5 Mon Sep 17 00:00:00 2001 From: Viacheslav Panasovets Date: Wed, 18 Jan 2023 12:12:34 +0100 Subject: [PATCH] Do not create endpoints if service of type ExternalName (#114814) --- .../endpoint/endpoints_controller.go | 6 + .../endpoint/endpoints_controller_test.go | 69 +++++++++++ .../endpointslice/endpointslice_controller.go | 6 + .../endpointslice_controller_test.go | 72 +++++++++++ test/integration/endpoints/endpoints_test.go | 116 +++++++++++++++++- 5 files changed, 268 insertions(+), 1 deletion(-) diff --git a/pkg/controller/endpoint/endpoints_controller.go b/pkg/controller/endpoint/endpoints_controller.go index 1d1d8f20a9a..cde9028c5c1 100644 --- a/pkg/controller/endpoint/endpoints_controller.go +++ b/pkg/controller/endpoint/endpoints_controller.go @@ -378,6 +378,12 @@ func (e *Controller) syncService(ctx context.Context, key string) error { return nil } + if service.Spec.Type == v1.ServiceTypeExternalName { + // services with Type ExternalName receive no endpoints from this controller; + // Ref: https://issues.k8s.io/105986 + return nil + } + if service.Spec.Selector == nil { // services without a selector receive no endpoints from this controller; // these services will receive the endpoints that are created out-of-band via the REST API. diff --git a/pkg/controller/endpoint/endpoints_controller_test.go b/pkg/controller/endpoint/endpoints_controller_test.go index f10698aa671..1bfe983b6bc 100644 --- a/pkg/controller/endpoint/endpoints_controller_test.go +++ b/pkg/controller/endpoint/endpoints_controller_test.go @@ -479,6 +479,75 @@ func TestSyncEndpointsHeadlessServiceLabel(t *testing.T) { endpointsHandler.ValidateRequestCount(t, 0) } +func TestSyncServiceExternalNameType(t *testing.T) { + serviceName := "testing-1" + namespace := metav1.NamespaceDefault + + testCases := []struct { + desc string + service *v1.Service + }{ + { + desc: "External name with selector and ports should not receive endpoints", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: serviceName, Namespace: namespace}, + Spec: v1.ServiceSpec{ + Selector: map[string]string{"foo": "bar"}, + Ports: []v1.ServicePort{{Port: 80}}, + Type: v1.ServiceTypeExternalName, + }, + }, + }, + { + desc: "External name with ports should not receive endpoints", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: serviceName, Namespace: namespace}, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{{Port: 80}}, + Type: v1.ServiceTypeExternalName, + }, + }, + }, + { + desc: "External name with selector should not receive endpoints", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: serviceName, Namespace: namespace}, + Spec: v1.ServiceSpec{ + Selector: map[string]string{"foo": "bar"}, + Type: v1.ServiceTypeExternalName, + }, + }, + }, + { + desc: "External name without selector and ports should not receive endpoints", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: serviceName, Namespace: namespace}, + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeExternalName, + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + testServer, endpointsHandler := makeTestServer(t, namespace) + + defer testServer.Close() + endpoints := newController(testServer.URL, 0*time.Second) + err := endpoints.serviceStore.Add(tc.service) + if err != nil { + t.Fatalf("Error adding service to service store: %v", err) + } + err = endpoints.syncService(context.TODO(), namespace+"/"+serviceName) + if err != nil { + t.Fatalf("Error syncing service: %v", err) + } + endpointsHandler.ValidateRequestCount(t, 0) + }) + } +} + func TestSyncEndpointsProtocolUDP(t *testing.T) { ns := "other" testServer, endpointsHandler := makeTestServer(t, ns) diff --git a/pkg/controller/endpointslice/endpointslice_controller.go b/pkg/controller/endpointslice/endpointslice_controller.go index 1e21a6fac49..5910d6cd3bd 100644 --- a/pkg/controller/endpointslice/endpointslice_controller.go +++ b/pkg/controller/endpointslice/endpointslice_controller.go @@ -328,6 +328,12 @@ func (c *Controller) syncService(key string) error { return nil } + if service.Spec.Type == v1.ServiceTypeExternalName { + // services with Type ExternalName receive no endpoints from this controller; + // Ref: https://issues.k8s.io/105986 + return nil + } + if service.Spec.Selector == nil { // services without a selector receive no endpoint slices from this controller; // these services will receive endpoint slices that are created out-of-band via the REST API. diff --git a/pkg/controller/endpointslice/endpointslice_controller_test.go b/pkg/controller/endpointslice/endpointslice_controller_test.go index f024effe524..093f83fa04c 100644 --- a/pkg/controller/endpointslice/endpointslice_controller_test.go +++ b/pkg/controller/endpointslice/endpointslice_controller_test.go @@ -134,6 +134,78 @@ func TestSyncServiceNoSelector(t *testing.T) { assert.Len(t, client.Actions(), 0) } +func TestServiceExternalNameTypeSync(t *testing.T) { + serviceName := "testing-1" + namespace := metav1.NamespaceDefault + + testCases := []struct { + desc string + service *v1.Service + }{ + { + desc: "External name with selector and ports should not receive endpoint slices", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: serviceName, Namespace: namespace}, + Spec: v1.ServiceSpec{ + Selector: map[string]string{"foo": "bar"}, + Ports: []v1.ServicePort{{Port: 80}}, + Type: v1.ServiceTypeExternalName, + }, + }, + }, + { + desc: "External name with ports should not receive endpoint slices", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: serviceName, Namespace: namespace}, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{{Port: 80}}, + Type: v1.ServiceTypeExternalName, + }, + }, + }, + { + desc: "External name with selector should not receive endpoint slices", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: serviceName, Namespace: namespace}, + Spec: v1.ServiceSpec{ + Selector: map[string]string{"foo": "bar"}, + Type: v1.ServiceTypeExternalName, + }, + }, + }, + { + desc: "External name without selector and ports should not receive endpoint slices", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: serviceName, Namespace: namespace}, + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeExternalName, + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + client, esController := newController([]string{"node-1"}, time.Duration(0)) + + pod := newPod(1, namespace, true, 0, false) + err := esController.podStore.Add(pod) + assert.NoError(t, err) + + err = esController.serviceStore.Add(tc.service) + assert.NoError(t, err) + + err = esController.syncService(fmt.Sprintf("%s/%s", namespace, serviceName)) + assert.NoError(t, err) + assert.Len(t, client.Actions(), 0) + + sliceList, err := client.DiscoveryV1().EndpointSlices(namespace).List(context.TODO(), metav1.ListOptions{}) + assert.NoError(t, err) + assert.Len(t, sliceList.Items, 0, "Expected 0 endpoint slices") + }) + } +} + // Ensure SyncService for service with pending deletion results in no action func TestSyncServicePendingDeletion(t *testing.T) { ns := metav1.NamespaceDefault diff --git a/test/integration/endpoints/endpoints_test.go b/test/integration/endpoints/endpoints_test.go index 368456d51f1..e0f445279a3 100644 --- a/test/integration/endpoints/endpoints_test.go +++ b/test/integration/endpoints/endpoints_test.go @@ -158,6 +158,113 @@ func TestEndpointUpdates(t *testing.T) { } +// TestExternalNameToClusterIPTransition tests that Service of type ExternalName +// does not get endpoints, and after transition to ClusterIP, service gets endpoint, +// without headless label +func TestExternalNameToClusterIPTransition(t *testing.T) { + // Disable ServiceAccount admission plugin as we don't have serviceaccount controller running. + server := kubeapiservertesting.StartTestServerOrDie(t, nil, []string{"--disable-admission-plugins=ServiceAccount"}, framework.SharedEtcd()) + defer server.TearDownFn() + + client, err := clientset.NewForConfig(server.ClientConfig) + if err != nil { + t.Fatalf("Error creating clientset: %v", err) + } + + informers := informers.NewSharedInformerFactory(client, 0) + + epController := endpoint.NewEndpointController( + informers.Core().V1().Pods(), + informers.Core().V1().Services(), + informers.Core().V1().Endpoints(), + client, + 0) + + // Start informer and controllers + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + informers.Start(ctx.Done()) + go epController.Run(ctx, 1) + + // Create namespace + ns := framework.CreateNamespaceOrDie(client, "test-endpoints-updates", t) + defer framework.DeleteNamespaceOrDie(client, ns, t) + + // Create a pod with labels + pod := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: ns.Name, + Labels: labelMap(), + }, + Spec: v1.PodSpec{ + NodeName: "fakenode", + Containers: []v1.Container{ + { + Name: "fake-name", + Image: "fakeimage", + }, + }, + }, + } + + createdPod, err := client.CoreV1().Pods(ns.Name).Create(ctx, pod, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("Failed to create pod %s: %v", pod.Name, err) + } + + // Set pod IPs + createdPod.Status = v1.PodStatus{ + Phase: v1.PodRunning, + PodIPs: []v1.PodIP{{IP: "1.1.1.1"}, {IP: "2001:db8::"}}, + } + _, err = client.CoreV1().Pods(ns.Name).UpdateStatus(ctx, createdPod, metav1.UpdateOptions{}) + if err != nil { + t.Fatalf("Failed to update status of pod %s: %v", pod.Name, err) + } + + // Create an ExternalName service associated to the pod + svc := newExternalNameService(ns.Name, "foo1") + svc1, err := client.CoreV1().Services(ns.Name).Create(ctx, svc, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("Failed to create service %s: %v", svc.Name, err) + } + + err = wait.PollImmediate(1*time.Second, 10*time.Second, func() (bool, error) { + endpoints, err := client.CoreV1().Endpoints(ns.Name).Get(ctx, svc.Name, metav1.GetOptions{}) + if err == nil { + t.Errorf("expected no endpoints for externalName service, got: %v", endpoints) + return true, nil + } + return false, nil + }) + if err == nil { + t.Errorf("expected error waiting for endpoints") + } + + // update service to ClusterIP type and verify endpoint was created + svc1.Spec.Type = v1.ServiceTypeClusterIP + _, err = client.CoreV1().Services(ns.Name).Update(ctx, svc1, metav1.UpdateOptions{}) + if err != nil { + t.Fatalf("Failed to update service %s: %v", svc1.Name, err) + } + + if err := wait.PollImmediate(1*time.Second, wait.ForeverTestTimeout, func() (bool, error) { + ep, err := client.CoreV1().Endpoints(ns.Name).Get(ctx, svc1.Name, metav1.GetOptions{}) + if err != nil { + t.Logf("no endpoints found, error: %v", err) + return false, nil + } + t.Logf("endpoint %s was successfully created", svc1.Name) + if _, ok := ep.Labels[v1.IsHeadlessService]; ok { + t.Errorf("ClusterIP endpoint should not have headless label, got: %v", ep) + } + return true, nil + }); err != nil { + t.Fatalf("endpoints not found: %v", err) + } +} + // TestEndpointWithTerminatingPod tests that terminating pods are NOT included in Endpoints. // This capability is only available in the newer EndpointSlice API and there are no plans to // include it for Endpoints. This test can be removed in the future if we decide to include @@ -346,5 +453,12 @@ func newService(namespace, name string) *v1.Service { }, }, } - +} + +// newExternalNameService returns an ExternalName service with selector and exposing ports +func newExternalNameService(namespace, name string) *v1.Service { + svc := newService(namespace, name) + svc.Spec.Type = v1.ServiceTypeExternalName + svc.Spec.ExternalName = "google.com" + return svc }