Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Source Endpoints de-duplication (dedupSource) performed before AdjustEndpoints #4494

Open
krasoffski opened this issue May 22, 2024 · 0 comments
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@krasoffski
Copy link

krasoffski commented May 22, 2024

What happened:
Controller RunOnce get Records and than Source Endpoints.
After this, it performs AdjustEndpoints for Source Endpoints.

Here is simplified part.

func (c *Controller) RunOnce(ctx context.Context) error {
	lastReconcileTimestamp.SetToCurrentTime()

	records, err := c.Registry.Records(ctx)
	if err != nil {
		registryErrorsTotal.Inc()
		deprecatedRegistryErrors.Inc()
		return err
	}
	ctx = context.WithValue(ctx, provider.RecordsContextKey, records)

	endpoints, err := c.Source.Endpoints(ctx)
	if err != nil {
		return err
	}
	endpoints, err = c.Registry.AdjustEndpoints(endpoints)
	if err != nil {
		return fmt.Errorf("adjusting endpoints: %w", err)
	}
	registryFilter := c.Registry.GetDomainFilter()

}

During getting Source Endpoints: endpoints, err := c.Source.Endpoints(ctx) also performed de-duplication based on following attributes: DNSName, Targets and SetIdentifier.

Here is de-duplication simplified code:

for _, ep := range endpoints {
	identifier := ep.DNSName + " / " + ep.SetIdentifier + " / " + ep.Targets.String()

	if _, ok := collected[identifier]; ok {
		log.Debugf("Removing duplicate endpoint %s", ep)
		continue
	}

	collected[identifier] = true
	result = append(result, ep)
}

Thus, some Source Endpoints from sources are deleted before they are updated by AdjustEndpoints.
This leads to unexpected deletion de-duplicated records from DNS because they are de-deplicated before call AdjustEndpoints.
For example, SetIdentifier can be set or updated via AdjustEndpoints call to provider, after this Source Endpoint can became unique but has been de-duplicated before.

Or vice-versa case, provider can change SetIdentifier and Endpoint after AdjustEndpoints will be (should be) de-duplicated.

Finally, changed SetIdentifier used in planKey which with looks like a broken logic when we might perform de-duplication using different SetIdentifier value.

What you expected to happen:
I expect that Source Endpoints de-duplication performed after AdjustEndpoints call.

How to reproduce it (as minimally and precisely as possible):
It is clear from code.

Environment:

  • External-DNS version: v0.14.1
  • DNS provider: custom

BTW, I know there is annotation for setting SetIdentifier property, but it not solve this logic with AdjustEndpoints.

@krasoffski krasoffski added the kind/bug Categorizes issue or PR as related to a bug. label May 22, 2024
@krasoffski krasoffski changed the title Endpoint de-duplication (dedupSource) performed before AdjustEndpoints Source Endpoints de-duplication (dedupSource) performed before AdjustEndpoints May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

1 participant