From 8f2358eeaa59fe369eebc6186403f95c2d66e49b Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Thu, 23 Dec 2021 06:41:55 -0500 Subject: [PATCH] Add podman rm --depend This option causes Podman to not only remove the specified containers but all of the containers that depend on the specified containers. Fixes: https://github.com/containers/podman/issues/10360 Also ran codespell on the code Signed-off-by: Daniel J Walsh --- cmd/podman/containers/rm.go | 3 +- cmd/podman/root.go | 4 +- cmd/podman/utils/error.go | 2 +- cmd/winpath/main.go | 4 +- docs/source/markdown/podman-rm.1.md | 9 ++ libpod/runtime_ctr.go | 31 +++++++ pkg/api/handlers/compat/containers.go | 13 ++- pkg/api/handlers/compat/images_build.go | 2 +- pkg/api/handlers/swagger/swagger.go | 7 ++ pkg/api/handlers/types.go | 11 +++ pkg/api/server/register_containers.go | 17 +++- pkg/bindings/containers/containers.go | 11 +-- pkg/bindings/containers/types.go | 1 + .../containers/types_remove_options.go | 15 ++++ pkg/bindings/test/containers_test.go | 47 +++++----- pkg/domain/entities/containers.go | 6 +- pkg/domain/entities/engine_container.go | 2 +- pkg/domain/entities/reports/containers.go | 28 ++++++ pkg/domain/infra/abi/containers.go | 37 +++++--- pkg/domain/infra/tunnel/containers.go | 90 ++++++++++--------- pkg/specgen/generate/ports.go | 2 +- pkg/specgen/podspecgen.go | 2 +- pkg/specgen/specgen.go | 2 +- test/apiv2/12-imagesMore.at | 2 +- test/apiv2/20-containers.at | 8 +- test/apiv2/22-stop.at | 4 +- test/apiv2/25-containersMore.at | 4 +- .../python/rest_api/test_v2_0_0_container.py | 2 +- test/system/001-basic.bats | 2 +- test/system/055-rm.bats | 12 +++ 30 files changed, 267 insertions(+), 113 deletions(-) create mode 100644 pkg/domain/entities/reports/containers.go diff --git a/cmd/podman/containers/rm.go b/cmd/podman/containers/rm.go index 70cb766937..cede0ba146 100644 --- a/cmd/podman/containers/rm.go +++ b/cmd/podman/containers/rm.go @@ -61,7 +61,8 @@ func rmFlags(cmd *cobra.Command) { flags.BoolVarP(&rmOptions.All, "all", "a", false, "Remove all containers") flags.BoolVarP(&rmOptions.Ignore, "ignore", "i", false, "Ignore errors when a specified container is missing") - flags.BoolVarP(&rmOptions.Force, "force", "f", false, "Force removal of a running or unusable container. The default is false") + flags.BoolVarP(&rmOptions.Force, "force", "f", false, "Force removal of a running or unusable container") + flags.BoolVar(&rmOptions.Depend, "depend", false, "Remove container and all containers that depend on the selected container") timeFlagName := "time" flags.UintVarP(&stopTimeout, timeFlagName, "t", containerConfig.Engine.StopTimeout, "Seconds to wait for stop before killing the container") _ = cmd.RegisterFlagCompletionFunc(timeFlagName, completion.AutocompleteNone) diff --git a/cmd/podman/root.go b/cmd/podman/root.go index bccc559ce3..b695443c25 100644 --- a/cmd/podman/root.go +++ b/cmd/podman/root.go @@ -137,7 +137,7 @@ func persistentPreRunE(cmd *cobra.Command, args []string) error { runtimeFlag := cmd.Root().Flags().Lookup("runtime") if runtimeFlag == nil { return errors.Errorf( - "Unexcpected error setting runtime to '%s' for restore", + "setting runtime to '%s' for restore", *runtime, ) } @@ -217,7 +217,7 @@ func persistentPreRunE(cmd *cobra.Command, args []string) error { context := cmd.Root().LocalFlags().Lookup("context") if context.Value.String() != "default" { - return errors.New("Podman does not support swarm, the only --context value allowed is \"default\"") + return errors.New("podman does not support swarm, the only --context value allowed is \"default\"") } if !registry.IsRemote() { if cmd.Flag("cpu-profile").Changed { diff --git a/cmd/podman/utils/error.go b/cmd/podman/utils/error.go index aab1da675c..b3b54876f4 100644 --- a/cmd/podman/utils/error.go +++ b/cmd/podman/utils/error.go @@ -27,7 +27,7 @@ func (o OutputErrors) PrintErrors() (lastError error) { instead returns a message and we cast it to a new error. Following function performs parsing on build error and returns - exit status which was exepected for this current build + exit status which was expected for this current build */ func ExitCodeFromBuildError(errorMsg string) (int, error) { if strings.Contains(errorMsg, "exit status") { diff --git a/cmd/winpath/main.go b/cmd/winpath/main.go index 494d1cf3c1..6fbe728375 100644 --- a/cmd/winpath/main.go +++ b/cmd/winpath/main.go @@ -114,7 +114,7 @@ func addPathToRegistry(dir string) error { return err } -// Removes all occurences of a directory path from the Windows path stored in the registry +// Removes all occurrences of a directory path from the Windows path stored in the registry func removePathFromRegistry(path string) error { k, err := registry.OpenKey(registry.CURRENT_USER, Environment, registry.READ|registry.WRITE) if err != nil { @@ -155,7 +155,7 @@ func removePathFromRegistry(path string) error { return err } -// Sends a notification message to all top level windows informing them the environmental setings have changed. +// Sends a notification message to all top level windows informing them the environmental settings have changed. // Applications such as the Windows command prompt and powershell will know to stop caching stale values on // subsequent restarts. Since applications block the sender when receiving a message, we set a 3 second timeout func broadcastEnvironmentChange() { diff --git a/docs/source/markdown/podman-rm.1.md b/docs/source/markdown/podman-rm.1.md index c599c8687c..f3807d2f7a 100644 --- a/docs/source/markdown/podman-rm.1.md +++ b/docs/source/markdown/podman-rm.1.md @@ -18,6 +18,10 @@ Running or unusable containers will not be removed without the **-f** option. Remove all containers. Can be used in conjunction with **-f** as well. +#### **--depend** + +Remove selected container and recursively remove all containers that depend on it. + #### **--cidfile** Read container ID from the specified file and remove the container. Can be specified multiple times. @@ -56,6 +60,11 @@ Remove a container by its name *mywebserver* $ podman rm mywebserver ``` +Remove a *mywebserver* container and all of the containers that depend on it +``` +$ podman rm --depend mywebserver +``` + Remove several containers by name and container id. ``` $ podman rm mywebserver myflaskserver 860a4b23 diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 59a1fd1534..2891eb783c 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -915,6 +915,37 @@ func (r *Runtime) evictContainer(ctx context.Context, idOrName string, removeVol return id, cleanupErr } +// RemoveDepend removes all dependencies for a container +func (r *Runtime) RemoveDepend(ctx context.Context, rmCtr *Container, force bool, removeVolume bool, timeout *uint) ([]*reports.RmReport, error) { + rmReports := make([]*reports.RmReport, 0) + deps, err := r.state.ContainerInUse(rmCtr) + if err != nil { + if err == define.ErrCtrRemoved { + return rmReports, nil + } + return rmReports, err + } + for _, cid := range deps { + ctr, err := r.state.Container(cid) + if err != nil { + if err == define.ErrNoSuchCtr { + continue + } + return rmReports, err + } + + reports, err := r.RemoveDepend(ctx, ctr, force, removeVolume, timeout) + if err != nil { + return rmReports, err + } + rmReports = append(rmReports, reports...) + } + report := reports.RmReport{Id: rmCtr.ID()} + report.Err = r.removeContainer(ctx, rmCtr, force, removeVolume, false, timeout) + + return append(rmReports, &report), nil +} + // GetContainer retrieves a container by its ID func (r *Runtime) GetContainer(id string) (*Container, error) { r.lock.RLock() diff --git a/pkg/api/handlers/compat/containers.go b/pkg/api/handlers/compat/containers.go index ad341c3ab5..4539199d3a 100644 --- a/pkg/api/handlers/compat/containers.go +++ b/pkg/api/handlers/compat/containers.go @@ -36,6 +36,7 @@ func RemoveContainer(w http.ResponseWriter, r *http.Request) { query := struct { Force bool `schema:"force"` Ignore bool `schema:"ignore"` + Depend bool `schema:"depend"` Link bool `schema:"link"` Timeout *uint `schema:"timeout"` DockerVolumes bool `schema:"v"` @@ -57,6 +58,7 @@ func RemoveContainer(w http.ResponseWriter, r *http.Request) { if utils.IsLibpodRequest(r) { options.Volumes = query.LibpodVolumes options.Timeout = query.Timeout + options.Depend = query.Depend } else { if query.Link { utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, @@ -71,7 +73,7 @@ func RemoveContainer(w http.ResponseWriter, r *http.Request) { // code. containerEngine := abi.ContainerEngine{Libpod: runtime} name := utils.GetName(r) - report, err := containerEngine.ContainerRm(r.Context(), []string{name}, options) + reports, err := containerEngine.ContainerRm(r.Context(), []string{name}, options) if err != nil { if errors.Cause(err) == define.ErrNoSuchCtr { utils.ContainerNotFound(w, name, err) @@ -81,8 +83,8 @@ func RemoveContainer(w http.ResponseWriter, r *http.Request) { utils.InternalServerError(w, err) return } - if len(report) > 0 && report[0].Err != nil { - err = report[0].Err + if len(reports) > 0 && reports[0].Err != nil { + err = reports[0].Err if errors.Cause(err) == define.ErrNoSuchCtr { utils.ContainerNotFound(w, name, err) return @@ -90,7 +92,10 @@ func RemoveContainer(w http.ResponseWriter, r *http.Request) { utils.InternalServerError(w, err) return } - + if utils.IsLibpodRequest(r) { + utils.WriteResponse(w, http.StatusOK, reports) + return + } utils.WriteResponse(w, http.StatusNoContent, nil) } diff --git a/pkg/api/handlers/compat/images_build.go b/pkg/api/handlers/compat/images_build.go index 0fcac53309..2d296b5ce9 100644 --- a/pkg/api/handlers/compat/images_build.go +++ b/pkg/api/handlers/compat/images_build.go @@ -138,7 +138,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) { // if layers field not set assume its not from a valid podman-client // could be a docker client, set `layers=true` since that is the default - // expected behviour + // expected behaviour if !utils.IsLibpodRequest(r) { if _, found := r.URL.Query()["layers"]; !found { query.Layers = true diff --git a/pkg/api/handlers/swagger/swagger.go b/pkg/api/handlers/swagger/swagger.go index 9844839b71..7868ff2063 100644 --- a/pkg/api/handlers/swagger/swagger.go +++ b/pkg/api/handlers/swagger/swagger.go @@ -111,6 +111,13 @@ type swagLibpodInspectImageResponse struct { } } +// Rm containers +// swagger:response DocsLibpodContainerRmReport +type swagLibpodContainerRmReport struct { + // in: body + Body []handlers.LibpodContainersRmReport +} + // Prune containers // swagger:response DocsContainerPruneReport type swagContainerPruneReport struct { diff --git a/pkg/api/handlers/types.go b/pkg/api/handlers/types.go index f850db3d8c..588758b2cc 100644 --- a/pkg/api/handlers/types.go +++ b/pkg/api/handlers/types.go @@ -53,6 +53,17 @@ type LibpodContainersPruneReport struct { PruneError string `json:"Err,omitempty"` } +type LibpodContainersRmReport struct { + ID string `json:"Id"` + // Error which occurred during Rm operation (if any). + // This field is optional and may be omitted if no error occurred. + // + // Extensions: + // x-omitempty: true + // x-nullable: true + RmError string `json:"Err,omitempty"` +} + type Info struct { docker.Info BuildahVersion string diff --git a/pkg/api/server/register_containers.go b/pkg/api/server/register_containers.go index 601e1251b6..4d19c04d4a 100644 --- a/pkg/api/server/register_containers.go +++ b/pkg/api/server/register_containers.go @@ -817,9 +817,22 @@ func (s *APIServer) registerContainersHandlers(r *mux.Router) error { // required: true // description: the name or ID of the container // - in: query + // name: depend + // type: boolean + // description: additionally remove containers that depend on the container to be removed + // - in: query // name: force // type: boolean - // description: need something + // description: force stop container if running + // - in: query + // name: ignore + // type: boolean + // description: ignore errors when the container to be removed does not existxo + // - in: query + // name: timeout + // type: integer + // default: 10 + // description: number of seconds to wait before killing container when force removing // - in: query // name: v // type: boolean @@ -827,6 +840,8 @@ func (s *APIServer) registerContainersHandlers(r *mux.Router) error { // produces: // - application/json // responses: + // 200: + // $ref: "#/responses/DocsLibpodContainerRmReport" // 204: // description: no error // 400: diff --git a/pkg/bindings/containers/containers.go b/pkg/bindings/containers/containers.go index 14a173025e..0148e62cbb 100644 --- a/pkg/bindings/containers/containers.go +++ b/pkg/bindings/containers/containers.go @@ -78,25 +78,26 @@ func Prune(ctx context.Context, options *PruneOptions) ([]*reports.PruneReport, // The volumes bool dictates that a container's volumes should also be removed. // The All option indicates that all containers should be removed // The Ignore option indicates that if a container did not exist, ignore the error -func Remove(ctx context.Context, nameOrID string, options *RemoveOptions) error { +func Remove(ctx context.Context, nameOrID string, options *RemoveOptions) ([]*reports.RmReport, error) { if options == nil { options = new(RemoveOptions) } + var reports []*reports.RmReport conn, err := bindings.GetClient(ctx) if err != nil { - return err + return reports, err } params, err := options.ToParams() if err != nil { - return err + return reports, err } response, err := conn.DoRequest(ctx, nil, http.MethodDelete, "/containers/%s", params, nil, nameOrID) if err != nil { - return err + return reports, err } defer response.Body.Close() - return response.Process(nil) + return reports, response.Process(&reports) } // Inspect returns low level information about a Container. The nameOrID can be a container name diff --git a/pkg/bindings/containers/types.go b/pkg/bindings/containers/types.go index 81a53a549a..db3eb3e1b7 100644 --- a/pkg/bindings/containers/types.go +++ b/pkg/bindings/containers/types.go @@ -138,6 +138,7 @@ type PruneOptions struct { //go:generate go run ../generator/generator.go RemoveOptions // RemoveOptions are optional options for removing containers type RemoveOptions struct { + Depend *bool Ignore *bool Force *bool Volumes *bool diff --git a/pkg/bindings/containers/types_remove_options.go b/pkg/bindings/containers/types_remove_options.go index 1e52e819d1..7fa198d2f3 100644 --- a/pkg/bindings/containers/types_remove_options.go +++ b/pkg/bindings/containers/types_remove_options.go @@ -17,6 +17,21 @@ func (o *RemoveOptions) ToParams() (url.Values, error) { return util.ToParams(o) } +// WithDepend set field Depend to given value +func (o *RemoveOptions) WithDepend(value bool) *RemoveOptions { + o.Depend = &value + return o +} + +// GetDepend returns value of field Depend +func (o *RemoveOptions) GetDepend() bool { + if o.Depend == nil { + var z bool + return z + } + return *o.Depend +} + // WithIgnore set field Ignore to given value func (o *RemoveOptions) WithIgnore(value bool) *RemoveOptions { o.Ignore = &value diff --git a/pkg/bindings/test/containers_test.go b/pkg/bindings/test/containers_test.go index b6c06756bc..cab032a407 100644 --- a/pkg/bindings/test/containers_test.go +++ b/pkg/bindings/test/containers_test.go @@ -175,7 +175,7 @@ var _ = Describe("Podman containers ", func() { Expect(err).To(BeNil()) err = containers.Pause(bt.conn, cid, nil) Expect(err).To(BeNil()) - err = containers.Remove(bt.conn, cid, nil) + _, err = containers.Remove(bt.conn, cid, nil) Expect(err).ToNot(BeNil()) code, _ := bindings.CheckResponseCode(err) Expect(code).To(BeNumerically("==", http.StatusInternalServerError)) @@ -188,8 +188,10 @@ var _ = Describe("Podman containers ", func() { Expect(err).To(BeNil()) err = containers.Pause(bt.conn, cid, nil) Expect(err).To(BeNil()) - err = containers.Remove(bt.conn, cid, new(containers.RemoveOptions).WithForce(true)) + rmResponse, err := containers.Remove(bt.conn, cid, new(containers.RemoveOptions).WithForce(true)) Expect(err).To(BeNil()) + Expect(len(reports.RmReportsErrs(rmResponse))).To(Equal(0)) + Expect(len(reports.RmReportsIds(rmResponse))).To(Equal(1)) }) It("podman stop a paused container by name", func() { @@ -669,7 +671,8 @@ var _ = Describe("Podman containers ", func() { }) It("podman remove bogus container", func() { - err = containers.Remove(bt.conn, "foobar", nil) + _, err := containers.Remove(bt.conn, "foobar", nil) + Expect(err).ToNot(BeNil()) code, _ := bindings.CheckResponseCode(err) Expect(code).To(BeNumerically("==", http.StatusNotFound)) }) @@ -679,7 +682,7 @@ var _ = Describe("Podman containers ", func() { _, err := bt.RunTopContainer(&name, nil) Expect(err).To(BeNil()) // Removing running container should fail - err = containers.Remove(bt.conn, name, nil) + _, err = containers.Remove(bt.conn, name, nil) Expect(err).ToNot(BeNil()) code, _ := bindings.CheckResponseCode(err) Expect(code).To(BeNumerically("==", http.StatusInternalServerError)) @@ -690,7 +693,7 @@ var _ = Describe("Podman containers ", func() { cid, err := bt.RunTopContainer(&name, nil) Expect(err).To(BeNil()) // Removing running container should fail - err = containers.Remove(bt.conn, cid, nil) + _, err = containers.Remove(bt.conn, cid, nil) Expect(err).ToNot(BeNil()) code, _ := bindings.CheckResponseCode(err) Expect(code).To(BeNumerically("==", http.StatusInternalServerError)) @@ -700,22 +703,22 @@ var _ = Describe("Podman containers ", func() { var name = "top" _, err := bt.RunTopContainer(&name, nil) Expect(err).To(BeNil()) - // Removing running container should fail - err = containers.Remove(bt.conn, name, new(containers.RemoveOptions).WithForce(true)) + // Removing running container should succeed + rmResponse, err := containers.Remove(bt.conn, name, new(containers.RemoveOptions).WithForce(true)) Expect(err).To(BeNil()) - //code, _ := bindings.CheckResponseCode(err) - //Expect(code).To(BeNumerically("==", http.StatusInternalServerError)) + Expect(len(reports.RmReportsErrs(rmResponse))).To(Equal(0)) + Expect(len(reports.RmReportsIds(rmResponse))).To(Equal(1)) }) It("podman forcibly remove running container by ID", func() { var name = "top" cid, err := bt.RunTopContainer(&name, nil) Expect(err).To(BeNil()) - // Removing running container should fail - err = containers.Remove(bt.conn, cid, new(containers.RemoveOptions).WithForce(true)) + // Forcably Removing running container should succeed + rmResponse, err := containers.Remove(bt.conn, cid, new(containers.RemoveOptions).WithForce(true)) Expect(err).To(BeNil()) - //code, _ := bindings.CheckResponseCode(err) - //Expect(code).To(BeNumerically("==", http.StatusInternalServerError)) + Expect(len(reports.RmReportsErrs(rmResponse))).To(Equal(0)) + Expect(len(reports.RmReportsIds(rmResponse))).To(Equal(1)) }) It("podman remove running container and volume by name", func() { @@ -723,7 +726,7 @@ var _ = Describe("Podman containers ", func() { _, err := bt.RunTopContainer(&name, nil) Expect(err).To(BeNil()) // Removing running container should fail - err = containers.Remove(bt.conn, name, new(containers.RemoveOptions).WithVolumes(true)) + _, err = containers.Remove(bt.conn, name, new(containers.RemoveOptions).WithVolumes(true)) Expect(err).ToNot(BeNil()) code, _ := bindings.CheckResponseCode(err) Expect(code).To(BeNumerically("==", http.StatusInternalServerError)) @@ -734,7 +737,7 @@ var _ = Describe("Podman containers ", func() { cid, err := bt.RunTopContainer(&name, nil) Expect(err).To(BeNil()) // Removing running container should fail - err = containers.Remove(bt.conn, cid, new(containers.RemoveOptions).WithVolumes(true)) + _, err = containers.Remove(bt.conn, cid, new(containers.RemoveOptions).WithVolumes(true)) Expect(err).ToNot(BeNil()) code, _ := bindings.CheckResponseCode(err) Expect(code).To(BeNumerically("==", http.StatusInternalServerError)) @@ -744,11 +747,11 @@ var _ = Describe("Podman containers ", func() { var name = "top" _, err := bt.RunTopContainer(&name, nil) Expect(err).To(BeNil()) - // Removing running container should fail - err = containers.Remove(bt.conn, name, new(containers.RemoveOptions).WithVolumes(true).WithForce(true)) + // Forcibly Removing running container should succeed + rmResponse, err := containers.Remove(bt.conn, name, new(containers.RemoveOptions).WithVolumes(true).WithForce(true)) Expect(err).To(BeNil()) - //code, _ := bindings.CheckResponseCode(err) - //Expect(code).To(BeNumerically("==", http.StatusInternalServerError)) + Expect(len(reports.RmReportsErrs(rmResponse))).To(Equal(0)) + Expect(len(reports.RmReportsIds(rmResponse))).To(Equal(1)) }) It("podman forcibly remove running container and volume by ID", func() { @@ -756,10 +759,10 @@ var _ = Describe("Podman containers ", func() { cid, err := bt.RunTopContainer(&name, nil) Expect(err).To(BeNil()) // Removing running container should fail - err = containers.Remove(bt.conn, cid, new(containers.RemoveOptions).WithForce(true).WithVolumes(true)) + rmResponse, err := containers.Remove(bt.conn, cid, new(containers.RemoveOptions).WithForce(true).WithVolumes(true)) Expect(err).To(BeNil()) - //code, _ := bindings.CheckResponseCode(err) - //Expect(code).To(BeNumerically("==", http.StatusInternalServerError)) + Expect(len(reports.RmReportsErrs(rmResponse))).To(Equal(0)) + Expect(len(reports.RmReportsIds(rmResponse))).To(Equal(1)) }) It("List containers with filters", func() { diff --git a/pkg/domain/entities/containers.go b/pkg/domain/entities/containers.go index ae441b7f31..e3f8f1b7c0 100644 --- a/pkg/domain/entities/containers.go +++ b/pkg/domain/entities/containers.go @@ -129,6 +129,7 @@ type RestartReport struct { type RmOptions struct { All bool + Depend bool Force bool Ignore bool Latest bool @@ -136,11 +137,6 @@ type RmOptions struct { Volumes bool } -type RmReport struct { - Err error - Id string //nolint -} - type ContainerInspectReport struct { *define.InspectContainerData } diff --git a/pkg/domain/entities/engine_container.go b/pkg/domain/entities/engine_container.go index 383e420986..7ce4dd0f69 100644 --- a/pkg/domain/entities/engine_container.go +++ b/pkg/domain/entities/engine_container.go @@ -40,7 +40,7 @@ type ContainerEngine interface { ContainerRename(ctr context.Context, nameOrID string, options ContainerRenameOptions) error ContainerRestart(ctx context.Context, namesOrIds []string, options RestartOptions) ([]*RestartReport, error) ContainerRestore(ctx context.Context, namesOrIds []string, options RestoreOptions) ([]*RestoreReport, error) - ContainerRm(ctx context.Context, namesOrIds []string, options RmOptions) ([]*RmReport, error) + ContainerRm(ctx context.Context, namesOrIds []string, options RmOptions) ([]*reports.RmReport, error) ContainerRun(ctx context.Context, opts ContainerRunOptions) (*ContainerRunReport, error) ContainerRunlabel(ctx context.Context, label string, image string, args []string, opts ContainerRunlabelOptions) error ContainerStart(ctx context.Context, namesOrIds []string, options ContainerStartOptions) ([]*ContainerStartReport, error) diff --git a/pkg/domain/entities/reports/containers.go b/pkg/domain/entities/reports/containers.go new file mode 100644 index 0000000000..54bcd092ba --- /dev/null +++ b/pkg/domain/entities/reports/containers.go @@ -0,0 +1,28 @@ +package reports + +type RmReport struct { + Id string `json:"Id"` //nolint + Err error `json:"Err,omitempty"` +} + +func RmReportsIds(r []*RmReport) []string { + ids := make([]string, 0, len(r)) + for _, v := range r { + if v == nil || v.Id == "" { + continue + } + ids = append(ids, v.Id) + } + return ids +} + +func RmReportsErrs(r []*RmReport) []error { + errs := make([]error, 0, len(r)) + for _, v := range r { + if v == nil || v.Err == nil { + continue + } + errs = append(errs, v.Err) + } + return errs +} diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index bf4dcff629..a4522698ec 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -301,27 +301,27 @@ func (ic *ContainerEngine) removeContainer(ctx context.Context, ctr *libpod.Cont return err } -func (ic *ContainerEngine) ContainerRm(ctx context.Context, namesOrIds []string, options entities.RmOptions) ([]*entities.RmReport, error) { - reports := []*entities.RmReport{} +func (ic *ContainerEngine) ContainerRm(ctx context.Context, namesOrIds []string, options entities.RmOptions) ([]*reports.RmReport, error) { + rmReports := []*reports.RmReport{} names := namesOrIds // Attempt to remove named containers directly from storage, if container is defined in libpod // this will fail and code will fall through to removing the container from libpod.` tmpNames := []string{} for _, ctr := range names { - report := entities.RmReport{Id: ctr} + report := reports.RmReport{Id: ctr} report.Err = ic.Libpod.RemoveStorageContainer(ctr, options.Force) switch errors.Cause(report.Err) { case nil: // remove container names that we successfully deleted - reports = append(reports, &report) + rmReports = append(rmReports, &report) case define.ErrNoSuchCtr, define.ErrCtrExists: // There is still a potential this is a libpod container tmpNames = append(tmpNames, ctr) default: if _, err := ic.Libpod.LookupContainer(ctr); errors.Cause(err) == define.ErrNoSuchCtr { // remove container failed, but not a libpod container - reports = append(reports, &report) + rmReports = append(rmReports, &report) continue } // attempt to remove as a libpod container @@ -340,23 +340,34 @@ func (ic *ContainerEngine) ContainerRm(ctx context.Context, namesOrIds []string, for _, ctr := range names { logrus.Debugf("Evicting container %q", ctr) - report := entities.RmReport{Id: ctr} + report := reports.RmReport{Id: ctr} _, err := ic.Libpod.EvictContainer(ctx, ctr, options.Volumes) if err != nil { if options.Ignore && errors.Cause(err) == define.ErrNoSuchCtr { logrus.Debugf("Ignoring error (--allow-missing): %v", err) - reports = append(reports, &report) + rmReports = append(rmReports, &report) continue } report.Err = err - reports = append(reports, &report) + rmReports = append(rmReports, &report) continue } - reports = append(reports, &report) + rmReports = append(rmReports, &report) } - return reports, nil + return rmReports, nil } + if !options.All && options.Depend { + // Add additional containers based on dependencies to container map + for _, ctr := range ctrs { + reports, err := ic.Libpod.RemoveDepend(ctx, ctr, options.Force, options.Volumes, options.Timeout) + if err != nil { + return rmReports, err + } + rmReports = append(rmReports, reports...) + } + return rmReports, nil + } errMap, err := parallelctr.ContainerOp(ctx, ctrs, func(c *libpod.Container) error { return ic.removeContainer(ctx, c, options) }) @@ -364,12 +375,12 @@ func (ic *ContainerEngine) ContainerRm(ctx context.Context, namesOrIds []string, return nil, err } for ctr, err := range errMap { - report := new(entities.RmReport) + report := new(reports.RmReport) report.Id = ctr.ID() report.Err = err - reports = append(reports, report) + rmReports = append(rmReports, report) } - return reports, nil + return rmReports, nil } func (ic *ContainerEngine) ContainerInspect(ctx context.Context, namesOrIds []string, options entities.InspectOptions) ([]*entities.ContainerInspectReport, []error, error) { diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index 2127f8749c..4f72eab963 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -182,9 +182,9 @@ func (ic *ContainerEngine) ContainerRestart(ctx context.Context, namesOrIds []st return reports, nil } -func (ic *ContainerEngine) ContainerRm(ctx context.Context, namesOrIds []string, opts entities.RmOptions) ([]*entities.RmReport, error) { +func (ic *ContainerEngine) ContainerRm(ctx context.Context, namesOrIds []string, opts entities.RmOptions) ([]*reports.RmReport, error) { // TODO there is no endpoint for container eviction. Need to discuss - options := new(containers.RemoveOptions).WithForce(opts.Force).WithVolumes(opts.Volumes).WithIgnore(opts.Ignore) + options := new(containers.RemoveOptions).WithForce(opts.Force).WithVolumes(opts.Volumes).WithIgnore(opts.Ignore).WithDepend(opts.Depend) if opts.Timeout != nil { options = options.WithTimeout(*opts.Timeout) } @@ -193,25 +193,31 @@ func (ic *ContainerEngine) ContainerRm(ctx context.Context, namesOrIds []string, if err != nil { return nil, err } - reports := make([]*entities.RmReport, 0, len(ctrs)) + rmReports := make([]*reports.RmReport, 0, len(ctrs)) for _, c := range ctrs { - reports = append(reports, &entities.RmReport{ - Id: c.ID, - Err: containers.Remove(ic.ClientCtx, c.ID, options), - }) + report, err := containers.Remove(ic.ClientCtx, c.ID, options) + if err != nil { + return rmReports, err + } + rmReports = append(rmReports, report...) } - return reports, nil + return rmReports, nil } - reports := make([]*entities.RmReport, 0, len(namesOrIds)) + rmReports := make([]*reports.RmReport, 0, len(namesOrIds)) for _, name := range namesOrIds { - reports = append(reports, &entities.RmReport{ - Id: name, - Err: containers.Remove(ic.ClientCtx, name, options), - }) + report, err := containers.Remove(ic.ClientCtx, name, options) + if err != nil { + rmReports = append(rmReports, &reports.RmReport{ + Id: name, + Err: err, + }) + continue + } + rmReports = append(rmReports, report...) } - return reports, nil + return rmReports, nil } func (ic *ContainerEngine) ContainerPrune(ctx context.Context, opts entities.ContainerPruneOptions) ([]*reports.PruneReport, error) { @@ -552,6 +558,27 @@ func startAndAttach(ic *ContainerEngine, name string, detachKeys *string, input, return <-attachErr } +func logIfRmError(id string, err error, reports []*reports.RmReport) { + logError := func(id string, err error) { + if errorhandling.Contains(err, define.ErrNoSuchCtr) || + errorhandling.Contains(err, define.ErrCtrRemoved) || + errorhandling.Contains(err, types.ErrLayerUnknown) { + logrus.Debugf("Container %s does not exist: %v", id, err) + } else { + logrus.Errorf("Removing container %s: %v", id, err) + } + } + if err != nil { + logError(id, err) + } else { + for _, report := range reports { + if report.Err != nil { + logError(report.Id, report.Err) + } + } + } +} + func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []string, options entities.ContainerStartOptions) ([]*entities.ContainerStartReport, error) { reports := []*entities.ContainerStartReport{} var exitCode = define.ExecErrorCodeGeneric @@ -590,14 +617,8 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri } removeOptions := new(containers.RemoveOptions).WithVolumes(true).WithForce(false) removeContainer := func(id string) { - if err := containers.Remove(ic.ClientCtx, id, removeOptions); err != nil { - if errorhandling.Contains(err, define.ErrNoSuchCtr) || - errorhandling.Contains(err, define.ErrCtrRemoved) { - logrus.Debugf("Container %s does not exist: %v", id, err) - } else { - logrus.Errorf("Removing container %s: %v", id, err) - } - } + reports, err := containers.Remove(ic.ClientCtx, id, removeOptions) + logIfRmError(id, err, reports) } // There can only be one container if attach was used @@ -674,15 +695,8 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri if err != nil { if ctr.AutoRemove { rmOptions := new(containers.RemoveOptions).WithForce(false).WithVolumes(true) - if err := containers.Remove(ic.ClientCtx, ctr.ID, rmOptions); err != nil { - if errorhandling.Contains(err, define.ErrNoSuchCtr) || - errorhandling.Contains(err, define.ErrCtrRemoved) || - errorhandling.Contains(err, types.ErrLayerUnknown) { - logrus.Debugf("Container %s does not exist: %v", ctr.ID, err) - } else { - logrus.Errorf("Removing container %s: %v", ctr.ID, err) - } - } + reports, err := containers.Remove(ic.ClientCtx, ctr.ID, rmOptions) + logIfRmError(ctr.ID, err, reports) } report.Err = errors.Wrapf(err, "unable to start container %q", name) report.ExitCode = define.ExitCode(err) @@ -741,7 +755,8 @@ func (ic *ContainerEngine) ContainerRun(ctx context.Context, opts entities.Conta report.ExitCode = define.ExitCode(err) if opts.Rm { - if rmErr := containers.Remove(ic.ClientCtx, con.ID, new(containers.RemoveOptions).WithForce(false).WithVolumes(true)); rmErr != nil { + reports, rmErr := containers.Remove(ic.ClientCtx, con.ID, new(containers.RemoveOptions).WithForce(false).WithVolumes(true)) + if rmErr != nil || reports[0].Err != nil { logrus.Debugf("unable to remove container %s after failing to start and attach to it", con.ID) } } @@ -759,15 +774,8 @@ func (ic *ContainerEngine) ContainerRun(ctx context.Context, opts entities.Conta } if !shouldRestart { - if err := containers.Remove(ic.ClientCtx, con.ID, new(containers.RemoveOptions).WithForce(false).WithVolumes(true)); err != nil { - if errorhandling.Contains(err, define.ErrNoSuchCtr) || - errorhandling.Contains(err, define.ErrCtrRemoved) || - errorhandling.Contains(err, types.ErrLayerUnknown) { - logrus.Debugf("Container %s does not exist: %v", con.ID, err) - } else { - logrus.Errorf("Removing container %s: %v", con.ID, err) - } - } + reports, err := containers.Remove(ic.ClientCtx, con.ID, new(containers.RemoveOptions).WithForce(false).WithVolumes(true)) + logIfRmError(con.ID, err, reports) } }() } diff --git a/pkg/specgen/generate/ports.go b/pkg/specgen/generate/ports.go index b60cc1e980..34b43a62e4 100644 --- a/pkg/specgen/generate/ports.go +++ b/pkg/specgen/generate/ports.go @@ -206,7 +206,7 @@ func ParsePortMapping(portMappings []types.PortMapping, exposePorts map[uint16][ } // we do no longer need the original port mappings - // set it to 0 length so we can resuse it to populate + // set it to 0 length so we can reuse it to populate // the slice again while keeping the underlying capacity portMappings = portMappings[:0] diff --git a/pkg/specgen/podspecgen.go b/pkg/specgen/podspecgen.go index 33e8422fdd..fdaa714da0 100644 --- a/pkg/specgen/podspecgen.go +++ b/pkg/specgen/podspecgen.go @@ -95,7 +95,7 @@ type PodNetworkConfig struct { // Map of networks names ot ids the container should join to. // You can request additional settings for each network, you can // set network aliases, static ips, static mac address and the - // network interface name for this container on the specifc network. + // network interface name for this container on the specific network. // If the map is empty and the bridge network mode is set the container // will be joined to the default network. Networks map[string]types.PerNetworkOptions diff --git a/pkg/specgen/specgen.go b/pkg/specgen/specgen.go index 5989456c96..6c1011a78f 100644 --- a/pkg/specgen/specgen.go +++ b/pkg/specgen/specgen.go @@ -426,7 +426,7 @@ type ContainerNetworkConfig struct { // Map of networks names ot ids the container should join to. // You can request additional settings for each network, you can // set network aliases, static ips, static mac address and the - // network interface name for this container on the specifc network. + // network interface name for this container on the specific network. // If the map is empty and the bridge network mode is set the container // will be joined to the default network. Networks map[string]nettypes.PerNetworkOptions diff --git a/test/apiv2/12-imagesMore.at b/test/apiv2/12-imagesMore.at index 3a5d5c0969..96eba1db55 100644 --- a/test/apiv2/12-imagesMore.at +++ b/test/apiv2/12-imagesMore.at @@ -53,7 +53,7 @@ t GET libpod/images/$IMAGE/json 200 \ .RepoTags[-1]=$IMAGE # Remove the registry container -t DELETE libpod/containers/registry?force=true 204 +t DELETE libpod/containers/registry?force=true 200 # Remove images t DELETE libpod/images/$IMAGE 200 \ diff --git a/test/apiv2/20-containers.at b/test/apiv2/20-containers.at index 554a905d4e..936597f72a 100644 --- a/test/apiv2/20-containers.at +++ b/test/apiv2/20-containers.at @@ -85,7 +85,7 @@ else fi fi -t DELETE libpod/containers/$cid 204 +t DELETE libpod/containers/$cid 200 .[0].Id=$cid # Issue #6799: it should be possible to start a container, even w/o args. t POST libpod/containers/create?name=test_noargs Image=${IMAGE} 201 \ @@ -100,7 +100,7 @@ t GET libpod/containers/${cid}/json 200 \ .State.Status~\\\(exited\\\|stopped\\\) \ .State.Running=false \ .State.ExitCode=0 -t DELETE libpod/containers/$cid 204 +t DELETE libpod/containers/$cid 200 .[0].Id=$cid CNAME=myfoo podman run -d --name $CNAME $IMAGE top @@ -190,8 +190,8 @@ t GET containers/myctr/json 200 \ t DELETE images/localhost/newrepo:latest?force=true 200 t DELETE images/localhost/newrepo:v1?force=true 200 t DELETE images/localhost/newrepo:v2?force=true 200 -t DELETE libpod/containers/$cid?force=true 204 -t DELETE libpod/containers/myctr 204 +t DELETE libpod/containers/$cid?force=true 200 .[0].Id=$cid +t DELETE libpod/containers/myctr 200 t DELETE libpod/containers/bogus 404 diff --git a/test/apiv2/22-stop.at b/test/apiv2/22-stop.at index 91bc9937d8..bde534b728 100644 --- a/test/apiv2/22-stop.at +++ b/test/apiv2/22-stop.at @@ -11,7 +11,7 @@ podman run -dt --name mytop $IMAGE top &>/dev/null t GET libpod/containers/mytop/json 200 .State.Status=running t POST libpod/containers/mytop/stop 204 t GET libpod/containers/mytop/json 200 .State.Status~\\\(exited\\\|stopped\\\) -t DELETE libpod/containers/mytop 204 +t DELETE libpod/containers/mytop 200 # stop, by ID # Remember that podman() hides all output; we need to get our CID via inspect @@ -21,4 +21,4 @@ t GET libpod/containers/mytop/json 200 .State.Status=running cid=$(jq -r .Id <<<"$output") t POST libpod/containers/$cid/stop 204 t GET libpod/containers/mytop/json 200 .State.Status~\\\(exited\\\|stopped\\\) -t DELETE libpod/containers/mytop 204 +t DELETE libpod/containers/mytop 200 diff --git a/test/apiv2/25-containersMore.at b/test/apiv2/25-containersMore.at index 0a049d8697..c9fda8c6f5 100644 --- a/test/apiv2/25-containersMore.at +++ b/test/apiv2/25-containersMore.at @@ -51,7 +51,7 @@ like "$output" ".*merged" "Check container mount" # Unmount the container t POST libpod/containers/foo/unmount 204 -t DELETE libpod/containers/foo?force=true 204 +t DELETE libpod/containers/foo?force=true 200 podman run $IMAGE true @@ -79,7 +79,7 @@ like "$output" ".*metadata:.*" "Check generated kube yaml(service=true) - metada like "$output" ".*spec:.*" "Check generated kube yaml(service=true) - spec" like "$output" ".*kind:\\sService.*" "Check generated kube yaml(service=true) - kind: Service" -t DELETE libpod/containers/$cid 204 +t DELETE libpod/containers/$cid 200 .[0].Id=$cid # Create 3 stopped containers to test containers prune podman run $IMAGE true diff --git a/test/apiv2/python/rest_api/test_v2_0_0_container.py b/test/apiv2/python/rest_api/test_v2_0_0_container.py index 101044bbb7..1b4597cf8c 100644 --- a/test/apiv2/python/rest_api/test_v2_0_0_container.py +++ b/test/apiv2/python/rest_api/test_v2_0_0_container.py @@ -99,7 +99,7 @@ def test_stats(self): def test_delete(self): r = requests.delete(self.uri(self.resolve_container("/containers/{}?force=true"))) - self.assertEqual(r.status_code, 204, r.text) + self.assertEqual(r.status_code, 200, r.text) def test_stop(self): r = requests.post(self.uri(self.resolve_container("/containers/{}/start"))) diff --git a/test/system/001-basic.bats b/test/system/001-basic.bats index 23489c1b59..9b0a71285a 100644 --- a/test/system/001-basic.bats +++ b/test/system/001-basic.bats @@ -41,7 +41,7 @@ function setup() { # This one must fail run_podman 125 --context=swarm version is "$output" \ - "Error: Podman does not support swarm, the only --context value allowed is \"default\"" \ + "Error: podman does not support swarm, the only --context value allowed is \"default\"" \ "--context=default or fail" } diff --git a/test/system/055-rm.bats b/test/system/055-rm.bats index 7fe81c0841..69663fafaa 100644 --- a/test/system/055-rm.bats +++ b/test/system/055-rm.bats @@ -58,6 +58,18 @@ load helpers run_podman rm -af } +@test "podman rm --depend" { + run_podman create $IMAGE + dependCid=$output + run_podman create --net=container:$dependCid $IMAGE + cid=$output + run_podman 125 rm $dependCid + is "$output" "Error: container $dependCid has dependent containers which must be removed before it:.*" "Fail to remove because of dependencies" + run_podman rm --depend $dependCid + is "$output" ".*$cid" "Container should have been removed" + is "$output" ".*$dependCid" "Depend container should have been removed" +} + # I'm sorry! This test takes 13 seconds. There's not much I can do about it, # please know that I think it's justified: podman 1.5.0 had a strange bug # in with exit status was not preserved on some code paths with 'rm -f'