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

Add Nodeport support in Helm #981

Open
kameshsampath opened this issue Feb 11, 2025 · 4 comments · May be fixed by #984
Open

Add Nodeport support in Helm #981

kameshsampath opened this issue Feb 11, 2025 · 4 comments · May be fixed by #984
Labels
enhancement New feature or request

Comments

@kameshsampath
Copy link

kameshsampath commented Feb 11, 2025

Is your feature request related to a problem? Please describe.

Currently the service type does not support specifying an arbitrary nodePort value for the services. Its practice with k8s deployment with Helm to allow specifying the nodePort number.

Describe the solution you'd like

The service.ports could be modified as

service:
   type: ClusterIP/NodePort
   ports:
      - polaris-service
            targetPort: 8181
            nodePort: 320081 # optional nodeport 
     - polaris-metrics
           targetPort: 8182
           nodePort: 320082 # optional nodeport 

Then the corresponding service template could be

spec:
  type: {{ .Values.service.type | default "ClusterIP" }}
  selector:
    {{- include "polaris.selectorLabels" . | nindent 4 }}
  ports:
    {{- range $name, $port := .Values.service.ports }}
    - port: {{ $port.port }}
      targetPort: {{ $port.port }}
      {{- if and (eq $.Values.service.type "NodePort") $port.nodePort }}
      nodePort: {{ $port.nodePort }}
      {{- end }}
      protocol: TCP
      name: {{ $portName }}
    {{- end }}
  sessionAffinity: {{ .Values.service.sessionAffinity }}

When we run helm template . -s templates/service.yaml -f /tmp/test-values.yaml with test values as

service:
  type: NodePort
  ports:
    polaris-service:
      port: 8181
      nodePort: 30181
    polaris-metrics:
      port: 8182
      nodePort: 30182

It will now generate service.yaml like:

---
# Source: polaris/templates/service.yaml
apiVersion: v1
kind: Service
metadata:
  name: release-name-polaris
  namespace: polaris
  labels:
    helm.sh/chart: polaris-0.1.0
    app.kubernetes.io/name: polaris
    app.kubernetes.io/instance: release-name
    app.kubernetes.io/version: "1.0.0-incubating-SNAPSHOT"
    app.kubernetes.io/managed-by: Helm
spec:
  type: NodePort
  selector:
    app.kubernetes.io/name: polaris
    app.kubernetes.io/instance: release-name
  ports:
    - port: 8182
      targetPort: 8182
      nodePort: 30182
      protocol: TCP
      name: polaris-metrics
    - port: 8181
      targetPort: 8181
      nodePort: 30181
      protocol: TCP
      name: polaris-service
  sessionAffinity: None

Describe alternatives you've considered

No response

Additional context

No response

@adutra
Copy link
Contributor

adutra commented Feb 11, 2025

Hi, thanks for submitting this!

In fact, adding a node port is already planned in the new Helm chart redesigned for Quarkus (#626):

nodePort: ~ # 30000

The corresponding template is here:

nodePort: {{ .nodePort }}

The only difference I see with your PR #982 is that we don't test if the service type is NodePort.

I suggest then that we merge #626 first (I was planning to do so today anyways), then if you feel like we should enhance the templates to test the service type before printing the node port, then you could rebase your PR. Is that OK?

@adutra
Copy link
Contributor

adutra commented Feb 11, 2025

@kameshsampath #626 is merged! If you want, please rebase your PR now, thanks!

@kameshsampath
Copy link
Author

thanks @adutra - let me rebase and just add the test part

kameshsampath added a commit to kameshsampath/polaris that referenced this issue Feb 11, 2025
@kameshsampath
Copy link
Author

@adutra - I rebased and pushed it back. Please review and merge if it looks good to you

kameshsampath added a commit to kameshsampath/polaris that referenced this issue Feb 11, 2025
@kameshsampath kameshsampath linked a pull request Feb 11, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants