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 v0.125.5 fails to build dependencies #1438

Closed
jheidbrink opened this issue Aug 28, 2020 · 7 comments · Fixed by #1439
Closed

Helmfile v0.125.5 fails to build dependencies #1438

jheidbrink opened this issue Aug 28, 2020 · 7 comments · Fixed by #1439
Labels

Comments

@jheidbrink
Copy link

Versions used: Helmfile 0.125.5 (0.125.4 works alright), Helm 3.2.4

When defining multiple releases for a chart with a dependency, Helmfile fails when updating the dependencies:

$ helmfilev0.125.5 diff

Building dependency release=release1, chart=chart-templates/example-with-dependency
Building dependency release=release2, chart=chart-templates/example-with-dependency
Hang tight while we grab the latest from your chart repositories...
...Successfully got an update from the "incubator" chart repository
...Successfully got an update from the "harbor" chart repository
...Successfully got an update from the "stable" chart repository
Update Complete. ⎈Happy Helming!⎈

Hang tight while we grab the latest from your chart repositories...
...Successfully got an update from the "harbor" chart repository
...Successfully got an update from the "incubator" chart repository
...Successfully got an update from the "stable" chart repository
Update Complete. ⎈Happy Helming!⎈
Saving 1 charts
Downloading nginx-ingress from repo https://kubernetes-charts.storage.googleapis.com
Deleting outdated charts

in ./helmfile.yaml: [command "/home/[...]/development/bin/helm" exited with non-zero status:

PATH:
  /home/[...]/development/bin/helm

ARGS:
  0: helm (4 bytes)
  1: dependency (10 bytes)
  2: build (5 bytes)
  3: chart-templates/example-with-dependency (39 bytes)

ERROR:
  exit status 1

EXIT STATUS
  1

STDERR:
  Error: unable to move current charts to tmp dir: link error: cannot rename chart-templates/example-with-dependency/charts to chart-templates/example-with-dependency/tmpcharts: rename chart-templates/example-with-dependency/charts chart-templates/example-with-dependency/tmpcharts: file exists

COMBINED OUTPUT:
  Hang tight while we grab the latest from your chart repositories...
  ...Successfully got an update from the "incubator" chart repository
  ...Successfully got an update from the "harbor" chart repository
  ...Successfully got an update from the "stable" chart repository
  Update Complete. ⎈Happy Helming!⎈
  Error: unable to move current charts to tmp dir: link error: cannot rename chart-templates/example-with-dependency/charts to chart-templates/example-with-dependency/tmpcharts: rename chart-templates/example-with-dependency/charts chart-templates/example-with-dependency/tmpcharts: file exists]

The command was run in a fresh clone of the repository.
This is the content of the files involved:

helmfile.yaml:

releases:
  - name: release1
    namespace: services
    chart: chart-templates/example-with-dependency
  - name: release2
    namespace: services
    chart: chart-templates/example-with-dependency

chart-templates/example-with-dependency/Chart.yaml:

apiVersion: v1
description: example-with-dependency
name: example-with-dependency
version: 0.1.0

chart-templates/example-with-dependency/requirements.yaml:

dependencies:
  - name: nginx-ingress
    repository: https://kubernetes-charts.storage.googleapis.com
    version: 1.28.2

chart-templates/example-with-dependency/requirements.lock:

dependencies:
- name: nginx-ingress
  repository: https://kubernetes-charts.storage.googleapis.com
  version: 1.28.2
digest: sha256:ed05ca41ee36de8615e6cf86456d086ea5914473c002ea9a7885ce0b1a73d829
generated: "2020-08-26T18:17:16.678972358+02:00"
@mumoshu
Copy link
Collaborator

mumoshu commented Aug 28, 2020

@jheidbrink I think this relaets to #541 and it happens when you tried to concurrently helm dep build the same local chart.

Probably I can enhance helmfile to gain exclusive lock on the local chart before running helm dep build so that it just works.

I'm marking this as a bug

@mumoshu mumoshu added the bug label Aug 28, 2020
@jheidbrink
Copy link
Author

I agree it's probably an issue with building helm dependencies for the same chart concurrently. I wouldn't say it's a duplicate of #541 though, as that one only happens when killing a previous helmfile process, or when running mulitple helmfile processes simultaneosly. The issue I described always happens. Also, it only got introduced with version 0.125.5, while #541 also affects earlier versions.

@jheidbrink
Copy link
Author

I'm wondering if one could simply remove duplicates in the list of charts to update the dependencies for.

@jheidbrink
Copy link
Author

By the way, our workaround for this is to first update all chart dependencies directly using helm, and then run helmfile with the --skip-deps parameter.

@mumoshu
Copy link
Collaborator

mumoshu commented Aug 28, 2020

. I wouldn't say it's a duplicate of #541 as that one sees the problem only after killing helmfile

@InspektorWarnebring Thanks. You're correct. I reread my comment before seeing your comment and edited my comment realizing I was wrong!

@mumoshu
Copy link
Collaborator

mumoshu commented Aug 28, 2020

I'm wondering if one could simply remove duplicates in the list of charts to update the dependencies for.

@jheidbrink Yes. Implementation-wise, it would have been the best if I could do so. Unfortunately, the helm deps build step currently reside in the complex "chartify" step(

if err := helm.BuildDeps(release.Name, chartPath); err != nil {
) and hence it wasn't trivial.

A short-term fix I managed to build in a timely manner is #1439. I'll keep it open until tomorrow morning and see if I can come up with a better one, and merge it anyway if I had no other idea.

mumoshu added a commit that referenced this issue Aug 28, 2020
mumoshu added a commit that referenced this issue Aug 29, 2020
* Fix race while running `helm dep build` on local chart

Fixes #1438
@mumoshu
Copy link
Collaborator

mumoshu commented Aug 29, 2020

@jheidbrink Merged #1439. It will be released as part of upcoming v0.125.9.

I'm wondering if one could simply remove duplicates in the list of charts to update the dependencies for.

BTW I think I've managed to implement it this way- See the latest commit in the PR :) Thanks for the feedback.

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.

2 participants