summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorOpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>2021-01-20 17:58:11 -0500
committerGitHub <noreply@github.com>2021-01-20 17:58:11 -0500
commit14443ccdfc19a7d719c63937ce76bfddf01f88e2 (patch)
tree3b3270237f9d3837668733e7364feb029037d437
parentfe4f9ba303affde0b838e19c862b45206ae2feed (diff)
parente7df73efadd36e2489954ee9a766a3fbfe4eafeb (diff)
downloadpodman-14443ccdfc19a7d719c63937ce76bfddf01f88e2.tar.gz
podman-14443ccdfc19a7d719c63937ce76bfddf01f88e2.tar.bz2
podman-14443ccdfc19a7d719c63937ce76bfddf01f88e2.zip
Merge pull request #9014 from rhatdan/rm
Fix handling of container remove
-rw-r--r--cmd/podman/containers/rm.go15
-rw-r--r--pkg/api/handlers/compat/containers.go45
-rw-r--r--pkg/bindings/containers/containers.go12
-rw-r--r--pkg/bindings/containers/types.go2
-rw-r--r--pkg/bindings/containers/types_remove_options.go32
-rw-r--r--pkg/domain/entities/containers.go11
-rw-r--r--pkg/domain/infra/abi/containers.go30
-rw-r--r--pkg/domain/infra/tunnel/containers.go8
-rw-r--r--test/e2e/rm_test.go34
-rw-r--r--test/system/040-ps.bats7
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