From 789dfca8045ce4f835bd04f9101ca353bc9069dc Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 11 Aug 2020 14:16:40 -0400 Subject: [PATCH 1/6] skip vaildations if --force is supplied --- cmd/minikube/cmd/start.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 2aa4668041b6..b3e19f655503 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -940,6 +940,10 @@ func validateCPUCount(drvName string) { // validateFlags validates the supplied flags against known bad combinations func validateFlags(cmd *cobra.Command, drvName string) { + if viper.GetBool(force) { + out.WarningT("minikube skips flag validations when --force is supplied; this may lead to unexpected behavior") + return + } if cmd.Flags().Changed(humanReadableDiskSize) { diskSizeMB, err := util.CalculateSizeInMB(viper.GetString(humanReadableDiskSize)) if err != nil { From 74309a83e61619bacd5ed76b6b7c2c073359d2e2 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 11 Aug 2020 14:22:10 -0400 Subject: [PATCH 2/6] include memory check --- cmd/minikube/cmd/start.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index b3e19f655503..e7121bb6e433 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -135,6 +135,9 @@ func runStart(cmd *cobra.Command, args []string) { } displayEnviron(os.Environ()) + if viper.GetBool(force) { + out.WarningT("minikube skips various validations when --force is supplied; this may lead to unexpected behavior") + } // if --registry-mirror specified when run minikube start, // take arg precedence over MINIKUBE_REGISTRY_MIRROR @@ -846,6 +849,9 @@ func validateMemoryHardLimit(drvName string) { // validateMemorySize validates the memory size matches the minimum recommended func validateMemorySize(req int, drvName string) { + if viper.GetBool(force) { + return + } sysLimit, containerLimit, err := memoryLimits(drvName) if err != nil { glog.Warningf("Unable to query memory limits: %v", err) @@ -941,7 +947,6 @@ func validateCPUCount(drvName string) { // validateFlags validates the supplied flags against known bad combinations func validateFlags(cmd *cobra.Command, drvName string) { if viper.GetBool(force) { - out.WarningT("minikube skips flag validations when --force is supplied; this may lead to unexpected behavior") return } if cmd.Flags().Changed(humanReadableDiskSize) { From 6ff308d89271be79e2ccd8c9ebff3a39eb01596a Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 11 Aug 2020 14:38:28 -0400 Subject: [PATCH 3/6] Run download only test with docker daemon unavailable --- test/integration/aaa_download_only_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/integration/aaa_download_only_test.go b/test/integration/aaa_download_only_test.go index 7c6940336365..61627f072fb5 100644 --- a/test/integration/aaa_download_only_test.go +++ b/test/integration/aaa_download_only_test.go @@ -165,7 +165,10 @@ func TestDownloadOnlyKic(t *testing.T) { args := []string{"start", "--download-only", "-p", profile, "--force", "--alsologtostderr"} args = append(args, StartArgs()...) - if _, err := Run(t, exec.CommandContext(ctx, Target(), args...)); err != nil { + cmd := exec.CommandContext(ctx, Target(), args...) + // make sure this works even if docker daemon isn't running + cmd.Env = append(os.Environ(), "DOCKER_HOST=/does/not/exist") + if _, err := Run(t, cmd); err != nil { t.Errorf("start with download only failed %q : %v", args, err) } From a69b41932c9a9ac8658e8d622623e1b211960c71 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 11 Aug 2020 14:56:19 -0400 Subject: [PATCH 4/6] improve error handling --- cmd/minikube/cmd/start.go | 66 +++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 38 deletions(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index e7121bb6e433..858c0690e8db 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -665,7 +665,7 @@ func validateDriver(ds registry.DriverState, existing *config.ClusterConfig) { } out.ErrLn("") - if !st.Installed && !viper.GetBool(force) { + if !st.Installed { if existing != nil { if old := hostDriver(existing); name == old { exit.WithCodeT(exit.Unavailable, "{{.driver}} does not appear to be installed, but is specified by an existing profile. Please run 'minikube delete' or install {{.driver}}", out.V{"driver": name}) @@ -673,10 +673,7 @@ func validateDriver(ds registry.DriverState, existing *config.ClusterConfig) { } exit.WithCodeT(exit.Unavailable, "{{.driver}} does not appear to be installed", out.V{"driver": name}) } - - if !viper.GetBool(force) { - exit.WithCodeT(exit.Unavailable, "Failed to validate '{{.driver}}' driver", out.V{"driver": name}) - } + skipErrorIfForceEnabled(exit.Unavailable, "Failed to validate '{{.driver}}' driver", out.V{"driver": name}) } } @@ -849,9 +846,6 @@ func validateMemoryHardLimit(drvName string) { // validateMemorySize validates the memory size matches the minimum recommended func validateMemorySize(req int, drvName string) { - if viper.GetBool(force) { - return - } sysLimit, containerLimit, err := memoryLimits(drvName) if err != nil { glog.Warningf("Unable to query memory limits: %v", err) @@ -862,11 +856,10 @@ func validateMemorySize(req int, drvName string) { // a more sane alternative to their high memory 80% minAdvised := 0.50 * float64(sysLimit) - if req < minUsableMem && !viper.GetBool(force) { - exit.WithCodeT(exit.Config, "Requested memory allocation {{.requested}}MB is less than the usable minimum of {{.minimum_memory}}MB", - out.V{"requested": req, "minimum_memory": minUsableMem}) + if req < minUsableMem { + skipErrorIfForceEnabled(exit.Config, "Requested memory allocation {{.requested}}MB is less than the usable minimum of {{.minimum_memory}}MB", out.V{"requested": req, "minimum_memory": minUsableMem}) } - if req < minRecommendedMem && !viper.GetBool(force) { + if req < minRecommendedMem { out.WarningT("Requested memory allocation ({{.requested}}MB) is less than the recommended minimum {{.recommended}}MB. Kubernetes may crash unexpectedly.", out.V{"requested": req, "recommended": minRecommendedMem}) } @@ -879,24 +872,21 @@ func validateMemorySize(req int, drvName string) { `, out.V{"container_limit": containerLimit, "system_limit": sysLimit}) } - if req > sysLimit && !viper.GetBool(force) { - out.T(out.Tip, "To suppress memory validations you can use --force flag.") - exit.WithCodeT(exit.Config, `Requested memory allocation {{.requested}}MB is more than your system limit {{.system_limit}}MB. Try specifying a lower memory: - - miniube start --memory={{.min_advised}}mb - -`, - out.V{"requested": req, "system_limit": sysLimit, "max_advised": int32(maxAdvised), "min_advised": minAdvised}) + if req > sysLimit { + message := `Requested memory allocation {{.requested}}MB is more than your system limit {{.system_limit}}MB. Try specifying a lower memory: + miniube start --memory={{.min_advised}}mb + +` + skipErrorIfForceEnabled(exit.Config, message, out.V{"requested": req, "system_limit": sysLimit, "max_advised": int32(maxAdvised), "min_advised": minAdvised}) } - if float64(req) > maxAdvised && !viper.GetBool(force) { + if float64(req) > maxAdvised { out.WarningT(`You are allocating {{.requested}}MB to memory and your system only has {{.system_limit}}MB. You might face issues. try specifying a lower memory: miniube start --memory={{.min_advised}}mb `, out.V{"requested": req, "system_limit": sysLimit, "min_advised": minAdvised}) - out.T(out.Tip, "To suppress and ignore this warning you can use --force flag.") } } @@ -915,8 +905,8 @@ func validateCPUCount(drvName string) { } else { cpuCount = viper.GetInt(cpus) } - if cpuCount < minimumCPUS && !viper.GetBool(force) { - exit.UsageT("Requested cpu count {{.requested_cpus}} is less than the minimum allowed of {{.minimum_cpus}}", out.V{"requested_cpus": cpuCount, "minimum_cpus": minimumCPUS}) + if cpuCount < minimumCPUS { + skipErrorIfForceEnabled(exit.BadUsage, "Requested cpu count {{.requested_cpus}} is less than the minimum allowed of {{.minimum_cpus}}", out.V{"requested_cpus": cpuCount, "minimum_cpus": minimumCPUS}) } if driver.IsKIC((drvName)) { @@ -938,25 +928,22 @@ func validateCPUCount(drvName string) { `) } out.T(out.Documentation, "https://docs.docker.com/config/containers/resource_constraints/") - exit.UsageT("Ensure your {{.driver_name}} system has enough CPUs. The minimum allowed is 2 CPUs.", out.V{"driver_name": driver.FullName(viper.GetString("driver"))}) - + skipErrorIfForceEnabled(exit.BadUsage, "Ensure your {{.driver_name}} system has enough CPUs. The minimum allowed is 2 CPUs.", out.V{"driver_name": driver.FullName(viper.GetString("driver"))}) } } } // validateFlags validates the supplied flags against known bad combinations func validateFlags(cmd *cobra.Command, drvName string) { - if viper.GetBool(force) { - return - } + if cmd.Flags().Changed(humanReadableDiskSize) { diskSizeMB, err := util.CalculateSizeInMB(viper.GetString(humanReadableDiskSize)) if err != nil { - exit.WithCodeT(exit.Config, "Validation unable to parse disk size '{{.diskSize}}': {{.error}}", out.V{"diskSize": viper.GetString(humanReadableDiskSize), "error": err}) + skipErrorIfForceEnabled(exit.Config, "Validation unable to parse disk size '{{.diskSize}}': {{.error}}", out.V{"diskSize": viper.GetString(humanReadableDiskSize), "error": err}) } - if diskSizeMB < minimumDiskSize && !viper.GetBool(force) { - exit.WithCodeT(exit.Config, "Requested disk size {{.requested_size}} is less than minimum of {{.minimum_size}}", out.V{"requested_size": diskSizeMB, "minimum_size": minimumDiskSize}) + if diskSizeMB < minimumDiskSize { + skipErrorIfForceEnabled(exit.Config, "Requested disk size {{.requested_size}} is less than minimum of {{.minimum_size}}", out.V{"requested_size": diskSizeMB, "minimum_size": minimumDiskSize}) } } @@ -974,7 +961,7 @@ func validateFlags(cmd *cobra.Command, drvName string) { } req, err := util.CalculateSizeInMB(viper.GetString(memory)) if err != nil { - exit.WithCodeT(exit.Config, "Unable to parse memory '{{.memory}}': {{.error}}", out.V{"memory": viper.GetString(memory), "error": err}) + skipErrorIfForceEnabled(exit.Config, "Unable to parse memory '{{.memory}}': {{.error}}", out.V{"memory": viper.GetString(memory), "error": err}) } validateMemorySize(req, drvName) } @@ -1152,11 +1139,7 @@ func validateKubernetesVersion(old *config.ClusterConfig) { if nvs.LT(oldestVersion) { out.WarningT("Specified Kubernetes version {{.specified}} is less than the oldest supported version: {{.oldest}}", out.V{"specified": nvs, "oldest": constants.OldestKubernetesVersion}) - if viper.GetBool(force) { - out.WarningT("Kubernetes {{.version}} is not supported by this release of minikube", out.V{"version": nvs}) - } else { - exit.WithCodeT(exit.Data, "Sorry, Kubernetes {{.version}} is not supported by this release of minikube. To use this version anyway, use the `--force` flag.", out.V{"version": nvs}) - } + skipErrorIfForceEnabled(exit.Data, "Kubernetes {{.version}} is not supported by this release of minikube", out.V{"version": nvs}) } if old == nil || old.KubernetesConfig.KubernetesVersion == "" { @@ -1219,3 +1202,10 @@ func getKubernetesVersion(old *config.ClusterConfig) string { return version.VersionPrefix + nvs.String() } + +func skipErrorIfForceEnabled(code int, message string, v out.V) { + if !viper.GetBool(force) { + exit.WithCodeT(code, message, v) + } + out.WarningT(message, v) +} From 506279c58522d7f0848a496415f457ab1c4df1de Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 11 Aug 2020 15:38:42 -0400 Subject: [PATCH 5/6] change function name --- cmd/minikube/cmd/start.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 858c0690e8db..2501da94def5 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -673,7 +673,7 @@ func validateDriver(ds registry.DriverState, existing *config.ClusterConfig) { } exit.WithCodeT(exit.Unavailable, "{{.driver}} does not appear to be installed", out.V{"driver": name}) } - skipErrorIfForceEnabled(exit.Unavailable, "Failed to validate '{{.driver}}' driver", out.V{"driver": name}) + exitIfNotForced(exit.Unavailable, "Failed to validate '{{.driver}}' driver", out.V{"driver": name}) } } @@ -857,7 +857,7 @@ func validateMemorySize(req int, drvName string) { minAdvised := 0.50 * float64(sysLimit) if req < minUsableMem { - skipErrorIfForceEnabled(exit.Config, "Requested memory allocation {{.requested}}MB is less than the usable minimum of {{.minimum_memory}}MB", out.V{"requested": req, "minimum_memory": minUsableMem}) + exitIfNotForced(exit.Config, "Requested memory allocation {{.requested}}MB is less than the usable minimum of {{.minimum_memory}}MB", out.V{"requested": req, "minimum_memory": minUsableMem}) } if req < minRecommendedMem { out.WarningT("Requested memory allocation ({{.requested}}MB) is less than the recommended minimum {{.recommended}}MB. Kubernetes may crash unexpectedly.", @@ -878,7 +878,7 @@ func validateMemorySize(req int, drvName string) { miniube start --memory={{.min_advised}}mb ` - skipErrorIfForceEnabled(exit.Config, message, out.V{"requested": req, "system_limit": sysLimit, "max_advised": int32(maxAdvised), "min_advised": minAdvised}) + exitIfNotForced(exit.Config, message, out.V{"requested": req, "system_limit": sysLimit, "max_advised": int32(maxAdvised), "min_advised": minAdvised}) } if float64(req) > maxAdvised { @@ -906,7 +906,7 @@ func validateCPUCount(drvName string) { cpuCount = viper.GetInt(cpus) } if cpuCount < minimumCPUS { - skipErrorIfForceEnabled(exit.BadUsage, "Requested cpu count {{.requested_cpus}} is less than the minimum allowed of {{.minimum_cpus}}", out.V{"requested_cpus": cpuCount, "minimum_cpus": minimumCPUS}) + exitIfNotForced(exit.BadUsage, "Requested cpu count {{.requested_cpus}} is less than the minimum allowed of {{.minimum_cpus}}", out.V{"requested_cpus": cpuCount, "minimum_cpus": minimumCPUS}) } if driver.IsKIC((drvName)) { @@ -928,7 +928,7 @@ func validateCPUCount(drvName string) { `) } out.T(out.Documentation, "https://docs.docker.com/config/containers/resource_constraints/") - skipErrorIfForceEnabled(exit.BadUsage, "Ensure your {{.driver_name}} system has enough CPUs. The minimum allowed is 2 CPUs.", out.V{"driver_name": driver.FullName(viper.GetString("driver"))}) + exitIfNotForced(exit.BadUsage, "Ensure your {{.driver_name}} system has enough CPUs. The minimum allowed is 2 CPUs.", out.V{"driver_name": driver.FullName(viper.GetString("driver"))}) } } } @@ -939,11 +939,11 @@ func validateFlags(cmd *cobra.Command, drvName string) { if cmd.Flags().Changed(humanReadableDiskSize) { diskSizeMB, err := util.CalculateSizeInMB(viper.GetString(humanReadableDiskSize)) if err != nil { - skipErrorIfForceEnabled(exit.Config, "Validation unable to parse disk size '{{.diskSize}}': {{.error}}", out.V{"diskSize": viper.GetString(humanReadableDiskSize), "error": err}) + exitIfNotForced(exit.Config, "Validation unable to parse disk size '{{.diskSize}}': {{.error}}", out.V{"diskSize": viper.GetString(humanReadableDiskSize), "error": err}) } if diskSizeMB < minimumDiskSize { - skipErrorIfForceEnabled(exit.Config, "Requested disk size {{.requested_size}} is less than minimum of {{.minimum_size}}", out.V{"requested_size": diskSizeMB, "minimum_size": minimumDiskSize}) + exitIfNotForced(exit.Config, "Requested disk size {{.requested_size}} is less than minimum of {{.minimum_size}}", out.V{"requested_size": diskSizeMB, "minimum_size": minimumDiskSize}) } } @@ -961,7 +961,7 @@ func validateFlags(cmd *cobra.Command, drvName string) { } req, err := util.CalculateSizeInMB(viper.GetString(memory)) if err != nil { - skipErrorIfForceEnabled(exit.Config, "Unable to parse memory '{{.memory}}': {{.error}}", out.V{"memory": viper.GetString(memory), "error": err}) + exitIfNotForced(exit.Config, "Unable to parse memory '{{.memory}}': {{.error}}", out.V{"memory": viper.GetString(memory), "error": err}) } validateMemorySize(req, drvName) } @@ -1139,7 +1139,7 @@ func validateKubernetesVersion(old *config.ClusterConfig) { if nvs.LT(oldestVersion) { out.WarningT("Specified Kubernetes version {{.specified}} is less than the oldest supported version: {{.oldest}}", out.V{"specified": nvs, "oldest": constants.OldestKubernetesVersion}) - skipErrorIfForceEnabled(exit.Data, "Kubernetes {{.version}} is not supported by this release of minikube", out.V{"version": nvs}) + exitIfNotForced(exit.Data, "Kubernetes {{.version}} is not supported by this release of minikube", out.V{"version": nvs}) } if old == nil || old.KubernetesConfig.KubernetesVersion == "" { @@ -1203,7 +1203,7 @@ func getKubernetesVersion(old *config.ClusterConfig) string { return version.VersionPrefix + nvs.String() } -func skipErrorIfForceEnabled(code int, message string, v out.V) { +func exitIfNotForced(code int, message string, v out.V) { if !viper.GetBool(force) { exit.WithCodeT(code, message, v) } From f122deeaf3c324e67194e29c67ae5c2b42f53291 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 11 Aug 2020 15:42:05 -0400 Subject: [PATCH 6/6] suggest --force for k8s version --- cmd/minikube/cmd/start.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 2501da94def5..2b1bcede9d1a 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -1139,6 +1139,9 @@ func validateKubernetesVersion(old *config.ClusterConfig) { if nvs.LT(oldestVersion) { out.WarningT("Specified Kubernetes version {{.specified}} is less than the oldest supported version: {{.oldest}}", out.V{"specified": nvs, "oldest": constants.OldestKubernetesVersion}) + if !viper.GetBool(force) { + out.WarningT("You can force an unsupported Kubernetes version via the --force flag") + } exitIfNotForced(exit.Data, "Kubernetes {{.version}} is not supported by this release of minikube", out.V{"version": nvs}) }