From 71394c43e725b6e109ec48fa449fd572c9b93c14 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Wed, 29 Jul 2020 13:40:09 -0700 Subject: [PATCH 1/7] advice user when they allocate too much memory --- cmd/minikube/cmd/start.go | 30 +++++++++++++++++++++++++----- cmd/minikube/cmd/start_flags.go | 3 +++ 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 7189231565d8..0d6cb5968b18 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -829,6 +829,10 @@ func validateMemorySize(req int, drvName string) { glog.Warningf("Unable to query memory limits: %v", err) } + // maximm ram they should allocate to minikube to prevent #8708 + maxAdvised := 0.79 * float64(sysLimit) + 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}}MB", out.V{"requested": req, "mininum": minUsableMem}) @@ -855,6 +859,27 @@ 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 float64(req) > maxAdvised && !viper.GetBool(force) { + 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.") + } + } // validateCPUCount validates the cpu count matches the minimum recommended @@ -921,11 +946,6 @@ func validateFlags(cmd *cobra.Command, drvName string) { validateCPUCount(drvName) if cmd.Flags().Changed(memory) { - 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}) - } - validateMemorySize(req, drvName) if !driver.HasResourceLimits(drvName) { out.WarningT("The '{{.name}}' driver does not respect the --memory flag", out.V{"name": drvName}) } diff --git a/cmd/minikube/cmd/start_flags.go b/cmd/minikube/cmd/start_flags.go index 2bcf5b212072..376881806517 100644 --- a/cmd/minikube/cmd/start_flags.go +++ b/cmd/minikube/cmd/start_flags.go @@ -393,6 +393,9 @@ func updateExistingConfigFromFlags(cmd *cobra.Command, existing *config.ClusterC cc := *existing + // validate the memory size in case user changed their system memory limits (example change docker desktop or upgraded memory.) + validateMemorySize(cc.Memory, cc.Driver) + if cmd.Flags().Changed(containerRuntime) { cc.KubernetesConfig.ContainerRuntime = viper.GetString(containerRuntime) } From e59a026450140edcabfc7c19ae27332e6beefda2 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Wed, 29 Jul 2020 13:45:29 -0700 Subject: [PATCH 2/7] update comments --- cmd/minikube/cmd/start.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 0d6cb5968b18..f394c1cfb942 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -829,8 +829,9 @@ func validateMemorySize(req int, drvName string) { glog.Warningf("Unable to query memory limits: %v", err) } - // maximm ram they should allocate to minikube to prevent #8708 + // maximm percent of their ram they could allocate to minikube to prevent #8708 maxAdvised := 0.79 * float64(sysLimit) + // a more sane alternative to their high memory 80% minAdvised := 0.50 * float64(sysLimit) if req < minUsableMem && !viper.GetBool(force) { From d782f80e23449a641a95325e081fdd56a08b73e8 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Mon, 3 Aug 2020 13:27:49 -0700 Subject: [PATCH 3/7] bring back check for dry run --- cmd/minikube/cmd/start.go | 5 +++++ cmd/minikube/cmd/start_flags.go | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index f394c1cfb942..ff17a4f22599 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -950,6 +950,11 @@ func validateFlags(cmd *cobra.Command, drvName string) { if !driver.HasResourceLimits(drvName) { out.WarningT("The '{{.name}}' driver does not respect the --memory flag", out.V{"name": drvName}) } + 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}) + } + validateMemorySize(req, drvName) } if cmd.Flags().Changed(containerRuntime) { diff --git a/cmd/minikube/cmd/start_flags.go b/cmd/minikube/cmd/start_flags.go index 376881806517..18c48f4e60b5 100644 --- a/cmd/minikube/cmd/start_flags.go +++ b/cmd/minikube/cmd/start_flags.go @@ -236,12 +236,12 @@ func generateClusterConfig(cmd *cobra.Command, existing *config.ClusterConfig, k exit.UsageT("{{.driver_name}} has only {{.container_limit}}MB memory but you specified {{.specified_memory}}MB", out.V{"container_limit": containerLimit, "specified_memory": mem, "driver_name": driver.FullName(drvName)}) } + validateMemorySize(mem, drvName) + } else { glog.Infof("Using suggested %dMB memory alloc based on sys=%dMB, container=%dMB", mem, sysLimit, containerLimit) } - validateMemorySize(mem, drvName) - diskSize, err := pkgutil.CalculateSizeInMB(viper.GetString(humanReadableDiskSize)) if err != nil { exit.WithCodeT(exit.Config, "Generate unable to parse disk size '{{.diskSize}}': {{.error}}", out.V{"diskSize": viper.GetString(humanReadableDiskSize), "error": err}) From b71a7dceca0fbf03d4b0797604bd9e7480cb170b Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Mon, 3 Aug 2020 14:40:59 -0700 Subject: [PATCH 4/7] use cached hostinfo instead --- cmd/minikube/cmd/start.go | 45 ++++++++++++++++---------- cmd/minikube/cmd/start_flags.go | 3 +- pkg/minikube/machine/info.go | 56 +++++++++++++++++++++++++++++---- pkg/minikube/machine/start.go | 2 +- 4 files changed, 81 insertions(+), 25 deletions(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index ff17a4f22599..62a54d3f0582 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -37,7 +37,6 @@ import ( "github.com/pkg/errors" "github.com/shirou/gopsutil/cpu" gopshost "github.com/shirou/gopsutil/host" - "github.com/shirou/gopsutil/mem" "github.com/spf13/cobra" "github.com/spf13/viper" cmdcfg "k8s.io/minikube/cmd/minikube/cmd/config" @@ -764,13 +763,13 @@ func validateUser(drvName string) { } } -// memoryLimits returns the amount of memory allocated to the system and hypervisor +// memoryLimits returns the amount of memory allocated to the system and hypervisor , the return value is in MB func memoryLimits(drvName string) (int, int, error) { - v, err := mem.VirtualMemory() + info, err := machine.CachedHostInfo() if err != nil { return -1, -1, err } - sysLimit := int(v.Total / 1024 / 1024) + sysLimit := int(info.Memory) containerLimit := 0 if driver.IsKIC(drvName) { @@ -822,6 +821,29 @@ func suggestMemoryAllocation(sysLimit int, containerLimit int, nodes int) int { return suggested } +// validateMemoryHardLimit checks if the user system has enough memory at all ! +func validateMemoryHardLimit(drvName string) { + s, c, err := memoryLimits(drvName) + if err != nil { + glog.Warningf("Unable to query memory limits: %v", err) + out.T(out.Conflict, "Failed to verify system memory limits.") + } + if s < 2200 { + out.WarningT("Your system has only {{.memory_amount}}MB memory. This might not work minimum required is 2000MB.", out.V{"memory_amount": s}) + } + if driver.IsDockerDesktop(drvName) { + // in Docker Desktop if you allocate 2 GB the docker info shows: Total Memory: 1.945GiB which becomes 1991 when we calculate the MBs + // thats why it is not same number as other drivers which is 2 GB + if c < 1991 { + out.WarningT(`Increase Docker for Desktop memory to at least 2.5GB or more: + + Docker for Desktop > Settings > Resources > Memory + +`) + } + } +} + // validateMemorySize validates the memory size matches the minimum recommended func validateMemorySize(req int, drvName string) { sysLimit, containerLimit, err := memoryLimits(drvName) @@ -843,22 +865,12 @@ func validateMemorySize(req int, drvName string) { out.V{"requested": req, "recommended": minRecommendedMem}) } - if driver.IsDockerDesktop(drvName) { - // in Docker Desktop if you allocate 2 GB the docker info shows: Total Memory: 1.945GiB which becomes 1991 when we calculate the MBs - // thats why it is not same number as other drivers which is 2 GB - if containerLimit < 1991 { - out.WarningT(`Increase Docker for Desktop memory to at least 2.5GB or more: - - Docker for Desktop > Settings > Resources > Memory - -`) - } else if containerLimit < 2997 && sysLimit > 8000 { // for users with more than 8 GB advice 3 GB - out.WarningT(`Your system has {{.system_limit}}MB memory but Docker has only {{.container_limit}}MB. For a better performance increase to at least 3GB. + if driver.IsDockerDesktop(drvName) && containerLimit < 2997 && sysLimit > 8000 { // for users with more than 8 GB advice 3 GB + out.WarningT(`Your system has {{.system_limit}}MB memory but Docker has only {{.container_limit}}MB. For a better performance increase to at least 3GB. Docker for Desktop > Settings > Resources > Memory `, out.V{"container_limit": containerLimit, "system_limit": sysLimit}) - } } if req > sysLimit && !viper.GetBool(force) { @@ -945,6 +957,7 @@ func validateFlags(cmd *cobra.Command, drvName string) { } } validateCPUCount(drvName) + validateMemoryHardLimit(drvName) if cmd.Flags().Changed(memory) { if !driver.HasResourceLimits(drvName) { diff --git a/cmd/minikube/cmd/start_flags.go b/cmd/minikube/cmd/start_flags.go index 18c48f4e60b5..bb06d392c16b 100644 --- a/cmd/minikube/cmd/start_flags.go +++ b/cmd/minikube/cmd/start_flags.go @@ -236,9 +236,8 @@ func generateClusterConfig(cmd *cobra.Command, existing *config.ClusterConfig, k exit.UsageT("{{.driver_name}} has only {{.container_limit}}MB memory but you specified {{.specified_memory}}MB", out.V{"container_limit": containerLimit, "specified_memory": mem, "driver_name": driver.FullName(drvName)}) } - validateMemorySize(mem, drvName) - } else { + validateMemorySize(mem, drvName) glog.Infof("Using suggested %dMB memory alloc based on sys=%dMB, container=%dMB", mem, sysLimit, containerLimit) } diff --git a/pkg/minikube/machine/info.go b/pkg/minikube/machine/info.go index ee505c295255..020e227c2435 100644 --- a/pkg/minikube/machine/info.go +++ b/pkg/minikube/machine/info.go @@ -30,7 +30,7 @@ import ( "k8s.io/minikube/pkg/minikube/out/register" ) -type hostInfo struct { +type HostInfo struct { Memory int64 CPUs int DiskSize int64 @@ -40,24 +40,26 @@ func megs(bytes uint64) int64 { return int64(bytes / 1024 / 1024) } -func getHostInfo() (*hostInfo, error) { - i, err := cpu.Info() +// CachedHostInfo returns system information such as memory,CPU, DiskSize +func CachedHostInfo() (*HostInfo, error) { + i, err := cachedCPUInfo() if err != nil { glog.Warningf("Unable to get CPU info: %v", err) return nil, err } - v, err := mem.VirtualMemory() + v, err := cachedSysMemLimit() if err != nil { glog.Warningf("Unable to get mem info: %v", err) return nil, err } - d, err := disk.Usage("/") + + d, err := cachedDisInfo() if err != nil { glog.Warningf("Unable to get disk info: %v", err) return nil, err } - var info hostInfo + var info HostInfo info.CPUs = len(i) info.Memory = megs(v.Total) info.DiskSize = megs(d.Total) @@ -97,3 +99,45 @@ func logRemoteOsRelease(r command.Runner) { glog.Infof("Remote host: %s", osReleaseInfo.PrettyName) } + +var cachedSystemMemoryLimit *mem.VirtualMemoryStat +var cachedSystemMemoryErr *error + +// cachedSysMemLimit will return a chaced limit for the system's virtual memory. +func cachedSysMemLimit() (*mem.VirtualMemoryStat, error) { + if cachedSystemMemoryLimit == nil { + v, err := mem.VirtualMemory() + cachedSystemMemoryLimit = v + cachedSystemMemoryErr = &err + } + return cachedSystemMemoryLimit, *cachedSystemMemoryErr +} + +var cachedDiskInfo *disk.UsageStat +var cachedDiskInfoeErr *error + +// cachedSysMemLimit will return a cached disk usage info +func cachedDisInfo() (disk.UsageStat, error) { + if cachedDiskInfo == nil { + d, err := disk.Usage("/") + cachedDiskInfo = d + cachedDiskInfoeErr = &err + } + return *cachedDiskInfo, *cachedDiskInfoeErr +} + +var cachedCPU *[]cpu.InfoStat +var cachedCPUErr *error + +// cachedCPUInfo will return a cached cpu info +func cachedCPUInfo() ([]cpu.InfoStat, error) { + if cachedCPU == nil { + i, err := cpu.Info() + cachedCPU = &i + cachedCPUErr = &err + if err != nil { + return nil, *cachedCPUErr + } + } + return *cachedCPU, *cachedCPUErr +} diff --git a/pkg/minikube/machine/start.go b/pkg/minikube/machine/start.go index 291f34662fa6..a52e2481f354 100644 --- a/pkg/minikube/machine/start.go +++ b/pkg/minikube/machine/start.go @@ -251,7 +251,7 @@ func acquireMachinesLock(name string) (mutex.Releaser, error) { func showHostInfo(cfg config.ClusterConfig) { machineType := driver.MachineType(cfg.Driver) if driver.BareMetal(cfg.Driver) { - info, err := getHostInfo() + info, err := CachedHostInfo() if err == nil { register.Reg.SetStep(register.RunningLocalhost) out.T(out.StartingNone, "Running on localhost (CPUs={{.number_of_cpus}}, Memory={{.memory_size}}MB, Disk={{.disk_size}}MB) ...", out.V{"number_of_cpus": info.CPUs, "memory_size": info.Memory, "disk_size": info.DiskSize}) From e87400db535128900d54bc15996f17fade4b57f5 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Mon, 3 Aug 2020 15:34:32 -0700 Subject: [PATCH 5/7] addres review comments --- cmd/minikube/cmd/start.go | 2 +- pkg/minikube/machine/info.go | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 62a54d3f0582..6eb412770972 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -826,7 +826,7 @@ func validateMemoryHardLimit(drvName string) { s, c, err := memoryLimits(drvName) if err != nil { glog.Warningf("Unable to query memory limits: %v", err) - out.T(out.Conflict, "Failed to verify system memory limits.") + out.WarningT("Failed to verify system memory limits.") } if s < 2200 { out.WarningT("Your system has only {{.memory_amount}}MB memory. This might not work minimum required is 2000MB.", out.V{"memory_amount": s}) diff --git a/pkg/minikube/machine/info.go b/pkg/minikube/machine/info.go index 020e227c2435..da8464bc4fc4 100644 --- a/pkg/minikube/machine/info.go +++ b/pkg/minikube/machine/info.go @@ -30,6 +30,7 @@ import ( "k8s.io/minikube/pkg/minikube/out/register" ) +// HostInfo holds information on the user's machine type HostInfo struct { Memory int64 CPUs int @@ -103,7 +104,7 @@ func logRemoteOsRelease(r command.Runner) { var cachedSystemMemoryLimit *mem.VirtualMemoryStat var cachedSystemMemoryErr *error -// cachedSysMemLimit will return a chaced limit for the system's virtual memory. +// cachedSysMemLimit will return a cached limit for the system's virtual memory. func cachedSysMemLimit() (*mem.VirtualMemoryStat, error) { if cachedSystemMemoryLimit == nil { v, err := mem.VirtualMemory() @@ -116,7 +117,7 @@ func cachedSysMemLimit() (*mem.VirtualMemoryStat, error) { var cachedDiskInfo *disk.UsageStat var cachedDiskInfoeErr *error -// cachedSysMemLimit will return a cached disk usage info +// cachedDisInfo will return a cached disk usage info func cachedDisInfo() (disk.UsageStat, error) { if cachedDiskInfo == nil { d, err := disk.Usage("/") From e783af2bef3c4a7cac4be2b9c17a3e746f7d3836 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Mon, 3 Aug 2020 22:44:38 -0700 Subject: [PATCH 6/7] fill up cpu memory if config is missing --- cmd/minikube/cmd/start_flags.go | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/cmd/minikube/cmd/start_flags.go b/cmd/minikube/cmd/start_flags.go index bb06d392c16b..3ecbfff8c8f1 100644 --- a/cmd/minikube/cmd/start_flags.go +++ b/cmd/minikube/cmd/start_flags.go @@ -392,9 +392,6 @@ func updateExistingConfigFromFlags(cmd *cobra.Command, existing *config.ClusterC cc := *existing - // validate the memory size in case user changed their system memory limits (example change docker desktop or upgraded memory.) - validateMemorySize(cc.Memory, cc.Driver) - if cmd.Flags().Changed(containerRuntime) { cc.KubernetesConfig.ContainerRuntime = viper.GetString(containerRuntime) } @@ -411,19 +408,34 @@ func updateExistingConfigFromFlags(cmd *cobra.Command, existing *config.ClusterC cc.MinikubeISO = viper.GetString(isoURL) } + if cc.Memory == 0 { + glog.Info("Existing config file was missing memory. (could be an old minikube config), will use the default value") + memInMB, err := pkgutil.CalculateSizeInMB(viper.GetString(memory)) + if err != nil { + glog.Warningf("error calculate memory size in mb : %v", err) + } + cc.Memory = memInMB + } + if cmd.Flags().Changed(memory) { memInMB, err := pkgutil.CalculateSizeInMB(viper.GetString(memory)) if err != nil { glog.Warningf("error calculate memory size in mb : %v", err) } - if memInMB != existing.Memory { + if memInMB != cc.Memory { out.WarningT("You cannot change the memory size for an exiting minikube cluster. Please first delete the cluster.") } - } + // validate the memory size in case user changed their system memory limits (example change docker desktop or upgraded memory.) + validateMemorySize(cc.Memory, cc.Driver) + + if cc.CPUs == 0 { + glog.Info("Existing config file was missing cpu. (could be an old minikube config), will use the default value") + cc.CPUs = viper.GetInt(cpus) + } if cmd.Flags().Changed(cpus) { - if viper.GetInt(cpus) != existing.CPUs { + if viper.GetInt(cpus) != cc.CPUs { out.WarningT("You cannot change the CPUs for an existing minikube cluster. Please first delete the cluster.") } } From 98c2157242ecfd227b07938e854e945c4fd6609c Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Tue, 4 Aug 2020 09:46:53 -0700 Subject: [PATCH 7/7] address review comments --- pkg/minikube/machine/info.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/minikube/machine/info.go b/pkg/minikube/machine/info.go index da8464bc4fc4..15e94558c7d7 100644 --- a/pkg/minikube/machine/info.go +++ b/pkg/minikube/machine/info.go @@ -54,7 +54,7 @@ func CachedHostInfo() (*HostInfo, error) { return nil, err } - d, err := cachedDisInfo() + d, err := cachedDiskInfo() if err != nil { glog.Warningf("Unable to get disk info: %v", err) return nil, err @@ -114,17 +114,17 @@ func cachedSysMemLimit() (*mem.VirtualMemoryStat, error) { return cachedSystemMemoryLimit, *cachedSystemMemoryErr } -var cachedDiskInfo *disk.UsageStat +var cachedDisk *disk.UsageStat var cachedDiskInfoeErr *error -// cachedDisInfo will return a cached disk usage info -func cachedDisInfo() (disk.UsageStat, error) { - if cachedDiskInfo == nil { +// cachedDiskInfo will return a cached disk usage info +func cachedDiskInfo() (disk.UsageStat, error) { + if cachedDisk == nil { d, err := disk.Usage("/") - cachedDiskInfo = d + cachedDisk = d cachedDiskInfoeErr = &err } - return *cachedDiskInfo, *cachedDiskInfoeErr + return *cachedDisk, *cachedDiskInfoeErr } var cachedCPU *[]cpu.InfoStat