From d59bd66e1b6657aa6574d8f10da369d70a418e9b Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 30 Jan 2018 15:01:45 -0800 Subject: [PATCH 1/5] daemon.cleanupContainer: nullify container RWLayer upon release ReleaseRWLayer can and should only be called once (unless it returns an error), but might be called twice in case of a failure from `system.EnsureRemoveAll(container.Root)`. This results in the following error: > Error response from daemon: driver "XXX" failed to remove root filesystem for YYY: layer not retained The obvious fix is to set container.RWLayer to nil as soon as ReleaseRWLayer() succeeds. Signed-off-by: Kir Kolyshkin Upstream-commit: e9b9e4ace294230c6b8eb010eda564a2541c4564 Component: engine --- components/engine/daemon/delete.go | 1 + 1 file changed, 1 insertion(+) diff --git a/components/engine/daemon/delete.go b/components/engine/daemon/delete.go index f16d38beb3..c6f8efec45 100644 --- a/components/engine/daemon/delete.go +++ b/components/engine/daemon/delete.go @@ -128,6 +128,7 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, forceRemo container.SetRemovalError(e) return e } + container.RWLayer = nil } if err := system.EnsureRemoveAll(container.Root); err != nil { From 6744cda5265619fb2d60d7a09138be6dd322c5f2 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 31 Jan 2018 01:30:56 -0800 Subject: [PATCH 2/5] bump docker/go-connections to 98e7d807e5d804e4e42a98d74d1dd695321224ef Signed-off-by: Sebastiaan van Stijn Upstream-commit: a6d35a822eed940260c74e2e7eea6455953ffabe Component: engine --- components/engine/vendor.conf | 2 +- .../docker/go-connections/tlsconfig/config.go | 22 ++++++++++++++----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/components/engine/vendor.conf b/components/engine/vendor.conf index dc06f6a6fb..38ebca2602 100644 --- a/components/engine/vendor.conf +++ b/components/engine/vendor.conf @@ -16,7 +16,7 @@ github.com/vdemeester/shakers 24d7f1d6a71aa5d9cbe7390e4afb66b7eef9e1b3 golang.org/x/net 7dcfb8076726a3fdd9353b6b8a1f1b6be6811bd6 golang.org/x/sys 95c6576299259db960f6c5b9b69ea52422860fce github.com/docker/go-units 9e638d38cf6977a37a8ea0078f3ee75a7cdb2dd1 -github.com/docker/go-connections 3ede32e2033de7505e6500d6c868c2b9ed9f169d +github.com/docker/go-connections 98e7d807e5d804e4e42a98d74d1dd695321224ef golang.org/x/text f72d8390a633d5dfb0cc84043294db9f6c935756 github.com/stretchr/testify 4d4bfba8f1d1027c4fdbe371823030df51419987 github.com/pmezard/go-difflib v1.0.0 diff --git a/components/engine/vendor/github.com/docker/go-connections/tlsconfig/config.go b/components/engine/vendor/github.com/docker/go-connections/tlsconfig/config.go index 1b31bbb8b1..f11f166a44 100644 --- a/components/engine/vendor/github.com/docker/go-connections/tlsconfig/config.go +++ b/components/engine/vendor/github.com/docker/go-connections/tlsconfig/config.go @@ -65,22 +65,34 @@ var allTLSVersions = map[uint16]struct{}{ } // ServerDefault returns a secure-enough TLS configuration for the server TLS configuration. -func ServerDefault() *tls.Config { - return &tls.Config{ - // Avoid fallback to SSL protocols < TLS1.0 +func ServerDefault(ops ...func(*tls.Config)) *tls.Config { + tlsconfig := &tls.Config{ + // Avoid fallback by default to SSL protocols < TLS1.0 MinVersion: tls.VersionTLS10, PreferServerCipherSuites: true, CipherSuites: DefaultServerAcceptedCiphers, } + + for _, op := range ops { + op(tlsconfig) + } + + return tlsconfig } // ClientDefault returns a secure-enough TLS configuration for the client TLS configuration. -func ClientDefault() *tls.Config { - return &tls.Config{ +func ClientDefault(ops ...func(*tls.Config)) *tls.Config { + tlsconfig := &tls.Config{ // Prefer TLS1.2 as the client minimum MinVersion: tls.VersionTLS12, CipherSuites: clientCipherSuites, } + + for _, op := range ops { + op(tlsconfig) + } + + return tlsconfig } // certPool returns an X.509 certificate pool from `caFile`, the certificate file. From 877ec711a0d960168c5f4b56eb6ce4dcfe8e065d Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Tue, 30 Jan 2018 17:26:56 +0000 Subject: [PATCH 3/5] Fix issue of ExitCode and PID not show up in Task.Status.ContainerStatus This fix tries to address the issue raised in 36139 where ExitCode and PID does not show up in Task.Status.ContainerStatus The issue was caused by `json:",omitempty"` in PID and ExitCode which interprate 0 as null. This is confusion as ExitCode 0 does have a meaning. This fix removes `json:",omitempty"` in ExitCode and PID, but changes ContainerStatus to pointer so that ContainerStatus does not show up at all if no content. If ContainerStatus does have a content, then ExitCode and PID will show up (even if they are 0). This fix fixes 36139. Signed-off-by: Yong Tang Upstream-commit: 9247e09944a4c7f3c2f3f20f180c047a19fb6bae Component: engine --- components/engine/api/types/swarm/task.go | 18 ++++++------ .../engine/daemon/cluster/convert/task.go | 8 ++++-- .../docker_cli_service_create_test.go | 28 +++++++++---------- 3 files changed, 28 insertions(+), 26 deletions(-) diff --git a/components/engine/api/types/swarm/task.go b/components/engine/api/types/swarm/task.go index ff11b07e74..079ec3690f 100644 --- a/components/engine/api/types/swarm/task.go +++ b/components/engine/api/types/swarm/task.go @@ -162,19 +162,19 @@ const ( // TaskStatus represents the status of a task. type TaskStatus struct { - Timestamp time.Time `json:",omitempty"` - State TaskState `json:",omitempty"` - Message string `json:",omitempty"` - Err string `json:",omitempty"` - ContainerStatus ContainerStatus `json:",omitempty"` - PortStatus PortStatus `json:",omitempty"` + Timestamp time.Time `json:",omitempty"` + State TaskState `json:",omitempty"` + Message string `json:",omitempty"` + Err string `json:",omitempty"` + ContainerStatus *ContainerStatus `json:",omitempty"` + PortStatus PortStatus `json:",omitempty"` } // ContainerStatus represents the status of a container. type ContainerStatus struct { - ContainerID string `json:",omitempty"` - PID int `json:",omitempty"` - ExitCode int `json:",omitempty"` + ContainerID string + PID int + ExitCode int } // PortStatus represents the port status of a task's host ports whose diff --git a/components/engine/daemon/cluster/convert/task.go b/components/engine/daemon/cluster/convert/task.go index bedf2dba9e..be3bacf11c 100644 --- a/components/engine/daemon/cluster/convert/task.go +++ b/components/engine/daemon/cluster/convert/task.go @@ -42,9 +42,11 @@ func TaskFromGRPC(t swarmapi.Task) (types.Task, error) { task.Status.Timestamp, _ = gogotypes.TimestampFromProto(t.Status.Timestamp) if containerStatus != nil { - task.Status.ContainerStatus.ContainerID = containerStatus.ContainerID - task.Status.ContainerStatus.PID = int(containerStatus.PID) - task.Status.ContainerStatus.ExitCode = int(containerStatus.ExitCode) + task.Status.ContainerStatus = &types.ContainerStatus{ + ContainerID: containerStatus.ContainerID, + PID: int(containerStatus.PID), + ExitCode: int(containerStatus.ExitCode), + } } // NetworksAttachments diff --git a/components/engine/integration-cli/docker_cli_service_create_test.go b/components/engine/integration-cli/docker_cli_service_create_test.go index ac7eff713c..d690b7e45f 100644 --- a/components/engine/integration-cli/docker_cli_service_create_test.go +++ b/components/engine/integration-cli/docker_cli_service_create_test.go @@ -29,10 +29,10 @@ func (s *DockerSwarmSuite) TestServiceCreateMountVolume(c *check.C) { task := tasks[0] waitAndAssert(c, defaultReconciliationTimeout, func(c *check.C) (interface{}, check.CommentInterface) { - if task.NodeID == "" || task.Status.ContainerStatus.ContainerID == "" { + if task.NodeID == "" || task.Status.ContainerStatus == nil { task = d.GetTask(c, task.ID) } - return task.NodeID != "" && task.Status.ContainerStatus.ContainerID != "", nil + return task.NodeID != "" && task.Status.ContainerStatus != nil, nil }, checker.Equals, true) // check container mount config @@ -143,10 +143,10 @@ func (s *DockerSwarmSuite) TestServiceCreateWithSecretSourceTargetPaths(c *check task := tasks[0] waitAndAssert(c, defaultReconciliationTimeout, func(c *check.C) (interface{}, check.CommentInterface) { - if task.NodeID == "" || task.Status.ContainerStatus.ContainerID == "" { + if task.NodeID == "" || task.Status.ContainerStatus == nil { task = d.GetTask(c, task.ID) } - return task.NodeID != "" && task.Status.ContainerStatus.ContainerID != "", nil + return task.NodeID != "" && task.Status.ContainerStatus != nil, nil }, checker.Equals, true) for testName, testTarget := range testPaths { @@ -193,10 +193,10 @@ func (s *DockerSwarmSuite) TestServiceCreateWithSecretReferencedTwice(c *check.C task := tasks[0] waitAndAssert(c, defaultReconciliationTimeout, func(c *check.C) (interface{}, check.CommentInterface) { - if task.NodeID == "" || task.Status.ContainerStatus.ContainerID == "" { + if task.NodeID == "" || task.Status.ContainerStatus == nil { task = d.GetTask(c, task.ID) } - return task.NodeID != "" && task.Status.ContainerStatus.ContainerID != "", nil + return task.NodeID != "" && task.Status.ContainerStatus != nil, nil }, checker.Equals, true) for _, target := range []string{"target1", "target2"} { @@ -290,10 +290,10 @@ func (s *DockerSwarmSuite) TestServiceCreateWithConfigSourceTargetPaths(c *check task := tasks[0] waitAndAssert(c, defaultReconciliationTimeout, func(c *check.C) (interface{}, check.CommentInterface) { - if task.NodeID == "" || task.Status.ContainerStatus.ContainerID == "" { + if task.NodeID == "" || task.Status.ContainerStatus == nil { task = d.GetTask(c, task.ID) } - return task.NodeID != "" && task.Status.ContainerStatus.ContainerID != "", nil + return task.NodeID != "" && task.Status.ContainerStatus != nil, nil }, checker.Equals, true) for testName, testTarget := range testPaths { @@ -340,10 +340,10 @@ func (s *DockerSwarmSuite) TestServiceCreateWithConfigReferencedTwice(c *check.C task := tasks[0] waitAndAssert(c, defaultReconciliationTimeout, func(c *check.C) (interface{}, check.CommentInterface) { - if task.NodeID == "" || task.Status.ContainerStatus.ContainerID == "" { + if task.NodeID == "" || task.Status.ContainerStatus == nil { task = d.GetTask(c, task.ID) } - return task.NodeID != "" && task.Status.ContainerStatus.ContainerID != "", nil + return task.NodeID != "" && task.Status.ContainerStatus != nil, nil }, checker.Equals, true) for _, target := range []string{"target1", "target2"} { @@ -372,10 +372,10 @@ func (s *DockerSwarmSuite) TestServiceCreateMountTmpfs(c *check.C) { task := tasks[0] waitAndAssert(c, defaultReconciliationTimeout, func(c *check.C) (interface{}, check.CommentInterface) { - if task.NodeID == "" || task.Status.ContainerStatus.ContainerID == "" { + if task.NodeID == "" || task.Status.ContainerStatus == nil { task = d.GetTask(c, task.ID) } - return task.NodeID != "" && task.Status.ContainerStatus.ContainerID != "", nil + return task.NodeID != "" && task.Status.ContainerStatus != nil, nil }, checker.Equals, true) // check container mount config @@ -428,10 +428,10 @@ func (s *DockerSwarmSuite) TestServiceCreateWithNetworkAlias(c *check.C) { task := tasks[0] waitAndAssert(c, defaultReconciliationTimeout, func(c *check.C) (interface{}, check.CommentInterface) { - if task.NodeID == "" || task.Status.ContainerStatus.ContainerID == "" { + if task.NodeID == "" || task.Status.ContainerStatus == nil { task = d.GetTask(c, task.ID) } - return task.NodeID != "" && task.Status.ContainerStatus.ContainerID != "", nil + return task.NodeID != "" && task.Status.ContainerStatus != nil, nil }, checker.Equals, true) // check container alias config From 6f8c4ca576dc74d023460624728efe1c5b087584 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Wed, 31 Jan 2018 00:58:32 +0000 Subject: [PATCH 4/5] Remove docker_api_update_unix_test.go Signed-off-by: Yong Tang Upstream-commit: 8786b09c81e4eed540924ef415ecdfb5f9affa01 Component: engine --- .../docker_api_update_unix_test.go | 45 ------------------- 1 file changed, 45 deletions(-) delete mode 100644 components/engine/integration-cli/docker_api_update_unix_test.go diff --git a/components/engine/integration-cli/docker_api_update_unix_test.go b/components/engine/integration-cli/docker_api_update_unix_test.go deleted file mode 100644 index e2e1b092be..0000000000 --- a/components/engine/integration-cli/docker_api_update_unix_test.go +++ /dev/null @@ -1,45 +0,0 @@ -// +build !windows - -package main - -import ( - "strings" - - "github.com/docker/docker/api/types/container" - "github.com/docker/docker/client" - "github.com/docker/docker/integration-cli/checker" - "github.com/go-check/check" - "golang.org/x/net/context" -) - -func (s *DockerSuite) TestAPIUpdateContainer(c *check.C) { - testRequires(c, DaemonIsLinux) - testRequires(c, memoryLimitSupport) - testRequires(c, swapMemorySupport) - - name := "apiUpdateContainer" - updateConfig := container.UpdateConfig{ - Resources: container.Resources{ - Memory: 314572800, - MemorySwap: 524288000, - }, - } - dockerCmd(c, "run", "-d", "--name", name, "-m", "200M", "busybox", "top") - cli, err := client.NewEnvClient() - c.Assert(err, check.IsNil) - defer cli.Close() - - _, err = cli.ContainerUpdate(context.Background(), name, updateConfig) - - c.Assert(err, check.IsNil) - - c.Assert(inspectField(c, name, "HostConfig.Memory"), checker.Equals, "314572800") - file := "/sys/fs/cgroup/memory/memory.limit_in_bytes" - out, _ := dockerCmd(c, "exec", name, "cat", file) - c.Assert(strings.TrimSpace(out), checker.Equals, "314572800") - - c.Assert(inspectField(c, name, "HostConfig.MemorySwap"), checker.Equals, "524288000") - file = "/sys/fs/cgroup/memory/memory.memsw.limit_in_bytes" - out, _ = dockerCmd(c, "exec", name, "cat", file) - c.Assert(strings.TrimSpace(out), checker.Equals, "524288000") -} From 2fdf0e5bd370eebe48ad0c36032713f74f83d04d Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Wed, 31 Jan 2018 16:10:41 +0000 Subject: [PATCH 5/5] Migrate TestAPIUpdateContainer from integration-cli to api tests This fix migrates TestAPIUpdateContainer from integration-cli to api tests Signed-off-by: Yong Tang Upstream-commit: 490edd35829a7dd1144da50105fc377d41a52fc0 Component: engine --- .../container/update_linux_test.go | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/components/engine/integration/container/update_linux_test.go b/components/engine/integration/container/update_linux_test.go index bb9906ac7d..53ed10692c 100644 --- a/components/engine/integration/container/update_linux_test.go +++ b/components/engine/integration/container/update_linux_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "fmt" + "io/ioutil" "strconv" "strings" "testing" @@ -11,10 +12,67 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/strslice" + "github.com/docker/docker/client" "github.com/docker/docker/integration/util/request" "github.com/docker/docker/pkg/stdcopy" + "github.com/gotestyourself/gotestyourself/poll" + "github.com/gotestyourself/gotestyourself/skip" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +func TestUpdateMemory(t *testing.T) { + skip.If(t, testEnv.DaemonInfo.OSType != "linux") + skip.If(t, !testEnv.DaemonInfo.MemoryLimit) + skip.If(t, !testEnv.DaemonInfo.SwapLimit) + + defer setupTest(t)() + client := request.NewAPIClient(t) + ctx := context.Background() + + c, err := client.ContainerCreate(ctx, + &container.Config{ + Cmd: []string{"top"}, + Image: "busybox", + }, + &container.HostConfig{ + Resources: container.Resources{ + Memory: 200 * 1024 * 1024, + }, + }, + nil, + "", + ) + require.NoError(t, err) + + err = client.ContainerStart(ctx, c.ID, types.ContainerStartOptions{}) + require.NoError(t, err) + + poll.WaitOn(t, containerIsInState(ctx, client, c.ID, "running"), poll.WithDelay(100*time.Millisecond)) + + _, err = client.ContainerUpdate(ctx, c.ID, container.UpdateConfig{ + Resources: container.Resources{ + Memory: 314572800, + MemorySwap: 524288000, + }, + }) + require.NoError(t, err) + + inspect, err := client.ContainerInspect(ctx, c.ID) + require.NoError(t, err) + assert.Equal(t, inspect.HostConfig.Memory, int64(314572800)) + assert.Equal(t, inspect.HostConfig.MemorySwap, int64(524288000)) + + body, err := getContainerSysFSValue(ctx, client, c.ID, "/sys/fs/cgroup/memory/memory.limit_in_bytes") + require.NoError(t, err) + assert.Equal(t, strings.TrimSpace(body), "314572800") + + body, err = getContainerSysFSValue(ctx, client, c.ID, "/sys/fs/cgroup/memory/memory.memsw.limit_in_bytes") + require.NoError(t, err) + assert.Equal(t, strings.TrimSpace(body), "524288000") +} + func TestUpdateCPUQUota(t *testing.T) { t.Parallel() @@ -106,3 +164,33 @@ func TestUpdateCPUQUota(t *testing.T) { } } + +func getContainerSysFSValue(ctx context.Context, client client.APIClient, cID string, path string) (string, error) { + var b bytes.Buffer + + ex, err := client.ContainerExecCreate(ctx, cID, + types.ExecConfig{ + AttachStdout: true, + Cmd: strslice.StrSlice([]string{"cat", path}), + }, + ) + if err != nil { + return "", err + } + + resp, err := client.ContainerExecAttach(ctx, ex.ID, + types.ExecStartCheck{ + Detach: false, + Tty: false, + }, + ) + if err != nil { + return "", err + } + + defer resp.Close() + + b.Reset() + _, err = stdcopy.StdCopy(&b, ioutil.Discard, resp.Reader) + return b.String(), err +}