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

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

Merged
merged 6 commits into from
Oct 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 4 additions & 2 deletions pkg/provision/buildroot.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ Requires= minikube-automount.service docker.socket

[Service]
Type=notify

`
if noPivot {
log.Warn("Using fundamentally insecure --no-pivot option")
Expand All @@ -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 }}
tstromberg marked this conversation as resolved.
Show resolved Hide resolved
ExecReload=/bin/kill -s HUP $MAINPID

# Having non-zero Limit*s causes performance problems due to accounting overhead
Expand Down
24 changes: 23 additions & 1 deletion test/integration/functional_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ func TestFunctional(t *testing.T) {
{"PersistentVolumeClaim", validatePersistentVolumeClaim},
{"TunnelCmd", validateTunnelCmd},
{"SSHCmd", validateSSHCmd},
{"MySQL", validateMySQL},
}
for _, tc := range tests {
tc := tc
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand Down
31 changes: 26 additions & 5 deletions test/integration/start_stop_delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"fmt"
"os/exec"
"path/filepath"
"strconv"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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)
}
}
})
}
Expand Down
35 changes: 35 additions & 0 deletions test/integration/testdata/mysql.yaml
Original file line number Diff line number Diff line change
@@ -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