TXT registry: metadata format

[mcharriere]

Description

This PR is the continuation of the work done in #3774

It includes a new option to select between 2 different modes:

• transition: Old and new formats are created.
• only-metadata: only metadata format is created.

I mostly resolved conflicts with master. Docs are still missing, but I think that all of the comments in the previous PR have been addressed. Including the integration with the AWS Alias behavior change.

Ref https://app.slack.com/client/T09NY5SBT/C771MKDKQ

Fixes #3757

Checklist

• Unit tests updated
• End user documentation updated

[k8s-ci-robot]

Hi @mcharriere. 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.

[danielloader]

Would be good to keep this going, having duplicated TXT records following both formats is creating a lot of visual pollution in the zone.

[mloiseleur]

/ok-to-test

[mloiseleur]

/lgtm

[szuecs]

The reference to slack does not work for me in the browser. Can you post a link to the discussion for weblicents?

[szuecs]

This looks like an enum and we should use this:

type TXTFormat int

const (
	TXTFormatTransition TXTFormat = iota + 1
	TXTFormatOnlyMetadata
)

func (t TXTFormat) String() string {
	switch t {
	case TXTFormatTransition:
		return "transition"
	case TXTFormatOnlyMetadata:
                return "only-metadata"
	}
        return "unknown"
}

[mcharriere]

I decided to simplify and stick to strings, in alignment with the code style of the project (as in here)

Validation of these values is done as per user input, as calling external-dns with an invalid input fails with:

build/external-dns --txt-format invalid-format
FATA[0000] flag parsing error: enum value must be one of transition,only-metadata, got 'invalid-format'

[szuecs]

better to have a reference to a doc where "new" (version 2) is defined

[mcharriere]

AFAICT there are no docs referencing this format in the master branch, but all I can find is this doc that was removed some time ago.
The new term was introduced during the proposal and it's probably not the best way to describe it, as this PR adds yet another new format.
I could link that file or we could name the format differently. wdyt?

[szuecs]

I would say write a section in a doc about it and reference that.

[mcharriere]

Added a reference.

[szuecs]

so if we do things wrong we hide errors by choosing TXTFormatOnlyMetadata, IMO wrong choice.
Be explicit and have an UknownFormat for example in the enum that is the 0-value.

[szuecs]

which version ? old/new?
The comment I can get from the variable name txtMetadata.

[mcharriere]

As old and new age badly, the name metadata for this format was introduced (see the PR and issues referenced above). This is the format that is supposed to replace the other 2 at some point in time, meaning that the old and new should be removed.

[szuecs]

Then I would just drop the comment.

[mcharriere]

I opted for keeping the comment, the purpose is mostly to demarcate the 3 different code blocks (each TXTFormat). To make it consistent I added a reference to the docs that describes the Metadata TXTFormat.

[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 ask for approval from mloiseleur. For more information see the Kubernetes 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

[mcharriere]

I just rebased to master to keep up to date.

@mloiseleur @szuecs when you have some time, could you please take a look into this PR? thank you!

[mloiseleur]

/lgtm
/assign @Raffo