opensearch-project / helm-charts

:wheel_of_dharma: A community repository for Helm Charts of OpenSearch Project.
https://opensearch.org/docs/latest/opensearch/install/helm/
Apache License 2.0
170 stars 228 forks source link

[BUG][opensearch-dashboards] livenessProbe.tcpSocket: Forbidden: may not specify more than 1 handler type #421

Closed sastorsl closed 1 year ago

sastorsl commented 1 year ago

Describe the bug NB! To be clear, this is not really a bug, but rather an issue when reconfiguring the livenessProbe to another handler than specified in the default values.yaml This issue is to enable others to see the issue, and to document how to properly be able to change the handler if required / wanted.

Affected:

Configuring / attempting to override the handler for the livenessProbe from tcpSocket to httpGet, or another handler type than the one in the default values will result in an error.

Due to the way helm merges the default parameters for livenessProbe in values.yaml with the values provided by me, the user one ends up with two handlers.

This results in an error message.

  Warning  error  32s (x2 over 5m33s)  helm-controller  reconciliation failed: Helm upgrade failed: failed to create resource: Deployment.apps "opensearch-dashboards" is invalid: spec.template.spec.containers[0].livenessProbe.tcpSocket: Forbidden: may not specify more than 1 handler type

To Reproduce

  1. Configure a httpGet handler in the user supplied values
  2. Deploy
  3. See error

Example

livenessProbe:
  failureThreshold: 6
  httpGet:
    path: /auth/openid/login?nextUrl=%2F&livenessProbe
    port: https  # Name of the k8s opensearch-dashboards service port name, not the scheme
    scheme: HTTP
  initialDelaySeconds: 60
  periodSeconds: 15
  successThreshold: 1
  timeoutSeconds: 5

Expected behavior That the probe was reconfigured and started working.

Chart Name opensearch-dashboards

Screenshots See reproduce info above.

Host/Environment (please complete the following information):

Additional context N/A

sastorsl commented 1 year ago

The underlying issue is that helm merge objects when merging the default values.yaml and user supplied values. Since tcpSocket is already defined adding a httpGet handler must result in two handlers being configured, which is not allowed.

This is of course as expected / how helm and default values should / do work, but it can be problematic to deal with.

One can see examples of the same type of issue here (merging values): https://github.com/helm/helm/issues/9597

sastorsl commented 1 year ago

Solution

One has to explicitly set the default value to remove / override the default handler.

livenessProbe:
  failureThreshold: 6
  httpGet:
    path: /auth/openid/login?nextUrl=%2F&livenessProbe
    port: https  # Name of the k8s service port, not the scheme
    scheme: HTTP
  initialDelaySeconds: 60
  periodSeconds: 15
  successThreshold: 1
  tcpSocket: null            # <------------------- set to null
  timeoutSeconds: 5

NB! tcpSocket: {} will not work, since then the value from values.yaml will be merged again.

sastorsl commented 1 year ago

Another thing to be observing is that you can end up with a probe warning if you are calling a URL which does a redirect (301, 302 http redirect / Location)

Warning  ProbeWarning    2m23s (x57 over 16m)  kubelet            Liveness probe warning:

This can be safely ignored (yes, I hate it...). See https://github.com/kubernetes/kubernetes/issues/103877 for reference.