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.

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

support helm 3.7 #134

Merged
merged 2 commits into from
Dec 8, 2023
Merged

support helm 3.7 #134

merged 2 commits into from
Dec 8, 2023

Conversation

avimoondra
Copy link
Contributor

@avimoondra avimoondra commented Dec 8, 2023

Tested the same commands here: #131 with helm 3.7

Repro of issues

# Download Helm v3.7.1 for Mac (amd64)
curl -LO https://get.helm.sh/helm-v3.7.1-darwin-amd64.tar.gz

# Extract the tarball
tar -zxvf helm-v3.7.1-darwin-amd64.tar.gz

# Move the helm binary to a directory in your PATH and set it as "helm-3-7"
sudo mv darwin-amd64/helm /usr/local/bin/helm-3-7

# Verify the installation
helm-3-7 version
# version.BuildInfo{Version:"v3.7.1", GitCommit:"1d11fcb5d3f3bf00dbe6fe31b8412839a96b3dc4", GitTreeState:"clean", GoVersion:"go1.16.9"}

helm-3-7 upgrade --install -f ./onprem-instances/values.base.yaml -f ./onprem-instances/values.joe-test-new.retool.dev.yaml joe-test-new retool/retool --version 6.0.9
Error: UPGRADE FAILED: template: retool/templates/deployment_jobs.yaml:1:7: executing "retool/templates/deployment_jobs.yaml" at <include "retool.jobRunner.enabled" .>: error calling include: template: retool/templates/_helpers.tpl:132:58: executing "retool.jobRunner.enabled" at <eq .Values.jobRunner.enabled true>: error calling eq: incompatible types for comparison

Tested as such:

helm-3-7 template -f ~/retool-helm/charts/retool/values.yaml foo ~/retool-helm/charts/retool --set config.encryptionKey="foo" --set image.tag="5.6.10" --set replicaCount=2  --debug | rg replicas -C 7

install.go:178: [debug] Original chart version: ""
install.go:199: [debug] CHART PATH: /Users/avimoondra/retool-helm/charts/retool

  name: foo-retool
  labels:
    helm.sh/chart: retool-6.0.9
    app.kubernetes.io/name: retool
    app.kubernetes.io/instance: foo
    app.kubernetes.io/managed-by: Helm
spec:
  replicas: 2
  selector:
    matchLabels:
      app.kubernetes.io/name: retool
      app.kubernetes.io/instance: foo
  revisionHistoryLimit: 3
  template:
    metadata:
--
  name: foo-retool-jobs-runner
  labels:
    helm.sh/chart: retool-6.0.9
    app.kubernetes.io/name: retool
    app.kubernetes.io/instance: foo
    app.kubernetes.io/managed-by: Helm

@@ -129,7 +129,7 @@ Usage: (include "retool.jobRunner.enabled" .)
*/}}
{{- define "retool.jobRunner.enabled" -}}
{{- $output := "" -}}
{{- if or (gt (int (toString (.Values.replicaCount))) 1) (eq .Values.jobRunner.enabled true) }}
{{- if or (gt (int (toString (.Values.replicaCount))) 1) (default false .Values.jobRunner.enabled) }}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was fine as .Values.jobRunner.enabled as well, but added default for clarity.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so the "" and "1" output are fine, just the default needs to be changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{{- if or (gt (int (toString (.Values.replicaCount))) 1) Values.jobRunner.enabled }}

even that was ok

but yea, the "" and "1" is non-truthy and truthy respectively

@@ -141,7 +141,7 @@ Usage: (include "retool.workflows.enabled" .)
*/}}
{{- define "retool.workflows.enabled" -}}
{{- $output := "" -}}
{{- if or (eq .Values.workflows.enabled true) (eq .Values.workflows.enabled false) -}}
{{- if or (eq (toString (default "" .Values.workflows.enabled)) "true") (eq (toString (default "" .Values.workflows.enabled)) "false") -}}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this condition represents if the enabled field was set explicitly or not, hence cast to strings. previous version didn't work for helm 3.7

@avimoondra avimoondra requested a review from anna-yn December 8, 2023 18:13
@avimoondra avimoondra merged commit 3250f73 into main Dec 8, 2023
12 checks passed
@avimoondra avimoondra deleted the avimoondra/fix-for-helm-3-7 branch December 8, 2023 18:55
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

Successfully merging this pull request may close these issues.

2 participants