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

catalog/from-k8s: adds support for ClusterIP Service #18

Closed
wants to merge 1 commit into from

Conversation

jipperinbham
Copy link
Contributor

addresses #4

In order to get the correct port, it has to loop through the endpoint ports while looping through the subsets. Also, in order to locate the correct "main" port, I moved the main variable outside the if ports condition to be referenced in the switch.

I believe this covers all the cases for ClusterIP services:

  • 1 port w/ no name
  • multiple ports (take first port defined in the service)
  • multiple ports with annotated override

@jipperinbham
Copy link
Contributor Author

I think this will also address #8 as the current release will use the NodePort for the Service port which doesn't work.

@leosunmo
Copy link

This would be immensely useful for me, I hope it doesn't fall through the cracks!

@darnould
Copy link

Can confirm that this made the from-k8s catalog sync functional for my use-case with ClusterIP services.

Well done @jipperinbham and thank you for your efforts. Very keen to see this merged/resolved in a release, HashiCorp! cc/ @mitchellh

@adilyse
Copy link
Contributor

adilyse commented Dec 7, 2018

Huge thanks to @jipperinbham for doing the work to support ClusterIP services! A version of this has now been merged with the addition of an option to turn off ClusterIp syncing if anyone should so desire.

@adilyse adilyse closed this Dec 7, 2018
ndhanushkodi pushed a commit to ndhanushkodi/consul-k8s that referenced this pull request Jul 9, 2021
Configure affinity to be a variable to allow overriding
wilkermichael added a commit that referenced this pull request Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants