roboll / helmfile

Deploy Kubernetes Helm Charts
MIT License
4.05k stars 565 forks source link

feat: Predictable Helmfile template #932

Open mumoshu opened 5 years ago

mumoshu commented 5 years ago

TL;DR;

I want to add a new helmfile.yaml field to make templating helmfile configs easier.

Problem

Helmfile's double-rendering has opened a wide variety of use-cases that requires you to write a dynamic Helmfile configs.

The fact it works by recursively rendering the template twice to avoid a chicken-and-egg problem.

The problem is that Helmfile needs to load environment values to render the template but to load the values you need to render the template at first. It breaks in many ways and something results in behaviors that are hard to understand. See the incomplete list of those issues to get more context.

Solution

I'd like to propose an alternative format for Helmfile config that doesn't suffer from the chicken-and-egg problem.

Let's see.

Option 1

In the first option, we add the new template.releases field to helmfile.yaml, and a new template function nindenrt.

helmfile.yaml:

values:
- foo: bar

environments:
  prod:
    value:
    - prod.yaml

template:
  # RELEASES: This is the alternative version of `releases` that is rendered by the template
  # You can write a template in the same way as I have been to.
  releases: |
  - {{ tpl (readFile "release.tpl") (dict "name" "myapp1") | nindent 2}}
  # You can add more fields to be merged
  - {{ tpl (readFile "release.tpl") (dict "name" "myapp1") | nindent 2}}
    needs:
    - myapp1
  {{ tpl (readFile "comon-releases.tpl") . }}

Option 2

In the second option, we add a new field named template.helpers in addition to all the things in the first option.

helmfile.yaml:

values:
- foo: bar

environments:
  prod:
    value:
    - prod.yaml

template:
  # HELPERS: Template snippets that are reusable from multiple Helmfile configs
  #
  # Any files listed here can be called with {{ tempalte "BASENAME" }}
  # See https://golang.org/pkg/html/template/#ParseFiles
  helpers:
  - helpers/release.tpl
  - helpers/common-releases.tpl
  # RELEASES: This is the alternative version of `releases` that is rendered by the template
  # In addition to `.Environment.Values`, you can acess template snippets declared above.
  releases: |
  - {{ template "release.tpl" (dict "name" "myapp1") | nindent 2}}
  # You can add more fields to be merged
  - {{ template "release.tpl" (dict "name" "myapp1") | nindent 2}}
    needs:
    - myapp1
  {{ template "common-releases.tpl" . }}

Note that the nindent template function is recently added one that is becoming widely used across many official Helm charts. I'd like to adopt the pattern so that anyone has some experience in reading and writing charts is able to get started with the new way of writing Helmfile configs easily.

helpers/release.tpl:

name: {{ .Name }}
chart: ...
{{ if .Namespace }}
namespace: {{ .Namespace }}
{{ end }}

helpers/common-releases.tpl:

- name: logging
  chart: stable/fluent-bit
- name: servicemesh
  charts: istio/istio

Notice that the new template, template.helpers and template.releases fields are just plain YAML primitives.

This means that Helmfile can load the whole file as a valid YAML. It makes Helmfile possible to load environment values from it, and then render the template. We don't need the double-rendering anymore, which makes the result more predictable.

Option 3

In the third option, we add a new field named releasesTemplate as a alternative to the template.releases in the first option.

This is more consistent with existing fields like installedTemplate and waitTemplate than other options

helmfile.yaml:

values:
- foo: bar

environments:
  prod:
    value:
    - prod.yaml

releasesTemplate: |
  - {{ template "release.tpl" (dict "name" "myapp1") | nindent 2}}
  # You can add more fields to be merged
  - {{ template "release.tpl" (dict "name" "myapp1") | nindent 2}}
    needs:
    - myapp1
  {{ template "common-releases.tpl" . }}

As similar to the option 2, we may add something like templateContext to allow declaraing template helpers to be loaded into the context.

Notes

Acknowledgement

Double Rendering was added in #308 a year ago. It has been a great way to make your configs dynamic and DRY. It did serve many happy users.

I'm happy and impressed about that this is the feature requested and added by the community.

Thanks a lot for all the people involved in the feature, and especially the original author of it, @davidovich. Without it, I couldn't have come up with this idea, and more importantly Helmfile wouldn't have been popular so much ☺️

I hope this proposal makes the existing users of Helmfile and Double Rendering even more happier, while making on-boarding experiences for new-comers easier as well.

davidovich commented 5 years ago

