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

docker/podman: warn if allocated memory is below limit #8718

Merged
merged 16 commits into from
Jul 20, 2020
70 changes: 62 additions & 8 deletions cmd/minikube/cmd/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ var (
insecureRegistry []string
apiServerNames []string
apiServerIPs []net.IP
kicSysInfo *oci.SysInfo
)

func init() {
Expand Down Expand Up @@ -752,12 +753,13 @@ func memoryLimits(drvName string) (int, int, error) {
containerLimit := 0

if driver.IsKIC(drvName) {
s, err := oci.DaemonInfo(drvName)
s, err := cachedKicSystemInfo(drvName)
if err != nil {
return -1, -1, err
}
containerLimit = int(s.TotalMemory / 1024 / 1024)
}

return sysLimit, containerLimit, nil
}

Expand Down Expand Up @@ -800,11 +802,13 @@ func suggestMemoryAllocation(sysLimit int, containerLimit int, nodes int) int {
}

// validateMemorySize validates the memory size matches the minimum recommended
func validateMemorySize() {
req, err := util.CalculateSizeInMB(viper.GetString(memory))
func validateMemorySize(req int, drvName string) {

sysLimit, containerLimit, err := memoryLimits(drvName)
if err != nil {
exit.WithCodeT(exit.Config, "Unable to parse memory '{{.memory}}': {{.error}}", out.V{"memory": viper.GetString(memory), "error": err})
glog.Warningf("Unable to query memory limits: %v", err)
}

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 @@ -813,12 +817,28 @@ func validateMemorySize() {
out.T(out.Notice, "Requested memory allocation ({{.requested}}MB) is less than the recommended minimum {{.recommended}}MB. Kubernetes may crash unexpectedly.",
out.V{"requested": req, "recommended": minRecommendedMem})
}

if driver.IsDockerDesktop(drvName) {
if containerLimit < 1991 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this re-use minRecommendedMem?

Copy link
Member Author

Choose a reason for hiding this comment

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

docker info for when it is 2 GB:
Total Memory: 1.945GiB

out.T(out.Tip, `Increase Docker for Desktop memory to at least 2.5 GB or more:
medyagh marked this conversation as resolved.
Show resolved Hide resolved

Docker for Desktop > Settings > Resources > Memory

`)
} else if containerLimit < 2997 && sysLimit > 8000 { // for users with more than 8 GB advice 3 GB
out.T(out.Tip, `Your system has {{.system_limit}}MB memory but Docker has only {{.container_limit}}MB. For a better performance increase to at least 3 GB.

Docker for Desktop > Settings > Resources > Memory

`, out.V{"container_limit": containerLimit, "system_limit": sysLimit})
}
}
}

// validateCPUCount validates the cpu count matches the minimum recommended
func validateCPUCount(local bool) {
func validateCPUCount(drvName string) {
var cpuCount int
if local {
if driver.BareMetal(drvName) {
// Uses the gopsutil cpu package to count the number of physical cpu cores
ci, err := cpu.Counts(false)
if err != nil {
Expand All @@ -832,6 +852,25 @@ func validateCPUCount(local bool) {
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 driver.IsKIC((drvName)) {
si, err := cachedKicSystemInfo(drvName)
if err != nil {
out.WarningT("Failed to verify '{{.driver_name}} info', ensure your {{.driver_name}} is running healthy.", out.V{"driver_namee": drvName})
medyagh marked this conversation as resolved.
Show resolved Hide resolved
}
medyagh marked this conversation as resolved.
Show resolved Hide resolved
if si.CPUs < 2 {
if drvName == oci.Docker {
out.T(out.Conflict, `Your Docker Desktop has less than 2 CPUs. Increase CPUs for Docker Desktop.

Docker icon > Settings > Resources > CPUs

`)
}
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.NameForHumans(viper.GetString("driver"))})

}
}
}

// validateFlags validates the supplied flags against known bad combinations
Expand All @@ -848,14 +887,18 @@ func validateFlags(cmd *cobra.Command, drvName string) {
}

if cmd.Flags().Changed(cpus) {
validateCPUCount(driver.BareMetal(drvName))
if !driver.HasResourceLimits(drvName) {
out.WarningT("The '{{.name}}' driver does not respect the --cpus flag", out.V{"name": drvName})
}
}
validateCPUCount(drvName)

if cmd.Flags().Changed(memory) {
validateMemorySize()
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})
}
Expand Down Expand Up @@ -1101,3 +1144,14 @@ func getKubernetesVersion(old *config.ClusterConfig) string {

return version.VersionPrefix + nvs.String()
}

// cachedKicSystemInfo will return docker/podman info. it will call docker/podam info only once per minikube start to avoid perforamce side effects
func cachedKicSystemInfo(ociBin string) (oci.SysInfo, error) {
medyagh marked this conversation as resolved.
Show resolved Hide resolved
var err error
if kicSysInfo == nil {
si, err := oci.DaemonInfo(ociBin)
kicSysInfo = &si
return *kicSysInfo, err
medyagh marked this conversation as resolved.
Show resolved Hide resolved
}
return *kicSysInfo, err
}
5 changes: 5 additions & 0 deletions cmd/minikube/cmd/start_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,11 +232,16 @@ func generateClusterConfig(cmd *cobra.Command, existing *config.ClusterConfig, k
if err != nil {
exit.WithCodeT(exit.Config, "Generate unable to parse memory '{{.memory}}': {{.error}}", out.V{"memory": viper.GetString(memory), "error": err})
}
if driver.IsKIC(drvName) && mem > containerLimit {
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.NameForHumans(drvName)})
medyagh marked this conversation as resolved.
Show resolved Hide resolved
}

} 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})
Expand Down
28 changes: 28 additions & 0 deletions pkg/minikube/driver/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"strings"

"github.com/golang/glog"
"k8s.io/minikube/pkg/drivers/kic/oci"
"k8s.io/minikube/pkg/minikube/config"
"k8s.io/minikube/pkg/minikube/registry"
)
Expand Down Expand Up @@ -105,6 +106,17 @@ func IsKIC(name string) bool {
return name == Docker || name == Podman
}

// IsKIC checks if the driver is a Docker for Desktop (Docker on windows or MacOs)
// for linux and exotic archs, this will be false
func IsDockerDesktop(name string) bool {
if IsKIC(name) {
medyagh marked this conversation as resolved.
Show resolved Hide resolved
if runtime.GOOS == "darwin" || runtime.GOOS == "windows" {
return true
}
}
return false
}

// IsMock checks if the driver is a mock
func IsMock(name string) bool {
return name == Mock
Expand Down Expand Up @@ -156,6 +168,22 @@ func NeedsShutdown(name string) bool {
return false
}

// NameForHumans will return the human-known and formatted name for the driver
func NameForHumans(name string) string {
medyagh marked this conversation as resolved.
Show resolved Hide resolved
switch name {
case oci.Docker:
if IsDockerDesktop(name) {
return "Docker for Desktop"
}
return "Docker Service"

case oci.Podman:
return "Podman Service"
medyagh marked this conversation as resolved.
Show resolved Hide resolved
default:
return strings.Title(name)
}
}

// FlagHints are hints for what default options should be used for this driver
type FlagHints struct {
ExtraOptions []string
Expand Down
5 changes: 3 additions & 2 deletions pkg/minikube/node/advice.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/spf13/viper"
"k8s.io/minikube/pkg/drivers/kic/oci"
"k8s.io/minikube/pkg/minikube/bootstrapper/kubeadm"
"k8s.io/minikube/pkg/minikube/driver"
"k8s.io/minikube/pkg/minikube/exit"
"k8s.io/minikube/pkg/minikube/out"
)
Expand All @@ -45,7 +46,7 @@ func MaybeExitWithAdvice(err error) {

if errors.Is(err, oci.ErrCPUCountLimit) {
out.ErrLn("")
out.ErrT(out.Conflict, "{{.name}} doesn't have enough CPUs. ", out.V{"name": viper.GetString("driver")})
out.ErrT(out.Conflict, "{{.name}} doesn't have enough CPUs. ", out.V{"name": driver.NameForHumans(viper.GetString("driver"))})
if runtime.GOOS != "linux" && viper.GetString("driver") == "docker" {
out.T(out.Warning, "Please consider changing your Docker Desktop's resources.")
out.T(out.Documentation, "https://docs.docker.com/config/containers/resource_constraints/")
Expand All @@ -54,7 +55,7 @@ func MaybeExitWithAdvice(err error) {
if cpuCount == 2 {
out.T(out.Tip, "Please ensure your system has {{.cpu_counts}} CPU cores.", out.V{"cpu_counts": viper.GetInt(cpus)})
} else {
out.T(out.Tip, "Please ensure your {{.driver_name}} system has access to {{.cpu_counts}} CPU cores or reduce the number of the specified CPUs", out.V{"driver_name": viper.GetString("driver"), "cpu_counts": viper.GetInt(cpus)})
out.T(out.Tip, "Please ensure your {{.driver_name}} system has access to {{.cpu_counts}} CPU cores or reduce the number of the specified CPUs", out.V{"driver_name": driver.NameForHumans(viper.GetString("driver")), "cpu_counts": viper.GetInt(cpus)})
}
}
exit.UsageT("Ensure your {{.driver_name}} system has enough CPUs. The minimum allowed is 2 CPUs.", out.V{"driver_name": viper.GetString("driver")})
Expand Down