Chart Grooming

[jrnt30]

• Changed the default workflow-controller installation to use the ServiceAccount that is created and bound.
• Customized the instanceID logic:
  • No longer defaults to installed (this was very difficult to see/understand when coming from starter tutorials)
  • Kept logic to allow for release name or explicit mappings but changed structure a bit
• Added in optional configuration for:
  • CRD Install hook's ServiceAccount to allow clean install if your
    default roles aren't privledged
  • Optional Pod and Service annotations
  • Controller logging level configuration
• Minio Customizations
  • Changed the Secret configuration to properly represent the path of a secret instead of the actual contents
  • Changed the names of the secret and service that are represented to mirror that of the underlying chart

[CLAassistant]

All committers have signed the CLA.

[jrnt30]

@louis-murray Wanted to make sure I wasn't missing anything with your recent changes. The comments explain some of the issues I was having.

The one major "philosophical" change was the removal of the instanceID as disabled by default. Curious if you think there is a sufficient justification to set it on by default. As a newcomer to Argo but experienced Helm operator, it was a nuance that I completely overlooked that took several hours and a helping hand from @jessesuen to figure out.

Thanks!

[jessesuen]

@jrnt30 thanks for contributing this -- we really appreciate the helm expertise!

The one major "philosophical" change was the removal of the instanceID as disabled by default. Curious if you think there is a sufficient justification to set it on by default.

I agree, and prefer that the default behavior should be the one with the least amount of surprise. It also matches the getting started guide, which instructs users to install it cluster-wide. Running multiple workflow controllers with different instance-id's is a bit of an advanced use case, so we should accommodate the unfamiliar users better.

That said, I believe the original rational for turning it on by default, is because the workflow-controller doesn't tolerate multiple instances running at the same time very well. Basically if you have two controllers running, they will fight over the same workflows. Is there a best practice / technique within helm charts to protect against that?

[jrnt30]

Helm itself does not have any facility it provides to "lock" a deployment from happening really. The only real guarantee we have there is by using the {{.Release.Name}} as a differentiator as the original author did.

My observation for these types of things for Helm charts in general is to trend towards experimentation by reducing the cognitive overhead of the install and enabling users to enhance the chart via the knobs to tweak. I can certainly see the value of the instanceID being configurable after you explained it to me however it was something that I completely overlooked.

[jessesuen]

Hey @jrnt30 I think this looks good. I merged the disabling of minio from #13 so if you can just rebase to resolve the conflict, I'll go ahead and bring this in.