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

* Make ha standby service generate random nodeport - #463

Closed
wants to merge 5 commits into from

Conversation

shayfisher
Copy link

In order to prevent collision with the ha active svc

  • Make the generic server svc generate a random
    nodeport if using HA mode, otherwise use the
    value supplied by the user on the chart

This addresses this issue:
#344

In order to prevent collision with the ha active svc
* Make the generic server svc generate a random
nodeport if using HA mode, otherwise use the
value supplied by the user on the chart
@hashicorp-cla
Copy link

hashicorp-cla commented Feb 26, 2021

CLA assistant check
All committers have signed the CLA.

random port when running in ha
nodePort if running in HA - we care about the
active service only
@shayfisher
Copy link
Author

This PR should make the user able to pick the nodePort port for the active svc, while generating port values for other svc that are also being created as nodePort (standby and generic server nodeport)

@shayfisher
Copy link
Author

Hi @jasonodonnell,
Do you know if I could see the reason the build failed?
It has a generic error with nothing informative.
I'm not sure that this contribution is what the devs meant, but it solves the problem we are facing.
The other way around it is probably to be very verbose in the chart and let the user configure the type for each service individually.
Would love to hear how to make progress on this!
Thanks

@jasonodonnell
Copy link
Contributor

Hi @shayfisher, sorry for the delay! I think this error is coming from CircleCI pipeline so I will look into it.

About this PR: does it not make more sense to simply change the port of the standby service instead of generating one? It's probably better to be explicit with the port rather than generating due to firewall rules you might want to apply. Thoughts?

@shayfisher
Copy link
Author

Hi @jasonodonnell ,
It does make more sense but I didn't know if you guys prefer to leave less choices for the end user in order to make the chart less verbose.
If I understand you correctly, a change should be made to introduce a new chart variable for the port of the standby service if NodePort was selected.
This way the user selects both the active and the standby ports.
Confirm and I will make the change.
Thanks

@jasonodonnell
Copy link
Contributor

@shahbazn Yes, I think a configurable on the port number makes sense.

@shahbazn
Copy link
Contributor

shahbazn commented May 4, 2021

Hey @jasonodonnell maybe you wanted to tag @shayfisher in the last comment?

@jasonodonnell
Copy link
Contributor

My apologies @shahbazn, indeed I made a mistake 😄

@swenson
Copy link
Contributor

swenson commented Aug 1, 2022

I believe this PR was superseded by #610

@swenson swenson closed this Aug 1, 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.

None yet

5 participants