From c0874e2a7eaf44af672e089faa822b672124a135 Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Wed, 18 Sep 2019 13:18:15 -0700 Subject: [PATCH 1/4] Add --interactive flag support --- cmd/minikube/cmd/start.go | 31 ++----- pkg/drivers/drivers.go | 166 +++++++++++++++++++++--------------- pkg/drivers/drivers_test.go | 8 +- pkg/minikube/out/style.go | 2 +- 4 files changed, 108 insertions(+), 99 deletions(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index f8c46436695a..a868cdfb1202 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -105,6 +105,7 @@ const ( hostDNSResolver = "host-dns-resolver" waitUntilHealthy = "wait" force = "force" + interactive = "interactive" waitTimeout = "wait-timeout" nativeSSH = "native-ssh" ) @@ -140,6 +141,7 @@ func initMinikubeFlags() { viper.AutomaticEnv() startCmd.Flags().Bool(force, false, "Force minikube to perform possibly dangerous operations") + startCmd.Flags().Bool(interactive, true, "Allow user prompts for more information") startCmd.Flags().Int(cpus, constants.DefaultCPUS, "Number of CPUs allocated to the minikube VM.") startCmd.Flags().String(memory, constants.DefaultMemorySize, "Amount of RAM allocated to the minikube VM (format: [], where unit = b, k, m or g).") @@ -290,7 +292,10 @@ func runStart(cmd *cobra.Command, args []string) { validateFlags(driver) validateUser(driver) - installOrUpdateDriver(driver) + if err := drivers.InstallOrUpdate(driver, viper.GetBool(interactive)); err != nil { + glog.Errorf("error: %v", err) + out.WarningT("Unable to update {{.driver}} driver: {{.error}}", out.V{"driver": driver, "error": err}) + } k8sVersion, isUpgrade := getKubernetesVersion(oldConfig) config, err := generateCfgFromFlags(cmd, k8sVersion, driver) @@ -1055,27 +1060,3 @@ func configureMounts() { func saveConfig(clusterCfg *cfg.Config) error { return cfg.CreateProfile(viper.GetString(cfg.MachineProfile), clusterCfg) } - -func installOrUpdateDriver(driver string) { - var driverExecutable string - switch driver { - case constants.DriverKvm2: - driverExecutable = fmt.Sprintf("docker-machine-driver-%s", constants.DriverKvm2) - case constants.DriverHyperkit: - driverExecutable = fmt.Sprintf("docker-machine-driver-%s", constants.DriverHyperkit) - default: // driver doesn't install or update - return - } - - minikubeVersion, err := version.GetSemverVersion() - if err != nil { - out.WarningT("Error parsing minikube version: {{.error}}", out.V{"error": err}) - return - } - - targetDir := constants.MakeMiniPath("bin") - err = drivers.InstallOrUpdate(driverExecutable, targetDir, minikubeVersion) - if err != nil { - out.WarningT("Error downloading driver: {{.error}}", out.V{"error": err}) - } -} diff --git a/pkg/drivers/drivers.go b/pkg/drivers/drivers.go index 52913dc925ee..9a227ede1097 100644 --- a/pkg/drivers/drivers.go +++ b/pkg/drivers/drivers.go @@ -22,7 +22,6 @@ import ( "io/ioutil" "os" "os/exec" - "path" "path/filepath" "regexp" "strings" @@ -38,6 +37,7 @@ import ( "github.com/pkg/errors" "k8s.io/minikube/pkg/version" + "k8s.io/minikube/pkg/minikube/constants" "k8s.io/minikube/pkg/minikube/out" "k8s.io/minikube/pkg/util" ) @@ -123,14 +123,14 @@ func MakeDiskImage(d *drivers.BaseDriver, boot2dockerURL string, diskSize int) e return errors.Wrapf(err, "createRawDiskImage(%s)", diskPath) } machPath := d.ResolveStorePath(".") - if err := fixPermissions(machPath); err != nil { + if err := fixMachinePermissions(machPath); err != nil { return errors.Wrapf(err, "fixing permissions on %s", machPath) } } return nil } -func fixPermissions(path string) error { +func fixMachinePermissions(path string) error { glog.Infof("Fixing permissions on %s ...", path) if err := os.Chown(path, syscall.Getuid(), syscall.Getegid()); err != nil { return errors.Wrap(err, "chown dir") @@ -149,44 +149,110 @@ func fixPermissions(path string) error { } // InstallOrUpdate downloads driver if it is not present, or updates it if there's a newer version -func InstallOrUpdate(driver, destination string, v semver.Version) error { - glog.Infof("InstallOrUpdate(%s): dest=%s, version=%s, PATH=%s", driver, destination, v, os.Getenv("PATH")) +func InstallOrUpdate(driver string, interactive bool) error { + if driver != constants.DriverKvm2 && driver != constants.DriverHyperkit { + return nil + } - _, err := exec.LookPath(driver) - // if file driver doesn't exist, download it + v, err := version.GetSemverVersion() if err != nil { - glog.Infof("LookPath %s: %v", driver, err) - return download(driver, v, destination) + out.WarningT("Error parsing minikube version: {{.error}}", out.V{"error": err}) + return err } - cmd := exec.Command(driver, "version") - output, err := cmd.Output() - // if driver doesnt support 'version', it is old, download it + executable := fmt.Sprintf("docker-machine-driver-%s", driver) + path, err := validateDriver(executable, v) if err != nil { - glog.Infof("%s version: %v", driver, err) - return download(driver, v, destination) + glog.Warningf("%s: %v", driver, executable) + path = filepath.Join(constants.MakeMiniPath("bin"), executable) + derr := download(executable, path, v) + if derr != nil { + return derr + } + } + return fixDriverPermissions(driver, path, interactive) +} + +// fixDriverPermissions fixes the permissions on a driver +func fixDriverPermissions(driver string, path string, interactive bool) error { + // Everything is easy but hyperkit + if driver != constants.DriverHyperkit { + return os.Chmod(path, 0755) + } + + // Hyperkit land + info, err := os.Stat(path) + if err != nil { + return err + } + + cmds := []*exec.Cmd{} + owner := info.Sys().(*syscall.Stat_t).Uid + m := info.Mode() + glog.Infof("%s owner: %d - permissions: %s", path, owner, m) + if owner != 0 { + cmds = append(cmds, exec.Command("sudo", "chown", "root:wheel", path)) + } + + if m&os.ModeSetuid == 0 { + cmds = append(cmds, exec.Command("sudo", "chmod", "u+s", path)) } - ev := ExtractVMDriverVersion(string(output)) + // No work to be done! + if len(cmds) == 0 { + return nil + } + + var example strings.Builder + for _, c := range cmds { + example.WriteString(fmt.Sprintf(" $ %s \n", strings.Join(c.Args, " "))) + } + + out.T(out.Permissions, "The '{{.driver}}' driver requires elevated permissions. The following commands will be executed:\n\n{{ .example }}\n", out.V{"driver": driver, "example": example.String()}) + for _, c := range cmds { + testArgs := append([]string{"-n"}, c.Args[1:]...) + test := exec.Command("sudo", testArgs...) + glog.Infof("testing: %v", test.Args) + if err := test.Run(); err != nil { + glog.Infof("%v may require a password: %v", c.Args, err) + if !interactive { + return fmt.Errorf("%v requires a password, and --interactive=false", c.Args) + } + } + glog.Infof("running: %v", c.Args) + err := c.Run() + if err != nil { + return errors.Wrapf(err, "%v", c.Args) + } + } + return nil +} + +// validateDriver validates if a driver appears to be up-to-date and installed properly +func validateDriver(driver string, v semver.Version) (string, error) { + path, err := exec.LookPath(driver) + if err != nil { + return path, err + } - // if the driver doesn't return any version, download it + output, err := exec.Command(path, "version").Output() + if err != nil { + return path, err + } + + ev := extractVMDriverVersion(string(output)) if len(ev) == 0 { - glog.Infof("%s: unable to extract version from %q", driver, output) - return download(driver, v, destination) + return path, fmt.Errorf("%s: unable to extract version from %q", driver, output) } vmDriverVersion, err := semver.Make(ev) if err != nil { - return errors.Wrap(err, "can't parse driver version") + return path, errors.Wrap(err, "can't parse driver version") } - - // if the current driver version is older, download newer if vmDriverVersion.LT(v) { - glog.Infof("%s is version %s, want %s", driver, vmDriverVersion, v) - return download(driver, v, destination) + return path, fmt.Errorf("%s is version %s, want %s", driver, vmDriverVersion, v) } - - return nil + return path, nil } func driverWithChecksumURL(driver string, v semver.Version) string { @@ -194,50 +260,31 @@ func driverWithChecksumURL(driver string, v semver.Version) string { return fmt.Sprintf("%s?checksum=file:%s.sha256", base, base) } -func download(driver string, v semver.Version, destination string) error { - // supports kvm2 and hyperkit - if driver != "docker-machine-driver-kvm2" && driver != "docker-machine-driver-hyperkit" { - return nil - } - +// download an arbitrary driver +func download(driver string, destination string, v semver.Version) error { out.T(out.FileDownload, "Downloading driver {{.driver}}:", out.V{"driver": driver}) - targetFilepath := path.Join(destination, driver) - os.Remove(targetFilepath) + os.Remove(destination) url := driverWithChecksumURL(driver, v) client := &getter.Client{ Src: url, - Dst: targetFilepath, + Dst: destination, Mode: getter.ClientModeFile, Options: []getter.ClientOption{getter.WithProgress(util.DefaultProgressBar)}, } glog.Infof("Downloading: %+v", client) - if err := client.Get(); err != nil { return errors.Wrapf(err, "download failed: %s", url) } - - err := os.Chmod(targetFilepath, 0755) - if err != nil { - return errors.Wrap(err, "chmod error") - } - - if driver == "docker-machine-driver-hyperkit" { - err := setHyperKitPermissions(targetFilepath) - if err != nil { - return errors.Wrap(err, "setting hyperkit permission") - } - } - return nil } -// ExtractVMDriverVersion extracts the driver version. +// extractVMDriverVersion extracts the driver version. // KVM and Hyperkit drivers support the 'version' command, that display the information as: // version: vX.X.X // commit: XXXX // This method returns the version 'vX.X.X' or empty if the version isn't found. -func ExtractVMDriverVersion(s string) string { +func extractVMDriverVersion(s string) string { versionRegex := regexp.MustCompile(`version:(.*)`) matches := versionRegex.FindStringSubmatch(s) @@ -248,22 +295,3 @@ func ExtractVMDriverVersion(s string) string { v := strings.TrimSpace(matches[1]) return strings.TrimPrefix(v, version.VersionPrefix) } - -func setHyperKitPermissions(driverPath string) error { - msg := fmt.Sprintf("A new hyperkit driver was installed. It needs elevated permissions to run. The following commands will be executed:\n\n $ sudo chown root:wheel %s\n $ sudo chmod u+s %s\n", driverPath, driverPath) - out.T(out.Permissions, msg, out.V{}) - - cmd := exec.Command("sudo", "chown", "root:wheel", driverPath) - err := cmd.Run() - if err != nil { - return errors.Wrap(err, "chown root:wheel") - } - - cmd = exec.Command("sudo", "chmod", "u+s", driverPath) - err = cmd.Run() - if err != nil { - return errors.Wrap(err, "chmod u+s") - } - - return nil -} diff --git a/pkg/drivers/drivers_test.go b/pkg/drivers/drivers_test.go index c4d9aafe03b0..10e42e3e4466 100644 --- a/pkg/drivers/drivers_test.go +++ b/pkg/drivers/drivers_test.go @@ -50,24 +50,24 @@ func Test_createDiskImage(t *testing.T) { } func TestExtractVMDriverVersion(t *testing.T) { - v := ExtractVMDriverVersion("") + v := extractVMDriverVersion("") if len(v) != 0 { t.Error("Expected empty string") } - v = ExtractVMDriverVersion("random text") + v = extractVMDriverVersion("random text") if len(v) != 0 { t.Error("Expected empty string") } expectedVersion := "1.2.3" - v = ExtractVMDriverVersion("version: v1.2.3") + v = extractVMDriverVersion("version: v1.2.3") if expectedVersion != v { t.Errorf("Expected version: %s, got: %s", expectedVersion, v) } - v = ExtractVMDriverVersion("version: 1.2.3") + v = extractVMDriverVersion("version: 1.2.3") if expectedVersion != v { t.Errorf("Expected version: %s, got: %s", expectedVersion, v) } diff --git a/pkg/minikube/out/style.go b/pkg/minikube/out/style.go index 44ae64efe8f6..4791787413b6 100644 --- a/pkg/minikube/out/style.go +++ b/pkg/minikube/out/style.go @@ -78,7 +78,7 @@ var styles = map[StyleEnum]style{ Documentation: {Prefix: "📘 "}, Issues: {Prefix: "⁉️ "}, Issue: {Prefix: " ▪ ", LowPrefix: lowIndent}, // Indented bullet - Check: {Prefix: "✔️ "}, + Check: {Prefix: "✅ "}, Celebration: {Prefix: "🎉 "}, Workaround: {Prefix: "👉 ", LowPrefix: lowIndent}, From 197039c23d9dc76d8fc2ffa71cd3d0a403bb8f85 Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Wed, 18 Sep 2019 13:46:10 -0700 Subject: [PATCH 2/4] Fix tests --- cmd/minikube/cmd/start.go | 12 +++++++++--- pkg/drivers/drivers.go | 10 ++-------- test/integration/driver_install_or_update_test.go | 4 ++-- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 88fac543cae5..fbeb406e89ac 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -292,9 +292,15 @@ func runStart(cmd *cobra.Command, args []string) { validateFlags(driver) validateUser(driver) - if err := drivers.InstallOrUpdate(driver, viper.GetBool(interactive)); err != nil { - glog.Errorf("error: %v", err) - out.WarningT("Unable to update {{.driver}} driver: {{.error}}", out.V{"driver": driver, "error": err}) + + v, err := version.GetSemverVersion() + if err != nil { + out.WarningT("Error parsing minikube version: {{.error}}", out.V{"error": err}) + } else { + if err := drivers.InstallOrUpdate(driver, constants.MakeMiniPath("bin"), v, viper.GetBool(interactive)); err != nil { + glog.Errorf("error: %v", err) + out.WarningT("Unable to update {{.driver}} driver: {{.error}}", out.V{"driver": driver, "error": err}) + } } k8sVersion, isUpgrade := getKubernetesVersion(oldConfig) diff --git a/pkg/drivers/drivers.go b/pkg/drivers/drivers.go index 9a227ede1097..710c3eea890e 100644 --- a/pkg/drivers/drivers.go +++ b/pkg/drivers/drivers.go @@ -149,22 +149,16 @@ func fixMachinePermissions(path string) error { } // InstallOrUpdate downloads driver if it is not present, or updates it if there's a newer version -func InstallOrUpdate(driver string, interactive bool) error { +func InstallOrUpdate(driver string, directory string, v semver.Version, interactive bool) error { if driver != constants.DriverKvm2 && driver != constants.DriverHyperkit { return nil } - v, err := version.GetSemverVersion() - if err != nil { - out.WarningT("Error parsing minikube version: {{.error}}", out.V{"error": err}) - return err - } - executable := fmt.Sprintf("docker-machine-driver-%s", driver) path, err := validateDriver(executable, v) if err != nil { glog.Warningf("%s: %v", driver, executable) - path = filepath.Join(constants.MakeMiniPath("bin"), executable) + path = filepath.Join(directory, executable) derr := download(executable, path, v) if derr != nil { return derr diff --git a/test/integration/driver_install_or_update_test.go b/test/integration/driver_install_or_update_test.go index 682b872d0c13..933f250d4ecc 100644 --- a/test/integration/driver_install_or_update_test.go +++ b/test/integration/driver_install_or_update_test.go @@ -84,7 +84,7 @@ func TestKVMDriverInstallOrUpdate(t *testing.T) { t.Fatalf("Expected new semver. test: %v, got: %v", tc.name, err) } - err = drivers.InstallOrUpdate("docker-machine-driver-kvm2", dir, newerVersion) + err = drivers.InstallOrUpdate("docker-machine-driver-kvm2", dir, newerVersion, true) if err != nil { t.Fatalf("Failed to update driver to %v. test: %s, got: %v", newerVersion, tc.name, err) } @@ -147,7 +147,7 @@ func TestHyperKitDriverInstallOrUpdate(t *testing.T) { t.Fatalf("Expected new semver. test: %v, got: %v", tc.name, err) } - err = drivers.InstallOrUpdate("docker-machine-driver-hyperkit", dir, newerVersion) + err = drivers.InstallOrUpdate("docker-machine-driver-hyperkit", dir, newerVersion, true) if err != nil { t.Fatalf("Failed to update driver to %v. test: %s, got: %v", newerVersion, tc.name, err) } From 19f3533c937714a4c1d25af9d72c42690eb0575f Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Wed, 18 Sep 2019 14:24:18 -0700 Subject: [PATCH 3/4] Use find instead of syscall_t because Go --- Makefile | 4 ++-- pkg/drivers/drivers.go | 38 +++++++++++++++----------------------- 2 files changed, 17 insertions(+), 25 deletions(-) diff --git a/Makefile b/Makefile index 3fa9ff14c8f7..b4125489bd62 100755 --- a/Makefile +++ b/Makefile @@ -15,8 +15,8 @@ # Bump these on release - and please check ISO_VERSION for correctness. VERSION_MAJOR ?= 1 VERSION_MINOR ?= 4 -VERSION_BUILD ?= 0 -RAW_VERSION=$(VERSION_MAJOR).$(VERSION_MINOR).0 +VERSION_BUILD ?= 0-beta.2 +RAW_VERSION=$(VERSION_MAJOR).$(VERSION_MINOR).${VERSION_BUILD} VERSION ?= v$(RAW_VERSION) # Default to .0 for higher cache hit rates, as build increments typically don't require new ISO versions diff --git a/pkg/drivers/drivers.go b/pkg/drivers/drivers.go index 710c3eea890e..da7edaf2d833 100644 --- a/pkg/drivers/drivers.go +++ b/pkg/drivers/drivers.go @@ -157,7 +157,7 @@ func InstallOrUpdate(driver string, directory string, v semver.Version, interact executable := fmt.Sprintf("docker-machine-driver-%s", driver) path, err := validateDriver(executable, v) if err != nil { - glog.Warningf("%s: %v", driver, executable) + glog.Warningf("%s: %v", executable, err) path = filepath.Join(directory, executable) derr := download(executable, path, v) if derr != nil { @@ -169,32 +169,22 @@ func InstallOrUpdate(driver string, directory string, v semver.Version, interact // fixDriverPermissions fixes the permissions on a driver func fixDriverPermissions(driver string, path string, interactive bool) error { - // Everything is easy but hyperkit + // This method only supports hyperkit so far (because it's complicated) if driver != constants.DriverHyperkit { - return os.Chmod(path, 0755) - } - - // Hyperkit land - info, err := os.Stat(path) - if err != nil { - return err - } - - cmds := []*exec.Cmd{} - owner := info.Sys().(*syscall.Stat_t).Uid - m := info.Mode() - glog.Infof("%s owner: %d - permissions: %s", path, owner, m) - if owner != 0 { - cmds = append(cmds, exec.Command("sudo", "chown", "root:wheel", path)) + return nil } - if m&os.ModeSetuid == 0 { - cmds = append(cmds, exec.Command("sudo", "chmod", "u+s", path)) + // Using the find command for hyperkit is far easier than cross-platform uid checks in Go. + stdout, err := exec.Command("find", path, "-uid", "0", "-perm", "4755").Output() + glog.Infof("stdout: %s", stdout) + if err == nil && strings.TrimSpace(string(stdout)) == path { + glog.Infof("%s looks good", path) + return nil } - // No work to be done! - if len(cmds) == 0 { - return nil + cmds := []*exec.Cmd{ + exec.Command("sudo", "chown", "root:wheel", path), + exec.Command("sudo", "chmod", "u+s", path), } var example strings.Builder @@ -224,6 +214,7 @@ func fixDriverPermissions(driver string, path string, interactive bool) error { // validateDriver validates if a driver appears to be up-to-date and installed properly func validateDriver(driver string, v semver.Version) (string, error) { + glog.Infof("Validating %s, PATH=%s", driver, os.Getenv("PATH")) path, err := exec.LookPath(driver) if err != nil { return path, err @@ -270,7 +261,8 @@ func download(driver string, destination string, v semver.Version) error { if err := client.Get(); err != nil { return errors.Wrapf(err, "download failed: %s", url) } - return nil + // Give downloaded drivers a baseline decent file permission + return os.Chmod(destination, 0755) } // extractVMDriverVersion extracts the driver version. From d0bff3872d0709ab373b1442033d4f4a62abce6c Mon Sep 17 00:00:00 2001 From: tstromberg Date: Wed, 18 Sep 2019 15:08:53 -0700 Subject: [PATCH 4/4] Pass driver name rather than executable name --- test/integration/driver_install_or_update_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/driver_install_or_update_test.go b/test/integration/driver_install_or_update_test.go index 933f250d4ecc..de11940873cb 100644 --- a/test/integration/driver_install_or_update_test.go +++ b/test/integration/driver_install_or_update_test.go @@ -84,7 +84,7 @@ func TestKVMDriverInstallOrUpdate(t *testing.T) { t.Fatalf("Expected new semver. test: %v, got: %v", tc.name, err) } - err = drivers.InstallOrUpdate("docker-machine-driver-kvm2", dir, newerVersion, true) + err = drivers.InstallOrUpdate("kvm2", dir, newerVersion, true) if err != nil { t.Fatalf("Failed to update driver to %v. test: %s, got: %v", newerVersion, tc.name, err) } @@ -147,7 +147,7 @@ func TestHyperKitDriverInstallOrUpdate(t *testing.T) { t.Fatalf("Expected new semver. test: %v, got: %v", tc.name, err) } - err = drivers.InstallOrUpdate("docker-machine-driver-hyperkit", dir, newerVersion, true) + err = drivers.InstallOrUpdate("hyperkit", dir, newerVersion, true) if err != nil { t.Fatalf("Failed to update driver to %v. test: %s, got: %v", newerVersion, tc.name, err) }