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

Helmfile hooks being run during helmfile lint #552

Closed
patrickmslatteryvt opened this issue Apr 10, 2019 · 6 comments
Closed

Helmfile hooks being run during helmfile lint #552

patrickmslatteryvt opened this issue Apr 10, 2019 · 6 comments

Comments

@patrickmslatteryvt
Copy link

patrickmslatteryvt commented Apr 10, 2019

We implemented helmfile hooks in our helmfiles chart to install cert-manager and the actual install works perfectly. However what does not work is helmfile lint against that chart, see below.
Ideally the hooks should not run during a lint operation.

helmfile --log-level=debug --file=./ --selector=enabled=true,infra=true,app=cert-manager lint

helmfiles/v8-deploy.d/012.cert-manager.yaml

releases:
  - name: cert-manager
    namespace: clu-inf-all
    chart: stable/cert-manager
    wait: true
    version: {{ readFile "versions/infra/versions.yaml" | fromYaml | get "cert-manager" }}
    values:
      - values/infra/cert-manager.yaml.gotmpl
    hooks:
      - events: ["prepare"]
        command: "kubectl"
        args: ["apply", "--validate=true", "--filename=values/infra/cert-manager-crds.yaml"]
      - events: ["prepare"]
        command: "kubectl"
        args: ["label", "namespace", "clu-inf-all", "certmanager.k8s.io/disable-validation=true"]

Output:

processing file "012.cert-manager.yaml" in directory "."
first-pass rendering input of "012.cert-manager.yaml":
 0: releases:
 1:   - name: cert-manager
 2:     namespace: clu-inf-all
 3:     chart: stable/cert-manager
 4:     wait: true
 5:     version: {{ readFile "versions/infra/versions.yaml" | fromYaml | get "cert-manager" }}
 6:     values:
 7:       - values/infra/cert-manager.yaml.gotmpl
 8:     hooks:
 9:       - events: ["prepare"]
10:         command: "kubectl"
11:         args: ["apply", "--validate=true", "--filename=values/infra/cert-manager-crds.yaml"]
12:       - events: ["prepare"]
13:         command: "kubectl"
14:         args: ["label", "namespace", "clu-inf-all", "certmanager.k8s.io/disable-validation=true"]
15:       - events: ["prepare"]
16:         command: "sh"
17:         args: ["../../scripts/dns-zone-client-secret.sh"]
18:     labels:
19:       app: cert-manager
20:       enabled: true
21:       infra: true
22:       domain: "wildcard-v8-mywebgrocer-com"
23:

second-pass rendering result of "012.cert-manager.yaml":
 0: releases:
 1:   - name: cert-manager
 2:     namespace: clu-inf-all
 3:     chart: stable/cert-manager
 4:     wait: true
 5:     version: v0.6.6
 6:     values:
 7:       - values/infra/cert-manager.yaml.gotmpl
 8:     hooks:
 9:       - events: ["prepare"]
10:         command: "kubectl"
11:         args: ["apply", "--validate=true", "--filename=values/infra/cert-manager-crds.yaml"]
12:       - events: ["prepare"]
13:         command: "kubectl"
14:         args: ["label", "namespace", "clu-inf-all", "certmanager.k8s.io/disable-validation=true"]
15:       - events: ["prepare"]
16:         command: "sh"
17:         args: ["../../scripts/dns-zone-client-secret.sh"]
18:     labels:
19:       app: cert-manager
20:       enabled: true
21:       infra: true
22:       domain: "wildcard-v8-mywebgrocer-com"
23:

1 release(s) matching enabled=true,infra=true found in 012.cert-manager.yaml

012.cert-manager.yaml: basePath=/home/pslattery/source/bitbucket/Kubernetes/v8-deploy/helmfiles/v8-deploy.d
hook[kubectl]: triggered by event "prepare"

hook[kubectl]: customresourcedefinition.apiextensions.k8s.io/certificates.certmanager.k8s.io unchanged
customresourcedefinition.apiextensions.k8s.io/issuers.certmanager.k8s.io unchanged
customresourcedefinition.apiextensions.k8s.io/clusterissuers.certmanager.k8s.io unchanged
customresourcedefinition.apiextensions.k8s.io/orders.certmanager.k8s.io unchanged
customresourcedefinition.apiextensions.k8s.io/challenges.certmanager.k8s.io unchanged


012.cert-manager.yaml: basePath=/home/pslattery/source/bitbucket/Kubernetes/v8-deploy/helmfiles/v8-deploy.d
hook[kubectl]: triggered by event "prepare"

hook[kubectl]: Error from server (NotFound): namespaces "clu-inf-all" not found


err: release "cert-manager" in "012.cert-manager.yaml" failed: hook[kubectl]: command `kubectl` failed: exit status 1
hook[kubectl]: command `kubectl` failed: exit status 1
@rajiteh
Copy link
Contributor

rajiteh commented May 6, 2019

This is a serious gotcha, I think lint implies that it is going to be a read-only operation, but you will execute commands in your hooks against the currently active kubernetes environment.

@mumoshu
Copy link
Collaborator

mumoshu commented May 7, 2019

@rajiteh Hey! Thanks for chiming in.

I think I have the same sentiment as yours about lint being a read-only operation. But the problem here(as I think) is that you do something mutates the actual cluster. prepare hook is there for you to prepare a helm chart. You shouldn't use it to mutate the cluster state.

Maybe you'd better raise a feature request to add presync hook?

@mumoshu
Copy link
Collaborator

mumoshu commented May 7, 2019

@patrickmslatteryvt Thanks for reporting this! I think the same comment I made for @rajiteh would apply to your use-case, too.

@rajiteh
Copy link
Contributor

rajiteh commented May 7, 2019

You shouldn't use it to mutate the cluster state.

Thanks for the insight @mumoshu! I wasn't aware of this. In my helmfile, I had a prepare hook to create docker pull secrets and tls secrets via kubectl apply .... This resulted in me accidentally updating secrets in a different environment than I ran the helmfile lint -e <env> with. :(

Also am I correct to assume that running helmfile diff will invoke hooks as well?
UPDATE: yes it does.

Maybe you'd better raise a feature request to add presync hook?

Yes, will do this. I think it's also better for the documentation to reflect the behaviour of hooks when linting as well.

On a related note, if helmfile exposes a template var that describes the current operation type (i.e in Helm's templating you have .Release.IsUpgrade and .Release.IsInstall), perhaps a .Helmfile.IsLint/IsApply/etc might help out here too.

Update: Just realized that .HelmfileCommand already does this. Sorry!

@rajiteh
Copy link
Contributor

rajiteh commented May 7, 2019

@patrickmslatteryvt @mumoshu I have a proposed a PR for the presync hook here: #580

@mumoshu
Copy link
Collaborator

mumoshu commented May 9, 2019

Closing this as #580 has been merged. Thanks for your supports @rajiteh @patrickmslatteryvt!

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

No branches or pull requests

3 participants