helm / helm-classic

⚠️(OBSOLETE) Helm Classic v1
https://github.com/helm/helm
Other
574 stars 54 forks source link

2 generators run initially... then 4 subsequently #393

Open krancour opened 8 years ago

krancour commented 8 years ago

I'm not sure I understand why this happens:

[kent@mbp ~]$ helm fetch deis/deis-dev
---> Fetched chart into workspace /Users/kent/.helm/workspace/charts/deis-dev
---> Done
[kent@mbp ~]$ helm generate deis-dev
---> Ran 2 generators.
[kent@mbp ~]$ helm generate deis-dev
---> Ran 4 generators.
technosophos commented 8 years ago

I'm gonna guess that whatever gets generated also has helm:generate in the headers

krancour commented 8 years ago

Shouldn't helm generate only be looking in the tpl dir for templates? If the generated manifests still mentioned helm:generate (albeit in a comment), shouldn't those be ignored? I'm probably misunderstanding something. fwiw, this is with the official deis-dev chart, so the issue should be easily reproducable.

mboersma commented 8 years ago

Maybe the generated files shouldn't contain the helm:generate line?

technosophos commented 8 years ago

We don't strip anything out of a file because we don't want to presume intent.

Is this still a problem somewhere? I'd like to take a look.

technosophos commented 8 years ago

Alright, so I reproduced the problem with the deis-dev chart.

The reason it is happening is because the helm:generate header is inside of a manifest file, and that file operates on itself as input.

There are two design decisions that we made that are resulting in this problem. Both decisions were made intentionally, but I'd be open to reconsidering.

  1. helm generate operates on an entire chart, including the directory manifests. While we don't recommend people store templates in manifests, we don't prevent the generator from checking that directory. We could do this.
  2. helm generate itself NEVER modifies a file. So it will not strip a generate header from output, or do any other modification. And the template engine doesn't remove helm:generate headers. While we could change this behavior, I'm not all that keen on this.

Thoughts?

mboersma commented 8 years ago

I thought requiring templates to live only in tpl/ (and ignoring them in manifests/) is too much magical behavior. At least I wouldn't expect it.

Stripping the helm:generate header seemed reasonable to me. Or you could rewrite it as helm:generated and otherwise keep it intact. The problem is obviously when there's a template actually in the destination manifests/ directory, then helm:generate would only work once under this proposal, and you'd have to "reset" the templates by editing them to say helm:generate again...ugly.

I guess on balance solution 1. seems saner. Or there's:

krancour commented 8 years ago

If we leave it as is, it remains a head scratching moment for anyone who just wants to regenerate secrets and such on the chart they already have (meaning no update) to bootstrap a second cluster or replacement cluster.

technosophos commented 8 years ago

Or... we could use the --exclude flag that's been in there the whole time, but which I forgot about.

$ helm generate --exclude=manifests