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.

Sign up for GitHub

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

Jump to bottom

Publish externalIPs of Nodes running Pods in headless service #1391

Closed
wants to merge 6 commits into from
Closed

Publish externalIPs of Nodes running Pods in headless service #1391

wants to merge 6 commits into from

Conversation

coufalja
Copy link

Added feature to publish Node ExternalIPs for headless services.

Use case:

  • Running K8s cluster on AWS
  • Nodes are having assigned both Internal and External IPs
  • Running Pods with hostPorts
  • Pods created are assigned those internal IPs into their status.hostIP which makes --publish-host-ip unusable
  • New --publish-host-external-ip flag will instead of status.hostIP publish assigned node ExternalIP(s)

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 24, 2020
@k8s-ci-robot
Copy link
Contributor

Welcome @coufalja!

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. 😃

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 24, 2020
@k8s-ci-robot k8s-ci-robot requested review from hjacobs and linki January 24, 2020 08:52
@coufalja
Copy link
Author

Docker image for testing purposes: wanderadock/external-dns:external-ips

@martinohansen martinohansen mentioned this pull request Jan 25, 2020
Open
njuettner
njuettner requested changes Feb 4, 2020
View reviewed changes
main.go Outdated Show resolved Hide resolved
@coufalja
Copy link
Author

coufalja commented Feb 5, 2020

/assign @linki

@linki
Copy link
Member

linki commented Feb 17, 2020

@coufalja Thank you!

@linki
Copy link
Member

linki commented Feb 17, 2020

/lgtm
/cc @njuettner

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 17, 2020
@domingusj
Copy link

Would this change be similar/related to #1077?

@domingusj domingusj mentioned this pull request Feb 17, 2020
Closed
@coufalja
Copy link
Author

Yes, it is pretty related, the approach is different though. If you guys think that doing it #1077 way is better I might re-do this according to it as it seems that #1077 is dead?

@mindw
Copy link

mindw commented Feb 18, 2020

@coufalja as users of externdDNS the annotation is preferable - it is explicit and allows to use both externalIps and internalIps using the same external-dns instance.

Thank you for your time and efforts on this PR :)

@domingusj
Copy link

I also think an annotation would be preferable

@coufalja coufalja force-pushed the publish-extrenal-ips branch from 51d536b to 7492751 Compare February 18, 2020 15:32
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 18, 2020
@k8s-ci-robot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@coufalja coufalja force-pushed the publish-extrenal-ips branch from 7492751 to 7eef000 Compare February 18, 2020 15:32
@coufalja
Copy link
Author

coufalja commented Feb 18, 2020 •
edited
Loading

Cool, WDYT now?

New image for testing wanderadock/external-dns:external-ips-v2

@linki
Copy link
Member

linki commented Feb 18, 2020

@domingusj @coufalja @mindw Thanks for pointing this out. We also prefer the annotation approach.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 5, 2020
@domingusj
Copy link

@coufalja anything I can do to help with the rebase/merge conflicts? Would love to see this added in the next release. Thanks again for your work on this!

@coufalja coufalja force-pushed the publish-extrenal-ips branch from 6e75adc to c90c70c Compare March 23, 2020 15:43
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 23, 2020
@coufalja
Copy link
Author

coufalja commented Mar 23, 2020 •
edited
Loading

@domingusj I attempted to rebase but now I am not able to build the project even locally due to

provider/ovh.go:50:15: undefined: DomainFilter

Due to #1469 it seems

@coufalja
Copy link
Author

@stefanlasiewski @linki Could you guys take another look? I replaced the usage of a new annotation with an already existing access one, or should I open it as a new PR?

@coufalja coufalja force-pushed the publish-extrenal-ips branch from 2c6b377 to ba7117c Compare April 1, 2021 13:55
@coufalja
Use existing Access annotation
89df027 
Signed-off-by: jakub.coufal <jakub.coufal@wandera.com>
@coufalja coufalja force-pushed the publish-extrenal-ips branch from ba7117c to 89df027 Compare April 6, 2021 06:35
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 5, 2021
@unixfox
Copy link

unixfox commented Jul 5, 2021

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 5, 2021
@ArchiFleKs
Copy link

Any change of having this merged soon ?

@ArchiFleKs
Copy link

@coufalja I just tried rebasing against latest release. It works as expected :). Could you try updating your PR for a chance of this getting merged ?

@coufalja
Copy link
Author

@ArchiFleKs updated

@k8s-triage-robot
Copy link

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:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 28, 2021
@mhoran
Copy link

mhoran commented Nov 28, 2021

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 28, 2021
@NicklasWallgren
Copy link
Contributor

Would be great if you could include support for Ingress as well.

@k8s-triage-robot
Copy link

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:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 10, 2022
@mhoran
Copy link

mhoran commented Mar 10, 2022

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 10, 2022
@k8s-ci-robot
Copy link
Contributor

@coufalja: 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.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 6, 2022
@k8s-triage-robot
Copy link

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:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 4, 2022
@mhoran
Copy link

mhoran commented Aug 4, 2022

Similar functionality is included in v0.12.0 via #2115.

/close

@k8s-ci-robot
Copy link
Contributor

@mhoran: Closed this PR.

In response to this:

Similar functionality is included in v0.12.0 via #2115.

/close

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.

joejulian pushed a commit to joejulian/redpanda that referenced this pull request Mar 10, 2023
@RafalKorepta
(split) operator: Change the configurator to respect subdomain
27a5f1d 
If subdomain is not empty, then external listener is configured.
The annotations in the headless service is used to register all
pods in DNS provider. The externalDNS does not support AWS
external IP, but there is open PR for addressing that.

During the testing the code from
kubernetes-sigs/external-dns#1391 was used.

REF
https://hub.docker.com/layers/wanderadock/external-dns/v0.7.6-pub-external-ips/images/sha256-dc59c17c70504bb86843bf9b71f649eb3220aa075751a7a11e46977cbdaedbcb?context=explore
joejulian pushed a commit to joejulian/redpanda that referenced this pull request Mar 24, 2023
@RafalKorepta
operator: operator: Change the configurator to respect subdomain
d783ff2 
If subdomain is not empty, then external listener is configured.
The annotations in the headless service is used to register all
pods in DNS provider. The externalDNS does not support AWS
external IP, but there is open PR for addressing that.

During the testing the code from
kubernetes-sigs/external-dns#1391 was used.

REF
https://hub.docker.com/layers/wanderadock/external-dns/v0.7.6-pub-external-ips/images/sha256-dc59c17c70504bb86843bf9b71f649eb3220aa075751a7a11e46977cbdaedbcb?context=explore
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hjacobs hjacobs Awaiting requested review from hjacobs

@linki linki Awaiting requested review from linki

@njuettner njuettner Awaiting requested review from njuettner

Assignees

@linki linki

@Raffo Raffo

Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.