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

feat: Integrate ExternalDNS with VirtualServer resources #2800

Merged
merged 27 commits into from
Jul 5, 2022

Conversation

ciarams87
Copy link
Member

@ciarams87 ciarams87 commented Jun 28, 2022

Proposed changes

This PR creates an ExternalDNS block in the VirtualServer resource and uses it to create and configure DNSEndpoint objects which will be picked up an ExternalDNS deployment. ExternalDNS can then use the information in DNSEndpoint object to create, update, or delete, as appropriate, DNS records with the configured provider. See https://kubernetes-sigs.github.io/external-dns/v0.12.0/.

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

@github-actions github-actions bot added dependencies Pull requests that update a dependency file documentation Pull requests/issues for documentation enhancement Pull requests for new features/feature enhancements labels Jun 28, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jun 28, 2022

Codecov Report

Merging #2800 (4faa900) into main (27d6036) will decrease coverage by 0.71%.
The diff coverage is 22.31%.

❗ Current head 4faa900 differs from pull request most recent head bface28. Consider uploading reports for the commit bface28 to get more accurate results

@@            Coverage Diff             @@
##             main    #2800      +/-   ##
==========================================
- Coverage   53.79%   53.08%   -0.72%     
==========================================
  Files          55       58       +3     
  Lines       15273    15625     +352     
==========================================
+ Hits         8216     8294      +78     
- Misses       6779     7052     +273     
- Partials      278      279       +1     
Impacted Files Coverage Δ
cmd/nginx-ingress/flags.go 26.58% <0.00%> (-0.52%) ⬇️
cmd/nginx-ingress/main.go 0.00% <0.00%> (ø)
internal/externaldns/controller.go 0.00% <0.00%> (ø)
internal/externaldns/handlers.go 0.00% <0.00%> (ø)
internal/k8s/controller.go 11.78% <0.00%> (-0.03%) ⬇️
internal/externaldns/sync.go 36.31% <36.31%> (ø)
pkg/apis/configuration/validation/virtualserver.go 94.36% <72.22%> (-0.39%) ⬇️
pkg/apis/externaldns/validation/externaldns.go 95.89% <100.00%> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

glog.V(3).Infof("external DNS endpoint resource isnot owned by this object. refusing to update non-owned resource")
return nil, nil, nil
}
if !extdnsendpointNeedsUpdate(existingDNSEndpoint, dnsEndpoint) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would change this function name to isUpdateRequired since this function is already in the context of ExternalDNS

@ciarams87 ciarams87 force-pushed the extdns-controller branch 4 times, most recently from 40113ef to c4b66b4 Compare July 4, 2022 09:51
@ciarams87 ciarams87 marked this pull request as ready for review July 4, 2022 10:13
Copy link
Contributor

@shaun-nx shaun-nx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@ciarams87 ciarams87 mentioned this pull request Jul 4, 2022
@ciarams87 ciarams87 merged commit 439abda into main Jul 5, 2022
@ciarams87 ciarams87 deleted the extdns-controller branch July 5, 2022 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file documentation Pull requests/issues for documentation enhancement Pull requests for new features/feature enhancements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants