diff --git a/pkg/minikube/bootstrapper/certs.go b/pkg/minikube/bootstrapper/certs.go index 611b5416cce8..d0731b24cd01 100644 --- a/pkg/minikube/bootstrapper/certs.go +++ b/pkg/minikube/bootstrapper/certs.go @@ -70,7 +70,7 @@ func SetupCerts(cmd command.Runner, k8s config.KubernetesConfig) error { // // If another process updates the shared certificate, it's invalid. // TODO: Instead of racey manipulation of a shared certificate, use per-profile certs - spec := lock.UserMutexSpec(filepath.Join(localPath, "certs")) + spec := lock.PathMutexSpec(filepath.Join(localPath, "certs")) glog.Infof("acquiring lock: %+v", spec) releaser, err := mutex.Acquire(spec) if err != nil { diff --git a/pkg/minikube/kubeconfig/settings.go b/pkg/minikube/kubeconfig/settings.go index 0cd09ec542ec..85b8f7ca9d3f 100644 --- a/pkg/minikube/kubeconfig/settings.go +++ b/pkg/minikube/kubeconfig/settings.go @@ -119,7 +119,7 @@ func PopulateFromSettings(cfg *Settings, apiCfg *api.Config) error { // activeContext is true when minikube is the CurrentContext // If no CurrentContext is set, the given name will be used. func Update(kcs *Settings) error { - spec := lock.UserMutexSpec(filepath.Join(kcs.filePath(), "settings.Update")) + spec := lock.PathMutexSpec(filepath.Join(kcs.filePath(), "settings.Update")) glog.Infof("acquiring lock: %+v", spec) releaser, err := mutex.Acquire(spec) if err != nil { diff --git a/pkg/util/lock/lock.go b/pkg/util/lock/lock.go index 5083fa0d8cb4..8d2e833ab0cf 100644 --- a/pkg/util/lock/lock.go +++ b/pkg/util/lock/lock.go @@ -17,12 +17,10 @@ limitations under the License. package lock import ( + "crypto/sha1" "fmt" "io/ioutil" "os" - "os/user" - "regexp" - "strings" "time" "github.com/golang/glog" @@ -31,42 +29,27 @@ import ( "github.com/pkg/errors" ) -var ( - // nonString is characters to strip from lock names - nonString = regexp.MustCompile(`[\W_]+`) - // forceID is a user id for consistent testing - forceID = "" -) - // WriteFile decorates ioutil.WriteFile with a file lock and retry func WriteFile(filename string, data []byte, perm os.FileMode) error { - spec := UserMutexSpec(filename) - glog.Infof("acquiring lock for %s: %+v", filename, spec) + spec := PathMutexSpec(filename) + glog.Infof("WriteFile acquiring %s: %+v", filename, spec) releaser, err := mutex.Acquire(spec) if err != nil { - return errors.Wrapf(err, "error acquiring lock for %s", filename) + return errors.Wrapf(err, "failed to acquire lock for %s: %+v", filename, spec) } defer releaser.Release() if err = ioutil.WriteFile(filename, data, perm); err != nil { - return errors.Wrapf(err, "error writing file %s", filename) + return errors.Wrapf(err, "writefile failed for %s", filename) } return err } -// UserMutexSpec returns a mutex spec that will not collide with other users -func UserMutexSpec(path string) mutex.Spec { - id := forceID - if forceID == "" { - u, err := user.Current() - if err == nil { - id = u.Uid - } - } - +// PathMutexSpec returns a mutex spec for a path +func PathMutexSpec(path string) mutex.Spec { s := mutex.Spec{ - Name: getMutexNameForPath(fmt.Sprintf("%s-%s", path, id)), + Name: fmt.Sprintf("mk%x", sha1.Sum([]byte(path)))[0:40], Clock: clock.WallClock, // Poll the lock twice a second Delay: 500 * time.Millisecond, @@ -75,16 +58,3 @@ func UserMutexSpec(path string) mutex.Spec { } return s } - -func getMutexNameForPath(path string) string { - // juju requires that names match ^[a-zA-Z][a-zA-Z0-9-]*$", and be under 40 chars long. - n := strings.Trim(nonString.ReplaceAllString(path, "-"), "-") - // we need to always guarantee an alphanumeric prefix - prefix := "m" - - // Prefer the last 40 chars, as paths tend get more specific toward the end - if len(n) >= 40 { - return prefix + n[len(n)-39:] - } - return prefix + n -} diff --git a/pkg/util/lock/lock_test.go b/pkg/util/lock/lock_test.go index 8bb97d046578..8ec94e5f24b1 100644 --- a/pkg/util/lock/lock_test.go +++ b/pkg/util/lock/lock_test.go @@ -16,11 +16,13 @@ limitations under the License. package lock -import "testing" +import ( + "testing" -func TestUserMutexSpec(t *testing.T) { - forceID = "test" + "github.com/juju/mutex" +) +func TestUserMutexSpec(t *testing.T) { var tests = []struct { description string path string @@ -29,41 +31,53 @@ func TestUserMutexSpec(t *testing.T) { { description: "standard", path: "/foo/bar", - expected: "mfoo-bar-test", }, { description: "deep directory", path: "/foo/bar/baz/bat", - expected: "mfoo-bar-baz-bat-test", }, { description: "underscores", path: "/foo_bar/baz", - expected: "mfoo-bar-baz-test", }, { description: "starts with number", path: "/foo/2bar/baz", - expected: "mfoo-2bar-baz-test", }, { description: "starts with punctuation", path: "/.foo/bar", - expected: "mfoo-bar-test", }, { description: "long filename", path: "/very-very-very-very-very-very-very-very-long/bar", - expected: "m-very-very-very-very-very-long-bar-test", + }, + { + description: "Windows kubeconfig", + path: `C:\Users\admin/.kube/config`, + }, + { + description: "Windows json", + path: `C:\Users\admin\.minikube\profiles\containerd-20191210T212325.7356633-8584\config.json`, }, } + seen := map[string]string{} + for _, tc := range tests { t.Run(tc.description, func(t *testing.T) { - got := UserMutexSpec(tc.path) - if got.Name != tc.expected { - t.Errorf("%s mutex name = %q, expected %q", tc.path, got.Name, tc.expected) + got := PathMutexSpec(tc.path) + if len(got.Name) != 40 { + t.Errorf("%s is not 40 chars long", got.Name) + } + if seen[got.Name] != "" { + t.Fatalf("lock name collision between %s and %s", tc.path, seen[got.Name]) + } + m, err := mutex.Acquire(got) + if err != nil { + t.Errorf("acquire for spec %+v failed: %v", got, err) } + m.Release() }) } }