I also think double-rendering has made it's time. I'm very glad that @mumoshu tried the experiment, and this is OSS at it's best: build upon ideas to find better ones.

On the proposal:

Instead of template.helpers would you consider using: template.context, template.template-context or template.render-context or variations on the context theme ? I like template.template-context as it hints on the usage of the {{ template ... }} feature of go that you mention.

Also, I think I prefer option 2 for it's usage of native primitives.

osterman commented 5 years ago

I accept that the current double rendering is a big kludge but has served its purpose well. Remember what Helmfile was like when only the values where templates?

For me the appeal to Helmfile from the beginning was how easy it was to get started. Immediate gratification. I admit I haven’t stayed current with some of the more advanced features of Helmfiles that have come out this year, but part of that reason is I haven’t invested to understand the use-cases it’s trying to solve. I like the self contained nature of Helmfiles. I like that for our intents they’ve almost become the analog of “terraform modules” for helm.

I think all this begs the bigger question of why the heck are we still templating YAML? The argument “pro” yaml templating has been based on how WYSIWYG. To this extent helm go templating and Helmfile templating is satisfactory. While there are some obvious edge cases where it doesn’t perform as expected, looking at a Helmfile it’s very easy to see what’s going on.

The proposed changes per the examples move to obfuscate all this advanced YAML templating into multiple layers of abstractions. I have no doubt that with the sheer genius of @mumoshu & community here, there will be an elegant solution for the problem, but are we solving the right problem?

In competitive sailing on an upwind course, you will never beat the boat ahead of you if you make the same tacks. The best chance at winning is to phase shift and make different tacks at different times by reading the situation.

Perhaps the problem is that we are not phase shifting. We use go templates to generate YAML for values, but also go templates for Helmfile YAML configuration. We don’t like that we are doing double rendering, so we want to fix that in our next tack. But we never get ahead of the problem, because our tacks are off. While this is good for consistency, it’s maybe not the winning move?

Maybe instead we should do what HashiCorp did by inventing HCL. Chose a configuration that is out of phase with what we want to configure and at the same time better at describing what we want to configure using a DSL.

This is a dangerous topic. And I don’t want to start any language wars. I love the self contained Helmfile binary. I know there are some more esoteric ways of elegantly expressing configurations with Jsonnet. But seeing as how helm 3 as adopted Lua, maybe we should do the same? Lua is relatively easily understood by developers and we can develop libraries. It’s the ultimate escape hatch. Write the configuration logic in a Lua DSL that produces the YAML for helm. I bet there’s even a way to template the YAML in lua for those that still want WYSIWYG.

Anyways, thanks @mumoshu for all the countless hours and endless dedication to this project. I think you have previously suggested phase shifting to another configuration language for Helmfile but met resistance. You spend a lot more time thinking about this than I, so I trust your judgement.

mumoshu commented 5 years ago

@osterman Hey! Thanks as always for your thoughtful comment.

are we solving the right problem?

"Yes", in short term. But "No" in longer term.

My goal for this proposal is, more or less, to improve Helmfile so that templating YAML becomes easier.

Seeing how Double Rendering has been adopted by users and seeing it's still used, I believe improving it provides a real benefit.

Also, I'm committed to maintaining Helmfile's templating capability until it turns out totally wrong.

In other words, I'm awaiting a real option that totally replaces YAML templating.

looking at a Helmfile it’s very easy to see what’s going on.

For the rendering part of Helmfile - not really.

After experiencing various debugging sessions that turned out related to Double Rendering, I'd say it isn't easy. The bigger/more complex your Helmfile gets, it is likely that the user steps into an edge-case that we really don't understand. The latest example is https://sweetops.slack.com/archives/CE5NGCB9Q/p1571818484076100.

Assuming it doesn't disappear in a year or two, improving it like I proposed here seemed to make sense.

Now, let's start talking about longer-term options.

why the heck are we still templating YAML?

I have the same sentiment as yours!

I'd say there are a few aspects:

  1. YAML templating is still a go-to option for providing a dynamic configuration facility to a single-binary tool
  2. It's widely used
  3. People love a single-binary tool so much. They don't eagerly try generating helmfile.yaml with an external tool(like jsonnet, cue, etc.)

I have been looking for an alternative to Helmfile's YAML template that satisfies the following conditions:

If I were to add "everyone knows" to the list, the only option would be Go template. But as I didn't I've considered various options like:

None of these met all the conditions I listed above, though.

The next best things I believe we can do are:

  1. Add more config options to Helmfile so that 80% of use-cases can be addressed without templating.
  2. Make the helmfile.yaml structure more friendly to configuration languages (ex. #893)
  3. Wait until the best possible option arrives(for something embeddable into helmfile
  4. // Please add anything I missed

For 3, it can be LUA once it lands on Helm 3.x, or even ECMAScript once everyone starts Infa-as-Code solutions based on ECMAScript and TypeScript like Pulumi or AWS CDK. But not now.

Probably we can do 1 and 2 while evangelizing one of the configuration languages we have today. But for that LUA wouldn't be the option right now. As Helm 3 doesn't have LUA support yet.

Do you have preferences, comments, etc. on any of the above?

Thanks in advance for your support, as always!

mumoshu commented 5 years ago

@davidovich Thanks!

It's encouraging to me that you, the original author of the feature, are very supportive. "build upon ideas to find better ones." <- I like this.

Re context it makes sense.

On a side note, I was wondering if we could add support for alternative configuration languages other than Go template under template. Regardless of which language we chose, it is likely that we need to pass some additional context to language's runtime. context would help that, too.

mumoshu commented 5 years ago

I like the self contained nature of Helmfiles. I like that for our intents they’ve almost become the analog of “terraform modules” for helm.

Btw, @osterman, I believe all the recent additions to Helmfile are for augmenting the helmfile-as-a-module use-case. Probably we are on the same page there.

It's like, bases is for writing a module in a DRY manner by reusing and distributing the common part as a "base" module, and layering for writing a complex module in a DRY manner.

The latest addition of DAG(#914) extends what a single module can do, by making it possible to declare the order of updates that was possible only with sub-helmfiles.

mumoshu commented 5 years ago

Also, #906 makes modules more self-contained, as you don't need an external tool to read secrets and sets envvars required for the module anymore.

mumoshu commented 5 years ago

Another double-rendering gotcha to share: #957

mumoshu commented 5 years ago

Added option 3.

mumoshu commented 5 years ago

@osterman Please correct me if I'm wrong, but you're not using double rendering at all in https://github.com/cloudposse/helmfiles/tree/master/releases, right?

I consider the pattern in cloudposse/helmfiles to avoid state values and environments(hence no need for double rendering) is the best practice when you have external tool(s) to feed environment-specific configuration values(via envvars, with e.g. chamber).

So this doesn't affect your use-case, and your usage of helmfile will remain as the best-practice for certain use-cases.

On the other hand, this feature provides a better alternative to Double Rendering for folks who doesn't want external tool(s) to feed environment-specific config.

osterman commented 5 years ago

Hrmmmm I am not 100% certain, but maybe we are not? @goruha @nuru

I certainly don’t mind any changes if we can continue to use what we have! :-) I just don’t want to have to update our dozens and dozens of releases and test the changes.

mumoshu commented 5 years ago

Just to be clear, if you don't have environments and values in the top-level of your helmfile.yaml's, you aren't relying on Double Rendering.

osterman commented 5 years ago

@mumoshu thanks for clarifying. I don't think we do that often, but we loved it for this use-case:

https://github.com/cloudposse/helmfiles/blob/master/releases/keycloak-gatekeeper.yaml#L13-L35

And then define a list of releases in with our own schema rather than the values schema of the chart. This lets us then stamp out gatekeeper instances without using a proxy injector.

We define an environment with a bunch of releases

services:
  - name: dashboard
    portalName: "Kubernetes Dashboard - Staging"
    host: dashboard.{{- env "KOPS_CLUSTER_NAME" }}
    forecastleGroupName: portal
    portalIcon: "https://kubernetes.io/images/favicon.png"
    useTLS: true
    skipUpstreamTlsVerify: true
    upstream: https://kubernetes-dashboard.kube-system.svc.cluster.local
    rules:
      - "uri=/*|roles=kube-admin,observer,dashboard|require-any-role=true"
    debug: false
    replicas: 2
    forecastleInstanceName: {{ env "FORECASTLE_INSTANCE_NAME" | default "default" }}
  - name: forecastle
    replicas: 2
    host: portal.{{- env "KOPS_CLUSTER_NAME" }}
    forecastleGroupName: portal
    portalIcon: "https://cloudposse.com/wp-content/uploads/2019/09/portal.png"
    useTLS: true
    upstream: 'http://forecastle.{{- env "FORECASTLE_NAMESPACE" | default "kube-system" -}}.svc.cluster.local'
    rules:
      - "uri=/*|roles=observer,user,portal|require-any-role=true"
    forecastleInstanceName: {{ env "FORECASTLE_INSTANCE_NAME" | default "default" }}
mumoshu commented 5 years ago

@osterman Thanks! That's a neat trick.

It does rely on Double Rendering. If this proposal is implemented, before Double Rendering is actually removed after the deprecation period, you'd need to move environments and releases under template like, for the option 1:

template:
  environments: |
    default:
      values:
      - {{ env "KEYCLOAK_GATEKEEPER_SERVICES_YAML" | default "keycloak-gatekeeper-services.yaml" }}
  releases: |
    {{ range $index, $service := .Environment.Values.services }}
    - name: "key-gate-{{- $service.name }}"
    *snip*

Or for the option 3:

environmentsTemplate: |
  default:
    values:
    - {{ env "KEYCLOAK_GATEKEEPER_SERVICES_YAML" | default "keycloak-gatekeeper-services.yaml" }}
releasesTemplate: |
  {{ range $index, $service := .Environment.Values.services }}
  - name: "key-gate-{{- $service.name }}"
  *snip*

This way any helmfile.yaml becomes a valid YAML, which makes Double Rendering unnecessary and hopefully clears out all the related gotchas.

osterman commented 5 years ago

@mumoshu okay, that should be fine so long as there's an easy workaround like that.

osterman commented 5 years ago

@goruha @nuru heads up 👆

mumoshu commented 4 years ago

Another double-rendering gotcha: https://sweetops.slack.com/archives/CE5NGCB9Q/p1576188568400200

mumoshu commented 4 years ago

Another double-rendering gotcha: https://sweetops.slack.com/archives/CE5NGCB9Q/p1576510812010200

TBeijen commented 4 years ago

As can already be observed in this thread: A lot of people (myself included) might not be sure if they're actually using double rendering.

They (at least I do) follow examples and experience things generally to work. And if not, then it can become a bit frustrating but let's consider that the outlier.

Regardless of option 1, 2 or 3, explicitly opting in for the '2nd pass' to me looks like a big improvement (Being a python guy, the second rule of the Zen of Python, Explicit is better than implicit., usually resonates with me)

Am I wrong in summarizing the proposals as a 'controlled, smaller scoped, explicitly opted in for, 2nd pass rendering'? Option 1 as showed in the comment above looks quite intuitive to me


Regarding the comment about exploring alternatives to YAML: I feel one of the strenghts of Helmfile currently is it's parallels to Helm itself. At least that's how I try to explain it to my team members:

So, declarative defining one or multiple releases in what would otherwise be a highly customized set of glue code that binds values files, Helm plugins, Helm command line arguments and whatnot.

Being able to apply all the learnings from Helm to Helmfile as well is very appealing (at least it is to me). So, if Helm adds LUA, it makes sense to use it in Helmfile as well. Other templating languages: Slippery slope that might distract from Helmfile's core strength. As long as it's possible to organize Helmfile files in a flexible way, there's always room to bolt on whatever processing one needs outside of Helmfile (decrypting usin tool X, templating releases based on data connector Y, the sky is the limit).

Ok, bit of a disjointed brain-dump. Hope it helps nevertheless. ;)

Btw: Wouldn't it be more consistent to use .gotmpl instead of .tpl in the examples?

osterman commented 4 years ago

@mumoshu: I'll standby whatever you think is the best course of action. It sounds like it's coming up all the time and impacting others. If we (cloudposse) need to adjust our helmfiles to accommodate these changes, thats fine! We can smooth whatever rough edges come from it (if any) in subsequent PRs to helmfile.

birdx0810 commented 2 years ago

Hi, I'm new to helmfile and although the double rendering issue feature isn't mentioned in anywhere in the docs. After a few tries we would figure out that the way helmfile works isn't as intuitive as we thought it was. Any thoughts on writing a simple doc regarding how double-rendering works under the hood? Or maybe best practices/tutorials for starting a new project/scaling up etc.

This could prevent newcomers from kicking themselves in the foot every time we write templates for new projects. I'm not a Go developer so reading and tracing the code might be a little exhaustive for me alone unless I have some sort of head start on how things work.

Cheers.

mumoshu commented 2 years ago

@birdx0810 Hey. We're going to deprecate double-rendering(although we won't delete it anytime soon). See https://github.com/helmfile/helmfile/blob/main/docs/proposals/towards-1.0.md#the-changes-in-10 for more context. That said, the rule is simple- don't make circular-dependency among your go template generated content and YAML. If you're going to use environments, make it sure that you have separated helmfile parts with ---s so that you don't need to rely on double-rendering.

mumoshu commented 2 years ago

Also note that we've already moved to another repository