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

[Helm] Choose NodePort values for controller.service.type = LoadBalancer #5333

Open
Falltrades opened this issue Apr 2, 2024 · 2 comments · May be fixed by #5335
Open

[Helm] Choose NodePort values for controller.service.type = LoadBalancer #5333

Falltrades opened this issue Apr 2, 2024 · 2 comments · May be fixed by #5335
Labels
backlog Pull requests/issues that are backlog items proposal An issue that proposes a feature request

Comments

@Falltrades
Copy link

Is your feature request related to a problem? Please describe.
Currently when we have controller.service.type set to LoadBalancer, it allocates random NodePort for httpPort and httpsPort.
Sometimes we would like to choose specific ports ourselves.

Describe the solution you'd like
I would like to be able to set the nodePort number like the case when controller.service.type is set to NodePort in charts/nginx-ingress/values.yaml:

      ## The custom NodePort for the HTTP port. Requires controller.service.type set to NodePort.
      # nodePort: 80

...

      ## The custom NodePort for the HTTPS port. Requires controller.service.type set to NodePort.
      # nodePort: 443

Actually there is not much change, we only need to add a case in charts/nginx-ingress/templates/controller-service.yaml:

  {{- if eq .Values.controller.service.type "NodePort" }}
    nodePort: {{ .Values.controller.service.httpPort.nodePort }}
  {{- end }}

...

  {{- if eq .Values.controller.service.type "NodePort" }}
    nodePort: {{ .Values.controller.service.httpsPort.nodePort }}
  {{- end }}

Like this:

  {{- if or (eq .Values.controller.service.type "LoadBalancer") (eq .Values.controller.service.type "NodePort") }}
    nodePort: {{ .Values.controller.service.httpPort.nodePort }}
  {{- end }}

...

  {{- if or (eq .Values.controller.service.type "LoadBalancer") (eq .Values.controller.service.type "NodePort") }}
    nodePort: {{ .Values.controller.service.httpsPort.nodePort }}
  {{- end }}

Describe alternatives you've considered
None since the solution was quite straightforward.

Additional context
None.
@Falltrades Falltrades added the proposal An issue that proposes a feature request label Apr 2, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 2, 2024

Hi @Falltrades thanks for reporting!

Be sure to check out the docs and the Contributing Guidelines while you wait for a human to take a look at this 🙂

Cheers!

@Falltrades Falltrades linked a pull request Apr 2, 2024 that will close this issue
Open
6 tasks
@vepatel
Copy link
Contributor

vepatel commented Apr 4, 2024

Hi @Falltrades, thanks for the issue and PR, we will be discussing this issue in out next triage meeting on Monday 8th April, please feel free to join if you'd like (Zoom details are on the project homepage).
-Venktesh

@vepatel vepatel added the backlog Pull requests/issues that are backlog items label Apr 8, 2024
@brianehlert brianehlert linked a pull request Apr 8, 2024 that will close this issue
[Helm] Choose NodePort values for controller.service.type = LoadBalancer #5335
Open
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
backlog Pull requests/issues that are backlog items proposal An issue that proposes a feature request
Projects
NGINX Ingress Controller
Status: Todo ☑
Milestone
No milestone
2 participants
@vepatel @Falltrades