Merge pull request #6987 from tstromberg/memory-scaling2

Round suggested memory alloc by 100MB for VM's

cmd/minikube/cmd/start.go

@@ -116,7 +116,7 @@ const (
 	minUsableMem            = 1024 // Kubernetes will not start with less than 1GB
 	minRecommendedMem       = 2000 // Warn at no lower than existing configurations
 	minimumCPUS             = 2
-	minimumDiskSize         = "2000mb"
+	minimumDiskSize         = 2000
 	autoUpdate              = "auto-update-drivers"
 	hostOnlyNicType         = "host-only-nic-type"
 	natNicType              = "nat-nic-type"
@@ -642,43 +642,62 @@ func validateUser(drvName string) {
 	}
 }
 
-// defaultMemorySize calculates the default memory footprint in MB
+// memoryLimits returns the amount of memory allocated to the system and hypervisor
-func defaultMemorySize(drvName string) int {
+func memoryLimits(drvName string) (int, int, error) {
-	fallback := 2200
-	maximum := 6000
-
 	v, err := mem.VirtualMemory()
 	if err != nil {
-		return fallback
+		return -1, -1, err
 	}
-	available := v.Total / 1024 / 1024
+	sysLimit := int(v.Total / 1024 / 1024)
+	containerLimit := 0
 
-	// For KIC, do not allocate more memory than the container has available (+ some slack)
 	if driver.IsKIC(drvName) {
 		s, err := oci.DaemonInfo(drvName)
 		if err != nil {
-			return fallback
+			return -1, -1, err
 		}
-		maximum = int(s.TotalMemory/1024/1024) - 128
+		containerLimit = int(s.TotalMemory / 1024 / 1024)
+	}
+	return sysLimit, containerLimit, nil
+}
+
+// suggestMemoryAllocation calculates the default memory footprint in MB
+func suggestMemoryAllocation(sysLimit int, containerLimit int) int {
+	fallback := 2200
+	maximum := 6000
+
+	if sysLimit > 0 && fallback > sysLimit {
+		return sysLimit
 	}
 
-	suggested := int(available / 4)
+	// If there are container limits, add tiny bit of slack for non-minikube components
+	if containerLimit > 0 {
+		if fallback > containerLimit {
+			return containerLimit
+		}
+		maximum = containerLimit - 48
+	}
+
+	// Suggest 25% of RAM, rounded to nearest 100MB. Hyper-V requires an even number!
+	suggested := int(float32(sysLimit)/400.0) * 100
 
 	if suggested > maximum {
-		suggested = maximum
+		return maximum
 	}
 
 	if suggested < fallback {
-		suggested = fallback
+		return fallback
 	}
 
-	glog.Infof("Selecting memory default of %dMB, given %dMB available and %dMB maximum", suggested, available, maximum)
 	return suggested
 }
 
 // validateMemorySize validates the memory size matches the minimum recommended
 func validateMemorySize() {
-	req := pkgutil.CalculateSizeInMB(viper.GetString(memory))
+	req, err := pkgutil.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})
+	}
 	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})
@@ -711,9 +730,13 @@ func validateCPUCount(local bool) {
 // validateFlags validates the supplied flags against known bad combinations
 func validateFlags(cmd *cobra.Command, drvName string) {
 	if cmd.Flags().Changed(humanReadableDiskSize) {
-		diskSizeMB := pkgutil.CalculateSizeInMB(viper.GetString(humanReadableDiskSize))
+		diskSizeMB, err := pkgutil.CalculateSizeInMB(viper.GetString(humanReadableDiskSize))
-		if diskSizeMB < pkgutil.CalculateSizeInMB(minimumDiskSize) && !viper.GetBool(force) {
+		if err != nil {
-			exit.WithCodeT(exit.Config, "Requested disk size {{.requested_size}} is less than minimum of {{.minimum_size}}", out.V{"requested_size": diskSizeMB, "minimum_size": pkgutil.CalculateSizeInMB(minimumDiskSize)})
+			exit.WithCodeT(exit.Config, "Validation unable to parse disk size '{{.diskSize}}': {{.error}}", out.V{"diskSize": viper.GetString(humanReadableDiskSize), "error": err})
+		}
+
+		if diskSizeMB < minimumDiskSize && !viper.GetBool(force) {
+			exit.WithCodeT(exit.Config, "Requested disk size {{.requested_size}} is less than minimum of {{.minimum_size}}", out.V{"requested_size": diskSizeMB, "minimum_size": minimumDiskSize})
 		}
 	}
 
@@ -821,9 +844,20 @@ func generateCfgFromFlags(cmd *cobra.Command, k8sVersion string, drvName string)
 		kubeNodeName = "m01"
 	}
 
-	mem := defaultMemorySize(drvName)
+	sysLimit, containerLimit, err := memoryLimits(drvName)
-	if viper.GetString(memory) != "" {
+	if err != nil {
-		mem = pkgutil.CalculateSizeInMB(viper.GetString(memory))
+		glog.Warningf("Unable to query memory limits: %v", err)
+	}
+
+	mem := suggestMemoryAllocation(sysLimit, containerLimit)
+	if cmd.Flags().Changed(memory) {
+		mem, err = pkgutil.CalculateSizeInMB(viper.GetString(memory))
+		if err != nil {
+			exit.WithCodeT(exit.Config, "Generate unable to parse memory '{{.memory}}': {{.error}}", out.V{"memory": viper.GetString(memory), "error": err})
+		}
+
+	} else {
+		glog.Infof("Using suggested %dMB memory alloc based on sys=%dMB, container=%dMB", mem, sysLimit, containerLimit)
 	}
 
 	// Create the initial node, which will necessarily be a control plane
@@ -835,6 +869,11 @@ func generateCfgFromFlags(cmd *cobra.Command, k8sVersion string, drvName string)
 		Worker:            true,
 	}
 
+	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})
+	}
+
 	cfg := config.ClusterConfig{
 		Name:                    viper.GetString(config.ProfileName),
 		KeepContext:             viper.GetBool(keepContext),
@@ -842,7 +881,7 @@ func generateCfgFromFlags(cmd *cobra.Command, k8sVersion string, drvName string)
 		MinikubeISO:             viper.GetString(isoURL),
 		Memory:                  mem,
 		CPUs:                    viper.GetInt(cpus),
-		DiskSize:                pkgutil.CalculateSizeInMB(viper.GetString(humanReadableDiskSize)),
+		DiskSize:                diskSize,
 		Driver:                  drvName,
 		HyperkitVpnKitSock:      viper.GetString(vpnkitSock),
 		HyperkitVSockPorts:      viper.GetStringSlice(vsockPorts),

cmd/minikube/cmd/start_test.go

@@ -71,8 +71,9 @@ func TestGetKuberneterVersion(t *testing.T) {
 }
 
 func TestGenerateCfgFromFlagsHTTPProxyHandling(t *testing.T) {
-	viper.SetDefault(memory, defaultMemorySize)
+	// Set default disk size value in lieu of flag init
 	viper.SetDefault(humanReadableDiskSize, defaultDiskSize)
+
 	originalEnv := os.Getenv("HTTP_PROXY")
 	defer func() {
 		err := os.Setenv("HTTP_PROXY", originalEnv)
@@ -124,3 +125,34 @@ func TestGenerateCfgFromFlagsHTTPProxyHandling(t *testing.T) {
 		})
 	}
 }
