-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Expose ExternalIP of Node as annotation on headless service #1077
Conversation
Welcome @aaronhenshaw! |
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.
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. |
I signed it |
This feature is very useful for people that don't use LoadBalancer from cloud providers. I think supporting Headless ClusterIP + HostPort + ExternalIP is the way to go for this use case. |
source/service.go
Outdated
@@ -365,6 +380,11 @@ func (sc *serviceSource) generateEndpoints(svc *v1.Service, hostname string, pro | |||
log.Warn(err) | |||
} | |||
|
|||
externalIp := getExternalIpFromAnnotations(svc.Annotations) | |||
if err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be an err coming from getExternalIpFromAnnotations
or should externalIp be checked instead? (as this is, this is checking the err coming from getTTLFromAnnotations
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the check, not sure it's necessary.
The only thing I could do would be if its not "true" but it exists I could throw an error. What do you think?
77f4264
to
7758ceb
Compare
Thanks @aaronhenshaw, can't wait to check this out! The little script snippet in #632 has been chugging along without issue for months now 😃 |
Hi, any progress on this? |
I don't have any other feedback. I don't have approval privileges. The ball is on one of the approvers I guess. |
@zzh8829 any chance you could post the steps you used to create the modified container image? I'd also love to have this feature |
I guess this project is falling in unmaintained status, if PR are ignored |
@domingusj i cloned this branch and then compiled dockerfile. |
I also did an image myself. Its based on the latest master. you can check the repo and image out here https://github.com/TheUltimateC0der/external-dns |
Thank you @zzh8829 and @TheUltimateC0der. I'll do some testing this week. And hopefully your PR will get approved and released soon @aaronhenshaw! Thanks for the contribution! |
Hey any updates here. We would love to get this merged and move back to using the external-dns main releases! Thanks! |
The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
/remove-lifecycle stale |
#2115 merged, which might suffice as an alternative. |
@aaronhenshaw: 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. |
The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
/remove-lifecycle stale |
The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
/remove-lifecycle stale |
The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
/remove-lifecycle stale |
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
/remove-lifecycle stale |
@aaronhenshaw Do you think you can rebase this PR ? |
Since author is not answering, I'm closing this PR. |
@mloiseleur: Closed this PR. In response to this:
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. |
This is still needed for self-hosted kubernetes clusters - MetalLB exposes the internal NAT IP as the loadbalancer IP, which is then getting published as an A record by external-dns. |
Use Case:
We need to have external-dns set our DNS to point to the ExternalIP of the node that the pod winds up on.
I believe this is kind of mentioned in #632