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.

Sign up for GitHub

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

fix(helm_resource): no parallel in helm_repo #501

Closed
wants to merge 1 commit into from
Closed

fix(helm_resource): no parallel in helm_repo #501

wants to merge 1 commit into from

Conversation

nschmeller
Copy link

Multiple invocations of helm_repo may not be concurrency safe because
of the execution of helm repo add. This commit unsets the
allow_parallel argument in helm_repo to prevent this bug.

Resolves #500

@nschmeller nschmeller force-pushed the nschmeller/helm-repo-disallow-parallel branch 2 times, most recently from 657bc9b to fe7713b Compare May 25, 2023 23:18
@nschmeller
fix(helm_resource): no parallel in helm_repo
a76d9f7 
Multiple invocations of `helm_repo` may not be concurrency safe because
of the execution of `helm repo add`. This commit unsets the
`allow_parallel` argument in `helm_repo` to prevent this bug.

Resolves tilt-dev#500

Signed-off-by: Nick Schmeller <[email protected]>
@nschmeller nschmeller force-pushed the nschmeller/helm-repo-disallow-parallel branch from fe7713b to a76d9f7 Compare May 25, 2023 23:25
@nicks
Copy link
Member

nicks commented Jun 5, 2023

hmmmm....i played around with this a bit. i'm not so sure this is the problem. helm repo add seems pretty thread-safe to me, and is using a file lock - https://github.com/helm/helm/blob/main/cmd/helm/repo_add.go#L125

are you sure this isn't just a mix-up with resource_deps? i.e., the ci run that you were looking at didn't have resource_deps setup? how often are you seeing this happen?

@nschmeller
Copy link
Author

are you sure this isn't just a mix-up with resource_deps? i.e., the ci run that you were looking at didn't have resource_deps setup? how often are you seeing this happen?

Just the once, but I'm pretty sure the Tiltfile definition was correct (or at least tilt ci had worked on a clean setup before and after). Maybe I'll drop this for now and if I see it again then re-raise it?

@nicks
Copy link
Member

nicks commented Jul 13, 2023

hmmm very mysterious. ya, let's close for now. we can re-evaluate if we see it again and try to pin down a repro case.

@nicks nicks closed this Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

helm_resource did not wait for resource_deps on helm_repo
2 participants
@nschmeller @nicks