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.

Sign up for GitHub

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

Jump to bottom

improve auto-select memory for multinode clusters #7928

Merged
merged 5 commits into from
Apr 29, 2020
Merged

improve auto-select memory for multinode clusters #7928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e61a18c
be smarter about allocating memory for multinode clusters
sharifelgamal Apr 28, 2020
b116cd1
lint
sharifelgamal Apr 28, 2020
de5e493
add unit tests
sharifelgamal Apr 28, 2020
dc0ceee
debugging for multinode test
sharifelgamal Apr 29, 2020
90cd6c3
don't run multinode tests in parallel
sharifelgamal Apr 29, 2020
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
6 changes: 6 additions & 0 deletions cmd/minikube/cmd/node_add.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package cmd

import (
"github.com/spf13/cobra"
"github.com/spf13/viper"
"k8s.io/minikube/pkg/minikube/config"
"k8s.io/minikube/pkg/minikube/driver"
"k8s.io/minikube/pkg/minikube/exit"
Expand Down Expand Up @@ -54,6 +55,11 @@ var nodeAddCmd = &cobra.Command{
KubernetesVersion: cc.KubernetesConfig.KubernetesVersion,
}

// Make sure to decrease the default amount of memory we use per VM if this is the first worker node
if len(cc.Nodes) == 1 && viper.GetString(memory) == "" {
medyagh marked this conversation as resolved.
Show resolved Hide resolved
cc.Memory = 2200
}

if err := node.Add(cc, n); err != nil {
_, err := maybeDeleteAndRetry(*cc, n, nil, err)
if err != nil {
Expand Down
6 changes: 5 additions & 1 deletion cmd/minikube/cmd/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@ func memoryLimits(drvName string) (int, int, error) {
}

// suggestMemoryAllocation calculates the default memory footprint in MB
func suggestMemoryAllocation(sysLimit int, containerLimit int) int {
func suggestMemoryAllocation(sysLimit int, containerLimit int, nodes int) int {
if mem := viper.GetInt(memory); mem != 0 {
return mem
}
Expand All @@ -737,6 +737,10 @@ func suggestMemoryAllocation(sysLimit int, containerLimit int) int {
// Suggest 25% of RAM, rounded to nearest 100MB. Hyper-V requires an even number!
suggested := int(float32(sysLimit)/400.0) * 100

if nodes > 1 {
suggested /= nodes
sharifelgamal marked this conversation as resolved.
Show resolved Hide resolved
}

if suggested > maximum {
return maximum
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/start_flags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func generateClusterConfig(cmd *cobra.Command, existing *config.ClusterConfig, k
glog.Warningf("Unable to query memory limits: %v", err)
}

mem := suggestMemoryAllocation(sysLimit, containerLimit)
mem := suggestMemoryAllocation(sysLimit, containerLimit, viper.GetInt(nodes))
sharifelgamal marked this conversation as resolved.
Show resolved Hide resolved
if cmd.Flags().Changed(memory) {
mem, err = pkgutil.CalculateSizeInMB(viper.GetString(memory))
if err != nil {
Expand Down
37 changes: 23 additions & 14 deletions cmd/minikube/cmd/start_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,25 +185,34 @@ func TestSuggestMemoryAllocation(t *testing.T) {
description string
sysLimit int
containerLimit int
nodes 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},
{"128GB sys", 128000, 0, 1, 6000},
{"64GB sys", 64000, 0, 1, 6000},
{"32GB sys", 32768, 0, 1, 6000},
{"16GB sys", 16384, 0, 1, 4000},
{"odd sys", 14567, 0, 1, 3600},
{"4GB sys", 4096, 0, 1, 2200},
{"2GB sys", 2048, 0, 1, 2048},
{"Unable to poll sys", 0, 0, 1, 2200},
{"128GB sys, 16GB container", 128000, 16384, 1, 16336},
{"64GB sys, 16GB container", 64000, 16384, 1, 16000},
{"16GB sys, 4GB container", 16384, 4096, 1, 4000},
{"4GB sys, 3.5GB container", 16384, 3500, 1, 3452},
{"16GB sys, 2GB container", 16384, 2048, 1, 2048},
{"16GB sys, unable to poll container", 16384, 0, 1, 4000},
{"128GB sys 2 nodes", 128000, 0, 2, 6000},
{"8GB sys 3 nodes", 8192, 0, 3, 2200},
{"16GB sys 2 nodes", 16384, 0, 2, 2200},
{"32GB sys 2 nodes", 32768, 0, 2, 4050},
{"odd sys 2 nodes", 14567, 0, 2, 2200},
{"4GB sys 2 nodes", 4096, 0, 2, 2200},
{"2GB sys 3 nodes", 2048, 0, 3, 2048},
}
for _, test := range tests {
t.Run(test.description, func(t *testing.T) {
got := suggestMemoryAllocation(test.sysLimit, test.containerLimit)
got := suggestMemoryAllocation(test.sysLimit, test.containerLimit, test.nodes)
if got != test.want {
t.Errorf("defaultMemorySize(sys=%d, container=%d) = %d, want: %d", test.sysLimit, test.containerLimit, got, test.want)
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/minikube/node/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,8 @@ func configureRuntimes(runner cruntime.CommandRunner, cc config.ClusterConfig, k
disableOthers = false
}

// Preload is overly invasive for bare metal, and caching is not meaningful. KIC handled elsewhere.
// Preload is overly invasive for bare metal, and caching is not meaningful.
// KIC handles preload elsewhere.
if driver.IsVM(cc.Driver) {
if err := cr.Preload(cc.KubernetesConfig); err != nil {
switch err.(type) {
Expand Down
9 changes: 4 additions & 5 deletions test/integration/multinode_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ func TestMultiNode(t *testing.T) {
if NoneDriver() {
t.Skip("none driver does not support multinode")
}
MaybeParallel(t)

type validatorFunc func(context.Context, *testing.T, string)
profile := UniqueProfileName("multinode")
Expand Down Expand Up @@ -65,7 +64,7 @@ func validateMultiNodeStart(ctx context.Context, t *testing.T, profile string) {
}

// Make sure minikube status shows 2 nodes
rr, err = Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "status"))
rr, err = Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "status", "--alsologtostderr"))
if err != nil {
t.Fatalf("failed to run minikube status. args %q : %v", rr.Command(), err)
}
Expand All @@ -89,7 +88,7 @@ func validateAddNodeToMultiNode(ctx context.Context, t *testing.T, profile strin
}

// Make sure minikube status shows 3 nodes
rr, err = Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "status"))
rr, err = Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "status", "--alsologtostderr"))
if err != nil {
t.Fatalf("failed to run minikube status. args %q : %v", rr.Command(), err)
}
Expand Down Expand Up @@ -121,7 +120,7 @@ func validateStopRunningNode(ctx context.Context, t *testing.T, profile string)
}

// Make sure minikube status shows 2 running nodes and 1 stopped one
rr, err = Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "status"))
rr, err = Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "status", "--alsologtostderr"))
if err != nil && rr.ExitCode != 7 {
t.Fatalf("failed to run minikube status. args %q : %v", rr.Command(), err)
}
Expand Down Expand Up @@ -177,7 +176,7 @@ func validateDeleteNodeFromMultiNode(ctx context.Context, t *testing.T, profile
}

// Make sure status is back down to 2 hosts
rr, err = Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "status"))
rr, err = Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "status", "--alsologtostderr"))
if err != nil {
t.Fatalf("failed to run minikube status. args %q : %v", rr.Command(), err)
}
Expand Down