Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Question] using environments feature #273

Closed
davidovich opened this issue Aug 31, 2018 · 13 comments
Closed

[Question] using environments feature #273

davidovich opened this issue Aug 31, 2018 · 13 comments
Labels

Comments

@davidovich
Copy link
Contributor

#253 brings environments control to helmfile. Nice. #266 allows inclusions of many helmfiles controlled by one master helmfile. Excellent.

I had just finished an implementation (with helmfile 0.25.3) which does something similar, but I am not sure how to use the new feature to implement my use-case. Below explains the current situation, if anyone can comment on an alternative, it would be very valuable.

.
├── helmfile.d
│   ├── 00_serviceA.yaml
│   ├── 01-serviceB.yaml
│   ├── 02-serviceC.yaml
├── local-charts
│   ├── serviceA
└── values
    ├── serviceA.yaml  # default values for the serviceA
    ├── serviceB.yaml  # default values for the serviceB
    ├── production
    │   ├── serviceA.yaml  # production values
    │   ├── serviceB.yaml
    │   ├── serviceC.yaml
    └── staging
        ├── serviceA.yaml  # staging values
        ├── serviceB.yaml
        └── serviceC.yaml

Below are the contents of the serviceA.yaml.

releases:
- name: serviceA
  namespace: myTeam
  chart: ../local-charts/serviceA
  values:
    - ../values/serviceA.yaml  # load default values
    - ../values/{{ requiredEnv "DEPLOYMENT_ENV" }}/serviceA.yaml

DEPLOYMENT_ENV is injected by the ci-system, but it would be nice that helmfile would work without it.

I am wondering if you have any input on how I should refactor or structure this to use the environments and/or helmfile inclusion features.

Note that each release can have or leave out defaults, and can have or leave out configuration for an environment. Right now, this structure forces me to have a duplicated file in the environment directory even if it is not required. I'd like to tend towards a more generalized structure if at all possible.

Thanks for all the work!

@mumoshu
Copy link
Collaborator

mumoshu commented Sep 3, 2018

@davidovich Hey. Thanks for trying helmfile!

One of expected use-cases of #247 is to transform your structure into something like:

.
├── helmfile.yaml (1)
├── apps/
    ├── serviceA/
    │   ├── helmfile.yaml # former `helmfile.d/00-serviceA.yaml
    │   ├── charts/
    │      ├── serviceA # former `local-charts/serviceA`
    │   ├── default/
    │      ├── values.yaml # default values that are inherited to staging and production values
    │   ├── environments/
    │      ├── staging/values.yaml # staging values
    │      ├── staging/secrets.yaml
    │      ├── production/values.yaml # production values
    │      ├── production/secrets.yaml
    ├── serviceB/
    │   ├── helmfile.yaml
    │   ├── environments/
    │      ├── staging/values.yaml
    │      ├── staging/secrets.yaml
    │      ├── production/values.yaml
    │      ├── production/secrets.yaml
    ├── serviceC/
         ├── helmfile.yaml
         ├── environments/
            ├── staging/values.yaml
            ├── staging/secrets.yaml
            ├── production/values.yaml
            ├── production/secrets.yaml

Whereas the helmfile.yaml (1) looks like:

