diff --git a/api/server/server.go b/api/server/server.go index 57ebebc0e0..1bb81241f2 100644 --- a/api/server/server.go +++ b/api/server/server.go @@ -244,8 +244,6 @@ func (s *Server) postAuth(version version.Version, w http.ResponseWriter, r *htt } func (s *Server) getVersion(version version.Version, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - w.Header().Set("Content-Type", "application/json") - v := &types.Version{ Version: dockerversion.VERSION, ApiVersion: api.APIVERSION, @@ -359,8 +357,6 @@ func (s *Server) getImagesJSON(version version.Version, w http.ResponseWriter, r } func (s *Server) getInfo(version version.Version, w http.ResponseWriter, r *http.Request, vars map[string]string) error { - w.Header().Set("Content-Type", "application/json") - info, err := s.daemon.SystemInfo() if err != nil { return err @@ -863,7 +859,7 @@ func (s *Server) postImagesLoad(version version.Version, w http.ResponseWriter, func (s *Server) postContainersCreate(version version.Version, w http.ResponseWriter, r *http.Request, vars map[string]string) error { if err := parseForm(r); err != nil { - return nil + return err } if err := checkForJson(r); err != nil { return err @@ -1288,7 +1284,7 @@ func (s *Server) postContainersCopy(version version.Version, w http.ResponseWrit func (s *Server) postContainerExecCreate(version version.Version, w http.ResponseWriter, r *http.Request, vars map[string]string) error { if err := parseForm(r); err != nil { - return nil + return err } name := vars["name"] @@ -1317,7 +1313,7 @@ func (s *Server) postContainerExecCreate(version version.Version, w http.Respons // TODO(vishh): Refactor the code to avoid having to specify stream config as part of both create and start. func (s *Server) postContainerExecStart(version version.Version, w http.ResponseWriter, r *http.Request, vars map[string]string) error { if err := parseForm(r); err != nil { - return nil + return err } var ( execName = vars["name"] diff --git a/daemon/container.go b/daemon/container.go index 4a56132244..3efa756e1a 100644 --- a/daemon/container.go +++ b/daemon/container.go @@ -437,7 +437,23 @@ func (container *Container) Kill() error { // 1. Send SIGKILL if err := container.killPossiblyDeadProcess(9); err != nil { - return err + // While normally we might "return err" here we're not going to + // because if we can't stop the container by this point then + // its probably because its already stopped. Meaning, between + // the time of the IsRunning() call above and now it stopped. + // Also, since the err return will be exec driver specific we can't + // look for any particular (common) error that would indicate + // that the process is already dead vs something else going wrong. + // So, instead we'll give it up to 2 more seconds to complete and if + // by that time the container is still running, then the error + // we got is probably valid and so we return it to the caller. + + if container.IsRunning() { + container.WaitStop(2 * time.Second) + if container.IsRunning() { + return err + } + } } // 2. Wait for the process to die, in last resort, try to kill the process directly diff --git a/daemon/container_linux.go b/daemon/container_linux.go index 3e50ed22fe..8dd839eb6f 100644 --- a/daemon/container_linux.go +++ b/daemon/container_linux.go @@ -959,21 +959,37 @@ func (container *Container) DisableLink(name string) { } func (container *Container) UnmountVolumes(forceSyscall bool) error { - for _, m := range container.MountPoints { - dest, err := container.GetResourcePath(m.Destination) + var volumeMounts []mountPoint + + for _, mntPoint := range container.MountPoints { + dest, err := container.GetResourcePath(mntPoint.Destination) if err != nil { return err } - if forceSyscall { - syscall.Unmount(dest, 0) + volumeMounts = append(volumeMounts, mountPoint{Destination: dest, Volume: mntPoint.Volume}) + } + + for _, mnt := range container.networkMounts() { + dest, err := container.GetResourcePath(mnt.Destination) + if err != nil { + return err } - if m.Volume != nil { - if err := m.Volume.Unmount(); err != nil { + volumeMounts = append(volumeMounts, mountPoint{Destination: dest}) + } + + for _, volumeMount := range volumeMounts { + if forceSyscall { + syscall.Unmount(volumeMount.Destination, 0) + } + + if volumeMount.Volume != nil { + if err := volumeMount.Volume.Unmount(); err != nil { return err } } } + return nil } diff --git a/daemon/delete.go b/daemon/delete.go index 830bbbe267..39d500550e 100644 --- a/daemon/delete.go +++ b/daemon/delete.go @@ -58,10 +58,6 @@ func (daemon *Daemon) ContainerRm(name string, config *ContainerRmConfig) error // Destroy unregisters a container from the daemon and cleanly removes its contents from the filesystem. func (daemon *Daemon) rm(container *Container, forceRemove bool) (err error) { - // stop collection of stats for the container regardless - // if stats are currently getting collected. - daemon.statsCollector.stopCollection(container) - if container.IsRunning() { if !forceRemove { return fmt.Errorf("Conflict, You cannot remove a running container. Stop the container before attempting removal or use -f") @@ -71,6 +67,10 @@ func (daemon *Daemon) rm(container *Container, forceRemove bool) (err error) { } } + // stop collection of stats for the container regardless + // if stats are currently getting collected. + daemon.statsCollector.stopCollection(container) + element := daemon.containers.Get(container.ID) if element == nil { return fmt.Errorf("Container %v not found - maybe it was already destroyed?", container.ID) diff --git a/daemon/volumes.go b/daemon/volumes.go index 2fdcd8311d..963bd58da3 100644 --- a/daemon/volumes.go +++ b/daemon/volumes.go @@ -8,7 +8,6 @@ import ( "path/filepath" "strings" - "github.com/docker/docker/daemon/execdriver" "github.com/docker/docker/pkg/chrootarchive" "github.com/docker/docker/runconfig" "github.com/docker/docker/volume" @@ -115,20 +114,6 @@ func validMountMode(mode string) bool { return validModes[mode] } -func (container *Container) specialMounts() []execdriver.Mount { - var mounts []execdriver.Mount - if container.ResolvConfPath != "" { - mounts = append(mounts, execdriver.Mount{Source: container.ResolvConfPath, Destination: "/etc/resolv.conf", Writable: !container.hostConfig.ReadonlyRootfs, Private: true}) - } - if container.HostnamePath != "" { - mounts = append(mounts, execdriver.Mount{Source: container.HostnamePath, Destination: "/etc/hostname", Writable: !container.hostConfig.ReadonlyRootfs, Private: true}) - } - if container.HostsPath != "" { - mounts = append(mounts, execdriver.Mount{Source: container.HostsPath, Destination: "/etc/hosts", Writable: !container.hostConfig.ReadonlyRootfs, Private: true}) - } - return mounts -} - func copyExistingContents(source, destination string) error { volList, err := ioutil.ReadDir(source) if err != nil { diff --git a/integration-cli/docker_api_containers_test.go b/integration-cli/docker_api_containers_test.go index 363d279120..daec87c497 100644 --- a/integration-cli/docker_api_containers_test.go +++ b/integration-cli/docker_api_containers_test.go @@ -254,6 +254,42 @@ func (s *DockerSuite) TestGetContainerStats(c *check.C) { } } +func (s *DockerSuite) TestContainerStatsRmRunning(c *check.C) { + out, _ := dockerCmd(c, "run", "-d", "busybox", "top") + id := strings.TrimSpace(out) + + buf := &channelBuffer{make(chan []byte, 1)} + defer buf.Close() + chErr := make(chan error) + go func() { + _, body, err := sockRequestRaw("GET", "/containers/"+id+"/stats?stream=1", nil, "application/json") + if err != nil { + chErr <- err + } + defer body.Close() + _, err = io.Copy(buf, body) + chErr <- err + }() + defer func() { + c.Assert(<-chErr, check.IsNil) + }() + + b := make([]byte, 32) + // make sure we've got some stats + _, err := buf.ReadTimeout(b, 2*time.Second) + c.Assert(err, check.IsNil) + + // Now remove without `-f` and make sure we are still pulling stats + _, err = runCommand(exec.Command(dockerBinary, "rm", id)) + c.Assert(err, check.Not(check.IsNil), check.Commentf("rm should have failed but didn't")) + _, err = buf.ReadTimeout(b, 2*time.Second) + c.Assert(err, check.IsNil) + dockerCmd(c, "rm", "-f", id) + + _, err = buf.ReadTimeout(b, 2*time.Second) + c.Assert(err, check.Not(check.IsNil)) +} + func (s *DockerSuite) TestGetStoppedContainerStats(c *check.C) { // TODO: this test does nothing because we are c.Assert'ing in goroutine var ( diff --git a/integration-cli/docker_cli_cp_test.go b/integration-cli/docker_cli_cp_test.go index d79615f241..fd63bdbf2b 100644 --- a/integration-cli/docker_cli_cp_test.go +++ b/integration-cli/docker_cli_cp_test.go @@ -595,3 +595,40 @@ func (s *DockerSuite) TestCpNameHasColon(c *check.C) { c.Fatalf("Wrong content in copied file %q, should be %q", content, "lololol\n") } } + +func (s *DockerSuite) TestCopyAndRestart(c *check.C) { + expectedMsg := "hello" + out, err := exec.Command(dockerBinary, "run", "-d", "busybox", "echo", expectedMsg).CombinedOutput() + if err != nil { + c.Fatal(string(out), err) + } + id := strings.TrimSpace(string(out)) + + if out, err = exec.Command(dockerBinary, "wait", id).CombinedOutput(); err != nil { + c.Fatalf("unable to wait for container: %s", err) + } + + status := strings.TrimSpace(string(out)) + if status != "0" { + c.Fatalf("container exited with status %s", status) + } + + tmpDir, err := ioutil.TempDir("", "test-docker-restart-after-copy-") + if err != nil { + c.Fatalf("unable to make temporary directory: %s", err) + } + defer os.RemoveAll(tmpDir) + + if _, err = exec.Command(dockerBinary, "cp", fmt.Sprintf("%s:/etc/issue", id), tmpDir).CombinedOutput(); err != nil { + c.Fatalf("unable to copy from busybox container: %s", err) + } + + if out, err = exec.Command(dockerBinary, "start", "-a", id).CombinedOutput(); err != nil { + c.Fatalf("unable to start busybox container after copy: %s - %s", err, out) + } + + msg := strings.TrimSpace(string(out)) + if msg != expectedMsg { + c.Fatalf("expected %q but got %q", expectedMsg, msg) + } +} diff --git a/integration-cli/docker_cli_kill_test.go b/integration-cli/docker_cli_kill_test.go index 47b4e47b9a..932a8efca5 100644 --- a/integration-cli/docker_cli_kill_test.go +++ b/integration-cli/docker_cli_kill_test.go @@ -42,11 +42,11 @@ func (s *DockerSuite) TestKillofStoppedContainer(c *check.C) { stopCmd := exec.Command(dockerBinary, "stop", cleanedContainerID) out, _, err = runCommandWithOutput(stopCmd) - c.Assert(err, check.IsNil, check.Commentf("failed to stop container: %s, %v", out, err)) + c.Assert(err, check.IsNil) killCmd := exec.Command(dockerBinary, "kill", "-s", "30", cleanedContainerID) _, _, err = runCommandWithOutput(killCmd) - c.Assert(err, check.Not(check.IsNil), check.Commentf("Kill succeeded on a stopped container")) + c.Assert(err, check.Not(check.IsNil), check.Commentf("Container %s is not running", cleanedContainerID)) } func (s *DockerSuite) TestKillDifferentUserContainer(c *check.C) { diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index 1c47e53dfc..20e8c6ee4c 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -3172,7 +3172,7 @@ func (s *DockerSuite) TestTwoContainersInNetHost(c *check.C) { } func (s *DockerSuite) TestRunUnshareProc(c *check.C) { - testRequires(c, Apparmor) + testRequires(c, Apparmor, NativeExecDriver) name := "acidburn" runCmd := exec.Command(dockerBinary, "run", "--name", name, "jess/unshare", "unshare", "-p", "-m", "-f", "-r", "--mount-proc=/proc", "mount") diff --git a/integration-cli/utils.go b/integration-cli/utils.go index f7ad61b0ac..0ec4e758cd 100644 --- a/integration-cli/utils.go +++ b/integration-cli/utils.go @@ -7,7 +7,6 @@ import ( "errors" "fmt" "io" - "net/http" "net/http/httptest" "os" "os/exec" @@ -275,26 +274,6 @@ type FileServer struct { *httptest.Server } -func fileServer(files map[string]string) (*FileServer, error) { - var handler http.HandlerFunc = func(w http.ResponseWriter, r *http.Request) { - if filePath, found := files[r.URL.Path]; found { - http.ServeFile(w, r, filePath) - } else { - http.Error(w, http.StatusText(404), 404) - } - } - - for _, file := range files { - if _, err := os.Stat(file); err != nil { - return nil, err - } - } - server := httptest.NewServer(handler) - return &FileServer{ - Server: server, - }, nil -} - // randomUnixTmpDirPath provides a temporary unix path with rand string appended. // does not create or checks if it exists. func randomUnixTmpDirPath(s string) string { @@ -337,3 +316,26 @@ func parseCgroupPaths(procCgroupData string) map[string]string { } return cgroupPaths } + +type channelBuffer struct { + c chan []byte +} + +func (c *channelBuffer) Write(b []byte) (int, error) { + c.c <- b + return len(b), nil +} + +func (c *channelBuffer) Close() error { + close(c.c) + return nil +} + +func (c *channelBuffer) ReadTimeout(p []byte, n time.Duration) (int, error) { + select { + case b := <-c.c: + return copy(p[0:], b), nil + case <-time.After(n): + return -1, fmt.Errorf("timeout reading from channel") + } +}