Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Duplicate handler when overriding readiness/liveness probes #1065

Open
rbrendler opened this issue May 3, 2024 · 3 comments · May be fixed by #1070
Open

Duplicate handler when overriding readiness/liveness probes #1065

rbrendler opened this issue May 3, 2024 · 3 comments · May be fixed by #1070

Comments

@rbrendler
Copy link

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
Copy link
Contributor

rainest commented May 9, 2024

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
Copy link

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 Manan-Kothari linked a pull request May 22, 2024 that will close this issue
Open
4 tasks
@Manan-Kothari
Copy link

Opened up a PR for a fix, but this looks like it's an actual helm bug helm/helm#12991

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@rainest @Manan-Kothari @rbrendler