Repository: kubernetes-sigs/external-dns (language: go)

Issue:
Same hostname, but different `set-identifier` is not creating multiple records
<!-- Please use this template while reporting a bug and provide as much info as possible. Not doing so may result in your bug not being addressed in a timely manner. Thanks!

-->

**What happened**:
Adding `external-dns.alpha.kubernetes.io/hostname` annotation to different service resources with the same hostname doesn't create multiple weighted DNS records.

We Istio ingress gateways, AWS load balancers and AWS Route53 weighted DNS.
We want to annotate Istio ingress gateway services with the following annotations to create Route53 DNS entries:
```yaml
# Istio ingress service at version 1
external-dns.alpha.kubernetes.io/alias: "true"
external-dns.alpha.kubernetes.io/hostname: "ingress.my-domain-1.com"
external-dns.alpha.kubernetes.io/set-identifier: "version-1"
external-dns.alpha.kubernetes.io/aws-weight: "100"
```
During Istio upgrades, we deploy the new version of Istio ingress gateways and add the following annotations:
```yaml
# Istio Istio ingress service at version 2
external-dns.alpha.kubernetes.io/alias: "true"
external-dns.alpha.kubernetes.io/hostname: "ingress.my-domain-1.com"
external-dns.alpha.kubernetes.io/set-identifier: "version-2"
external-dns.alpha.kubernetes.io/aws-weight: "0"
```

When testing this setup, version 1 of the service is deployed first and ExternalDNS creates the DNS record. However, when we later deploy the service at version 2, ExternalDNS "upserts" the DNS record with name`ingress.my-domain-1.com` (see logs), but doesn't create a new DNS record nor update the existing one.

<details>
<summary>ExternalDNS logs</summary>

```
time="2021-08-25T05:10:57Z" level=info msg="Applying provider record filter for domains: [my-domain-1.com. .my-domain-1.com. my-domain-1.com. .my-domain-1.com.]"
time="2021-08-25T05:10:57Z" level=info msg="Desired change: CREATE ingress.my-domain-1.com A [Id: /hostedzone/ABCDE]"
time="2021-08-25T05:10:57Z" level=info msg="Desired change: CREATE ingress.my-domain-1.commy-domain-1.com TXT [Id: /hostedzone/ABCDE]"
time="2021-08-25T05:10:58Z" level=info msg="2 record(s) in zone my-domain-1.com. [Id: /hostedzone/ABCDE] were successfully updated"
time="2021-08-25T05:12:11Z" level=info msg="Applying provider record filter for domains: [my-domain-1.com. .my-domain-1.com. my-domain-1.com. .my-domain-1.com.]"
time="2021-08-25T05:12:11Z" level=info msg="Desired change: UPSERT ingress.my-domain-1.com A [Id: /hostedzone/ABCDE]"
time="2021-08-25T05:12:11Z" level=info msg="Desired change: UPSERT ingress.my-domain-1.com TXT [Id: /hostedzone/ABCDE]"
time="2021-08-25T05:12:12Z" level=info msg="2 record(s) in zone my-domain-1.com. [Id: /hostedzone/ABCDE] were successfully updated"
```

</details>


**What you expected to happen**:
I expected to have two DNS records for `ingress.my-domain-1.com` with different record id and pointing to the load balancer that corresponds to the ingress service for each version, i.e:

```
ingress.my-domain-1.com  |  A  | Weighted  | 100 |  load-balacer-istio-version-1 (record id: version-1)
ingress.my-domain-1.com  | TXT | Weighted  | 100 |  "heritage=external-dns,external-dns/owner=cluster-a,external-dns/resource=service/istio-system/ingressgateway-1"
ingress.my-domain-1.com  |  A  | Weighted  |  0  |  load-balacer-istio-version-2  (record id: version-2)
ingress.my-domain-1.com  | TXT | Weighted  |  0  |  "heritage=external-dns,external-dns/owner=cluster-a,external-dns/resource=service/istio-system/ingressgateway-2"
```

**How to reproduce it (as minimally and precisely as possible)**:

Deploy two Istio ingress gateways and add the annotations described above.

**Anything else we need to know?**:
Both ingress services have the annotations.

**Environment**:
- DNS provider: aws
- chart version: 1.2.0
<details>
<summary>chart values</summary>

```yaml
values:
    fullnameOverride: external-dns
    serviceMonitor:
      enabled: true
    provider: aws
    triggerLoopOnEvent: true
    interval: 5m
    policy: sync
    registry: txt
    txtOwnerId: cluster-a
    domainFilters:
    - "my-domain-1.com"
    - "my-domain-2.com"
    sources:
      - service
      - ingress
```
</details>


Discussion / hints:

Fix patch:
diff --git a/source/service.go b/source/service.go
index 31c4f26a53..4600b6cdb6 100644
--- a/source/service.go
+++ b/source/service.go
@@ -221,6 +221,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 {


Test patch:
diff --git a/source/service_test.go b/source/service_test.go
index 9cf6d8f49d..933b1de2bf 100644
--- a/source/service_test.go
+++ b/source/service_test.go
@@ -1324,7 +1324,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
@@ -1342,8 +1342,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{
@@ -1364,10 +1364,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{
@@ -1388,14 +1388,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{
@@ -1405,6 +1405,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) {
@@ -1414,12 +1438,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{
@@ -1428,7 +1449,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,
 					},