Skip to content

Missing default values for heartbeat in Helm chart #1135

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.

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

Closed
koki-sato opened this issue Feb 25, 2025 · 1 comment
Closed

Missing default values for heartbeat in Helm chart #1135

koki-sato opened this issue Feb 25, 2025 · 1 comment
Assignees

Comments

@koki-sato
Copy link

Describe the bug

Helm chart v0.26.0 does not have default values for heartbeat (added in #1116), so it is possible to generate a manifest that contains env with empty values.

Steps to reproduce

$ helm upgrade --dry-run --install aws-node-termination-handler \
  --namespace kube-system \
  --set enableSqsTerminationDraining=true \
  oci://public.ecr.aws/aws-ec2/helm/aws-node-termination-handler --version 0.26.0
...
---
# Source: aws-node-termination-handler/templates/deployment.yaml
apiVersion: apps/v1
kind: Deployment
spec:
  template:
    spec:
      containers:
        - name: aws-node-termination-handler
          env:
            - name: HEARTBEAT_INTERVAL
              value:
            - name: HEARTBEAT_UNTIL
              value:

Expected outcome

I think it's a good idea to set -1 as the default values for the helm chart according to the following:

flag.IntVar(&config.HeartbeatInterval, "heartbeat-interval", getIntEnv(heartbeatIntervalKey, -1), "The time period in seconds between consecutive heartbeat signals. Valid range: 30-3600 seconds (30 seconds to 1 hour).")
flag.IntVar(&config.HeartbeatUntil, "heartbeat-until", getIntEnv(heartbeatUntilKey, -1), "The duration in seconds over which heartbeat signals are sent. Valid range: 60-172800 seconds (1 minute to 48 hours).")

Application Logs

N/A

Environment

  • NTH App Version: 1.24.0
  • NTH Mode (IMDS/Queue processor): Queue processor
  • OS/Arch:
  • Kubernetes version:
  • Installation method: Helm
@LikithaVemulapalli
Copy link
Contributor

We have a fix in the latest release. Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants