Skip to content

Conversation

ciarams87
Copy link
Contributor

Proposed changes

There can be a race condition creating a new DNS endpoint when running multiple replicas of the Ingress Controller. This occurs when inbetween checking if the DNSEndpoint already exists and issuing the create request, another replica has already created the resource. The resource is created correctly, but the owning VirtualServer appears in an unhealthy state, and we see (an) event(s) like:

Warning BadConfig 14m nginx-ingress-controller Error creating DNSEndpoint for VirtualServer resource dnsendpoints.externaldns.nginx.org "dev" already exists

on the resource.

This PR resolves the issue by requeuing the resource when we get an already exists error back for a create request.

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@ciarams87 ciarams87 requested a review from a team as a code owner March 13, 2023 17:15
@codecov-commenter
Copy link

codecov-commenter commented Mar 14, 2023

Codecov Report

Merging #3637 (b9c258d) into main (3fab418) will decrease coverage by 0.02%.
The diff coverage is 25.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main    #3637      +/-   ##
==========================================
- Coverage   52.22%   52.21%   -0.02%     
==========================================
  Files          59       59              
  Lines       16873    16877       +4     
==========================================
  Hits         8812     8812              
- Misses       7764     7768       +4     
  Partials      297      297              
Impacted Files Coverage Δ
internal/externaldns/sync.go 37.56% <25.00%> (-0.82%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@github-actions github-actions bot added the tests Pull requests that update tests label Mar 14, 2023
@ciarams87 ciarams87 merged commit 4740b65 into main Mar 14, 2023
@ciarams87 ciarams87 deleted the fix-ed-race branch March 14, 2023 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tests Pull requests that update tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants