From 2c4f0e56a8c7e01ba39bba7a062351f7830f3bac Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Thu, 22 Feb 2018 10:51:50 +0100 Subject: [PATCH 1/3] Fix Dockerfile.e2e build This images is used to run integration and integration-cli tests on anything that implements the docker api :). The image wasn't building anywore :D Signed-off-by: Vincent Demeester Upstream-commit: 9c46b587eec84f6d9b525ecca1d8b66a51055c12 Component: engine --- components/engine/Dockerfile.e2e | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/components/engine/Dockerfile.e2e b/components/engine/Dockerfile.e2e index 88edc5a907..ce13170e32 100644 --- a/components/engine/Dockerfile.e2e +++ b/components/engine/Dockerfile.e2e @@ -21,13 +21,14 @@ RUN contrib/download-frozen-image-v2.sh /output/docker-frozen-images \ debian:jessie@sha256:287a20c5f73087ab406e6b364833e3fb7b3ae63ca0eb3486555dc27ed32c6e60 \ hello-world:latest@sha256:be0cd392e45be79ffeffa6b05338b98ebb16c87b255f48e297ec7f98e123905c -# Download Docker CLI binary -COPY hack/dockerfile hack/dockerfile -RUN hack/dockerfile/install.sh dockercli +# Install dockercli +# Please edit hack/dockerfile/install/.installer to update them. +COPY hack/dockerfile/install hack/dockerfile/install +RUN ./hack/dockerfile/install/install.sh dockercli # Set tag and add sources ARG DOCKER_GITCOMMIT -ENV DOCKER_GITCOMMIT=$DOCKER_GITCOMMIT +ENV DOCKER_GITCOMMIT=${DOCKER_GITCOMMIT:-undefined} ADD . . # Build DockerSuite.TestBuild* dependency From 96898bda211f2269a2e5e5222783e41a0c126f97 Mon Sep 17 00:00:00 2001 From: John Howard Date: Tue, 9 Jan 2018 11:46:29 -0800 Subject: [PATCH 2/3] Windows: Pass back system errors on container exit Signed-off-by: John Howard While debugging #32838, it was found (https://github.com/moby/moby/issues/32838#issuecomment-356005845) that the utility VM in some circumstances was crashing. Unfortunately, this was silently thrown away, and as far as the build step (also applies to docker run) was concerned, the exit code was zero and the error was thrown away. Windows containers operate differently to containers on Linux, and there can be legitimate system errors during container shutdown after the init process exits. This PR handles this and passes the error all the way back to the client, and correctly causes a build step running a container which hits a system error to fail, rather than blindly trying to keep going, assuming all is good, and get a subsequent failure on a commit. With this change, assuming an error occurs, here's an example of a failure which previous was reported as a commit error: ``` The command 'powershell -Command $ErrorActionPreference = 'Stop'; $ProgressPreference = 'SilentlyContinue'; Install-WindowsFeature -Name Web-App-Dev ; Install-WindowsFeature -Name ADLDS; Install-WindowsFeature -Name Web-Mgmt-Compat; Install-WindowsFeature -Name Web-Mgmt-Service; Install-WindowsFeature -Name Web-Metabase; Install-WindowsFeature -Name Web-Lgcy-Scripting; Install-WindowsFeature -Name Web-WMI; Install-WindowsFeature -Name Web-WHC; Install-WindowsFeature -Name Web-Scripting-Tools; Install-WindowsFeature -Name Web-Net-Ext45; Install-WindowsFeature -Name Web-ASP; Install-WindowsFeature -Name Web-ISAPI-Ext; Install-WindowsFeature -Name Web-ISAPI-Filter; Install-WindowsFeature -Name Web-Default-Doc; Install-WindowsFeature -Name Web-Dir-Browsing; Install-WindowsFeature -Name Web-Http-Errors; Install-WindowsFeature -Name Web-Static-Content; Install-WindowsFeature -Name Web-Http-Redirect; Install-WindowsFeature -Name Web-DAV-Publishing; Install-WindowsFeature -Name Web-Health; Install-WindowsFeature -Name Web-Http-Logging; Install-WindowsFeature -Name Web-Custom-Logging; Install-WindowsFeature -Name Web-Log-Libraries; Install-WindowsFeature -Name Web-Request-Monitor; Install-WindowsFeature -Name Web-Http-Tracing; Install-WindowsFeature -Name Web-Stat-Compression; Install-WindowsFeature -Name Web-Dyn-Compression; Install-WindowsFeature -Name Web-Security; Install-WindowsFeature -Name Web-Windows-Auth; Install-WindowsFeature -Name Web-Basic-Auth; Install-WindowsFeature -Name Web-Url-Auth; Install-WindowsFeature -Name Web-WebSockets; Install-WindowsFeature -Name Web-AppInit; Install-WindowsFeature -Name NET-WCF-HTTP-Activation45; Install-WindowsFeature -Name NET-WCF-Pipe-Activation45; Install-WindowsFeature -Name NET-WCF-TCP-Activation45;' returned a non-zero code: 4294967295: container shutdown failed: container ba9c65054d42d4830fb25ef55e4ab3287550345aa1a2bb265df4e5bfcd79c78a encountered an error during WaitTimeout: failure in a Windows system call: The compute system exited unexpectedly. (0xc0370106) ``` Without this change, it would be incorrectly reported such as in this comment: https://github.com/moby/moby/issues/32838#issuecomment-309621097 ``` Step 3/8 : ADD buildtools C:/buildtools re-exec error: exit status 1: output: time="2017-06-20T11:37:38+10:00" level=error msg="hcsshim::ImportLayer failed in Win32: The system cannot find the path specified. (0x3) layerId=\\\\?\\C:\\ProgramData\\docker\\windowsfilter\\b41d28c95f98368b73fc192cb9205700e21 6691495c1f9ac79b9b04ec4923ea2 flavour=1 folder=C:\\Windows\\TEMP\\hcs232661915" hcsshim::ImportLayer failed in Win32: The system cannot find the path specified. (0x3) layerId=\\?\C:\ProgramData\docker\windowsfilter\b41d28c95f98368b73fc192cb9205700e216691495c1f9ac79b9b04ec4923ea2 flavour=1 folder=C:\Windows\TEMP\hcs232661915 ``` Upstream-commit: 8c52560ea4593935322c1d056124be44e234b934 Component: engine --- .../builder/dockerfile/containerbackend.go | 5 +++- .../engine/builder/dockerfile/dispatchers.go | 12 ++++++--- components/engine/container/state.go | 2 +- components/engine/daemon/monitor.go | 3 +++ .../libcontainerd/client_local_windows.go | 27 ++++++++++++++++++- components/engine/libcontainerd/types.go | 1 + 6 files changed, 43 insertions(+), 7 deletions(-) diff --git a/components/engine/builder/dockerfile/containerbackend.go b/components/engine/builder/dockerfile/containerbackend.go index a97d3221e9..2080fc0f4c 100644 --- a/components/engine/builder/dockerfile/containerbackend.go +++ b/components/engine/builder/dockerfile/containerbackend.go @@ -93,7 +93,7 @@ func (c *containerManager) Run(ctx context.Context, cID string, stdout, stderr i close(finished) logCancellationError(cancelErrCh, fmt.Sprintf("a non-zero code from ContainerWait: %d", status.ExitCode())) - return &statusCodeError{code: status.ExitCode(), err: err} + return &statusCodeError{code: status.ExitCode(), err: status.Err()} } close(finished) @@ -112,6 +112,9 @@ type statusCodeError struct { } func (e *statusCodeError) Error() string { + if e.err == nil { + return "" + } return e.err.Error() } diff --git a/components/engine/builder/dockerfile/dispatchers.go b/components/engine/builder/dockerfile/dispatchers.go index 828c91d0c8..99d4aa627e 100644 --- a/components/engine/builder/dockerfile/dispatchers.go +++ b/components/engine/builder/dockerfile/dispatchers.go @@ -348,11 +348,15 @@ func dispatchRun(d dispatchRequest, c *instructions.RunCommand) error { if err := d.builder.containerManager.Run(d.builder.clientCtx, cID, d.builder.Stdout, d.builder.Stderr); err != nil { if err, ok := err.(*statusCodeError); ok { // TODO: change error type, because jsonmessage.JSONError assumes HTTP + msg := fmt.Sprintf( + "The command '%s' returned a non-zero code: %d", + strings.Join(runConfig.Cmd, " "), err.StatusCode()) + if err.Error() != "" { + msg = fmt.Sprintf("%s: %s", msg, err.Error()) + } return &jsonmessage.JSONError{ - Message: fmt.Sprintf( - "The command '%s' returned a non-zero code: %d", - strings.Join(runConfig.Cmd, " "), err.StatusCode()), - Code: err.StatusCode(), + Message: msg, + Code: err.StatusCode(), } } return err diff --git a/components/engine/container/state.go b/components/engine/container/state.go index d54696cd16..93c550dcd6 100644 --- a/components/engine/container/state.go +++ b/components/engine/container/state.go @@ -29,7 +29,7 @@ type State struct { Dead bool Pid int ExitCodeValue int `json:"ExitCode"` - ErrorMsg string `json:"Error"` // contains last known error during container start or remove + ErrorMsg string `json:"Error"` // contains last known error during container start, stop, or remove StartedAt time.Time FinishedAt time.Time Health *Health diff --git a/components/engine/daemon/monitor.go b/components/engine/daemon/monitor.go index dbd14ebfbb..ce897d9397 100644 --- a/components/engine/daemon/monitor.go +++ b/components/engine/daemon/monitor.go @@ -69,6 +69,9 @@ func (daemon *Daemon) ProcessEvent(id string, e libcontainerd.EventType, ei libc c.RestartCount++ c.SetRestarting(&exitStatus) } else { + if ei.Error != nil { + c.SetError(ei.Error) + } c.SetStopped(&exitStatus) defer daemon.autoRemove(c) } diff --git a/components/engine/libcontainerd/client_local_windows.go b/components/engine/libcontainerd/client_local_windows.go index 51475b42d0..ef87f3ed91 100644 --- a/components/engine/libcontainerd/client_local_windows.go +++ b/components/engine/libcontainerd/client_local_windows.go @@ -1203,7 +1203,13 @@ func (c *client) shutdownContainer(ctr *container) error { if err != nil { c.logger.WithError(err).WithField("container", ctr.id). Debug("failed to shutdown container, terminating it") - return c.terminateContainer(ctr) + terminateErr := c.terminateContainer(ctr) + if terminateErr != nil { + c.logger.WithError(terminateErr).WithField("container", ctr.id). + Error("failed to shutdown container, and subsequent terminate also failed") + return fmt.Errorf("%s: subsequent terminate failed %s", err, terminateErr) + } + return err } return nil @@ -1234,6 +1240,8 @@ func (c *client) reapProcess(ctr *container, p *process) int { "process": p.id, }) + var eventErr error + // Block indefinitely for the process to exit. if err := p.hcsProcess.Wait(); err != nil { if herr, ok := err.(*hcsshim.ProcessError); ok && herr.Err != windows.ERROR_BROKEN_PIPE { @@ -1263,6 +1271,8 @@ func (c *client) reapProcess(ctr *container, p *process) int { if err := p.hcsProcess.Close(); err != nil { logger.WithError(err).Warnf("failed to cleanup hcs process resources") + exitCode = -1 + eventErr = fmt.Errorf("hcsProcess.Close() failed %s", err) } var pendingUpdates bool @@ -1286,13 +1296,27 @@ func (c *client) reapProcess(ctr *container, p *process) int { } if err := c.shutdownContainer(ctr); err != nil { + exitCode = -1 logger.WithError(err).Warn("failed to shutdown container") + thisErr := fmt.Errorf("failed to shutdown container: %s", err) + if eventErr != nil { + eventErr = fmt.Errorf("%s: %s", eventErr, thisErr) + } else { + eventErr = thisErr + } } else { logger.Debug("completed container shutdown") } if err := ctr.hcsContainer.Close(); err != nil { + exitCode = -1 logger.WithError(err).Error("failed to clean hcs container resources") + thisErr := fmt.Errorf("failed to terminate container: %s", err) + if eventErr != nil { + eventErr = fmt.Errorf("%s: %s", eventErr, thisErr) + } else { + eventErr = thisErr + } } } @@ -1305,6 +1329,7 @@ func (c *client) reapProcess(ctr *container, p *process) int { ExitCode: uint32(exitCode), ExitedAt: exitedAt, UpdatePending: pendingUpdates, + Error: eventErr, } c.logger.WithFields(logrus.Fields{ "container": ctr.id, diff --git a/components/engine/libcontainerd/types.go b/components/engine/libcontainerd/types.go index f27fd50313..97e4fee735 100644 --- a/components/engine/libcontainerd/types.go +++ b/components/engine/libcontainerd/types.go @@ -73,6 +73,7 @@ type EventInfo struct { OOMKilled bool // Windows Only field UpdatePending bool + Error error } // Backend defines callbacks that the client of the library needs to implement. From c82f8abc8051fbea907390434f63f0edec63f5bd Mon Sep 17 00:00:00 2001 From: Dennis Chen Date: Fri, 23 Feb 2018 08:32:54 +0000 Subject: [PATCH 3/3] Clean the teardown process of network test We need to clean the resources created in some test cases, else in some cases we'll get below error for other tests: > FAIL: docker_experimental_network_test.go:37: DockerNetworkSuite.TestDockerNetworkMacvlanPersistance > docker_experimental_network_test.go:44: > ... > Command: ip link add dm-dummy0 type dummy > ExitCode: 2 > Error: exit status 2 > Stdout: > Stderr: RTNETLINK answers: File exists > ... Logically, each test case should be independent, the failure of previous test case should not have side-effect for the test cases followed. Signed-off-by: Dennis Chen Upstream-commit: 57d85e7e54f7d074af8c496cba43ee18d3815207 Component: engine --- .../docker_experimental_network_test.go | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/components/engine/integration-cli/docker_experimental_network_test.go b/components/engine/integration-cli/docker_experimental_network_test.go index 888970a0a3..fb20331631 100644 --- a/components/engine/integration-cli/docker_experimental_network_test.go +++ b/components/engine/integration-cli/docker_experimental_network_test.go @@ -42,6 +42,8 @@ func (s *DockerNetworkSuite) TestDockerNetworkMacvlanPersistance(c *check.C) { master := "dm-dummy0" // simulate the master link the vlan tagged subinterface parent link will use createMasterDummy(c, master) + // cleanup the master interface that also collects the slave dev + defer deleteInterface(c, master) // create a network specifying the desired sub-interface name dockerCmd(c, "network", "create", "--driver=macvlan", "-o", "parent=dm-dummy0.60", "dm-persist") assertNwIsAvailable(c, "dm-persist") @@ -49,8 +51,6 @@ func (s *DockerNetworkSuite) TestDockerNetworkMacvlanPersistance(c *check.C) { s.d.Restart(c) // verify network is recreated from persistence assertNwIsAvailable(c, "dm-persist") - // cleanup the master interface that also collects the slave dev - deleteInterface(c, "dm-dummy0") } func (s *DockerNetworkSuite) TestDockerNetworkIpvlanPersistance(c *check.C) { @@ -60,6 +60,8 @@ func (s *DockerNetworkSuite) TestDockerNetworkIpvlanPersistance(c *check.C) { master := "di-dummy0" // simulate the master link the vlan tagged subinterface parent link will use createMasterDummy(c, master) + // cleanup the master interface that also collects the slave dev + defer deleteInterface(c, master) // create a network specifying the desired sub-interface name dockerCmd(c, "network", "create", "--driver=ipvlan", "-o", "parent=di-dummy0.70", "di-persist") assertNwIsAvailable(c, "di-persist") @@ -67,8 +69,6 @@ func (s *DockerNetworkSuite) TestDockerNetworkIpvlanPersistance(c *check.C) { s.d.Restart(c) // verify network is recreated from persistence assertNwIsAvailable(c, "di-persist") - // cleanup the master interface that also collects the slave dev - deleteInterface(c, "di-dummy0") } func (s *DockerNetworkSuite) TestDockerNetworkMacvlanSubIntCreate(c *check.C) { @@ -78,11 +78,11 @@ func (s *DockerNetworkSuite) TestDockerNetworkMacvlanSubIntCreate(c *check.C) { master := "dm-dummy0" // simulate the master link the vlan tagged subinterface parent link will use createMasterDummy(c, master) + // cleanup the master interface which also collects the slave dev + defer deleteInterface(c, master) // create a network specifying the desired sub-interface name dockerCmd(c, "network", "create", "--driver=macvlan", "-o", "parent=dm-dummy0.50", "dm-subinterface") assertNwIsAvailable(c, "dm-subinterface") - // cleanup the master interface which also collects the slave dev - deleteInterface(c, "dm-dummy0") } func (s *DockerNetworkSuite) TestDockerNetworkIpvlanSubIntCreate(c *check.C) { @@ -92,11 +92,11 @@ func (s *DockerNetworkSuite) TestDockerNetworkIpvlanSubIntCreate(c *check.C) { master := "di-dummy0" // simulate the master link the vlan tagged subinterface parent link will use createMasterDummy(c, master) + // cleanup the master interface which also collects the slave dev + defer deleteInterface(c, master) // create a network specifying the desired sub-interface name dockerCmd(c, "network", "create", "--driver=ipvlan", "-o", "parent=di-dummy0.60", "di-subinterface") assertNwIsAvailable(c, "di-subinterface") - // cleanup the master interface which also collects the slave dev - deleteInterface(c, "di-dummy0") } func (s *DockerNetworkSuite) TestDockerNetworkMacvlanOverlapParent(c *check.C) { @@ -105,6 +105,8 @@ func (s *DockerNetworkSuite) TestDockerNetworkMacvlanOverlapParent(c *check.C) { // master dummy interface 'dm' abbreviation represents 'docker macvlan' master := "dm-dummy0" createMasterDummy(c, master) + // cleanup the master interface which also collects the slave dev + defer deleteInterface(c, master) createVlanInterface(c, master, "dm-dummy0.40", "40") // create a network using an existing parent interface dockerCmd(c, "network", "create", "--driver=macvlan", "-o", "parent=dm-dummy0.40", "dm-subinterface") @@ -113,8 +115,6 @@ func (s *DockerNetworkSuite) TestDockerNetworkMacvlanOverlapParent(c *check.C) { out, _, err := dockerCmdWithError("network", "create", "--driver=macvlan", "-o", "parent=dm-dummy0.40", "dm-parent-net-overlap") // verify that the overlap returns an error c.Assert(err, check.NotNil, check.Commentf(out)) - // cleanup the master interface which also collects the slave dev - deleteInterface(c, "dm-dummy0") } func (s *DockerNetworkSuite) TestDockerNetworkIpvlanOverlapParent(c *check.C) { @@ -123,6 +123,8 @@ func (s *DockerNetworkSuite) TestDockerNetworkIpvlanOverlapParent(c *check.C) { // master dummy interface 'dm' abbreviation represents 'docker ipvlan' master := "di-dummy0" createMasterDummy(c, master) + // cleanup the master interface which also collects the slave dev + defer deleteInterface(c, master) createVlanInterface(c, master, "di-dummy0.30", "30") // create a network using an existing parent interface dockerCmd(c, "network", "create", "--driver=ipvlan", "-o", "parent=di-dummy0.30", "di-subinterface") @@ -131,8 +133,6 @@ func (s *DockerNetworkSuite) TestDockerNetworkIpvlanOverlapParent(c *check.C) { out, _, err := dockerCmdWithError("network", "create", "--driver=ipvlan", "-o", "parent=di-dummy0.30", "di-parent-net-overlap") // verify that the overlap returns an error c.Assert(err, check.NotNil, check.Commentf(out)) - // cleanup the master interface which also collects the slave dev - deleteInterface(c, "di-dummy0") } func (s *DockerNetworkSuite) TestDockerNetworkMacvlanMultiSubnet(c *check.C) { @@ -471,6 +471,7 @@ func (s *DockerSuite) TestDockerNetworkMacVlanExistingParent(c *check.C) { testRequires(c, DaemonIsLinux, macvlanKernelSupport, NotUserNamespace, NotArm, ExperimentalDaemon) netName := "dm-parent-exists" createMasterDummy(c, "dm-dummy0") + defer deleteInterface(c, "dm-dummy0") //out, err := createVlanInterface(c, "dm-parent", "dm-slave", "macvlan", "bridge") // create a network using an existing parent interface dockerCmd(c, "network", "create", "--driver=macvlan", "-o", "parent=dm-dummy0", netName) @@ -480,7 +481,6 @@ func (s *DockerSuite) TestDockerNetworkMacVlanExistingParent(c *check.C) { assertNwNotAvailable(c, netName) // verify the network delete did not delete the predefined link linkExists(c, "dm-dummy0") - deleteInterface(c, "dm-dummy0") } func (s *DockerSuite) TestDockerNetworkMacVlanSubinterface(c *check.C) { @@ -488,6 +488,8 @@ func (s *DockerSuite) TestDockerNetworkMacVlanSubinterface(c *check.C) { testRequires(c, DaemonIsLinux, macvlanKernelSupport, NotUserNamespace, NotArm, ExperimentalDaemon) netName := "dm-subinterface" createMasterDummy(c, "dm-dummy0") + // delete the parent interface which also collects the slave + defer deleteInterface(c, "dm-dummy0") createVlanInterface(c, "dm-dummy0", "dm-dummy0.20", "20") // create a network using an existing parent interface dockerCmd(c, "network", "create", "--driver=macvlan", "-o", "parent=dm-dummy0.20", netName) @@ -510,8 +512,6 @@ func (s *DockerSuite) TestDockerNetworkMacVlanSubinterface(c *check.C) { assertNwNotAvailable(c, netName) // verify the network delete did not delete the predefined sub-interface linkExists(c, "dm-dummy0.20") - // delete the parent interface which also collects the slave - deleteInterface(c, "dm-dummy0") } func createMasterDummy(c *check.C, master string) {