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

fix: call GatherEnvironmentInfo once in NodeAddToClusterMulti to avoid race condition causing error #1411

Merged

Conversation

ligfx
Copy link
Contributor

@ligfx ligfx commented Feb 21, 2024

fixes #1410

When multiple nodes are created, each call to NodeAddToCluster was calling GatherEnvironmentInfo, which was calling EnsureToolsNode() and then deleting the tools node. This causes some type of race condition. The work doesn't need to be done multiple times anyways, so just move it up to NodeAddToClusterMulti and do it once, before kicking off the goroutines.

(Also see #924 and #952 for previous similar issue)

@iwilltry42 iwilltry42 force-pushed the fix_1410_node_create_race_condition_tools_node branch from 3b6c866 to 94a14f9 Compare April 9, 2024 15:01
@iwilltry42 iwilltry42 self-requested a review July 4, 2024 09:33
Copy link
Member

@iwilltry42 iwilltry42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one comment, otherwise LGTM - Thank you!

pkg/client/node.go Outdated Show resolved Hide resolved
…d race condition causing error

fixes k3d-io#1410

When multiple nodes are created, each call to NodeAddToCluster was calling GatherEnvironmentInfo,
which was calling EnsureToolsNode() and then deleting the tools node. This causes some type of race
condition. The work doesn't need to be done multiple times anyways, so just move it up to
NodeAddToClusterMulti and do it once, before kicking off the goroutines.
In order to make it a non-breaking change for anyone import k3d in code and using this
function directly, per https://github.com/k3d-io/k3d/pull/1411/files#r1665464968
@ligfx ligfx force-pushed the fix_1410_node_create_race_condition_tools_node branch from 94a14f9 to 7ea2bb5 Compare July 10, 2024 14:15
@ligfx ligfx requested a review from iwilltry42 July 10, 2024 14:19
@iwilltry42
Copy link
Member

LGTM!
Thank you @ligfx !

@iwilltry42 iwilltry42 merged commit 0bedbc2 into k3d-io:main Jul 10, 2024
@iwilltry42 iwilltry42 added this to the v5.8.0 milestone Jul 10, 2024
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.

[BUG] removal of container k3d-tools is already in progress when adding nodes with replicas
2 participants