Merge pull request #5761 from tstromberg/ulimit

Set Docker open-files limit ( 'ulimit -n') to be consistent with other runtimes

pkg/provision/buildroot.go

@@ -107,7 +107,6 @@ Requires= minikube-automount.service docker.socket
 
 [Service]
 Type=notify
-
 `
 	if noPivot {
 		log.Warn("Using fundamentally insecure --no-pivot option")
@@ -127,8 +126,11 @@ Environment=DOCKER_RAMDISK=yes
 # a sequence of commands, which is not the desired behavior, nor is it valid -- systemd
 # will catch this invalid input and refuse to start the service with an error like:
 #  Service has more than one ExecStart= setting, which is only allowed for Type=oneshot services.
+
+# NOTE: default-ulimit=nofile is set to an arbitrary number for consistency with other
+# container runtimes. If left unlimited, it may result in OOM issues with MySQL.
 ExecStart=
-ExecStart=/usr/bin/dockerd -H tcp://0.0.0.0:{{.DockerPort}} -H unix:///var/run/docker.sock --tlsverify --tlscacert {{.AuthOptions.CaCertRemotePath}} --tlscert {{.AuthOptions.ServerCertRemotePath}} --tlskey {{.AuthOptions.ServerKeyRemotePath}} {{ range .EngineOptions.Labels }}--label {{.}} {{ end }}{{ range .EngineOptions.InsecureRegistry }}--insecure-registry {{.}} {{ end }}{{ range .EngineOptions.RegistryMirror }}--registry-mirror {{.}} {{ end }}{{ range .EngineOptions.ArbitraryFlags }}--{{.}} {{ end }}
+ExecStart=/usr/bin/dockerd -H tcp://0.0.0.0:{{.DockerPort}} -H unix:///var/run/docker.sock --default-ulimit=nofile=1048576:1048576 --tlsverify --tlscacert {{.AuthOptions.CaCertRemotePath}} --tlscert {{.AuthOptions.ServerCertRemotePath}} --tlskey {{.AuthOptions.ServerKeyRemotePath}} {{ range .EngineOptions.Labels }}--label {{.}} {{ end }}{{ range .EngineOptions.InsecureRegistry }}--insecure-registry {{.}} {{ end }}{{ range .EngineOptions.RegistryMirror }}--registry-mirror {{.}} {{ end }}{{ range .EngineOptions.ArbitraryFlags }}--{{.}} {{ end }}
 ExecReload=/bin/kill -s HUP $MAINPID
 
 # Having non-zero Limit*s causes performance problems due to accounting overhead

test/integration/functional_test.go

@@ -94,6 +94,7 @@ func TestFunctional(t *testing.T) {
 			{"PersistentVolumeClaim", validatePersistentVolumeClaim},
 			{"TunnelCmd", validateTunnelCmd},
 			{"SSHCmd", validateSSHCmd},
+			{"MySQL", validateMySQL},
 		}
 		for _, tc := range tests {
 			tc := tc
@@ -297,7 +298,7 @@ func validateCacheCmd(ctx context.Context, t *testing.T, profile string) {
 	if NoneDriver() {
 		t.Skipf("skipping: cache unsupported by none")
 	}
-	for _, img := range []string{"busybox", "busybox:1.28.4-glibc"} {
+	for _, img := range []string{"busybox", "busybox:1.28.4-glibc", "mysql:5.6"} {
 		rr, err := Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "cache", "add", img))
 		if err != nil {
 			t.Errorf("%s failed: %v", rr.Args, err)
@@ -478,6 +479,27 @@ func validateSSHCmd(ctx context.Context, t *testing.T, profile string) {
 	}
 }
 
+// validateMySQL validates a minimalist MySQL deployment
+func validateMySQL(ctx context.Context, t *testing.T, profile string) {
+	rr, err := Run(t, exec.CommandContext(ctx, "kubectl", "--context", profile, "replace", "--force", "-f", filepath.Join(*testdataDir, "mysql.yaml")))
+	if err != nil {
+		t.Fatalf("%s failed: %v", rr.Args, err)
+	}
+
+	// Retry, as mysqld first comes up without users configured. Scan for names in case of a reschedule.
+	mysql := func() error {
+		names, err := PodWait(ctx, t, profile, "default", "app=mysql", 5*time.Second)
+		if err != nil {
+			return err
+		}
+		rr, err = Run(t, exec.CommandContext(ctx, "kubectl", "--context", profile, "exec", names[0], "--", "mysql", "-ppassword", "-e", "show databases;"))
+		return err
+	}
+	if err = retry.Expo(mysql, 1*time.Second, 2*time.Minute); err != nil {
+		t.Errorf("mysql failing: %v", err)
+	}
+}
+
 // startHTTPProxy runs a local http proxy and sets the env vars for it.
 func startHTTPProxy(t *testing.T) (*http.Server, error) {
 	port, err := freeport.GetFreePort()

test/integration/start_stop_delete_test.go

@@ -23,6 +23,7 @@ import (
 	"fmt"
 	"os/exec"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -102,9 +103,27 @@ func TestStartStop(t *testing.T) {
 						t.Fatalf("%s failed: %v", rr.Args, err)
 					}
 
-					if _, err := PodWait(ctx, t, profile, "default", "integration-test=busybox", 2*time.Minute); err != nil {
+					names, err := PodWait(ctx, t, profile, "default", "integration-test=busybox", 2*time.Minute)
+					if err != nil {
 						t.Fatalf("wait: %v", err)
 					}
+
+					// Use this pod to confirm that the runtime resource limits are sane
+					rr, err = Run(t, exec.CommandContext(ctx, "kubectl", "--context", profile, "exec", names[0], "--", "/bin/sh", "-c", "ulimit -n"))
+					if err != nil {
+						t.Fatalf("ulimit: %v", err)
+					}
+
+					got, err := strconv.ParseInt(strings.TrimSpace(rr.Stdout.String()), 10, 64)
+					if err != nil {
+						t.Errorf("ParseInt(%q): %v", rr.Stdout.String(), err)
+					}
+
+					// Arbitrary value set by some container runtimes. If higher, apps like MySQL may make bad decisions.
+					expected := int64(1048576)
+					if got != expected {
+						t.Errorf("'ulimit -n' returned %d, expected %d", got, expected)
+					}
 				}
 
 				rr, err = Run(t, exec.CommandContext(ctx, Target(), "stop", "-p", profile, "--alsologtostderr", "-v=3"))
@@ -134,10 +153,12 @@ func TestStartStop(t *testing.T) {
 					t.Errorf("status = %q; want = %q", got, state.Running)
 				}
 
-				// Normally handled by cleanuprofile, but not fatal there
-				rr, err = Run(t, exec.CommandContext(ctx, Target(), "delete", "-p", profile))
-				if err != nil {
-					t.Errorf("%s failed: %v", rr.Args, err)
+				if !*cleanup {
+					// Normally handled by cleanuprofile, but not fatal there
+					rr, err = Run(t, exec.CommandContext(ctx, Target(), "delete", "-p", profile))
+					if err != nil {
+						t.Errorf("%s failed: %v", rr.Args, err)
+					}
 				}
 			})
 		}

test/integration/testdata/mysql.yaml

@@ -0,0 +1,35 @@
+apiVersion: v1
+kind: Service
+metadata:
+  name: mysql
+spec:
+  ports:
+  - port: 3306
+  selector:
+    app: mysql
+---
+apiVersion: apps/v1 # for versions before 1.9.0 use apps/v1beta2
+kind: Deployment
+metadata:
+  name: mysql
+spec:
+  selector:
+    matchLabels:
+      app: mysql
+  strategy:
+    type: Recreate
+  template:
+    metadata:
+      labels:
+        app: mysql
+    spec:
+      containers:
+      - image: mysql:5.6
+        name: mysql
+        env:
+          # Use secret in real usage
+        - name: MYSQL_ROOT_PASSWORD
+          value: password
+        ports:
+        - containerPort: 3306
+          name: mysql