Kong / charts

Helm chart for Kong
Apache License 2.0
250 stars 480 forks source link

Duplicate handler when overriding readiness/liveness probes #1065

Open rbrendler opened 6 months ago

rbrendler commented 6 months ago

When overriding liveness probe, I get my probe plus the default one defined. The extra handler triggers a K8S error.

Chart.yaml:

apiVersion: v2
name: kong
version: "1.0.0"
appVersion: "1.0.0"
description: "A Helm chart for Kong Gateway for Kubernetes"
dependencies:
  - name: kong
    version: 2.33.3
    repository: https://charts.konghq.com

values.yaml:

kong:
  livenessProbe:
    exec:
      command:
        - python3.10
        - /home/kong/scripts/liveness_probe.py

Running helm dep up and helm template . renders the following in my Deployment:

        livenessProbe:
          exec:
            command:
            - python3.10
            - /home/kong/scripts/liveness_probe.py
          failureThreshold: 3
          httpGet:
            path: /status
            port: status
            scheme: HTTP
          initialDelaySeconds: 5
          periodSeconds: 10
          successThreshold: 1
          timeoutSeconds: 5

Note that my exec handler is followed by the default Kong httpGet handler, which triggers an error in K8S.

Same code behaves as expected with startup probes, only liveness and readiness probes have a problem.

rainest commented 6 months ago

YAML is kinda stupid and can't handle the mutually exclusive relationship exec and httpGet have in practice. Helm's merge logic doesn't let you define a complete livenessProbe value, it applies anything you provide as overlay atop the defaults in values.yaml. startupProbe doesn't have this issue because it has no actual default, just an example comment.

Does setting httpGet: {} knock the default out? It should.

Manan-Kothari commented 6 months ago

YAML is kinda stupid and can't handle the mutually exclusive relationship exec and httpGet have in practice. Helm's merge logic doesn't let you define a complete livenessProbe value, it applies anything you provide as overlay atop the defaults in values.yaml. startupProbe doesn't have this issue because it has no actual default, just an example comment.

Does setting httpGet: {} knock the default out? It should.

Nope, just tested this out

values.yaml

  livenessProbe:
    httpGet: {}
    exec:
      command:
        - python3.10
        - /home/kong/scripts/liveness_probe.py
    initialDelaySeconds: 30
    periodSeconds: 10
    successThreshold: 1
    failureThreshold: 3

renders out to

        livenessProbe:
          exec:
            command:
            - python3.10
            - /home/kong/scripts/liveness_probe.py
          failureThreshold: 3
          httpGet:
            path: /status
            port: status
            scheme: HTTP
          initialDelaySeconds: 30
          periodSeconds: 10
          successThreshold: 1
          timeoutSeconds: 5
Manan-Kothari commented 6 months ago

Opened up a PR for a fix, but this looks like it's an actual helm bug https://github.com/helm/helm/issues/12991