Errors thrown when converting Route53 CNAME record from simple routing policy to weighted

[nitrocode]

What happened:

Same issue in #1411 and fixed by #1555 which was included in release v0.7.3.

We used this annotation on a kind: Service with type: LoadBalancer

    external-dns.alpha.kubernetes.io/hostname: echo-server.dev.example.internal

This created a record echo-server.dev.example.internal in the correct route53 hosted zone using a simple policy

We then added the following annotations to switch to a weighted record

    external-dns.alpha.kubernetes.io/aws-weight: "255"
    external-dns.alpha.kubernetes.io/set-identifier: bananas

And we saw this error

time="2022-11-08T22:07:11Z" level=error msg="InvalidChangeBatch: [RRSet with DNS name cname-echo-server.dev.example.internal., type TXT, SetIdentifier bananas cannot be created as a non-weighted set exists with the same name and type., RRSet with DNS name echo-server.dev.example.internal., type A, SetIdentifier bananas cannot be created as a non-weighted set exists with the same name and type., RRSet with DNS name echo-server.dev.example.internal., type TXT, SetIdentifier bananas cannot be created as a non-weighted set exists with the same name and type.]\n\tstatus code: 400, request id: "

What you expected to happen:

Switch from simple to weighted without throwing an error

How to reproduce it (as minimally and precisely as possible):

1. Install echo-server helm chart
2. Swap the Service from a ClusterIP to LoadBalancer
3. Add the respective annotations listed above
4. Tail the logs of external-dns to see the error thrown

Anything else we need to know?:

Environment:

• External-DNS version (use external-dns --version):

✗ k -n external-dns exec --stdin --tty external-dns-67957484c8-lkcx2 -- sh
~ $ external-dns --version

~ $ external-dns
FATA[0000] flag parsing error: required flag --source not provided
~ $ external-dns -v
FATA[0000] flag parsing error: unknown short flag '-v'
~ $ external-dns --version

~ $ external-dns version
FATA[0000] flag parsing error: unexpected version
~ $ external-dns -version
FATA[0000] flag parsing error: unknown short flag '-v'

• DNS provider: route53
• Others:

We're using the latest helm chart https://github.com/kubernetes-sigs/external-dns/releases/tag/external-dns-helm-chart-1.11.0 which uses v0.12.2 and we have not tested with v0.13.1

#1411 was closed a while ago but it seems like there may be a regression.

Our current workaround is to manually (clickops) change the AWS Route53 record from simple to weighted with the appropriate weight number and set identifier and then we see this in the external-dns logs

time="2022-11-08T22:14:15Z" level=info msg="Applying provider record filter for domains: [dev.example.internal. .dev.example.internal.]"
time="2022-11-08T22:14:15Z" level=info msg="All records are already up to date"

external-dns/provider/aws/aws.go

Line 436 in b83dbf8

func (p *AWSProvider) createUpdateChanges(newEndpoints, oldEndpoints []*endpoint.Endpoint) []*route53.Change {

external-dns/provider/aws/aws.go

Lines 443 to 452 in b83dbf8

	if new.RecordType != old.RecordType ||
	// Handle the case where an AWS ALIAS record is changing to/from a CNAME.
	(old.RecordType == endpoint.RecordTypeCNAME && useAlias(old, p.preferCNAME) != useAlias(new, p.preferCNAME)) {
	// The record type changed, so UPSERT will fail. Instead perform a DELETE followed by a CREATE.
	deletes = append(deletes, old)
	creates = append(creates, new)
	} else {
	// Safe to perform an UPSERT.
	updates = append(updates, new)
	}

So we want it to fall into the delete and create block but to do that it has to either meet one of these conditions

new.RecordType != old.RecordType

(
  old.RecordType == endpoint.RecordTypeCNAME
  && useAlias(old, p.preferCNAME) != useAlias(new, p.preferCNAME)
)

If it's the same record type, it will not meet one of the above conditions.

We have to add a new condition to hit the delete and create block if either

• old record does NOT have a weight and the new record does have a weight
• OR the old record does have a weight and the new record does NOT have a weight

There should also be adequate tests and currently there are none for the createUpdateChanges function.

This is affecting me as well, we are migrating the majority of our external_dns managed records from simple to weighted routing. Digging in a bit further, I see that #1411 was closed, but I'm not sure that it was actually ever resolved. In #1867 the createUpdateChanges func was created to address changing the record type, but not the record routing policy.

[bdols]

I'm seeing this with 0.13.5. I run with --upsert-only because the zone is shared with more than one cluster. If I run with --sync it will delete the non-weighted simple entry and create a new one. I can create a new issue for this, if needed, but I see the same error message from AWS. I ran it through a go debugger and it looks like this stems from the TXT registry is keyed off of a combination of the name and the identifier that is being added with the set-identifier annotation.

[ku524]

I have same problem in v0.13.4. I think this problem is not resolved.

[jessegonzalez]

@bdols @ku524 since this issue is closed you probably want to start another.