helmfiles:
- apps/*/helmfile.yaml

And apps/serviceA/helmfile.yaml looks like:

environments:
  staging:
  production:

releases:
- name: serviceA
  namespace: myTeam
  chart: ./charts/serviceA
  values:
    - ./default/values.yaml  # load default values
    - ./environments/{{ .Environment.Name }}/serviceA.yaml

this structure forces me to have a duplicated file in the environment directory even if it is not required.

To avoid that, you should rewrite your helmfile.yaml and values.yaml into something like:

environments:
  staging:
  # omit array items when all you want is values from `default/values.yaml`
  production:
    values:
    - environments/production/values.yaml

releases:
- name: serviceA
  namespace: myTeam
  chart: ./charts/serviceA
  values:
    - values.yaml.gotmpl

Whereas values.yaml.gotmpl is:

{{ merge default .Environment.Values (readFile "default/values.yaml" | fromYaml) }}

@mumoshu
Copy link
Collaborator

mumoshu commented Sep 3, 2018

You should specify the environment name by a flag helmfile --environment production sync. --environment defaults to default.

If you have specific use-case that requires helmfile to set the env according to an environment variable, please file an another feature request!

@davidovich
Copy link
Contributor Author

@mumoshu thank you very much for this response, it gives me many options, and a better understanding of the new features.

I don't know what you think, but it would maybe be beneficial to be able to reference releases in the environments: section. This would allow flexibility on each release in a declarative manner. Then, we could do:

environments:
  staging:
  production:
    serviceA:
      values:
      - env/production/serviceA/values.yaml

releases:
- name: serviceA
   ...

@davidovich
Copy link
Contributor Author

davidovich commented Sep 5, 2018

@mumoshu The proposed solution does not seem to work,

releases:
- name: serviceA
  namespace: myTeam
  chart: ./charts/serviceA
  values:
    - ./default/values.yaml  # load default values
    - ./environments/{{ .Environment.Name }}/serviceA.yaml

When I issue a helmfile -e staging diff, I get

err: release "serviceA" in "app/serviceA/helmfile.yaml" failed: stat [app_path]/serviceA/environments/serviceA.yaml: no such file or directory

As you see, {{ .Environment.Name }} seems empty

EDIT: this is fixed in 0.33.2 by commit c393449

@davidovich
Copy link
Contributor Author

davidovich commented Sep 5, 2018

In fact, you mention this exact issue in #254 (comment)

I have a solution (double parse), I will prepare a PR. I am not sure if there is an issue on this specific topic.

For reference, this is the PR #308.

@mumoshu
Copy link
Collaborator

mumoshu commented Sep 8, 2018

@davidovich Hey!

it would maybe be beneficial to be able to reference releases in the environments: section

Yeah, that's definitely beneficial and feasible!

Does it make the double render feature #308 unnecessary to you?

Although I find the double render feature super useful, I'm struggling about how we could settle it, escaped templates(#297 (comment)), mixed templating(#297 (comment)), and chart hooks(#297 (comment)) altogether, in the simplest possible yet effective form.

@davidovich
Copy link
Contributor Author

@mumoshu Hi!

I still think the double render is a more generalized approach. In fact, the idea came to me when I opened the code to see why I couldn't reference the .Environment.Name as you suggested (which was later fixed). I thought that having a complete env available could help other implementation ideas. I have a feeling that this fixes all the environment self-references.

Am I right in thinking that the double parse would fix all the attempts you mention to devise a syntax for templating ? We can take this to #308

@mumoshu
Copy link
Collaborator

mumoshu commented Sep 10, 2018

Am I right in thinking that the double parse would fix all the attempts you mention to devise a syntax for templating ?

Almost. Use-cases like chart hooks(#297) aren't covered with double rendering, but it requires us to execute a template at chart hook runtime.

@mumoshu
Copy link
Collaborator

mumoshu commented Sep 10, 2018

I still think the double render is a more generalized approach

Yeah, I believe so, too! My only concern is, honestly, it is just scary because I've already seen many edge-cases. But it doesn't negate the usefulness of double rendering.

So to move forward, would you mind if we do both (1) merge your double rendering (#308) AND (2) make helmfile.yaml templating optional by changing helmfile to require .gotmpl ext (#297 (comment))?.

This way, even in case we find any more edge-cases in double rendering that can't be fixed in a straightforward way, we can fall-back to add our own syntax/language to make vanilla helmfile.yaml programmable without gotmpl e.g. #197.

In case we find no more edge-cases in double rendering, it just remains the most versatile way to write your helmfile.

How dose it sound to you?

@davidovich
Copy link
Contributor Author

Use-cases like chart hooks(#297) aren't covered with double rendering, but it requires us to execute a template at chart hook runtime.

Why so? The first pass renders the environment, then you can use the result to describe your hooks, then the final render has all the info to run the hooks e.g.

command:  “./chartify -e {{ .Environment.Values.envRef }} {{ .Chart }}”

Would become:

command: “./chatify -e an_env_value a_chart”

@mumoshu
Copy link
Collaborator

mumoshu commented Sep 10, 2018

My thought is that {{ .Chart }} differs among each release in releases:. So it can't be fixed at the first/second render of helmfile.yaml.

@mumoshu
Copy link
Collaborator

mumoshu commented Sep 20, 2018

@davidovich Can we close this now?

@davidovich
Copy link
Contributor Author

Thanks @mumoshu, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants