external-dns icon indicating copy to clipboard operation
external-dns copied to clipboard

Fix templated prefix for updates and deletes

Open chrigl opened this issue 3 years ago • 1 comments

Description

This commit enables registry/txt.go to append the correct labels to endpoints with existing templated prefixed txt records.

e.g.

old.example.com A 10.0.0.1
prefix-a.old.example.com TXT "external-dns/owner=chris"

results in

endpoints.Endpoint {
  DNSName:    "old.example.com",
  Targets:    endpoint.Targets{"10.0.0.1"},
  RecordType: endpoint.RecordTypeA,
  Labels:     map[string]string {
    enpoint.OwnerLabelKey: "owner-2",
  },
}

Fixes #3031

Checklist

  • [x] Unit tests updated
  • [x] End user documentation updated (No need to change)

chrigl avatar Sep 20 '22 07:09 chrigl

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: chrigl Once this PR has been reviewed and has the lgtm label, please assign seanmalloy for approval by writing /assign @seanmalloy in a comment. 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:

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 avatar Sep 20 '22 07:09 k8s-ci-robot

Hi there! Any news here? We're being affected by this bug. Thank you.

minac avatar Oct 27 '22 14:10 minac

We're affected, too.

haslersn avatar Nov 27 '22 20:11 haslersn

As this is actively breaking functionality for people, is there any updates on this review?

markercm avatar Dec 09 '22 13:12 markercm

For us there's also frequently something that breaks due to this issue. We cannot migrate services between clusters or even between different ingress controllers without manually removing the old records directly from the authoritative DNS server.

haslersn avatar Dec 09 '22 13:12 haslersn

/auto-cc

markercm avatar Dec 13 '22 13:12 markercm

Any news? It's very important to finally fix this bug.

haslersn avatar Feb 26 '23 20:02 haslersn

@Raffo @seanmalloy @szuecs this is a bug that is affecting people, any updates on when you might review this PR?

markercm avatar Mar 17 '23 13:03 markercm

@chrigl this change looks good to me. Can you provide info that you have tested it? What else should I watch out for as reviewer?

Raffo avatar Mar 17 '23 18:03 Raffo

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/test-infra repository.

k8s-ci-robot avatar Apr 14 '23 05:04 k8s-ci-robot

This bug is still affecting us.

haslersn avatar Jun 06 '23 09:06 haslersn

@chrigl: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-external-dns-licensecheck 46262764cd888b3d785ba55e8e4f95a4d886aa81 link true /test pull-external-dns-licensecheck

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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/test-infra repository. I understand the commands that are listed here.

k8s-ci-robot avatar Jan 03 '24 18:01 k8s-ci-robot

There is no answer from PR author since 9 months, so I close this PR. Feel free to re-open or create a new one based on it if you need it. /close

mloiseleur avatar Jan 16 '24 10:01 mloiseleur

@mloiseleur: Closed this PR.

In response to this:

There is no answer from PR author since 9 months, so I close this PR. Feel free to re-open or create a new one based on it if you need it. /close

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/test-infra repository.

k8s-ci-robot avatar Jan 16 '24 10:01 k8s-ci-robot

The required changes are included in #3724 (merged).

haslersn avatar Mar 27 '24 16:03 haslersn