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 lint+template command try to fetch local chart in 0.119.1 #1328

Closed
Morriz opened this issue Jun 30, 2020 · 9 comments · Fixed by #1400
Closed

helmfile lint+template command try to fetch local chart in 0.119.1 #1328

Morriz opened this issue Jun 30, 2020 · 9 comments · Fixed by #1400
Labels

Comments

@Morriz
Copy link

Morriz commented Jun 30, 2020

I use hooks to create tmp charts, but when I try to install those like this, it tries to fetch them and fails:

releases:
  ...
  chart: /tmp/charts/base
  ...

Error:

Fetching /tmp/charts/base
in helmfile.tpl/helmfile-init.yaml: [command "/home/app/tools/helm" exited with non-zero status:

PATH:
  /home/app/tools/helm

ARGS:
  0: helm (4 bytes)
  1: fetch (5 bytes)
  2: /tmp/charts/base (16 bytes)
  3: --untar (7 bytes)
  4: --untardir (10 bytes)
  5: /tmp/859425149/base/latest/tmp/charts/base (42 bytes)

ERROR:
  exit status 1

EXIT STATUS
  1

STDERR:
  Error: repo  not found

COMBINED OUTPUT:
  Error: repo  not found]

This happens in version 0.119.1 but I had this working fine before in 0.114.0.

@Morriz Morriz changed the title helmfile tries to fetch local chart helmfile lint+template command tries to fetch local chart in 0.119.1 Jun 30, 2020
@Morriz Morriz changed the title helmfile lint+template command tries to fetch local chart in 0.119.1 helmfile lint+template command try to fetch local chart in 0.119.1 Jun 30, 2020
@Morriz
Copy link
Author

Morriz commented Jul 22, 2020

@mumoshu @roboll Can you guys look at this? I just tried the latest 0.123.0 and this issue still exists:

in helmfile.tpl/helmfile-init.yaml: [command "/home/app/tools/helm" exited with non-zero status:

PATH:
  /home/app/tools/helm

ARGS:
  0: helm (4 bytes)
  1: fetch (5 bytes)
  2: /tmp/charts/base (16 bytes)
  3: --untar (7 bytes)
  4: --untardir (10 bytes)
  5: /tmp/882486847/base/latest/tmp/charts/base (42 bytes)


ERROR:
--
19 | exit status 1
20 |  
21 | EXIT STATUS
22 | 1
23 |  
24 | STDERR:
25 | Error: repo  not found
26 |  
27 | COMBINED OUTPUT:
28 | Error: repo  not found]
29 | error: no objects passed to apply

@Morriz
Copy link
Author

Morriz commented Jul 22, 2020

Why does helmfile not see that this is a local chart? It starts with a forward slash.

Again, 0.114 is ok.

@mumoshu
Copy link
Collaborator

mumoshu commented Jul 22, 2020

@Morriz Feeling interesting that I can't reproduce this. I have /tmp/charts/base that is accessible from my user and helmfile list helmfile lint just works. I'm on Helmfile v0.123.0.

Could you confirm that /tmp/charts/base is readable to your user?

@tinyzimmer
Copy link

tinyzimmer commented Jul 29, 2020

@mumoshu I am having this issue also. I did not even have a /tmp/charts/base directory, but I made one and lint/template are still trying to fetch the local chart.

PATH:
  /usr/local/bin/helm

ARGS:
  0: helm (4 bytes)
  1: fetch (5 bytes)
  2: ./charts/my-chart (23 bytes)
  3: --untar (7 bytes)
  4: --untardir (10 bytes)
  5: /tmp/268970692/my-chart/charts/my-chart (58 bytes)

ERROR:
  exit status 1

EXIT STATUS
  1

STDERR:
  Error: repo . not found

COMBINED OUTPUT:
  Error: repo . not found]

I also don't see where /tmp/charts/base is even a thing. It appears to be just using standard ioutil.TempDir("", "").

$> helmfile --version
helmfile version v0.123.0

@mumoshu
Copy link
Collaborator

mumoshu commented Aug 1, 2020

@tinyzimmer Thanks! Helmfile do use ioutil.TempDir("", ""), but only for creating a temporary directory which is used as the destination directory of helm fetch, not the input. So I was supposing it would have nothing to do with this issue.

Re: why this might be happening, Helmfile runs a lengthy logic to detect if it needs to be fetched. If the string specified in chart: is a valid local path, it skips fetching according to

helmfile/pkg/state/state.go

Lines 870 to 871 in 14636e3

} else if pathExists(normalizeChart(st.basePath, release.Chart)) {
chartPath = normalizeChart(st.basePath, release.Chart)

Maybe there're edge-cases that logic doesn't work as expected? Do you have any insight?

@mumoshu mumoshu added the bug label Aug 1, 2020
@tinyzimmer
Copy link

tinyzimmer commented Aug 1, 2020

@mumoshu I'm not near the stuff I was working on at the moment, but I had eventually figured it out. And if I recall correctly, it was having the local paths quoted in the helmfile.yaml that was causing the behavior. But I'll double back if that wasn't the case.

@tinyzimmer
Copy link

tinyzimmer commented Aug 1, 2020

I just did a quick poke through that package. Is this potentially the cause?

https://github.com/roboll/helmfile/blob/master/pkg/state/util.go#L10

func isLocalChart(chart string) bool {
	regex, _ := regexp.Compile("^[.]?./")
        // ...
}

This would go against my understanding of how yaml works 😛, but if the quotes are part of the string when that's getting evaluated then that regex wouldn't match.

EDIT: I actually can't for the life of me reproduce this right now. I was almost positive it had to do with the quotes in the end, but I can't validate that theory at the moment. For whatever it's worth, the chart in question had dependent charts that had not been built yet. I tried recreating that situation and then running a template, but then I got a completely rational error 😆

  Error: found in Chart.yaml, but missing in charts/ directory: base-chart

I'll update later if I have any success reproducing again.

@Morriz
Copy link
Author

Morriz commented Aug 1, 2020

@Morriz Feeling interesting that I can't reproduce this. I have /tmp/charts/base that is accessible from my user and helmfile list just works. I'm on Helmfile v0.123.0.

Could you confirm that /tmp/charts/base is readable to your user?

I create these folders in a pre hook, so they don't exist beforehand. This worked up till the version I mentioned though. Something changed

@mumoshu
Copy link
Collaborator

mumoshu commented Aug 5, 2020

@Morriz Thank you so much for the info! That does help. It turns out we accidentally changed the meaning of prepare hook that is intended to be called BEFORE the pathExists check there.

https://github.com/roboll/helmfile/pull/1172/files#diff-b4bc77a2ad92a32b058aaa223195ab06L800

I'll fix it asap. Thanks again for your patience and support.

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
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants