feat: Make path overridable for startup probe and add timeoutSeconds property to all probe types

[nils1k]

This pull request contains the following changes:

• It adds support for overriding the path and port properties for the startup probe
• It adds support for the timeoutSeconds for all probe types
• It adds support for all timeout-related properties to the startup probe

[AlexanderBrevig]

This is a very breaking change. We must support all our existing configs. Maybe only use http for startup if the path is defined

[nils1k]

This is a very breaking change. We must support all our existing configs. Maybe only use http for startup if the path is defined

What would that look like in this case? Could we create the logic in the _helpers.tpl file, e.g.:

startupProbe:
  tcpSocket:
    port: {{ .probes.startup.port | default .internalPort }}
  {{- if .probes.startup.path }}
  httpGet:
    path: {{ .probes.startup.path }}
    port: {{ .probes.startup.port | default .internalPort }}
  {{- end }}

Could another option be to let people add probes and their properties as they please with something like this in deployment.yaml:

{{- with .Values.probes -}}

and then simply define any probes under the following key in values.yaml:

probes: {}

I think we would be able to keep backwards compatibility that way, or am I missing something?

[AlexanderBrevig]

Thinking about it, you could already use probes.spec

          probes:
            spec: # specify probes in accordance to k8s spec
              livenessProbe:
                tcpSocket:
                  port: admin

But to answer your question, the _helpers would look like this:

startupProbe:
  {{- if .probes.startup.path }}
  httpGet:
    path: {{ .probes.startup.path }}
    port: {{ .probes.startup.port | default .internalPort }}
  {{- else }}
  tcpSocket:
    port: {{ .probes.startup.port | default .internalPort }}
  {{- end }}

[nils1k]

probes.startup.path

Thanks! I've made some adjustments according to your suggestions, and added two more tests. Would you say the changes in this PR are really needed? My bad for not noticing the probes.spec; we could use that to accomplish what we want.

[AlexanderBrevig]

[...] Would you say the changes in this PR are really needed?

I must honestly say that I do not. We should get some more opinions maybe? @Chr1stian @olsove @bvestli @sindrel. Took the liberty of tagging a few contributors :)

[Chr1stian]

If this can be accomplished without changing the Chart, I dont see a need for it, but maybe document it just a little if others come looking for the same thing?

[bvestli]

I agree with @AlexanderBrevig

[AlexanderBrevig]

Thanks for the effort, but per discussion above this will not be part of "golden path" and an override of probes.spec will suffice.

[nils1k]

Sounds good 👍