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
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 @@ -752,12 +752,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 @@ -800,11 +801,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 +816,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 {
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.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 @@ -832,6 +853,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)})
}

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

}
}
}

// validateFlags validates the supplied flags against known bad combinations
Expand All @@ -848,14 +893,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