Add Nodeport support in Helm

[kameshsampath]

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]

Hi, thanks for submitting this!

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

https://github.com/apache/polaris/blob/c64771b4edd558516c817e0faac79de2d41c0932/helm/polaris/values.yaml#L103

The corresponding template is here:

https://github.com/apache/polaris/blob/ad0d4805b4ae2a0ce44e7ace80c3ae71abd041d9/helm/polaris/templates/service.yaml#L43

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]

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

[kameshsampath]

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

[kameshsampath]

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