Helm chart is not helm3-compatible

[funkypenguin]

Hey guys,

I'm afraid the helm chart doesn't seem to be helm3-compatible. We're trying to deploy manifests into a namespace, but the namespace is only created when the manifests are deployed. 🐔 , meet :egg:

Here's how to reproduce:

1. Setup fresh k3s instance
2. Install helm v3.0.2
3. Clone the repo
4. Attempt to deploy using the helm chart (helm install k-rail deploy/helm)

The error you'll get in response is:

root@leeloo1:/tmp/k-rail# helm install -n k-rail k-rail deploy/helm
Error: create: failed to create: namespaces "k-rail" not found
root@leeloo1:/tmp/k-rail#

If you pre-emptively create the namespace, then helm will complain as follows:

root@leeloo1:/tmp/k-rail# helm install -n k-rail k-rail deploy/helm
Error: rendered manifests contain a resource that already exists. Unable to continue with install: existing resource conflict: kind: Namespace, namespace: , name: k-rail
root@leeloo1:/tmp/k-rail#

If you don't supply a namespace to helm, it'll fail since the default namespace already exists :)

root@leeloo1:/tmp/k-rail# helm install k-rail deploy/helm
Error: rendered manifests contain a resource that already exists. Unable to continue with install: existing resource conflict: kind: Namespace, namespace: , name: default
root@leeloo1:/tmp/k-rail#

The only way I've been able to deploy in helm3 has been to remove the namespace from the helm template, pre-create it, and then to install the chart. This further caught me out because I didn't realize that I needed to label the namespace with name=k-rail in order for the namespace to be ignored by the mutatingwebhook.

May I suggest the following:

1. Remove the namespace from the chart. It will break a lot of stuff, and bundling a namespace into a chart doesn't seem to be supported by helm (anymore?). Additionally, a pattern I've seen used in CI/CD has been to employ serviceaccounts with just enough privileges to apply manifests within a namespace, but not to create a namespace
2. Change the exclusion namespace label from name to k-rail-inspection=false. It's confusing and redundant that for proper operation, I need a namespace named "k-rail", with a label "name" equal to "k-rail" 😜
3. Update deployment docs indicating that users must manually create namespace and label, before applying the chart

If you're happy with the above approach, I'll prepare a PR for you :)

Cheers! D

[funkypenguin]

Note that this chicken/egg problem may be resolved with Helm v3.0.3, released 4h ago, with this fix : https://github.com/helm/helm/pull/7266

[dustin-decker]

Hmm, I think Helm 2 required the namespace to be created.. I'm curious to find what the best practice is here and apply it. I suspect creating the namespace separately as you suggested may be the best option. I think name= or app= convention is pretty typical though and is what I expect to see for label selectors.

[alpe]

Note that this chicken/egg problem may be resolved with Helm v3.0.3, released 4h ago, with this fix : helm/helm#7266

I can confirm that the issue still exists with helm v3.0.3

[netflash]

Up. Can we at least remove the namespace? I can provide a PR for that

[funkypenguin]

If it helps, here's a version I'm using with the namespace removed:

https://github.com/funkypenguin/helm-k-rail

[dustin-decker]

The solution that @funkypenguin proposed sounds fine. If one of you makes a PR we will accept it. Thanks.

[funkypenguin]

@dustin-decker I'll get on it. To be clear, which part of my proposed solution sounds fine? Just the removal of the namespace from the template, or also the changing of the exclusion label?

(The reason I suggest changing the label, is that in my experience when dealing with excluding certain namespaces from mutatingwebhooks, you come across self-documenting labels like this:

kiali.io/member-of=istio-system
istio-injection=enabled
certmanager.k8s.io/disable-validation=true

[dustin-decker]

I meant all of the above,

• the exclusion label
• removal of namespace for helm3 compat

Thanks for #72.

Do we need to move the chart under ./chart rather than ./deploy for the helm repo to work? Currently the action triggers on changes to the charts folder.

[funkypenguin]

Re #72, that was an oversight, sorry! Either we move the chart to /charts/k-rail, which is what the action expects by default, or we update the workflow to use the current /deploy/helm path. I'm happy to submit a PR for either :)

[dustin-decker]

I think helm expects repos have a charts directory, so let's try that.

[dustin-decker]

As of https://github.com/cruise-automation/k-rail/releases/tag/v2.0.0, automated helm chart releases are working! Namespace was removed from the helm chart to make it helm3 compatible. The namespace exclusion label has changed (a breaking change, so hence the major version increase)

Thanks for your help @funkypenguin