Skip to content

avoid race condition in bootstrap certs for parallel runs #10118

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

Merged
merged 8 commits into from
Jan 13, 2021
Merged

avoid race condition in bootstrap certs for parallel runs #10118

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4b8b2bc
Fix flackness of docker-env in parrallel
azhao155 Jan 9, 2021
baf929b
Add file lock for protect certs generation
azhao155 Jan 13, 2021
cbf602e
change unlock to defer
azhao155 Jan 13, 2021
948c1b5
check unlock return value
azhao155 Jan 13, 2021
376399c
Change naming of lock and log format
azhao155 Jan 13, 2021
bbbf0d9
Add issue link and comment
azhao155 Jan 13, 2021
4e368f0
Modify comment
azhao155 Jan 13, 2021
e933166
Modify again
azhao155 Jan 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ require (
github.com/johanneswuerbach/nfsexports v0.0.0-20200318065542-c48c3734757f
github.com/juju/clock v0.0.0-20190205081909-9c5c9712527c
github.com/juju/errors v0.0.0-20190806202954-0232dcc7464d // indirect
github.com/juju/fslock v0.0.0-20160525022230-4d5c94c67b4b
github.com/juju/loggo v0.0.0-20190526231331-6e530bcce5d8 // indirect
github.com/juju/mutex v0.0.0-20180619145857-d21b13acf4bf
github.com/juju/retry v0.0.0-20180821225755-9058e192b216 // indirect
Expand Down
2 changes: 2 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,8 @@ github.com/juju/clock v0.0.0-20190205081909-9c5c9712527c h1:3UvYABOQRhJAApj9MdCN
github.com/juju/clock v0.0.0-20190205081909-9c5c9712527c/go.mod h1:nD0vlnrUjcjJhqN5WuCWZyzfd5AHZAC9/ajvbSx69xA=
github.com/juju/errors v0.0.0-20190806202954-0232dcc7464d h1:hJXjZMxj0SWlMoQkzeZDLi2cmeiWKa7y1B8Rg+qaoEc=
github.com/juju/errors v0.0.0-20190806202954-0232dcc7464d/go.mod h1:W54LbzXuIE0boCoNJfwqpmkKJ1O4TCTZMetAt6jGk7Q=
github.com/juju/fslock v0.0.0-20160525022230-4d5c94c67b4b h1:FQ7+9fxhyp82ks9vAuyPzG0/vVbWwMwLJ+P6yJI5FN8=
github.com/juju/fslock v0.0.0-20160525022230-4d5c94c67b4b/go.mod h1:HMcgvsgd0Fjj4XXDkbjdmlbI505rUPBs6WBMYg2pXks=
github.com/juju/loggo v0.0.0-20190526231331-6e530bcce5d8 h1:UUHMLvzt/31azWTN/ifGWef4WUqvXk0iRqdhdy/2uzI=
github.com/juju/loggo v0.0.0-20190526231331-6e530bcce5d8/go.mod h1:vgyd7OREkbtVEN/8IXZe5Ooef3LQePvuBm9UWj6ZL8U=
github.com/juju/mutex v0.0.0-20180619145857-d21b13acf4bf h1:2d3cilQly1OpAfZcn4QRuwDOdVoHsM4cDTkcKbmO760=
Expand Down
21 changes: 20 additions & 1 deletion pkg/minikube/machine/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (
"github.com/docker/machine/libmachine/state"
"github.com/docker/machine/libmachine/swarm"
"github.com/docker/machine/libmachine/version"
"github.com/juju/fslock"
"github.com/pkg/errors"
"k8s.io/klog/v2"
"k8s.io/minikube/pkg/minikube/command"
Expand Down Expand Up @@ -71,6 +72,7 @@ func NewAPIClient(miniHome ...string) (libmachine.API, error) {
storePath: storePath,
Filestore: persist.NewFilestore(storePath, certsDir, certsDir),
legacyClient: NewRPCClient(storePath, certsDir),
flock: fslock.New(localpath.MakeMiniPath("machine_client.lock")),
}, nil
}

Expand All @@ -81,6 +83,7 @@ type LocalClient struct {
storePath string
*persist.Filestore
legacyClient libmachine.API
flock *fslock.Lock
}

// NewHost creates a new Host
Expand Down Expand Up @@ -183,7 +186,23 @@ func (api *LocalClient) Create(h *host.Host) error {
}{
{
"bootstrapping certificates",
func() error { return cert.BootstrapCertificates(h.AuthOptions()) },
func() error {
// Due to https://github.com/docker/machine/issues/3634, we need to have lock for certs generation, otherwise
// it will break Docker-Env test in parallel tesing.
// CA cert and client cert should be generated atomically, otherwise might cause bad certificate error.
lockErr := api.flock.LockWithTimeout(time.Second * 5)
if lockErr != nil {
return fmt.Errorf("failed to acquire bootstrap client lock: %v " + lockErr.Error())
}
defer func() {
lockErr = api.flock.Unlock()
if lockErr != nil {
klog.Errorf("falied to release bootstrap cert client lock: %v", lockErr.Error())
}
}()
certErr := cert.BootstrapCertificates(h.AuthOptions())
return certErr
},
},
{
"precreate",
Expand Down