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.

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

Jump to bottom

skip validations if --force is supplied #8969

Merged
merged 6 commits into from
Aug 11, 2020
Merged

skip validations if --force is supplied #8969

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
789dfca
skip vaildations if --force is supplied
Aug 11, 2020
74309a8
include memory check
Aug 11, 2020
6ff308d
Run download only test with docker daemon unavailable
Aug 11, 2020
a69b419
improve error handling
Aug 11, 2020
506279c
change function name
Aug 11, 2020
f122dee
suggest --force for k8s version
Aug 11, 2020
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
63 changes: 31 additions & 32 deletions cmd/minikube/cmd/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
priyawadhwa marked this conversation as resolved.
Show resolved Hide resolved
}

// if --registry-mirror specified when run minikube start,
// take arg precedence over MINIKUBE_REGISTRY_MIRROR
Expand Down Expand Up @@ -662,18 +665,15 @@ 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})
}
}
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})
}
}

Expand Down Expand Up @@ -856,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})
}
Expand All @@ -873,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.")
}

}
Expand All @@ -909,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)) {
Expand All @@ -932,22 +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 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})
}
}

Expand All @@ -965,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)
}
Expand Down Expand Up @@ -1143,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 == "" {
Expand Down Expand Up @@ -1210,3 +1202,10 @@ func getKubernetesVersion(old *config.ClusterConfig) string {

return version.VersionPrefix + nvs.String()
}

func skipErrorIfForceEnabled(code int, message string, v out.V) {
priyawadhwa marked this conversation as resolved.
Show resolved Hide resolved
if !viper.GetBool(force) {
exit.WithCodeT(code, message, v)
Copy link
Member

Choose a reason for hiding this comment

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

hint to the user if they really want this, they can use --force flag
similar to what we had before

To use this version anyway, use the `--force` flag

Copy link
Author

Choose a reason for hiding this comment

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

I don't think we should be encouraging people to just use the --force flag if something isn't meeting minimum requirements -- it makes sense for k8s versions because users may be using <1.13 which is a reasonable use case, but not for anything else

}
out.WarningT(message, v)
}
5 changes: 4 additions & 1 deletion test/integration/aaa_download_only_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down