Publish ExternalIPs for LoadBalancer services

[burningalchemist]

Add feature to publish ExternalIPs for LoadBalancer services.

Use case:

• Running K8s cluster on Bare-metal behind NAT (1:1)
• MetalLB implements on-premises LoadBalancer and assigns loadBalancerIP from the given pool (internal network IP).
• Running Services of type 'LoadBalancer';
• By adding external IP address we can propagate a DNS entry with publicly available external IP address instead of the internal one.

Currently, ExternalIPs in LoadBalancer services are not processed at all, so there are no breaking changes as far as I can tell.

Note: ExternalIP value is defined separately, so it's out of the scope of ExternalDNS. The only thing needed is to prefer externalIP to loadBalancerIP and let ExternalDNS do the job.

[k8s-ci-robot]

Welcome @burningalchemist!

It looks like this is your first PR to kubernetes-sigs/external-dns 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/external-dns has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[njuettner]

Please make sure you changes include the right formatting, you can run make lint locally to see if there's something not correct.

source/service.go:492: File is not `goimports`-ed (goimports)
source/service_test.go:763: File is not `goimports`-ed (goimports)

[burningalchemist]

@njuettner I'm sorry for that. Will fix it shortly, thanks. 👍

[burningalchemist]

@njuettner @hjacobs Could you please take a look whenever you have time? :)

[k8s-ci-robot]

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
• If you have done the above and are still having issues with the CLA being reported as unsigned, please log a ticket with the Linux Foundation Helpdesk: https://support.linuxfoundation.org/
• Should you encounter any issues with the Linux Foundation Helpdesk, send a message to the backup e-mail support address at: login-issues@jira.linuxfoundation.org

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.

[burningalchemist]

@Raffo Could you please take a look at my PR when you have time or suggest someone else? I guess @njuettner is too busy. :)

[jodok]

would love to see this merged

[burningalchemist]

@seanmalloy could you please take a look at this PR? 😃

[seanmalloy]

@seanmalloy could you please take a look at this PR? 😃

@burningalchemist thanks for reaching out. I'm in the process of ramping on contributing to this project. It will probably take me a few weeks to get to this PR.

CC: @vinny-sabatini

[seanmalloy]

/kind feature

[burningalchemist]

@seanmalloy Thanks for your reply! I'll wait patiently. 👍 Stability wise it's been running smooth without issues since April. 🙂

[seanmalloy]

@burningalchemist can you please rebase this PR? I'm working on triaging size/M pull requests, so this one on my short list to review now. Thanks!

[burningalchemist]

@seanmalloy sure thing, will do shortly. 👍 UPD: Done.

[seanmalloy]

/assign

[vinny-sabatini]

LGTM

[seanmalloy]

@burningalchemist the code changes look good to me. I just have one question.

Do we want to have a configuration option to enable this feature? I think turning the functionality on by default would be considered a braking change, right?

/cc @tariq1890

[burningalchemist]

@seanmalloy I haven't yet discovered any cases where ExternalIPs attribute is used with external-dns annotations, and where LoadBalancerIP would be preferred instead to pass to DNS provider.

I'm happy to look into it and for the configuration flag, but I don't see any breaking changes from my side at the moment.

[tariq1890]

@burningalchemist Please add documentation for this. It would be important to mention that External IPs would take precedence over loadBalancer internal IPs

[burningalchemist]

@tariq1890 Yes, will do. Thanks!

[seanmalloy]

@seanmalloy I haven't yet discovered any cases where ExternalIPs attribute is used with external-dns annotations, and where LoadBalancerIP would be preferred instead to pass to DNS provider.

I'm happy to look into it and for the configuration flag, but I don't see any breaking changes from my side at the moment.

Give me some time to think about this and get back to you.

[burningalchemist]

@seanmalloy I'm waiting for your feedback, but maybe it's better indeed to cover this behaviour behind a feature toggle, be it publish-external-ip or prefer-external-ip or something else.🙂 I'm up to it, let me know what you think.

[seanmalloy]

@seanmalloy I'm waiting for your feedback, but maybe it's better indeed to cover this behaviour behind a feature toggle, be it publish-external-ip or prefer-external-ip or something else.🙂 I'm up to it, let me know what you think.

@burningalchemist I discussed with @vinny-sabatini and we are ok not having a feature toggle for this. Please note we are not the official project maintainers for external-dns.

Please add some user documentation for this and I will add my LGTM and then send it for final approval by the project maintainers.

Thanks!

[burningalchemist]

@seanmalloy Sounds great, I'll add it shortly. Thanks!

[burningalchemist]

@seanmalloy I've added notes to README.md. Seems ok to me. What do you think?

[seanmalloy]

/lgtm
/assign @Raffo @njuettner

[Raffo]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: burningalchemist, Raffo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [Raffo]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment