From aa3339cf39b88e4a4b3d830d9b0831d4abff1dcb Mon Sep 17 00:00:00 2001 From: Victor Vieux Date: Wed, 11 Dec 2013 15:23:38 -0800 Subject: [PATCH] move container_delete to job handle error remove useless errStr Docker-DCO-1.1-Signed-off-by: Victor Vieux (github: vieux) --- api.go | 24 +++++++------------- integration/server_test.go | 12 +++++++--- server.go | 45 ++++++++++++++++++++++++++++---------- 3 files changed, 50 insertions(+), 31 deletions(-) diff --git a/api.go b/api.go index 1128c7a2b7..5a82375620 100644 --- a/api.go +++ b/api.go @@ -71,13 +71,13 @@ func httpError(w http.ResponseWriter, err error) { // create appropriate error types with clearly defined meaning. if strings.Contains(err.Error(), "No such") { statusCode = http.StatusNotFound - } else if strings.HasPrefix(err.Error(), "Bad parameter") { + } else if strings.Contains(err.Error(), "Bad parameter") { statusCode = http.StatusBadRequest - } else if strings.HasPrefix(err.Error(), "Conflict") { + } else if strings.Contains(err.Error(), "Conflict") { statusCode = http.StatusConflict - } else if strings.HasPrefix(err.Error(), "Impossible") { + } else if strings.Contains(err.Error(), "Impossible") { statusCode = http.StatusNotAcceptable - } else if strings.HasPrefix(err.Error(), "Wrong login/password") { + } else if strings.Contains(err.Error(), "Wrong login/password") { statusCode = http.StatusUnauthorized } else if strings.Contains(err.Error(), "hasn't been activated") { statusCode = http.StatusForbidden @@ -618,18 +618,10 @@ func deleteContainers(srv *Server, version float64, w http.ResponseWriter, r *ht if vars == nil { return fmt.Errorf("Missing parameter") } - name := vars["name"] - - removeVolume, err := getBoolParam(r.Form.Get("v")) - if err != nil { - return err - } - removeLink, err := getBoolParam(r.Form.Get("link")) - if err != nil { - return err - } - - if err := srv.ContainerDestroy(name, removeVolume, removeLink); err != nil { + job := srv.Eng.Job("container_delete", vars["name"]) + job.Setenv("removeVolume", r.Form.Get("v")) + job.Setenv("removeLink", r.Form.Get("link")) + if err := job.Run(); err != nil { return err } w.WriteHeader(http.StatusNoContent) diff --git a/integration/server_test.go b/integration/server_test.go index 2650311c36..e225c4dd35 100644 --- a/integration/server_test.go +++ b/integration/server_test.go @@ -95,7 +95,9 @@ func TestCreateRm(t *testing.T) { t.Errorf("Expected 1 container, %v found", len(c)) } - if err = srv.ContainerDestroy(id, true, false); err != nil { + job := eng.Job("container_delete", id) + job.SetenvBool("removeVolume", true) + if err := job.Run(); err != nil { t.Fatal(err) } @@ -135,7 +137,9 @@ func TestCreateRmVolumes(t *testing.T) { t.Fatal(err) } - if err = srv.ContainerDestroy(id, true, false); err != nil { + job = eng.Job("container_delete", id) + job.SetenvBool("removeVolume", true) + if err := job.Run(); err != nil { t.Fatal(err) } @@ -211,7 +215,9 @@ func TestCreateStartRestartStopStartKillRm(t *testing.T) { } // FIXME: this failed once with a race condition ("Unable to remove filesystem for xxx: directory not empty") - if err := srv.ContainerDestroy(id, true, false); err != nil { + job = eng.Job("container_delete", id) + job.SetenvBool("removeVolume", true) + if err := job.Run(); err != nil { t.Fatal(err) } diff --git a/server.go b/server.go index c9c10fe0ae..cb7c673d24 100644 --- a/server.go +++ b/server.go @@ -116,6 +116,10 @@ func jobInitApi(job *engine.Job) engine.Status { job.Error(err) return engine.StatusErr } + if err := job.Eng.Register("container_delete", srv.ContainerDestroy); err != nil { + job.Error(err) + return engine.StatusErr + } return engine.StatusOK } @@ -1431,24 +1435,36 @@ func (srv *Server) ContainerRestart(name string, t int) error { return nil } -func (srv *Server) ContainerDestroy(name string, removeVolume, removeLink bool) error { +func (srv *Server) ContainerDestroy(job *engine.Job) engine.Status { + if len(job.Args) != 1 { + job.Errorf("Not enough arguments. Usage: %s CONTAINER\n", job.Name) + return engine.StatusErr + } + name := job.Args[0] + removeVolume := job.GetenvBool("removeVolume") + removeLink := job.GetenvBool("removeLink") + container := srv.runtime.Get(name) if removeLink { if container == nil { - return fmt.Errorf("No such link: %s", name) + job.Errorf("No such link: %s", name) + return engine.StatusErr } name, err := srv.runtime.getFullName(name) if err != nil { - return err + job.Error(err) + return engine.StatusErr } parent, n := path.Split(name) if parent == "/" { - return fmt.Errorf("Conflict, cannot remove the default name of the container") + job.Errorf("Conflict, cannot remove the default name of the container") + return engine.StatusErr } pe := srv.runtime.containerGraph.Get(parent) if pe == nil { - return fmt.Errorf("Cannot get parent %s for name %s", parent, name) + job.Errorf("Cannot get parent %s for name %s", parent, name) + return engine.StatusErr } parentContainer := srv.runtime.Get(pe.ID()) @@ -1461,14 +1477,16 @@ func (srv *Server) ContainerDestroy(name string, removeVolume, removeLink bool) } if err := srv.runtime.containerGraph.Delete(name); err != nil { - return err + job.Error(err) + return engine.StatusErr } - return nil + return engine.StatusOK } if container != nil { if container.State.IsRunning() { - return fmt.Errorf("Impossible to remove a running container, please stop it first") + job.Errorf("Impossible to remove a running container, please stop it first") + return engine.StatusErr } volumes := make(map[string]struct{}) @@ -1493,7 +1511,8 @@ func (srv *Server) ContainerDestroy(name string, removeVolume, removeLink bool) volumes[volumeId] = struct{}{} } if err := srv.runtime.Destroy(container); err != nil { - return fmt.Errorf("Cannot destroy container %s: %s", name, err) + job.Errorf("Cannot destroy container %s: %s", name, err) + return engine.StatusErr } srv.LogEvent("destroy", container.ID, srv.runtime.repositories.ImageName(container.Image)) @@ -1513,14 +1532,16 @@ func (srv *Server) ContainerDestroy(name string, removeVolume, removeLink bool) continue } if err := srv.runtime.volumes.Delete(volumeId); err != nil { - return err + job.Error(err) + return engine.StatusErr } } } } else { - return fmt.Errorf("No such container: %s", name) + job.Errorf("No such container: %s", name) + return engine.StatusErr } - return nil + return engine.StatusOK } var ErrImageReferenced = errors.New("Image referenced by a repository")