Skip to content

Commit

Permalink
Merge pull request #8718 from medyagh/check_docker_deskop
Browse files Browse the repository at this point in the history
docker/podman: warn if allocated memory is below limit
  • Loading branch information
medyagh authored Jul 20, 2020
2 parents c6000e6 + 13357d9 commit 2086bcb
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 21 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,11 @@ jobs:
echo "--------------------------"
docker version || true
echo "--------------------------"
docker info || true
docker info --format='{{json .}}' || true
echo "--------------------------"
docker system df || true
echo "--------------------------"
docker system info || true
docker system info --format='{{json .}}'|| true
echo "--------------------------"
docker ps || true
echo "--------------------------"
Expand Down
65 changes: 57 additions & 8 deletions cmd/minikube/cmd/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -759,12 +759,13 @@ func memoryLimits(drvName string) (int, int, error) {
containerLimit := 0

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

return sysLimit, containerLimit, nil
}

Expand Down Expand Up @@ -807,11 +808,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 @@ -820,12 +823,30 @@ 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) {
// 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.T(out.Tip, `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.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 3GB.
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 @@ -839,6 +860,30 @@ 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 := oci.CachedDaemonInfo(drvName)
if err != nil {
out.T(out.Confused, "Failed to verify '{{.driver_name}} info' will try again ...", out.V{"driver_name": drvName})
si, err = oci.DaemonInfo(drvName)
if err != nil {
exit.UsageT("Ensure your {{.driver_name}} is running and is healthy.", out.V{"driver_name": driver.FullName(drvName)})
}

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

}
}
}

// validateFlags validates the supplied flags against known bad combinations
Expand All @@ -855,14 +900,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
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.FullName(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})
Expand Down
26 changes: 17 additions & 9 deletions pkg/drivers/kic/oci/info.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,29 @@ type SysInfo struct {
OSType string // container's OsType (windows or linux)
}

var cachedSysInfo *SysInfo
var cachedSysInfoErr *error

// CachedDaemonInfo will run and return a docker/podman info only once per minikube run time. to avoid performance
func CachedDaemonInfo(ociBin string) (SysInfo, error) {
if cachedSysInfo == nil {
si, err := DaemonInfo(ociBin)
cachedSysInfo = &si
cachedSysInfoErr = &err
}
return *cachedSysInfo, *cachedSysInfoErr
}

// DaemonInfo returns common docker/podman daemon system info that minikube cares about
func DaemonInfo(ociBin string) (SysInfo, error) {
var info SysInfo
if ociBin == Podman {
p, err := podmanSystemInfo()
info.CPUs = p.Host.Cpus
info.TotalMemory = p.Host.MemTotal
info.OSType = p.Host.Os
return info, err
cachedSysInfo = &SysInfo{CPUs: p.Host.Cpus, TotalMemory: p.Host.MemTotal, OSType: p.Host.Os}
return *cachedSysInfo, err
}
d, err := dockerSystemInfo()
info.CPUs = d.NCPU
info.TotalMemory = d.MemTotal
info.OSType = d.OSType
return info, err
cachedSysInfo = &SysInfo{CPUs: d.NCPU, TotalMemory: d.MemTotal, OSType: d.OSType}
return *cachedSysInfo, err
}

// dockerSysInfo represents the output of docker system info --format '{{json .}}'
Expand Down
30 changes: 30 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,22 @@ func IsKIC(name string) bool {
return name == Docker || name == Podman
}

// IsDocker checks if the driver docker
func IsDocker(name string) bool {
return name == Docker
}

// 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 IsDocker(name) {
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 +173,19 @@ func NeedsShutdown(name string) bool {
return false
}

// FullName will return the human-known and title formatted name for the driver based on platform
func FullName(name string) string {
switch name {
case oci.Docker:
if IsDockerDesktop(name) {
return "Docker for Desktop"
}
return "Docker Service"
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.FullName(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.FullName(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

0 comments on commit 2086bcb

Please sign in to comment.