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

Lock bootstrap data with empty key to prevent conflicts #7215

Merged
merged 1 commit into from
Apr 5, 2023

Conversation

brandond
Copy link
Member

@brandond brandond commented Apr 5, 2023

Proposed Changes

Lock bootstrap data with empty key to prevent conflicts

Types of Changes

Bugfix

Verification

This has been hard to reproduce on demand, as the timing involved is very tight. Basic test would be to launch a multi-server cluster using an external SQL DB where all the servers are started as close to simultaneously as possible.

Testing

Linked Issues

User-Facing Change

When using an external datastore, K3s now locks the bootstrap key while creating initial cluster bootstrap data, preventing a race condition when multiple servers attempted to initialize the cluster simultaneously.

Further Comments

@brandond brandond requested a review from a team as a code owner April 5, 2023 01:08
@codecov-commenter
Copy link

Codecov Report

Patch coverage has no change and project coverage change: -0.02 ⚠️

Comparison is base (2992477) 9.84% compared to head (4b8b7a9) 9.83%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff            @@
##           master   #7215      +/-   ##
=========================================
- Coverage    9.84%   9.83%   -0.02%     
=========================================
  Files         147     147              
  Lines       10819   10831      +12     
=========================================
  Hits         1065    1065              
- Misses       9532    9544      +12     
  Partials      222     222              
Flag Coverage Δ
unittests 9.83% <0.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/cluster/bootstrap.go 0.00% <0.00%> (ø)
pkg/cluster/storage.go 0.00% <0.00%> (ø)
pkg/etcd/etcd.go 14.54% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@brandond brandond force-pushed the fix_bootstrap_race branch 2 times, most recently from ae574ca to 9748c4f Compare April 5, 2023 09:34
Comment on lines -276 to -298
ticker := time.NewTicker(5 * time.Second)
defer ticker.Stop()

RETRY:
for {
value, c.saveBootstrap, err = getBootstrapKeyFromStorage(ctx, storageClient, normalizedToken, token)
if err != nil {
if strings.Contains(err.Error(), "not supported for learner") {
for range ticker.C {
continue RETRY
}

}
return err
}
if value == nil {
return nil
}

dbRawData, err = decrypt(normalizedToken, value.Data)
if err != nil {
return err
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I moved this whole ugly for/continue loop into a wait.PollImmediateUntilWithContext in the getBootstrapKeyFromStorage function itself.

@brandond brandond force-pushed the fix_bootstrap_race branch from 9748c4f to 81f3968 Compare April 5, 2023 09:38
@brandond brandond force-pushed the fix_bootstrap_race branch from 81f3968 to 6f84acd Compare April 5, 2023 09:54
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.

4 participants