Allow customising liveness and readiness probes

[Epicsteve2]

Adds default livenessProbe and readinessProbes, along with customLivenessProbe and customReadinessProbe to the Trino chart for both the coordinator and workers.

[nickelozz]

@hashhar would it be possible to get someone to review this PR?

[hashhar]

I'll take a look this weekend. sorry about the delay.

[hashhar]

Maybe let's split this by moving the default definition into helpers.tpl and leaving values.yaml empty and have the deployment pick the default if values is empty.

that way it keeps the chart simple by providing good defaults and allows anyone who wants customisation to provide their own livenessProbe node.

Same for readinessProbe.

[Epicsteve2]

I don't see the need for using _helpers.tpl. Instead, why not use the helm default function?
So it would look like:

initialDelaySeconds: {{ .Values.coordinator.readinessProbe.initialDelaySeconds | default 20 }}
periodSeconds: {{ .Values.coordinator.readinessProbe.periodSeconds | default 10 }}
timeoutSeconds: {{ .Values.coordinator.readinessProbe.timeoutSeconds | default 5 }}
successThreshold: {{ .Values.coordinator.readinessProbe.successThreshold | default 6 }}
failureThreshold: {{ .Values.coordinator.readinessProbe.failureThreshold | default 1 }}

If this is the case, do you also want values.yaml to look like this?

livenessProbe: {}
  # enabled: true
  # initialDelaySeconds: 20
  # periodSeconds: 10
  # timeoutSeconds: 5
  # failureThreshold: 6
  # successThreshold: 1
readinessProbe: {}
  # enabled: true
  # initialDelaySeconds: 20
  # periodSeconds: 10
  # timeoutSeconds: 5
  # failureThreshold: 6
  # successThreshold: 1
customLivenessProbe: {}
customReadinessProbe: {}

I feel like that would make customLivenessProbe and livenessProbe too similar. We could also only have customLivenessProbe and remove livenessProbe instead...

[hashhar]

I'd say we can remove customLivenessProbe and just use one of them. Keeps things simpler (for consumers of the chart for sure, maybe not for authors which is fine).

[Epicsteve2]

Ok. I've removed customReadinessProbe, customLivenessProbe, readinessProbe.enabled, livenessProbe.enabled as well
All of the if statements are gone and it will use the defaults if the selected values don't exist.

[hashhar]

LGTM and let's reword commit to Allow customising liveness and readiness probes since the probes already existed.