Skip to content

Download remote chart to Archive dir #30

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

Merged
merged 1 commit into from
Feb 28, 2018

Conversation

codenio
Copy link
Contributor

@codenio codenio commented Feb 20, 2018

Downloading remote chart to Archive dir (~/.helm/cache/archive) would keep the current dir clean.

@elemental-lf
Copy link
Contributor

Sounds like a good idea to prevent clutter. Could there be any issues when running multiple helm instances in parallel? I haven't looked into DownloadTo to see if it does any locking or so.

@codenio
Copy link
Contributor Author

codenio commented Feb 22, 2018

I just made it to mimics the way, Helm works. It works fine to me (as am using them often)
BTW what do you mean by running multiple helm instances in parallel? could you be brief?

@elemental-lf
Copy link
Contributor

@aananthraj I mean multiple instances of helm running under the same user account at the same time and so using the same archive directory. I haven't looked into the Helm source code if it does anything to prevent races between different processes when both try to download the same chart to the archive directory at the same time and so possibly corrupting the file or leading to failed verifications of the chart. If locking is done in downloadTo we'd be fine. If this isn't the case then we'd need to mimic the locking logic in helm diff, too, when writing to the shared archive directory.

@codenio
Copy link
Contributor Author

codenio commented Feb 25, 2018

@elemental-lf , That was nice. Thanks..!!.

I have a query.

when both try to download the same chart to the archive directory at the same time and so possibly corrupting the file or leading to failed verifications of the chart.

This could happen in PWD as well right..?, I mean when charts are downloaded to current working directory.

upon inspection codes of helm and helm diff plugin, I would like to share the following infrence.

helm diff plugin uses DownloadTo func from Helm's official k8s.io/helm/pkg/downloader package.

If this isn't the case then we'd need to mimic the locking logic in helm diff, too, when writing to the shared archive directory

so logic would be exactly the same.

@elemental-lf
Copy link
Contributor

This would also happen with the download to the cwd. But if there is a locking problem (at all), then it would be confined. I took a quick look at Helm now and I don't see any looking, so we're probably fine (or not worse than Helm itself).

@codenio
Copy link
Contributor Author

codenio commented Feb 28, 2018

ya, exactly 😄 , can this be merged..? now.

@databus23 databus23 merged commit 1d3b781 into databus23:master Feb 28, 2018
@databus23
Copy link
Owner

Thanks!

@codenio
Copy link
Contributor Author

codenio commented Mar 1, 2018

Thanks @databus23 and @elemental-lf .
It would be helpful , If we could work together on PR #22 and #24 as well.

@codenio codenio deleted the bug-fix/fix-remote-repo-access branch April 26, 2018 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants