Add support for --no-hooks switch in Helm template

[PatTheSilent]

This will allow users disabling hooks when rendering manifests from helm templates.

A use-case is, for example, installing Loki on EKS from https://github.com/grafana/helm-charts/tree/main/charts/loki-stack
When running tk diff you'll get an error

Error from server (BadRequest): admission webhook "iam-for-pods.amazonaws.com" does not support dry run

This isn't an issue in Tanka but the scope of this functionality is not limited to it and I think you can very much find use-cases outside of this example.

A potential improvement here could be to allow passing any args to helm template maybe using an array without any validation.

[sh0rez]

Hey!

From helm template --help:

--no-hooks     prevent hooks from running during install

With "hook" I guess they mean Chart hooks, which appear to be Kubernetes Job resources underneath. Furthermore, these only run during install, but Tanka uses template.

How would disabling hooks aid in Admission controllers not implementing dry run?

[PatTheSilent]

@sh0rez it helps by not having anything to validate 😄
For example the test-success hook in loki-stack is a Pod that runs something. The Admission Controller that I've mentioned runs only for Pod objects. By disabling the hook there is no Pod manifest to validate by the Admission Controller and tk diff runs happily.

A problem with hooks implemented as Pods is also that when running with tanka there is nothing to delete that Pod after it's done its supposed job. Like https://github.com/localstack/helm-charts/tree/main/charts/localstack/templates/tests. This one is failing on my clusters for some reason and is running indefinitely, consuming resources.

And, like I've mentioned, I don't think this is a problem with tanka but with chart maintainers and charts in general. But it would be helpful to be able to disable those hooks until they're fixed or if you don't really want them running.

[Duologic]

Maybe we can simply hide the resource in jsonnet that is generated by helm template that triggers this behavior.

Can you share a minimal reproducible example? I'd like to try it out and see if how it behaves.

[PatTheSilent]

@Duologic yes, you can do that.
This is more or less what I currently have in my project. To get it working with mainline tanka you'd need to add that override that's commented out.

  lokiStack:
    helm.template('loki-stack', 'charts/loki-stack', {
      namespace: 'monitoring',
      NoHooks: true,
      values: {}
    })
//    + {
//      'pod_loki_stack_test': {} //disable the test, otherwise `diff` fails on admission webhooks
//    }

Overriding that in jsonnet is not a problem but it felt somewhat hacky, which motivated me to submit this PR.
I feel that we should have more control over the execution of helm template than we do have now. We already have IncludeCRDs as an option.

[Duologic]

I would say the jsonnet override is actually a good solution to workaround this.

@sh0rez You were more about tying down on helm template arguments, is that still the case? Are we slowly going to implement every argument? Or maybe just have a list of argurments in jsonnet to add arbitrary arguments (like the original implementation)?

[PatTheSilent]

@sh0rez @Duologic if there we some design decisions behind the current implementation, I was not aware of them. I have no intent on insisting on this feature as there is a decent workaround. If you feel it be better to have a list of arguments instead of strict parameters, I'm more than fine with it can can adjust this PR or close it altogether.

[Duologic]

I have no strong opinion on which direction to choose, I was hoping on some input from @sh0rez when he gets to this.

[sh0rez]

LGTM!

There was some confusion on my side, I did not expect helm template to include Helm hooks, as that makes limited sense to me, given no one is controlling hook rollout, but so be it.

@Duologic as to whether to open the floodgates on any flag – let's see how this goes. If we exceed like 10 flags or so, sure, let's do that instead then.

[PatTheSilent]

Thanks both!

@sh0rez believe me when I say I was no less confused than you by helm template behavior.