diff --git a/CHANGELOG.md b/CHANGELOG.md index 454786554a..2a75143041 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ BUG FIXES: `_CONNECT_SERVICE_PORT` weren't being set when the upstream annotation was used. [[GH-549](https://github.com/hashicorp/consul-k8s/issues/549)] * Connect: Fix a bug with leaving around ACL tokens after a service has been deregistered. [[GH-571](https://github.com/hashicorp/consul-k8s/issues/540)] * CRDs: Fix ProxyDefaults and ServiceDefaults resources not syncing with Consul < 1.10.0 [[GH-1023](https://github.com/hashicorp/consul-helm/issues/1023)] +* Connect: Skip service registration for duplicate services only on Kubernetes. [[GH-581](https://github.com/hashicorp/consul-k8s/pull/581)] ## 0.26.0 (June 22, 2021) diff --git a/connect-inject/endpoints_controller.go b/connect-inject/endpoints_controller.go index 3f47fe03a9..693b0a25a8 100644 --- a/connect-inject/endpoints_controller.go +++ b/connect-inject/endpoints_controller.go @@ -235,13 +235,13 @@ func (r *EndpointsController) registerServicesAndHealthCheck(ctx context.Context return err } for _, service := range services { - if service.ServiceMeta[MetaKeyKubeNS] != serviceEndpoints.Namespace { + if existingNS, ok := service.ServiceMeta[MetaKeyKubeNS]; ok && existingNS != serviceEndpoints.Namespace { // Log but don't return an error because we don't want to reconcile this endpoints object again. r.Log.Info("Skipping service registration because a service with the same name "+ "but a different Kubernetes namespace is already registered with Consul", "name", serviceRegistration.Name, MetaKeyKubeNS, serviceEndpoints.Namespace, - "existing-k8s-namespace", service.ServiceMeta[MetaKeyKubeNS]) + "existing-k8s-namespace", existingNS) return nil } } diff --git a/connect-inject/endpoints_controller_test.go b/connect-inject/endpoints_controller_test.go index e1b41b3c8d..0afeb2aada 100644 --- a/connect-inject/endpoints_controller_test.go +++ b/connect-inject/endpoints_controller_test.go @@ -4657,94 +4657,79 @@ func TestCreateServiceRegistrations_withTransparentProxy(t *testing.T) { func TestRegisterServicesAndHealthCheck_skipsWhenDuplicateServiceFound(t *testing.T) { t.Parallel() - cases := map[string]struct { - consulServiceMeta map[string]string - }{ - "different k8s namespace meta": { - consulServiceMeta: map[string]string{MetaKeyKubeNS: "some-other-ns"}, - }, - "no k8s namespace meta": { - consulServiceMeta: nil, - }, - } - - for name, c := range cases { - t.Run(name, func(t *testing.T) { - nodeName := "test-node" - consul, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { - c.NodeName = nodeName - }) - require.NoError(t, err) - defer consul.Stop() + nodeName := "test-node" + consul, err := testutil.NewTestServerConfigT(t, func(c *testutil.TestServerConfig) { + c.NodeName = nodeName + }) + require.NoError(t, err) + defer consul.Stop() - consul.WaitForServiceIntentions(t) - httpAddr := consul.HTTPAddr - clientConfig := &api.Config{ - Address: httpAddr, - } - consulClient, err := api.NewClient(clientConfig) - require.NoError(t, err) - addr := strings.Split(httpAddr, ":") - consulPort := addr[1] + consul.WaitForServiceIntentions(t) + httpAddr := consul.HTTPAddr + clientConfig := &api.Config{ + Address: httpAddr, + } + consulClient, err := api.NewClient(clientConfig) + require.NoError(t, err) + addr := strings.Split(httpAddr, ":") + consulPort := addr[1] - existingService := &api.AgentServiceRegistration{ - ID: "test-service", - Name: "test-service", - Port: 1234, - Address: "1.2.3.4", - Meta: c.consulServiceMeta, - } - err = consulClient.Agent().ServiceRegister(existingService) - require.NoError(t, err) - pod := createPod("test-pod", "1.1.1.1", true, true) + existingService := &api.AgentServiceRegistration{ + ID: "test-service", + Name: "test-service", + Port: 1234, + Address: "1.2.3.4", + Meta: map[string]string{MetaKeyKubeNS: "some-other-ns"}, + } + err = consulClient.Agent().ServiceRegister(existingService) + require.NoError(t, err) + pod := createPod("test-pod", "1.1.1.1", true, true) - endpointsAddress := corev1.EndpointAddress{ - IP: "1.2.3.4", - NodeName: &nodeName, - TargetRef: &corev1.ObjectReference{ - Kind: "Pod", - Name: pod.Name, - Namespace: pod.Namespace, - }, - } - endpoints := &corev1.Endpoints{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-service", - Namespace: "default", - }, - Subsets: []corev1.EndpointSubset{ - { - Addresses: []corev1.EndpointAddress{endpointsAddress}, - }, - }, - } - ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}} - fakeClient := fake.NewClientBuilder().WithRuntimeObjects(ns, pod, endpoints).Build() + endpointsAddress := corev1.EndpointAddress{ + IP: "1.2.3.4", + NodeName: &nodeName, + TargetRef: &corev1.ObjectReference{ + Kind: "Pod", + Name: pod.Name, + Namespace: pod.Namespace, + }, + } + endpoints := &corev1.Endpoints{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-service", + Namespace: "default", + }, + Subsets: []corev1.EndpointSubset{ + { + Addresses: []corev1.EndpointAddress{endpointsAddress}, + }, + }, + } + ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}} + fakeClient := fake.NewClientBuilder().WithRuntimeObjects(ns, pod, endpoints).Build() - ep := &EndpointsController{ - Log: logrtest.TestLogger{T: t}, - ConsulClient: consulClient, - ConsulPort: consulPort, - ConsulScheme: "http", - ConsulClientCfg: clientConfig, - AllowK8sNamespacesSet: mapset.NewSetWith("*"), - DenyK8sNamespacesSet: mapset.NewSetWith(), - Client: fakeClient, - } + ep := &EndpointsController{ + Log: logrtest.TestLogger{T: t}, + ConsulClient: consulClient, + ConsulPort: consulPort, + ConsulScheme: "http", + ConsulClientCfg: clientConfig, + AllowK8sNamespacesSet: mapset.NewSetWith("*"), + DenyK8sNamespacesSet: mapset.NewSetWith(), + Client: fakeClient, + } - err = ep.registerServicesAndHealthCheck(context.Background(), *endpoints, endpointsAddress, api.HealthPassing, make(map[string]bool)) - require.NoError(t, err) + err = ep.registerServicesAndHealthCheck(context.Background(), *endpoints, endpointsAddress, api.HealthPassing, make(map[string]bool)) + require.NoError(t, err) - // Check that the service is not registered with Consul. - _, _, err = consulClient.Agent().Service("test-pod-test-service", nil) - require.Error(t, err) - require.Contains(t, err.Error(), "Unexpected response code: 404 (unknown service ID") + // Check that the service is not registered with Consul. + _, _, err = consulClient.Agent().Service("test-pod-test-service", nil) + require.Error(t, err) + require.Contains(t, err.Error(), "Unexpected response code: 404 (unknown service ID") - _, _, err = consulClient.Agent().Service("test-pod-test-service-sidecar-proxy", nil) - require.Error(t, err) - require.Contains(t, err.Error(), "Unexpected response code: 404 (unknown service ID") - }) - } + _, _, err = consulClient.Agent().Service("test-pod-test-service-sidecar-proxy", nil) + require.Error(t, err) + require.Contains(t, err.Error(), "Unexpected response code: 404 (unknown service ID") } func TestGetTokenMetaFromDescription(t *testing.T) {