From 3f7322c602613a894f0deabf210cc6fa58a3cee2 Mon Sep 17 00:00:00 2001 From: Marcus Martins Date: Tue, 19 Dec 2017 13:23:21 -0800 Subject: [PATCH 1/4] Vendor docker/swarmkit to 713d79d Revendor swarmkit to 713d79dc8799b33465c58ed120b870c52eb5eb4f to include https://github.com/docker/swarmkit/pull/2473. Signed-off-by: Marcus Martins Upstream-commit: af73d31e60fd5c26d58bb8275785e628c3febdc0 Component: engine --- components/engine/vendor.conf | 2 +- .../manager/orchestrator/taskreaper/task_reaper.go | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/components/engine/vendor.conf b/components/engine/vendor.conf index 78330a284a..216370695a 100644 --- a/components/engine/vendor.conf +++ b/components/engine/vendor.conf @@ -114,7 +114,7 @@ github.com/dmcgowan/go-tar go1.10 github.com/stevvooe/ttrpc 76e68349ad9ab4d03d764c713826d31216715e4f # cluster -github.com/docker/swarmkit a6519e28ff2a558f5d32b2dab9fcb0882879b398 +github.com/docker/swarmkit 713d79dc8799b33465c58ed120b870c52eb5eb4f github.com/gogo/protobuf v0.4 github.com/cloudflare/cfssl 7fb22c8cba7ecaf98e4082d22d65800cf45e042a github.com/google/certificate-transparency d90e65c3a07988180c5b1ece71791c0b6506826e diff --git a/components/engine/vendor/github.com/docker/swarmkit/manager/orchestrator/taskreaper/task_reaper.go b/components/engine/vendor/github.com/docker/swarmkit/manager/orchestrator/taskreaper/task_reaper.go index bcef801f63..d702783833 100644 --- a/components/engine/vendor/github.com/docker/swarmkit/manager/orchestrator/taskreaper/task_reaper.go +++ b/components/engine/vendor/github.com/docker/swarmkit/manager/orchestrator/taskreaper/task_reaper.go @@ -96,10 +96,10 @@ func (tr *TaskReaper) Run(ctx context.Context) { // Serviceless tasks can be cleaned up right away since they are not attached to a service. tr.cleanup = append(tr.cleanup, t.ID) } - // tasks with desired state REMOVE that have progressed beyond SHUTDOWN can be cleaned up + // tasks with desired state REMOVE that have progressed beyond COMPLETE can be cleaned up // right away for _, t := range removeTasks { - if t.Status.State >= api.TaskStateShutdown { + if t.Status.State >= api.TaskStateCompleted { tr.cleanup = append(tr.cleanup, t.ID) } } @@ -138,10 +138,10 @@ func (tr *TaskReaper) Run(ctx context.Context) { if t.Status.State >= api.TaskStateOrphaned && t.ServiceID == "" { tr.cleanup = append(tr.cleanup, t.ID) } - // add tasks that have progressed beyond SHUTDOWN and have desired state REMOVE. These + // add tasks that have progressed beyond COMPLETE and have desired state REMOVE. These // tasks are associated with slots that were removed as part of a service scale down // or service removal. - if t.DesiredState == api.TaskStateRemove && t.Status.State >= api.TaskStateShutdown { + if t.DesiredState == api.TaskStateRemove && t.Status.State >= api.TaskStateCompleted { tr.cleanup = append(tr.cleanup, t.ID) } case api.EventUpdateCluster: @@ -282,6 +282,8 @@ func (tr *TaskReaper) tick() { // Stop stops the TaskReaper and waits for the main loop to exit. func (tr *TaskReaper) Stop() { + // TODO(dperny) calling stop on the task reaper twice will cause a panic + // because we try to close a channel that will already have been closed. close(tr.stopChan) <-tr.doneChan } From 2a820c0f5fbbdfb9aba67f101f07414630b52979 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Wed, 20 Dec 2017 14:02:59 +0100 Subject: [PATCH 2/4] fix #35843 regression on health check workingdir Signed-off-by: Nicolas De Loof Upstream-commit: 852a943c773382df09cdda4f29f9e93807523178 Component: engine --- components/engine/daemon/health.go | 1 + 1 file changed, 1 insertion(+) diff --git a/components/engine/daemon/health.go b/components/engine/daemon/health.go index f40c0ddf1f..9acf19043b 100644 --- a/components/engine/daemon/health.go +++ b/components/engine/daemon/health.go @@ -80,6 +80,7 @@ func (p *cmdProbe) run(ctx context.Context, d *Daemon, cntr *container.Container execConfig.Tty = false execConfig.Privileged = false execConfig.User = cntr.Config.User + execConfig.WorkingDir = cntr.Config.WorkingDir linkedEnv, err := d.setupLinkedContainers(cntr) if err != nil { From e0ed436e6c27af1a24ddf74d57e1b82d3a5e8874 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 20 Dec 2017 17:45:18 +0100 Subject: [PATCH 3/4] Add integration test for healthcheck workdir Signed-off-by: Sebastiaan van Stijn Upstream-commit: 5be2f2be243a52eb1b051c981bac5442b6e85606 Component: engine --- .../integration/container/health_test.go | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 components/engine/integration/container/health_test.go diff --git a/components/engine/integration/container/health_test.go b/components/engine/integration/container/health_test.go new file mode 100644 index 0000000000..8ed86a86e6 --- /dev/null +++ b/components/engine/integration/container/health_test.go @@ -0,0 +1,61 @@ +package container + +import ( + "context" + "testing" + "time" + + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/network" + "github.com/docker/docker/api/types/strslice" + "github.com/docker/docker/client" + "github.com/docker/docker/integration/util/request" + "github.com/gotestyourself/gotestyourself/poll" + "github.com/stretchr/testify/require" +) + +// TestHealthCheckWorkdir verifies that health-checks inherit the containers' +// working-dir. +func TestHealthCheckWorkdir(t *testing.T) { + defer setupTest(t)() + ctx := context.Background() + client := request.NewAPIClient(t) + + c, err := client.ContainerCreate(ctx, + &container.Config{ + Image: "busybox", + Tty: true, + WorkingDir: "/foo", + Cmd: strslice.StrSlice([]string{"top"}), + Healthcheck: &container.HealthConfig{ + Test: []string{"CMD-SHELL", "if [ \"$PWD\" = \"/foo\" ]; then exit 0; else exit 1; fi;"}, + Interval: 50 * time.Millisecond, + Retries: 3, + }, + }, + &container.HostConfig{}, + &network.NetworkingConfig{}, + "healthtest", + ) + require.NoError(t, err) + err = client.ContainerStart(ctx, c.ID, types.ContainerStartOptions{}) + require.NoError(t, err) + + poll.WaitOn(t, pollForHealthStatus(ctx, client, c.ID, types.Healthy), poll.WithDelay(100*time.Millisecond)) +} + +func pollForHealthStatus(ctx context.Context, client client.APIClient, containerID string, healthStatus string) func(log poll.LogT) poll.Result { + return func(log poll.LogT) poll.Result { + inspect, err := client.ContainerInspect(ctx, containerID) + + switch { + case err != nil: + return poll.Error(err) + case inspect.State.Health.Status == healthStatus: + return poll.Success() + default: + return poll.Continue("waiting for container to become %s", healthStatus) + } + } +} From 2af5d43021ac2bf29191e02806fdcff58030e275 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Wed, 22 Nov 2017 13:31:26 -0800 Subject: [PATCH 4/4] Fix environ substitutions in `docker commit --change ...` The building machinery was being handed an uninitialized container Config. This changes it to use the target container's Config. Resolves #30538 Signed-off-by: Anthony Sottile Upstream-commit: 0785836c4b440a8d4a5dfdb8df82e50f9f4d23a1 Component: engine --- .../api/server/router/image/image_routes.go | 4 -- .../engine/builder/dockerfile/builder.go | 3 +- components/engine/daemon/commit.go | 4 ++ .../integration-cli/docker_cli_commit_test.go | 10 +++- .../engine/integration/image/commit_test.go | 47 +++++++++++++++++++ 5 files changed, 62 insertions(+), 6 deletions(-) create mode 100644 components/engine/integration/image/commit_test.go diff --git a/components/engine/api/server/router/image/image_routes.go b/components/engine/api/server/router/image/image_routes.go index dabab3bcf9..fd95420ef8 100644 --- a/components/engine/api/server/router/image/image_routes.go +++ b/components/engine/api/server/router/image/image_routes.go @@ -13,7 +13,6 @@ import ( "github.com/docker/docker/api/server/httputils" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/backend" - "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/filters" "github.com/docker/docker/api/types/versions" "github.com/docker/docker/pkg/ioutils" @@ -46,9 +45,6 @@ func (s *imageRouter) postCommit(ctx context.Context, w http.ResponseWriter, r * if err != nil && err != io.EOF { //Do not fail if body is empty. return err } - if c == nil { - c = &container.Config{} - } commitCfg := &backend.ContainerCommitConfig{ ContainerCommitConfig: types.ContainerCommitConfig{ diff --git a/components/engine/builder/dockerfile/builder.go b/components/engine/builder/dockerfile/builder.go index b62d6fc024..20f1650825 100644 --- a/components/engine/builder/dockerfile/builder.go +++ b/components/engine/builder/dockerfile/builder.go @@ -396,7 +396,8 @@ func BuildFromConfig(config *container.Config, changes []string) (*container.Con } dispatchRequest := newDispatchRequest(b, dockerfile.EscapeToken, nil, newBuildArgs(b.options.BuildArgs), newStagesBuildResults()) - dispatchRequest.state.runConfig = config + // We make mutations to the configuration, ensure we have a copy + dispatchRequest.state.runConfig = copyRunConfig(config) dispatchRequest.state.imageID = config.Image for _, cmd := range commands { err := dispatch(dispatchRequest, cmd) diff --git a/components/engine/daemon/commit.go b/components/engine/daemon/commit.go index 005313224b..1bdbd6be4f 100644 --- a/components/engine/daemon/commit.go +++ b/components/engine/daemon/commit.go @@ -149,6 +149,10 @@ func (daemon *Daemon) Commit(name string, c *backend.ContainerCommitConfig) (str defer daemon.containerUnpause(container) } + if c.MergeConfigs && c.Config == nil { + c.Config = container.Config + } + newConfig, err := dockerfile.BuildFromConfig(c.Config, c.Changes) if err != nil { return "", err diff --git a/components/engine/integration-cli/docker_cli_commit_test.go b/components/engine/integration-cli/docker_cli_commit_test.go index 58a50ce0b8..057c2d6f14 100644 --- a/components/engine/integration-cli/docker_cli_commit_test.go +++ b/components/engine/integration-cli/docker_cli_commit_test.go @@ -121,11 +121,19 @@ func (s *DockerSuite) TestCommitChange(c *check.C) { "test", "test-commit") imageID = strings.TrimSpace(imageID) + // The ordering here is due to `PATH` being overridden from the container's + // ENV. On windows, the container doesn't have a `PATH` ENV variable so + // the ordering is the same as the cli. + expectedEnv := "[PATH=/foo DEBUG=true test=1]" + if testEnv.DaemonPlatform() == "windows" { + expectedEnv = "[DEBUG=true test=1 PATH=/foo]" + } + prefix, slash := getPrefixAndSlashFromDaemonPlatform() prefix = strings.ToUpper(prefix) // Force C: as that's how WORKDIR is normalized on Windows expected := map[string]string{ "Config.ExposedPorts": "map[8080/tcp:{}]", - "Config.Env": "[DEBUG=true test=1 PATH=/foo]", + "Config.Env": expectedEnv, "Config.Labels": "map[foo:bar]", "Config.Cmd": "[/bin/sh]", "Config.WorkingDir": prefix + slash + "opt", diff --git a/components/engine/integration/image/commit_test.go b/components/engine/integration/image/commit_test.go new file mode 100644 index 0000000000..13edbe1175 --- /dev/null +++ b/components/engine/integration/image/commit_test.go @@ -0,0 +1,47 @@ +package image + +import ( + "context" + "testing" + + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/container" + "github.com/docker/docker/integration/util/request" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCommitInheritsEnv(t *testing.T) { + defer setupTest(t)() + client := request.NewAPIClient(t) + ctx := context.Background() + + createResp1, err := client.ContainerCreate(ctx, &container.Config{Image: "busybox"}, nil, nil, "") + require.NoError(t, err) + + commitResp1, err := client.ContainerCommit(ctx, createResp1.ID, types.ContainerCommitOptions{ + Changes: []string{"ENV PATH=/bin"}, + Reference: "test-commit-image", + }) + require.NoError(t, err) + + image1, _, err := client.ImageInspectWithRaw(ctx, commitResp1.ID) + require.NoError(t, err) + + expectedEnv1 := []string{"PATH=/bin"} + assert.Equal(t, expectedEnv1, image1.Config.Env) + + createResp2, err := client.ContainerCreate(ctx, &container.Config{Image: image1.ID}, nil, nil, "") + require.NoError(t, err) + + commitResp2, err := client.ContainerCommit(ctx, createResp2.ID, types.ContainerCommitOptions{ + Changes: []string{"ENV PATH=/usr/bin:$PATH"}, + Reference: "test-commit-image", + }) + require.NoError(t, err) + + image2, _, err := client.ImageInspectWithRaw(ctx, commitResp2.ID) + require.NoError(t, err) + expectedEnv2 := []string{"PATH=/usr/bin:/bin"} + assert.Equal(t, expectedEnv2, image2.Config.Env) +}