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.

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

advice user when allocate more than 80% of their memory #8877

Merged
merged 7 commits into from
Aug 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
77 changes: 58 additions & 19 deletions cmd/minikube/cmd/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -822,13 +821,41 @@ 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.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})
}
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)
if err != nil {
glog.Warningf("Unable to query memory limits: %v", err)
}

// 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) {
exit.WithCodeT(exit.Config, "Requested memory allocation {{.requested}}MB is less than the usable minimum of {{.minimum}}MB",
out.V{"requested": req, "mininum": minUsableMem})
Expand All @@ -838,23 +865,34 @@ 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) {
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
Expand Down Expand Up @@ -919,16 +957,17 @@ func validateFlags(cmd *cobra.Command, drvName string) {
}
}
validateCPUCount(drvName)
validateMemoryHardLimit(drvName)

if cmd.Flags().Changed(memory) {
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))
sharifelgamal marked this conversation as resolved.
Show resolved Hide resolved
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})
}
}

if cmd.Flags().Changed(containerRuntime) {
Expand Down
24 changes: 19 additions & 5 deletions cmd/minikube/cmd/start_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,11 +237,10 @@ func generateClusterConfig(cmd *cobra.Command, existing *config.ClusterConfig, k
}

} else {
validateMemorySize(mem, drvName)
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})
Expand Down Expand Up @@ -409,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.")
}
}
Expand Down
57 changes: 51 additions & 6 deletions pkg/minikube/machine/info.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ import (
"k8s.io/minikube/pkg/minikube/out/register"
)

type hostInfo struct {
// HostInfo holds information on the user's machine
type HostInfo struct {
Memory int64
CPUs int
DiskSize int64
Expand All @@ -40,24 +41,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 := cachedDiskInfo()
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)
Expand Down Expand Up @@ -97,3 +100,45 @@ func logRemoteOsRelease(r command.Runner) {

glog.Infof("Remote host: %s", osReleaseInfo.PrettyName)
}

var cachedSystemMemoryLimit *mem.VirtualMemoryStat
var cachedSystemMemoryErr *error

// cachedSysMemLimit will return a cached 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
sharifelgamal marked this conversation as resolved.
Show resolved Hide resolved
}

var cachedDisk *disk.UsageStat
var cachedDiskInfoeErr *error

// cachedDiskInfo will return a cached disk usage info
func cachedDiskInfo() (disk.UsageStat, error) {
if cachedDisk == nil {
d, err := disk.Usage("/")
cachedDisk = d
cachedDiskInfoeErr = &err
}
return *cachedDisk, *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
}
2 changes: 1 addition & 1 deletion pkg/minikube/machine/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand Down