diff options
-rw-r--r-- | cmd/podman/containers/rm.go | 15 | ||||
-rw-r--r-- | pkg/api/handlers/compat/containers.go | 45 | ||||
-rw-r--r-- | pkg/bindings/containers/containers.go | 12 | ||||
-rw-r--r-- | pkg/bindings/containers/types.go | 2 | ||||
-rw-r--r-- | pkg/bindings/containers/types_remove_options.go | 32 | ||||
-rw-r--r-- | pkg/domain/entities/containers.go | 11 | ||||
-rw-r--r-- | pkg/domain/infra/abi/containers.go | 30 | ||||
-rw-r--r-- | pkg/domain/infra/tunnel/containers.go | 8 | ||||
-rw-r--r-- | test/e2e/rm_test.go | 34 | ||||
-rw-r--r-- | test/system/040-ps.bats | 7 |
10 files changed, 139 insertions, 57 deletions
diff --git a/cmd/podman/containers/rm.go b/cmd/podman/containers/rm.go index ee9dc4c78..ea616b6e5 100644 --- a/cmd/podman/containers/rm.go +++ b/cmd/podman/containers/rm.go @@ -3,6 +3,7 @@ package containers import ( "context" "fmt" + "io/ioutil" "strings" "github.com/containers/common/pkg/completion" @@ -54,6 +55,7 @@ var ( var ( rmOptions = entities.RmOptions{} + cidFiles = []string{} ) func rmFlags(cmd *cobra.Command) { @@ -65,7 +67,7 @@ func rmFlags(cmd *cobra.Command) { flags.BoolVarP(&rmOptions.Volumes, "volumes", "v", false, "Remove anonymous volumes associated with the container") cidfileFlagName := "cidfile" - flags.StringArrayVarP(&rmOptions.CIDFiles, cidfileFlagName, "", nil, "Read the container ID from the file") + flags.StringArrayVar(&cidFiles, cidfileFlagName, nil, "Read the container ID from the file") _ = cmd.RegisterFlagCompletionFunc(cidfileFlagName, completion.AutocompleteDefault) if !registry.IsRemote() { @@ -92,7 +94,16 @@ func init() { validate.AddLatestFlag(containerRmCommand, &rmOptions.Latest) } -func rm(_ *cobra.Command, args []string) error { +func rm(cmd *cobra.Command, args []string) error { + for _, cidFile := range cidFiles { + content, err := ioutil.ReadFile(string(cidFile)) + if err != nil { + return errors.Wrap(err, "error reading CIDFile") + } + id := strings.Split(string(content), "\n")[0] + args = append(args, id) + } + return removeContainers(args, rmOptions, true) } diff --git a/pkg/api/handlers/compat/containers.go b/pkg/api/handlers/compat/containers.go index 6e1945db1..438fda441 100644 --- a/pkg/api/handlers/compat/containers.go +++ b/pkg/api/handlers/compat/containers.go @@ -14,7 +14,9 @@ import ( "github.com/containers/podman/v2/libpod/define" "github.com/containers/podman/v2/pkg/api/handlers" "github.com/containers/podman/v2/pkg/api/handlers/utils" + "github.com/containers/podman/v2/pkg/domain/entities" "github.com/containers/podman/v2/pkg/domain/filters" + "github.com/containers/podman/v2/pkg/domain/infra/abi" "github.com/containers/podman/v2/pkg/ps" "github.com/containers/podman/v2/pkg/signal" "github.com/docker/docker/api/types" @@ -29,9 +31,11 @@ import ( func RemoveContainer(w http.ResponseWriter, r *http.Request) { decoder := r.Context().Value("decoder").(*schema.Decoder) query := struct { - Force bool `schema:"force"` - Vols bool `schema:"v"` - Link bool `schema:"link"` + All bool `schema:"all"` + Force bool `schema:"force"` + Ignore bool `schema:"ignore"` + Link bool `schema:"link"` + Volumes bool `schema:"v"` }{ // override any golang type defaults } @@ -49,34 +53,31 @@ func RemoveContainer(w http.ResponseWriter, r *http.Request) { } runtime := r.Context().Value("runtime").(*libpod.Runtime) + // Now use the ABI implementation to prevent us from having duplicate + // code. + containerEngine := abi.ContainerEngine{Libpod: runtime} name := utils.GetName(r) - con, err := runtime.LookupContainer(name) - if err != nil && errors.Cause(err) == define.ErrNoSuchCtr { - // Failed to get container. If force is specified, get the container's ID - // and evict it - if !query.Force { + options := entities.RmOptions{ + All: query.All, + Force: query.Force, + Volumes: query.Volumes, + Ignore: query.Ignore, + } + report, err := containerEngine.ContainerRm(r.Context(), []string{name}, options) + if err != nil { + if errors.Cause(err) == define.ErrNoSuchCtr { utils.ContainerNotFound(w, name, err) return } - if _, err := runtime.EvictContainer(r.Context(), name, query.Vols); err != nil { - if errors.Cause(err) == define.ErrNoSuchCtr { - logrus.Debugf("Ignoring error (--allow-missing): %q", err) - w.WriteHeader(http.StatusNoContent) - return - } - logrus.Warn(errors.Wrapf(err, "failed to evict container: %q", name)) - utils.InternalServerError(w, err) - return - } - w.WriteHeader(http.StatusNoContent) + utils.InternalServerError(w, err) return } - - if err := runtime.RemoveContainer(r.Context(), con, query.Force, query.Vols); err != nil { - utils.InternalServerError(w, err) + if report[0].Err != nil { + utils.InternalServerError(w, report[0].Err) return } + utils.WriteResponse(w, http.StatusNoContent, nil) } diff --git a/pkg/bindings/containers/containers.go b/pkg/bindings/containers/containers.go index 73e4d1d3d..40fcfbded 100644 --- a/pkg/bindings/containers/containers.go +++ b/pkg/bindings/containers/containers.go @@ -71,8 +71,10 @@ func Prune(ctx context.Context, options *PruneOptions) ([]*reports.PruneReport, } // Remove removes a container from local storage. The force bool designates -// that the container should be removed forcibly (example, even it is running). The volumes -// bool dictates that a container's volumes should also be removed. +// that the container should be removed forcibly (example, even it is running). +// 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 { if options == nil { options = new(RemoveOptions) @@ -85,9 +87,15 @@ func Remove(ctx context.Context, nameOrID string, options *RemoveOptions) error if v := options.GetVolumes(); options.Changed("Volumes") { params.Set("v", strconv.FormatBool(v)) } + if all := options.GetAll(); options.Changed("All") { + params.Set("all", strconv.FormatBool(all)) + } if force := options.GetForce(); options.Changed("Force") { params.Set("force", strconv.FormatBool(force)) } + if ignore := options.GetIgnore(); options.Changed("Ignore") { + params.Set("ignore", strconv.FormatBool(ignore)) + } response, err := conn.DoRequest(nil, http.MethodDelete, "/containers/%s", params, nil, nameOrID) if err != nil { return err diff --git a/pkg/bindings/containers/types.go b/pkg/bindings/containers/types.go index 43cb58a54..24604fa83 100644 --- a/pkg/bindings/containers/types.go +++ b/pkg/bindings/containers/types.go @@ -122,6 +122,8 @@ type PruneOptions struct { //go:generate go run ../generator/generator.go RemoveOptions // RemoveOptions are optional options for removing containers type RemoveOptions struct { + All *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 e21fb41f7..3ef32fa03 100644 --- a/pkg/bindings/containers/types_remove_options.go +++ b/pkg/bindings/containers/types_remove_options.go @@ -87,6 +87,38 @@ func (o *RemoveOptions) ToParams() (url.Values, error) { return params, nil } +// WithAll +func (o *RemoveOptions) WithAll(value bool) *RemoveOptions { + v := &value + o.All = v + return o +} + +// GetAll +func (o *RemoveOptions) GetAll() bool { + var all bool + if o.All == nil { + return all + } + return *o.All +} + +// WithIgnore +func (o *RemoveOptions) WithIgnore(value bool) *RemoveOptions { + v := &value + o.Ignore = v + return o +} + +// GetIgnore +func (o *RemoveOptions) GetIgnore() bool { + var ignore bool + if o.Ignore == nil { + return ignore + } + return *o.Ignore +} + // WithForce func (o *RemoveOptions) WithForce(value bool) *RemoveOptions { v := &value diff --git a/pkg/domain/entities/containers.go b/pkg/domain/entities/containers.go index d8576c101..4c1bd6a7d 100644 --- a/pkg/domain/entities/containers.go +++ b/pkg/domain/entities/containers.go @@ -128,12 +128,11 @@ type RestartReport struct { } type RmOptions struct { - All bool - CIDFiles []string - Force bool - Ignore bool - Latest bool - Volumes bool + All bool + Force bool + Ignore bool + Latest bool + Volumes bool } type RmReport struct { diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index a8f4d44a8..48a32817d 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -264,30 +264,30 @@ func (ic *ContainerEngine) ContainerRm(ctx context.Context, namesOrIds []string, reports := []*entities.RmReport{} names := namesOrIds - for _, cidFile := range options.CIDFiles { - content, err := ioutil.ReadFile(cidFile) - if err != nil { - return nil, errors.Wrap(err, "error reading CIDFile") - } - id := strings.Split(string(content), "\n")[0] - names = append(names, id) - } - // 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} - if err := ic.Libpod.RemoveStorageContainer(ctr, options.Force); err != nil { + report.Err = ic.Libpod.RemoveStorageContainer(ctr, options.Force) + switch errors.Cause(report.Err) { + case nil: // remove container names that we successfully deleted - tmpNames = append(tmpNames, ctr) - } else { reports = append(reports, &report) + case define.ErrNoSuchCtr: + // 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) + continue + } + // attempt to remove as a libpod container + tmpNames = append(tmpNames, ctr) } } - if len(tmpNames) < len(names) { - names = tmpNames - } + names = tmpNames ctrs, err := getContainersByContext(options.All, options.Latest, names, ic.Libpod) if err != nil && !(options.Ignore && errors.Cause(err) == define.ErrNoSuchCtr) { diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index 84a07f8e9..524b29553 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -173,14 +173,6 @@ func (ic *ContainerEngine) ContainerRestart(ctx context.Context, namesOrIds []st } func (ic *ContainerEngine) ContainerRm(ctx context.Context, namesOrIds []string, opts entities.RmOptions) ([]*entities.RmReport, error) { - for _, cidFile := range opts.CIDFiles { - content, err := ioutil.ReadFile(cidFile) - if err != nil { - return nil, errors.Wrap(err, "error reading CIDFile") - } - id := strings.Split(string(content), "\n")[0] - namesOrIds = append(namesOrIds, id) - } ctrs, err := getContainersByContext(ic.ClientCtx, opts.All, opts.Ignore, namesOrIds) if err != nil { return nil, err diff --git a/test/e2e/rm_test.go b/test/e2e/rm_test.go index 524c07cc6..ca142d7f3 100644 --- a/test/e2e/rm_test.go +++ b/test/e2e/rm_test.go @@ -215,6 +215,40 @@ var _ = Describe("Podman rm", func() { Expect(result.ExitCode()).To(Equal(125)) }) + It("podman rm --all", func() { + session := podmanTest.Podman([]string{"create", ALPINE, "ls"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(podmanTest.NumberOfContainers()).To(Equal(1)) + + session = podmanTest.Podman([]string{"create", ALPINE, "ls"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(podmanTest.NumberOfContainers()).To(Equal(2)) + + session = podmanTest.Podman([]string{"rm", "--all"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(podmanTest.NumberOfContainers()).To(Equal(0)) + }) + + It("podman rm --ignore", func() { + session := podmanTest.Podman([]string{"create", ALPINE, "ls"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + cid := session.OutputToStringArray()[0] + Expect(podmanTest.NumberOfContainers()).To(Equal(1)) + + session = podmanTest.Podman([]string{"rm", "bogus", cid}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(1)) + + session = podmanTest.Podman([]string{"rm", "--ignore", "bogus", cid}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + Expect(podmanTest.NumberOfContainers()).To(Equal(0)) + }) + It("podman rm bogus container", func() { session := podmanTest.Podman([]string{"rm", "bogus"}) session.WaitWithDefaultTimeout() diff --git a/test/system/040-ps.bats b/test/system/040-ps.bats index 0447122b1..0ae8b0ce0 100644 --- a/test/system/040-ps.bats +++ b/test/system/040-ps.bats @@ -111,8 +111,11 @@ EOF run_podman ps --storage -a is "${#lines[@]}" "2" "podman ps -a --storage sees buildah container" - # This is what deletes the container - # FIXME: why doesn't "podman rm --storage $cid" do anything? + # We can't rm it without -f, but podman should issue a helpful message + run_podman 2 rm "$cid" + is "$output" "Error: container .* is mounted and cannot be removed without using force: container state improper" "podman rm <buildah container> without -f" + + # With -f, we can remove it. run_podman rm -f "$cid" run_podman ps --storage -a |