allow custom readiness and liveness probes for vmselect and vminsert

[jifwin]

No description provided.

[Haleygo]

1. I think If those are extra readiness/liveness, they shouldn't override the default one;
2. Also need to add corresponding values in https://github.com/VictoriaMetrics/helm-charts/blob/master/charts/victoria-metrics-cluster/values.yaml#L72

[Haleygo]

I think we should modify the vminsert/vmselect probes like vmstorage does now, it will be more flexible to add extra probes or remove default one.

helm-charts/charts/victoria-metrics-cluster/templates/vmstorage-statefulset.yaml

Lines 79 to 82 in 9e7356a

	{{- with $.Values.vmstorage.probe.readiness }}
	readinessProbe:
	{{- toYaml . | nindent 12 }}
	{{- end }}

And can you help updating the changelog with some description as well?

[jifwin]

Good idea, but how to move this default probe logic to values.yml?

          readinessProbe:
            httpGet:
            {{- if index  .Values.vmselect.extraArgs "http.pathPrefix" }}
              path: {{ trimSuffix "/" (index .Values.vmselect.extraArgs "http.pathPrefix") }}/health
            {{- else }}
              path: /health
            {{- end }}

[Haleygo]

Good idea, but how to move this default probe logic to values.yml?

          readinessProbe:
            httpGet:
            {{- if index  .Values.vmselect.extraArgs "http.pathPrefix" }}
              path: {{ trimSuffix "/" (index .Values.vmselect.extraArgs "http.pathPrefix") }}/health
            {{- else }}
              path: /health
            {{- end }}

Yeah, thanks for pointing out. Seems like there is no easy way to unify vminsert&vmstorage's probes without possibility to break some users' installation now.
Ok, that's keep the way you proposed, please see reveiew comments here.

[jifwin]

I renamed 'extra' to 'custom' to clarify that it overrides the default probe.
Regarding values.yaml, do you want me to add an example of a custom probe, e.g. like this?

  probe:
    readiness:
      initialDelaySeconds: 5
      periodSeconds: 15
      timeoutSeconds: 5
      failureThreshold: 3
      #custom: # -- override for the default probe
      #  tcpSocket:
      #    port: http

[Haleygo]

it overrides the default probe.

Is there any specific reason to override the default probe here? I thought you want to add an extra probe.

And those new fields need to be added to values file too.

[jifwin]

I'm binding vmselect and vminsert to localhost for security reasons and using TLS sidecar to access these services on a diferent through a different port. Thus I need to change the definitions of probes to access that new port with HTTPS scheme. Also I want to replace tcpSocket probe with httpGet probe, since opening tcp to a sidecar proxy does not give much information, I need to actually call the service (vmselect/vminsert) behind the proxy.

[Haleygo]

@jifwin
I think this is the way to add this new feature without breaking old installation. Could you please add new vminsert.probe.readiness.custom field to the values file with example?
And help updating the changelog with some description as well?

[AndrewChubatiuk]

closing in a favour of #1212