+
+func TestSuggestMemoryAllocation(t *testing.T) {
+	var tests = []struct {
+		description    string
+		sysLimit       int
+		containerLimit int
+		want           int
+	}{
+		{"128GB sys", 128000, 0, 6000},
+		{"64GB sys", 64000, 0, 6000},
+		{"16GB sys", 16384, 0, 4000},
+		{"odd sys", 14567, 0, 3600},
+		{"4GB sys", 4096, 0, 2200},
+		{"2GB sys", 2048, 0, 2048},
+		{"Unable to poll sys", 0, 0, 2200},
+		{"128GB sys, 16GB container", 128000, 16384, 16336},
+		{"64GB sys, 16GB container", 64000, 16384, 16000},
+		{"16GB sys, 4GB container", 16384, 4096, 4000},
+		{"4GB sys, 3.5GB container", 16384, 3500, 3452},
+		{"2GB sys, 2GB container", 16384, 2048, 2048},
+		{"2GB sys, unable to poll container", 16384, 0, 4000},
+	}
+	for _, test := range tests {
+		t.Run(test.description, func(t *testing.T) {
+			got := suggestMemoryAllocation(test.sysLimit, test.containerLimit)
+			if got != test.want {
+				t.Errorf("defaultMemorySize(sys=%d, container=%d) = %d, want: %d", test.sysLimit, test.containerLimit, got, test.want)
+			}
+		})
+	}
+}

pkg/addons/addons.go

@@ -127,7 +127,10 @@ func enableOrDisableAddon(name, val, profile string) error {
 	if name == "istio" && enable {
 		minMem := 8192
 		minCpus := 4
-		memorySizeMB := pkgutil.CalculateSizeInMB(viper.GetString("memory"))
+		memorySizeMB, err := pkgutil.CalculateSizeInMB(viper.GetString("memory"))
+		if err != nil {
+			return errors.Wrap(err, "calculate memory")
+		}
 		cpuCount := viper.GetInt("cpus")
 		if memorySizeMB < minMem || cpuCount < minCpus {
 			out.WarningT("Enable istio needs {{.minMem}} MB of memory and {{.minCpus}} CPUs.", out.V{"minMem": minMem, "minCpus": minCpus})

pkg/util/utils.go

@@ -25,8 +25,6 @@ import (
 
 	units "github.com/docker/go-units"
 	"github.com/pkg/errors"
-	"k8s.io/minikube/pkg/minikube/exit"
-	"k8s.io/minikube/pkg/minikube/out"
 )
 
 const (
@@ -34,17 +32,17 @@ const (
 )
 
 // CalculateSizeInMB returns the number of MB in the human readable string
-func CalculateSizeInMB(humanReadableSize string) int {
+func CalculateSizeInMB(humanReadableSize string) (int, error) {
 	_, err := strconv.ParseInt(humanReadableSize, 10, 64)
 	if err == nil {
 		humanReadableSize += "mb"
 	}
 	size, err := units.FromHumanSize(humanReadableSize)
 	if err != nil {
-		exit.WithCodeT(exit.Config, "Invalid size passed in argument: {{.error}}", out.V{"error": err})
+		return 0, fmt.Errorf("FromHumanSize: %v", err)
 	}
 
-	return int(size / units.MB)
+	return int(size / units.MB), nil
 }
 
 // GetBinaryDownloadURL returns a suitable URL for the platform

pkg/util/utils_test.go

@@ -52,7 +52,10 @@ func TestCalculateSizeInMB(t *testing.T) {
 	}
 
 	for _, tt := range testData {
-		number := CalculateSizeInMB(tt.size)
+		number, err := CalculateSizeInMB(tt.size)
+		if err != nil {
+			t.Fatalf("unexpected err: %v", err)
+		}
 		if number != tt.expectedNumber {
 			t.Fatalf("Expected '%d'' but got '%d'", tt.expectedNumber, number)
 		}