Allow configuring labels, annotations, and ports for the multiplayer Service

[abooij]

The main use for this is that in our implementation, the multiplayer Service is exposed as a Network Endpoint Group in Google Cloud. This requires adding a particular annotation to the Service. So as a workaround, I am using the following Service definition, managed as a manifest outside of the Helm chart:

apiVersion: v1
kind: Service
metadata:
  name: my-retool-multiplayer-ws
  annotations:
    cloud.google.com/neg: '{"exposed_ports": {"3001": {"name": "retool-multiplayer-neg"}}}'
spec:
  type: ClusterIP
  ports:
    - port: 3001
      targetPort: 3001
      protocol: TCP
      name: http
  selector:
    retoolService: retool-multiplayer-ws

(Our setup is roughly along the lines of this guide.)

It'd be useful if this pattern could be supported through the Helm chart directly. It is already supported for the backend Service, simply by setting this in values.yaml:

service:
  type: ClusterIP
  externalPort: 3000
  internalPort: 3000
  annotations:
    cloud.google.com/neg: '{"exposed_ports": {"3000": {"name": "retool-backend-neg"}}}'
  labels: {}
  selector: {}

I am not a Kubernetes expert, and haven't messed with Helm charts before. So it's quite likely something needs adjusting. Also, this code is untested as of writing. Feel free to push changes to this branch.

[jjlgao]

There's no need for an if/else here since you defined a default value for externalPort in values.yaml! Feel free to just have the port: {{ .Values.service.externalPort }} here.

[abooij]

With this if/else, it's backwards compatible in case people don't have the externalPort in their current values.yaml. If we don't have it, then if people quietly upgrade their chart without adding to values.yaml, things would likely break.

[jjlgao]

Ah, thanks for the explanation!

Apologies for being unclear, since you set a default value in values.yaml, it should carry over if they did not define it. I thought our default values.yaml would still act as the final fallback set of values in the event that someone didn't have a value set, but I'm hazy now and don't remember for sure.

Since you're leaving this check, I did notice that you've used .Values.multiplayer.service.externalPort as the if condition, but .Values.service.externalPort as the actual value being set. Should they match?

[abooij]

Oh yeah that was a mistake, it should've all referred to the multiplayer part of the config. Thanks for fixing it directly.

[jjlgao]

Formatting generally looks good here; I may be able to help test this to make sure it works as expected. Would you be able to make the changes as suggested?

Unfortunately, I think it may be difficult for me to push directly to the branch as it's a fork (and I actually don't know how easy it would be for me to push to a fork).

[abooij]

Formatting generally looks good here; I may be able to help test this to make sure it works as expected. Would you be able to make the changes as suggested?

Unfortunately, I think it may be difficult for me to push directly to the branch as it's a fork (and I actually don't know how easy it would be for me to push to a fork).

I'm not sure what you mean with the difficulty. You can literally just checkout the branch and push to it.

[jjlgao]

Just a heads up that this is still on my radar, but I think we have a linting test that's broken. Will try to confirm that it's indeed not working first before rebasing and getting this in, but unfortunately need some more time.