Merge pull request #2259 from mper0003/issue-2245

service: fix merging of endpoints - same hostname different set-identifier
kubernetes-sigs · Nov 3, 2021 · 39931b7 · 39931b7
2 parents 4c66715 + 85c2d27
commit 39931b7
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 21 deletions.
diff --git a/source/service.go b/source/service.go
@@ -223,6 +223,7 @@ func (sc *serviceSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, e
 			lastMergedEndpoint := len(mergedEndpoints) - 1
 			if mergedEndpoints[lastMergedEndpoint].DNSName == endpoints[i].DNSName &&
 				mergedEndpoints[lastMergedEndpoint].RecordType == endpoints[i].RecordType &&
+				mergedEndpoints[lastMergedEndpoint].SetIdentifier == endpoints[i].SetIdentifier &&
 				mergedEndpoints[lastMergedEndpoint].RecordTTL == endpoints[i].RecordTTL {
 				mergedEndpoints[lastMergedEndpoint].Targets = append(mergedEndpoints[lastMergedEndpoint].Targets, endpoints[i].Targets[0])
 			} else {

diff --git a/source/service_test.go b/source/service_test.go
@@ -1085,7 +1085,7 @@ func testMultipleServicesEndpoints(t *testing.T) {
 		ignoreHostnameAnnotation bool
 		labels                   map[string]string
 		clusterIP                string
-		hostnames                map[string]string
+		services                 map[string]map[string]string
 		serviceTypesFilter       []string
 		expected                 []*endpoint.Endpoint
 		expectError              bool
@@ -1103,8 +1103,8 @@ func testMultipleServicesEndpoints(t *testing.T) {
 			false,
 			map[string]string{},
 			"",
-			map[string]string{
-				"1.2.3.4": "foo.example.org",
+			map[string]map[string]string{
+				"1.2.3.4": {hostnameAnnotationKey: "foo.example.org"},
 			},
 			[]string{},
 			[]*endpoint.Endpoint{
@@ -1125,10 +1125,10 @@ func testMultipleServicesEndpoints(t *testing.T) {
 			false,
 			map[string]string{},
 			"",
-			map[string]string{
-				"1.2.3.4": "foo.example.org",
-				"1.2.3.5": "foo.example.org",
-				"1.2.3.6": "foo.example.org",
+			map[string]map[string]string{
+				"1.2.3.4": {hostnameAnnotationKey: "foo.example.org"},
+				"1.2.3.5": {hostnameAnnotationKey: "foo.example.org"},
+				"1.2.3.6": {hostnameAnnotationKey: "foo.example.org"},
 			},
 			[]string{},
 			[]*endpoint.Endpoint{
@@ -1149,14 +1149,14 @@ func testMultipleServicesEndpoints(t *testing.T) {
 			false,
 			map[string]string{},
 			"",
-			map[string]string{
-				"1.2.3.5":  "foo.example.org",
-				"10.1.1.3": "bar.example.org",
-				"10.1.1.1": "bar.example.org",
-				"1.2.3.4":  "foo.example.org",
-				"10.1.1.2": "bar.example.org",
-				"20.1.1.1": "foobar.example.org",
-				"1.2.3.6":  "foo.example.org",
+			map[string]map[string]string{
+				"1.2.3.5":  {hostnameAnnotationKey: "foo.example.org"},
+				"10.1.1.3": {hostnameAnnotationKey: "bar.example.org"},
+				"10.1.1.1": {hostnameAnnotationKey: "bar.example.org"},
+				"1.2.3.4":  {hostnameAnnotationKey: "foo.example.org"},
+				"10.1.1.2": {hostnameAnnotationKey: "bar.example.org"},
+				"20.1.1.1": {hostnameAnnotationKey: "foobar.example.org"},
+				"1.2.3.6":  {hostnameAnnotationKey: "foo.example.org"},
 			},
 			[]string{},
 			[]*endpoint.Endpoint{
@@ -1166,6 +1166,30 @@ func testMultipleServicesEndpoints(t *testing.T) {
 			},
 			false,
 		},
+		{
+			"test that services with different set-identifier do not get merged together",
+			"",
+			"",
+			"testing",
+			"foo",
+			v1.ServiceTypeLoadBalancer,
+			"",
+			"",
+			false,
+			false,
+			map[string]string{},
+			"",
+			map[string]map[string]string{
+				"a.elb.com": {hostnameAnnotationKey: "foo.example.org", SetIdentifierKey: "a"},
+				"b.elb.com": {hostnameAnnotationKey: "foo.example.org", SetIdentifierKey: "b"},
+			},
+			[]string{},
+			[]*endpoint.Endpoint{
+				{DNSName: "foo.example.org", Targets: endpoint.Targets{"a.elb.com"}, Labels: map[string]string{endpoint.ResourceLabelKey: "service/testing/fooa.elb.com"}, SetIdentifier: "a"},
+				{DNSName: "foo.example.org", Targets: endpoint.Targets{"b.elb.com"}, Labels: map[string]string{endpoint.ResourceLabelKey: "service/testing/foob.elb.com"}, SetIdentifier: "b"},
+			},
+			false,
+		},
 	} {
 		tc := tc
 		t.Run(tc.title, func(t *testing.T) {
@@ -1175,12 +1199,9 @@ func testMultipleServicesEndpoints(t *testing.T) {
 			kubernetes := fake.NewSimpleClientset()
 
 			// Create services to test against
-			for serviceip, hostname := range tc.hostnames {
+			for lb, annotations := range tc.services {
 				ingresses := []v1.LoadBalancerIngress{}
-				ingresses = append(ingresses, v1.LoadBalancerIngress{IP: serviceip})
-
-				annotations := make(map[string]string)
-				annotations[hostnameAnnotationKey] = hostname
+				ingresses = append(ingresses, v1.LoadBalancerIngress{IP: lb})
 
 				service := &v1.Service{
 					Spec: v1.ServiceSpec{
@@ -1189,7 +1210,7 @@ func testMultipleServicesEndpoints(t *testing.T) {
 					},
 					ObjectMeta: metav1.ObjectMeta{
 						Namespace:   tc.svcNamespace,
-						Name:        tc.svcName + serviceip,
+						Name:        tc.svcName + lb,
 						Labels:      tc.labels,
 						Annotations: annotations,
 					},