Move "delete" to daemon/delete.go

This is part of an effort to break apart the deprecated server/ package

Docker-DCO-1.1-Signed-off-by: Solomon Hykes <solomon@docker.com> (github: shykes)

Fix issues with renaming container_delete to delete
Docker-DCO-1.1-Signed-off-by: Tibor Vass <teabee89@gmail.com> (github: tiborvass)

api/server/server.go

@@ -678,7 +678,7 @@ func deleteContainers(eng *engine.Engine, version version.Version, w http.Respon
 	if vars == nil {
 		return fmt.Errorf("Missing parameter")
 	}
-	job := eng.Job("container_delete", vars["name"])
+	job := eng.Job("delete", vars["name"])
 
 	if version.GreaterThanOrEqualTo("1.14") {
 		job.Setenv("stop", r.Form.Get("stop"))

api/server/server_unit_test.go

@@ -455,7 +455,7 @@ func TestDeleteContainers(t *testing.T) {
 	eng := engine.New()
 	name := "foo"
 	var called bool
-	eng.Register("container_delete", func(job *engine.Job) engine.Status {
+	eng.Register("delete", func(job *engine.Job) engine.Status {
 		called = true
 		if len(job.Args) == 0 {
 			t.Fatalf("Job arguments is empty")
@@ -480,7 +480,7 @@ func TestDeleteContainersWithStopAndKill(t *testing.T) {
 	}
 	eng := engine.New()
 	var called bool
-	eng.Register("container_delete", func(job *engine.Job) engine.Status {
+	eng.Register("delete", func(job *engine.Job) engine.Status {
 		called = true
 		return engine.StatusOK
 	})

daemon/daemon.go

@@ -147,6 +147,11 @@ func (daemon *Daemon) Install(eng *engine.Engine) error {
 	if err := eng.Register("logs", daemon.ContainerLogs); err != nil {
 		return err
 	}
+	// FIXME: rename "delete" to "rm" for consistency with the CLI command
+	// FIXME: rename ContainerDestroy to ContainerRm for consistency with the CLI command
+	if err := eng.Register("delete", daemon.ContainerDestroy); err != nil {
+		return err
+	}
 	return nil
 }
 
@@ -309,47 +314,6 @@ func (daemon *Daemon) LogToDisk(src *broadcastwriter.BroadcastWriter, dst, strea
 	return nil
 }
 
-// Destroy unregisters a container from the daemon and cleanly removes its contents from the filesystem.
-func (daemon *Daemon) Destroy(container *Container) error {
-	if container == nil {
-		return fmt.Errorf("The given container is <nil>")
-	}
-
-	element := daemon.containers.Get(container.ID)
-	if element == nil {
-		return fmt.Errorf("Container %v not found - maybe it was already destroyed?", container.ID)
-	}
-
-	if err := container.Stop(3); err != nil {
-		return err
-	}
-
-	// Deregister the container before removing its directory, to avoid race conditions
-	daemon.idIndex.Delete(container.ID)
-	daemon.containers.Delete(container.ID)
-
-	if _, err := daemon.containerGraph.Purge(container.ID); err != nil {
-		utils.Debugf("Unable to remove container from link graph: %s", err)
-	}
-
-	if err := daemon.driver.Remove(container.ID); err != nil {
-		return fmt.Errorf("Driver %s failed to remove root filesystem %s: %s", daemon.driver, container.ID, err)
-	}
-
-	initID := fmt.Sprintf("%s-init", container.ID)
-	if err := daemon.driver.Remove(initID); err != nil {
-		return fmt.Errorf("Driver %s failed to remove init filesystem %s: %s", daemon.driver, initID, err)
-	}
-
-	if err := os.RemoveAll(container.root); err != nil {
-		return fmt.Errorf("Unable to remove filesystem for %v: %v", container.ID, err)
-	}
-
-	selinuxFreeLxcContexts(container.ProcessLabel)
-
-	return nil
-}
-
 func (daemon *Daemon) restore() error {
 	var (
 		debug             = (os.Getenv("DEBUG") != "" || os.Getenv("TEST") != "")

daemon/delete.go

@@ -0,0 +1,181 @@
+package daemon
+
+import (
+	"fmt"
+	"log"
+	"os"
+	"path"
+	"path/filepath"
+	"strings"
+
+	"github.com/docker/docker/engine"
+	"github.com/docker/docker/utils"
+)
+
+// FIXME: rename to ContainerRemove for consistency with the CLI command.
+func (daemon *Daemon) ContainerDestroy(job *engine.Job) engine.Status {
+	if len(job.Args) != 1 {
+		return job.Errorf("Not enough arguments. Usage: %s CONTAINER\n", job.Name)
+	}
+	name := job.Args[0]
+	removeVolume := job.GetenvBool("removeVolume")
+	removeLink := job.GetenvBool("removeLink")
+	stop := job.GetenvBool("stop")
+	kill := job.GetenvBool("kill")
+
+	container := daemon.Get(name)
+
+	if removeLink {
+		if container == nil {
+			return job.Errorf("No such link: %s", name)
+		}
+		name, err := GetFullContainerName(name)
+		if err != nil {
+			job.Error(err)
+		}
+		parent, n := path.Split(name)
+		if parent == "/" {
+			return job.Errorf("Conflict, cannot remove the default name of the container")
+		}
+		pe := daemon.ContainerGraph().Get(parent)
+		if pe == nil {
+			return job.Errorf("Cannot get parent %s for name %s", parent, name)
+		}
+		parentContainer := daemon.Get(pe.ID())
+
+		if parentContainer != nil {
+			parentContainer.DisableLink(n)
+		}
+
+		if err := daemon.ContainerGraph().Delete(name); err != nil {
+			return job.Error(err)
+		}
+		return engine.StatusOK
+	}
+
+	if container != nil {
+		if container.State.IsRunning() {
+			if stop {
+				if err := container.Stop(5); err != nil {
+					return job.Errorf("Could not stop running container, cannot remove - %v", err)
+				}
+			} else if kill {
+				if err := container.Kill(); err != nil {
+					return job.Errorf("Could not kill running container, cannot remove - %v", err)
+				}
+			} else {
+				return job.Errorf("You cannot remove a running container. Stop the container before attempting removal or use -s or -k")
+			}
+		}
+		if err := daemon.Destroy(container); err != nil {
+			return job.Errorf("Cannot destroy container %s: %s", name, err)
+		}
+		job.Eng.Job("log", "destroy", container.ID, daemon.Repositories().ImageName(container.Image)).Run()
+
+		if removeVolume {
+			var (
+				volumes     = make(map[string]struct{})
+				binds       = make(map[string]struct{})
+				usedVolumes = make(map[string]*Container)
+			)
+
+			// the volume id is always the base of the path
+			getVolumeId := func(p string) string {
+				return filepath.Base(strings.TrimSuffix(p, "/layer"))
+			}
+
+			// populate bind map so that they can be skipped and not removed
+			for _, bind := range container.HostConfig().Binds {
+				source := strings.Split(bind, ":")[0]
+				// TODO: refactor all volume stuff, all of it
+				// it is very important that we eval the link or comparing the keys to container.Volumes will not work
+				//
+				// eval symlink can fail, ref #5244 if we receive an is not exist error we can ignore it
+				p, err := filepath.EvalSymlinks(source)
+				if err != nil && !os.IsNotExist(err) {
+					return job.Error(err)
+				}
+				if p != "" {
+					source = p
+				}
+				binds[source] = struct{}{}
+			}
+
+			// Store all the deleted containers volumes
+			for _, volumeId := range container.Volumes {
+				// Skip the volumes mounted from external
+				// bind mounts here will will be evaluated for a symlink
+				if _, exists := binds[volumeId]; exists {
+					continue
+				}
+
+				volumeId = getVolumeId(volumeId)
+				volumes[volumeId] = struct{}{}
+			}
+
+			// Retrieve all volumes from all remaining containers
+			for _, container := range daemon.List() {
+				for _, containerVolumeId := range container.Volumes {
+					containerVolumeId = getVolumeId(containerVolumeId)
+					usedVolumes[containerVolumeId] = container
+				}
+			}
+
+			for volumeId := range volumes {
+				// If the requested volu
+				if c, exists := usedVolumes[volumeId]; exists {
+					log.Printf("The volume %s is used by the container %s. Impossible to remove it. Skipping.\n", volumeId, c.ID)
+					continue
+				}
+				if err := daemon.Volumes().Delete(volumeId); err != nil {
+					return job.Errorf("Error calling volumes.Delete(%q): %v", volumeId, err)
+				}
+			}
+		}
+	} else {
+		return job.Errorf("No such container: %s", name)
+	}
+	return engine.StatusOK
+}
+
+// Destroy unregisters a container from the daemon and cleanly removes its contents from the filesystem.
+// FIXME: rename to Rm for consistency with the CLI command
+func (daemon *Daemon) Destroy(container *Container) error {
+	if container == nil {
+		return fmt.Errorf("The given container is <nil>")
+	}
+
+	element := daemon.containers.Get(container.ID)
+	if element == nil {
+		return fmt.Errorf("Container %v not found - maybe it was already destroyed?", container.ID)
+	}
+
+	if err := container.Stop(3); err != nil {
+		return err
+	}
+
+	// Deregister the container before removing its directory, to avoid race conditions
+	daemon.idIndex.Delete(container.ID)
+	daemon.containers.Delete(container.ID)
+
+	if _, err := daemon.containerGraph.Purge(container.ID); err != nil {
+		utils.Debugf("Unable to remove container from link graph: %s", err)
+	}
+
+	if err := daemon.driver.Remove(container.ID); err != nil {
+		return fmt.Errorf("Driver %s failed to remove root filesystem %s: %s", daemon.driver, container.ID, err)
+	}
+
+	initID := fmt.Sprintf("%s-init", container.ID)
+	if err := daemon.driver.Remove(initID); err != nil {
+		return fmt.Errorf("Driver %s failed to remove init filesystem %s: %s", daemon.driver, initID, err)
+	}
+
+	if err := os.RemoveAll(container.root); err != nil {
+		return fmt.Errorf("Unable to remove filesystem for %v: %v", container.ID, err)
+	}
+
+	selinuxFreeLxcContexts(container.ProcessLabel)
+
+	return nil
+}

integration/server_test.go

@@ -221,7 +221,7 @@ func TestCreateStartRestartStopStartKillRm(t *testing.T) {
 	}
 
 	// FIXME: this failed once with a race condition ("Unable to remove filesystem for xxx: directory not empty")
-	job = eng.Job("container_delete", id)
+	job = eng.Job("delete", id)
 	job.SetenvBool("removeVolume", true)
 	if err := job.Run(); err != nil {
 		t.Fatal(err)

server/container.go

@@ -8,11 +8,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"log"
-	"os"
 	"os/exec"
-	"path"
-	"path/filepath"
 	"strconv"
 	"strings"
 
@@ -223,131 +219,6 @@ func (srv *Server) Containers(job *engine.Job) engine.Status {
 	return engine.StatusOK
 }
 
-func (srv *Server) ContainerDestroy(job *engine.Job) engine.Status {
-	if len(job.Args) != 1 {
-		return job.Errorf("Not enough arguments. Usage: %s CONTAINER\n", job.Name)
-	}
-	name := job.Args[0]
-	removeVolume := job.GetenvBool("removeVolume")
-	removeLink := job.GetenvBool("removeLink")
-	stop := job.GetenvBool("stop")
-	kill := job.GetenvBool("kill")
-
-	container := srv.daemon.Get(name)
-
-	if removeLink {
-		if container == nil {
-			return job.Errorf("No such link: %s", name)
-		}
-		name, err := daemon.GetFullContainerName(name)
-		if err != nil {
-			job.Error(err)
-		}
-		parent, n := path.Split(name)
-		if parent == "/" {
-			return job.Errorf("Conflict, cannot remove the default name of the container")
-		}
-		pe := srv.daemon.ContainerGraph().Get(parent)
-		if pe == nil {
-			return job.Errorf("Cannot get parent %s for name %s", parent, name)
-		}
-		parentContainer := srv.daemon.Get(pe.ID())
-
-		if parentContainer != nil {
-			parentContainer.DisableLink(n)
-		}
-
-		if err := srv.daemon.ContainerGraph().Delete(name); err != nil {
-			return job.Error(err)
-		}
-		return engine.StatusOK
-	}
-
-	if container != nil {
-		if container.State.IsRunning() {
-			if stop {
-				if err := container.Stop(5); err != nil {
-					return job.Errorf("Could not stop running container, cannot remove - %v", err)
-				}
-			} else if kill {
-				if err := container.Kill(); err != nil {
-					return job.Errorf("Could not kill running container, cannot remove - %v", err)
-				}
-			} else {
-				return job.Errorf("You cannot remove a running container. Stop the container before attempting removal or use -s or -k")
-			}
-		}
-		if err := srv.daemon.Destroy(container); err != nil {
-			return job.Errorf("Cannot destroy container %s: %s", name, err)
-		}
-		srv.LogEvent("destroy", container.ID, srv.daemon.Repositories().ImageName(container.Image))
-
-		if removeVolume {
-			var (
-				volumes     = make(map[string]struct{})
-				binds       = make(map[string]struct{})
-				usedVolumes = make(map[string]*daemon.Container)
-			)
-
-			// the volume id is always the base of the path
-			getVolumeId := func(p string) string {
-				return filepath.Base(strings.TrimSuffix(p, "/layer"))
-			}
-
-			// populate bind map so that they can be skipped and not removed
-			for _, bind := range container.HostConfig().Binds {
-				source := strings.Split(bind, ":")[0]
-				// TODO: refactor all volume stuff, all of it
-				// it is very important that we eval the link or comparing the keys to container.Volumes will not work
-				//
-				// eval symlink can fail, ref #5244 if we receive an is not exist error we can ignore it
-				p, err := filepath.EvalSymlinks(source)
-				if err != nil && !os.IsNotExist(err) {
-					return job.Error(err)
-				}
-				if p != "" {
-					source = p
-				}
-				binds[source] = struct{}{}
-			}
-
-			// Store all the deleted containers volumes
-			for _, volumeId := range container.Volumes {
-				// Skip the volumes mounted from external
-				// bind mounts here will will be evaluated for a symlink
-				if _, exists := binds[volumeId]; exists {
-					continue
-				}
-
-				volumeId = getVolumeId(volumeId)
-				volumes[volumeId] = struct{}{}
-			}
-
-			// Retrieve all volumes from all remaining containers
-			for _, container := range srv.daemon.List() {
-				for _, containerVolumeId := range container.Volumes {
-					containerVolumeId = getVolumeId(containerVolumeId)
-					usedVolumes[containerVolumeId] = container
-				}
-			}
-
-			for volumeId := range volumes {
-				// If the requested volu
-				if c, exists := usedVolumes[volumeId]; exists {
-					log.Printf("The volume %s is used by the container %s. Impossible to remove it. Skipping.\n", volumeId, c.ID)
-					continue
-				}
-				if err := srv.daemon.Volumes().Delete(volumeId); err != nil {
-					return job.Errorf("Error calling volumes.Delete(%q): %v", volumeId, err)
-				}
-			}
-		}
-	} else {
-		return job.Errorf("No such container: %s", name)
-	}
-	return engine.StatusOK
-}
-
 func (srv *Server) ContainerCopy(job *engine.Job) engine.Status {
 	if len(job.Args) != 2 {
 		return job.Errorf("Usage: %s CONTAINER RESOURCE\n", job.Name)

server/init.go

@@ -86,25 +86,24 @@ func InitServer(job *engine.Job) engine.Status {
 	job.Eng.Hack_SetGlobalVar("httpapi.daemon", srv.daemon)
 
 	for name, handler := range map[string]engine.Handler{
-		"tag":              srv.ImageTag, // FIXME merge with "image_tag"
-		"info":             srv.DockerInfo,
-		"container_delete": srv.ContainerDestroy,
-		"image_export":     srv.ImageExport,
-		"images":           srv.Images,
-		"history":          srv.ImageHistory,
-		"viz":              srv.ImagesViz,
-		"container_copy":   srv.ContainerCopy,
-		"log":              srv.Log,
-		"changes":          srv.ContainerChanges,
-		"top":              srv.ContainerTop,
-		"load":             srv.ImageLoad,
-		"build":            srv.Build,
-		"pull":             srv.ImagePull,
-		"import":           srv.ImageImport,
-		"image_delete":     srv.ImageDelete,
-		"events":           srv.Events,
-		"push":             srv.ImagePush,
-		"containers":       srv.Containers,
+		"tag":            srv.ImageTag, // FIXME merge with "image_tag"
+		"info":           srv.DockerInfo,
+		"image_export":   srv.ImageExport,
+		"images":         srv.Images,
+		"history":        srv.ImageHistory,
+		"viz":            srv.ImagesViz,
+		"container_copy": srv.ContainerCopy,
+		"log":            srv.Log,
+		"changes":        srv.ContainerChanges,
+		"top":            srv.ContainerTop,
+		"load":           srv.ImageLoad,
+		"build":          srv.Build,
+		"pull":           srv.ImagePull,
+		"import":         srv.ImageImport,
+		"image_delete":   srv.ImageDelete,
+		"events":         srv.Events,
+		"push":           srv.ImagePush,
+		"containers":     srv.Containers,
 	} {
 		if err := job.Eng.Register(name, srv.handlerWrap(handler)); err != nil {
 			return job.Error(err)