Skip to content

/ helm

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

Jump to bottom

when killing `helm dep up` the next `dep up` fails #5567

Open
sgandon opened this issue Apr 8, 2019 · 19 comments
Open

when killing `helm dep up` the next `dep up` fails #5567

sgandon opened this issue Apr 8, 2019 · 19 comments
Labels
bug good first issue

Comments

@sgandon
Copy link

@sgandon sgandon commented Apr 8, 2019

when killing the process running helm dep up it sometimes leave a temporary folder named tmpcharts and this makes the next helm dep up fail with the following error.

Error: Unable to move current charts to tmp dir: rename /mychart/charts /mychart/tmpcharts: file exists

This is easily reproducable by creating a folder named tmpcharts inside an helm chart root folder and try to launch helm dep up

Of course a workaround is to remove the folder manually before running the helm dep up but that would be great that helm did that automatically somehow.

Output of helm version:

$helm version
Client: &version.Version{SemVer:"v2.12.2", GitCommit:"7d2b0c73d734f6586ed222a567c5d103fed435be", GitTreeState:"clean"}
Server: &version.Version{SemVer:"v2.13.1", GitCommit:"618447cbf203d147601b4b9bd7f8c37a5d39fbb4", GitTreeState:"clean"}

Output of kubectl version:

kubectl version
Client Version: version.Info{Major:"1", Minor:"13", GitVersion:"v1.13.3", GitCommit:"721bfa751924da8d1680787490c54b9179b1fed0", GitTreeState:"clean", BuildDate:"2019-02-04T04:48:03Z", GoVersion:"go1.11.5", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"13", GitVersion:"v1.13.0", GitCommit:"ddf47ac13c1a9483ea035a79cd7c10005ff21a6d", GitTreeState:"clean", BuildDate:"2018-12-03T20:56:12Z", GoVersion:"go1.11.2", Compiler:"gc", Platform:"linux/amd64"}

Cloud Provider/Platform (AKS, GKE, Minikube etc.): docker for mac

The code handling this is here :

helm/pkg/downloader/manager.go

Line 190 in 53d432f

func (m *Manager) downloadAll(deps []*chartutil.Dependency) error {

@sgandon sgandon mentioned this issue Apr 8, 2019
Open
@thomastaylor312 thomastaylor312 added the bug label Apr 8, 2019
@thomastaylor312
Copy link
Collaborator

@thomastaylor312 thomastaylor312 commented Apr 8, 2019

Good catch on this bug. Let me see if I can hammer out a quick bug fix this week
@thomastaylor312 thomastaylor312 added this to the 2.14.0 milestone Apr 8, 2019
@wknapik wknapik mentioned this issue May 2, 2019
Closed
@wknapik
Copy link

@wknapik wknapik commented May 2, 2019

Ran into this in 2.12.3 on Linux.
@sgandon
Copy link
Author

@sgandon sgandon commented May 2, 2019

I would have proposed a PR to fix this, be the first and only PR I did on this project took one year to get reviewed. So I'll pass.
@benoitf benoitf mentioned this issue Jul 1, 2019
Closed
@lindhe
Copy link
Contributor

@lindhe lindhe commented Feb 10, 2020

This is still an issue in 3.0.3.
@EnziinSystem
Copy link

@EnziinSystem EnziinSystem commented Apr 20, 2020

This is still an issue in v3.1.2+gd878d4d
@thomastaylor312 thomastaylor312 removed this from the 2.14.0 milestone May 5, 2020
@pankaj-akhade
Copy link

@pankaj-akhade pankaj-akhade commented May 15, 2020

My helm version is v3.2.1.

I am running a jenkins job where I do "helm dep update chart/

"
But I am getting the same error.
@MiaoRachelYu
Copy link

@MiaoRachelYu MiaoRachelYu commented May 28, 2020 •
edited

My helm version is v3.2.1. I do not kill "helm dep up" in the script before. Every time, I run with a new container, but when I execute "helm dep update chart" with a Jenkins job, it fails with the same error shown above.
@zak905
Copy link

@zak905 zak905 commented Aug 2, 2020

Any updates on this issue ?
@zak905
Copy link

@zak905 zak905 commented Aug 2, 2020

To solve this in my case, I had to: rm -rf charts and rm -rf tmpcharts/

It seems like helm assumes tmpcharts should not present, since it deletes it after the dependency updates, if one stops the operation in the middle this can happen.
@lindhe
Copy link
Contributor

@lindhe lindhe commented Aug 3, 2020

Sounds to me like tmpcharts should instead use some unique directory in /tmp similar to what mktemp does.
@bacongobbler
Copy link
Member

@bacongobbler bacongobbler commented Aug 14, 2020 •
edited

tmpcharts was introduced to fix a cross-device linking error as seen in #1472.

What we could do is include a defer in there to clean up the directory regardless of whether the operation succeeded or not.

Should be a simple issue to solve for those looking to contribute.
@bacongobbler
Copy link
Member

@bacongobbler bacongobbler commented Aug 14, 2020 •
edited

like so:

helm/pkg/action/lint.go

Line 87 in d0422e6

defer os.RemoveAll(tempDir)

and here's where that code resides:

helm/pkg/downloader/manager.go

Line 247 in d481bc6

tmpPath := filepath.Join(m.ChartPath, "tmpcharts")

@vikkyomkar
Copy link

@vikkyomkar vikkyomkar commented Aug 14, 2020

I would like to contribute to this issue. Pls let me know if its available to pick ?
@bacongobbler
Copy link
Member

@bacongobbler bacongobbler commented Aug 14, 2020

go ahead.
@tnleeuw
Copy link

@tnleeuw tnleeuw commented Aug 17, 2020

I get this error all the time trying to run helm, both with version 2.14.3 (which is what we have installed in our cluster) and when using the latest locally (v3.3.0). It is something with my system perhaps -- but it happens always, even though the directory tmpcharts does not exist before I run Helm.
The directory charts is created properly when it doesn't yet exist.

Creating a directory with a unique name, instead of assuming that the fixed name tmpcharts does not exist, is perhaps a more robust solution.

(Meanwhile I'm looking for the solution to my local issue)
@vikkyomkar vikkyomkar mentioned this issue Aug 19, 2020
Closed
@deathname
Copy link

@deathname deathname commented Oct 3, 2020

Can I work on this issue
@hickeyma
Copy link
Contributor

@hickeyma hickeyma commented Oct 5, 2020

@deathname Maybe ask @vikkyomkar if no longer working on it?
@vikkyomkar
Copy link

@vikkyomkar vikkyomkar commented Oct 5, 2020

Can I work on this issue

Pls go ahead. I dont have bandwidth for now
@deathname deathname mentioned this issue Oct 5, 2020
Open
2 of 2 tasks complete
@deathname
Copy link

@deathname deathname commented Oct 6, 2020

@hickeyma can you please have a look at #8846 created a PR for the fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug good first issue
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

added a line to remove tmpPath
You can’t perform that action at this time.