refactor: Pair of lists to list of pairs in update changes

[bachorp]

What does it do ?

This patch set refactors the Changes struct to reflect the relationship of UpdateOld and UpdateNew.

// Old Format
type ChangesV1 struct {
	Create    []*endpoint.Endpoint
	UpdateOld []*endpoint.Endpoint
	UpdateNew []*endpoint.Endpoint
	Delete    []*endpoint.Endpoint
}

// New Format
type Changes struct {
	Create []*endpoint.Endpoint
	Update []*Update
	Delete []*endpoint.Endpoint
}

type Update struct {
	Old *endpoint.Endpoint
	New *endpoint.Endpoint
}

Interestingly, there is one test case, TestApplyRecordsOnEncryptionKeyChangeWithKeyIdLabel, that does not work under the new regime, which hints at some other problem with either the test case or the implementation.

If need be, this PR can be split into a sequence of PRs, depending on maintainer feedback.

Motivation

New and Old parts of updates cannot be treated independently. Pairing corresponding Endpoint objects guarantees that matching parts will stick together. This allows for simpler code at use sites of Changes (mainly Provider.ApplyChanges) and evades some class of indexing bugs such as #5542.

More

• Yes, this PR title follows Conventional Commits
• Yes, I added unit tests
• Yes, I updated end user documentation accordingly

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: bachorp / name: Pascal Bachor (177a418, 4e18b4b, c4a3b46, 84ee04e, 48959eb, fc73f0a, 39ea4e9, 8005887, 2333e93)

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign raffo for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[k8s-ci-robot]

Welcome @bachorp!

It looks like this is your first PR to kubernetes-sigs/external-dns 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/external-dns has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @bachorp. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[ivankatliarchuk]

Maybe. Hard to say where this refactoring will provide benefits or not. Most likely yes.

Greater Encapsulation - definately something nice to have.
~2000 lines of changes, most likely a no-go. Needs to be sliced differently. But first worth to w8 for other maintainers opinion @mloiseleur, @szuecs and @Raffo

If this will be accepted, I think we should first made a decision about following PRs

1. fix(txt-registry): skip creation of already-existing TXT records (#4914) #5459
2. feat(events): raise k8s events with fake provider #5659

[mloiseleur]

This refactor makes sense to me.

[ivankatliarchuk]

/ok-to-test

[ivankatliarchuk]

Was it tested on real cluster? No changes detected?

[bachorp]

Was it tested on real cluster? No changes detected?

No. I could most probably do so in a couple weeks.

[k8s-ci-robot]

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[mloiseleur]

@bachorp We will release next week. If you can test (and rebase) this PR before, maybe it will make it. Otherwise it will be for the next release.

[bachorp]

@mloiseleur Thanks for the info! I just shaved off a few patches that could make it into this week's release. For the main part I would then target the one afterwards.