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

helmfile template fails when requirements.yaml specifies local path #1341

Open
sirianni opened this issue Jul 8, 2020 · 8 comments
Open

Comments

@sirianni
Copy link

sirianni commented Jul 8, 2020

We are unable to run several helmfile commands (hemfile template, helmfile lint). These commands all error with:

ARGS:
  0: helm (4 bytes)
  1: --kube-context (14 bytes)
  2: k8s-mz-monitoring-eks (21 bytes)
  3: dependency (10 bytes)
  4: build (5 bytes)
  5: /tmp/214786334/... (99 bytes)

ERROR:
  exit status 1

EXIT STATUS
  1

STDERR:
  Error: directory /tmp/214786334/.../... not found

We have a monorepo and are using a relative path in requirements.yaml. The issue appears to be that helmfile is downloading the chart from the repo into /tmp and running helm dependency build inside that directory. This is unnecessary since our chart already includes the subcharts in the /charts directory (via the --dependency-update option to helm package. Our chart otherwise works fine with helm so this seems to be a quirk of how helmfile operates.

Why can't helmfile template just run helm template? Why does it need to download the chart first and run helm dependency build in a temp dir?

I can't find any references other than this but I think it is a best practice to include all requirements in the chart TGZ package. If so, then running helm dependency build is unnecessary.

EDIT: I see that it needs to download the chart first because helm template (with helm 2) is not capable of operating against remote charts (see helm/helm#4401). Is the dependency build needed or can this be skipped if the subcharts are already packaged in the ./charts directory?

Seems similar to #1002

@sirianni
Copy link
Author

sirianni commented Jul 9, 2020

Actually I just found the --skip-deps parameter can be used to avoid this step. I'll leave this issue open for discussion however.

@mumoshu
Copy link
Collaborator

mumoshu commented Aug 1, 2020

@sirianni Hey! This is really interesting.

So I thought that I had to run helm dep build to make local charts with requirements.yaml but with no charts/*.tgz downloaded yet.

I don't have an elegant solution to this. Do we need to parse requirements.yaml and check the existence of corresponding charts/CHART_NAME-vCHART_VERSION.tgz files before committing to run helmfile dep build?

Oh, or maybe we can just tolerate helm dep build failure and defer the actual error until the main operation - helm template in this case - cuz it should fail anyway if dependencies are really missing.

WDYT? Any input is much appreciated.

@sirianni
Copy link
Author

sirianni commented Aug 3, 2020

Oh, or maybe we can just tolerate helm dep build failure and defer the actual error until the main operation

Thanks @mumoshu. I haven't fully dug into all the details, but this seems like a reasonable approach.

@mumoshu
Copy link
Collaborator

mumoshu commented Aug 5, 2020

@sirianni Thanks!

I'm going to implement the fix soon.

However after re-reading your original issue description, I realized that the exact error you've seen seems like due to Helmfile is somehow missing the temporary created created by Helmfile itself. I'll investigate that asap, too.

@mumoshu
Copy link
Collaborator

mumoshu commented Aug 5, 2020

@sirianni Hey! I was wondering if your original issue relates to #1328. Do you use prepare hooks to generate local charts dynamically?

mumoshu added a commit that referenced this issue Aug 5, 2020
In #1172, we accidentally changed the meaning of prepare hook that is intended to be called BEFORE the pathExists check. It broke the scenario where one used a prepare hook for generating the local chart dynamically. This fixes Helmfile not to fetch local chart generated by prepare hook.

In addition to that, this patch results in the following fixes:

- Fix an issue that `helmfile template` without `--skip-deps` fails while trying to run `helm dep build` on `helm fetch`ed chart, when the remote chart has outdated dependencies in the Chart.lock file. It should be up to the chart maintainer to update Chart.lock and the user should not be blocked due to that. So, after this patch `helm dep build` is run only on the local chart, not on fetched remote chart.
- Skip fetching chart on `helmfile template` when using Helm v3. `helm template` in helm v3 does support rendering remote charts so we do not need to fetch beforehand.

Fixes #1328
May relate to #1341
@sirianni
Copy link
Author

sirianni commented Aug 5, 2020

@sirianni Hey! I was wondering if your original issue relates to #1328. Do you use prepare hooks to generate local charts dynamically?

No. We are not generating charts dynamically.

@mumoshu
Copy link
Collaborator

mumoshu commented Aug 6, 2020

@sirianni Thanks for confirming!

I'm still unable to reproduce your specific issue, but in #1400 I've updated Helmfile to only run helm dep build(which is failing for you) only on local charts. In my understanding, it should fix the issue for you.

mumoshu added a commit that referenced this issue Aug 6, 2020
In #1172, we accidentally changed the meaning of prepare hook that is intended to be called BEFORE the pathExists check. It broke the scenario where one used a prepare hook for generating the local chart dynamically. This fixes Helmfile not to fetch local chart generated by prepare hook.

In addition to that, this patch results in the following fixes:

- Fix an issue that `helmfile template` without `--skip-deps` fails while trying to run `helm dep build` on `helm fetch`ed chart, when the remote chart has outdated dependencies in the Chart.lock file. It should be up to the chart maintainer to update Chart.lock and the user should not be blocked due to that. So, after this patch `helm dep build` is run only on the local chart, not on fetched remote chart.
- Skip fetching chart on `helmfile template` when using Helm v3. `helm template` in helm v3 does support rendering remote charts so we do not need to fetch beforehand.

Fixes #1328
May relate to #1341
@jfelten
Copy link

jfelten commented Nov 23, 2020

I just ran into this on a chart with a dependent chart when doing a helmfile lint

releases:
- name: hydra
    namespace: tools
    chart: ory/hydra
    values:
    - {{ env "LAB_ENV" }}/hydra-values.yaml
helm version
version.BuildInfo{Version:"v3.4.1", GitCommit:"c4e74854886b2efe3321e185578e6db9be0a6e29", GitTreeState:"clean", GoVersion:"go1.14.11"}
helmfile -version
helmfile version v0.125.0

output error:

Error: directory /tmp/244978559/hydra/latest/ory/hydra/hydra-maester not found

hydra-maester is a depedent chart of ory/hydra

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

No branches or pull requests

3 participants