summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDaniel J Walsh <dwalsh@redhat.com>2020-10-07 14:47:52 -0400
committerDaniel J Walsh <dwalsh@redhat.com>2020-10-09 06:06:08 -0400
commitfea78d5530f996d008912584761433023491f160 (patch)
tree61fc3f7a53b77d53d7d9e1100b3d1ae8497a3794
parent9ae873e60e149677db66782eaf2b4ed1402e97d2 (diff)
downloadpodman-fea78d5530f996d008912584761433023491f160.tar.gz
podman-fea78d5530f996d008912584761433023491f160.tar.bz2
podman-fea78d5530f996d008912584761433023491f160.zip
This PR allows users to remove external containers directly
Currenly if a user specifies the name or ID of an external storage container, we report an error to them. buildah from scratch working-container-2 podman rm working-container-2 Error: no container with name or ID working-container-2 found: no such container Since the user specified the correct name and the container is in storage we force them to specify --storage to remove it. This is a bad experience for the user. This change will just remove the container from storage. If the container is known by libpod, it will remove the container from libpod as well. The podman rm --storage option has been deprecated, and removed from docs. Also cleaned documented options that are not available to podman-remote. Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
-rw-r--r--cmd/podman/containers/rm.go13
-rw-r--r--docs/source/markdown/podman-rm.1.md9
-rw-r--r--pkg/domain/entities/containers.go1
-rw-r--r--pkg/domain/infra/abi/containers.go28
-rw-r--r--test/apiv2/20-containers.at2
-rw-r--r--test/e2e/network_test.go2
-rw-r--r--test/e2e/play_kube_test.go2
-rw-r--r--test/e2e/rm_test.go1
-rw-r--r--test/e2e/run_test.go2
-rw-r--r--test/system/055-rm.bats15
10 files changed, 40 insertions, 35 deletions
diff --git a/cmd/podman/containers/rm.go b/cmd/podman/containers/rm.go
index f8f12234d..a7739b3ba 100644
--- a/cmd/podman/containers/rm.go
+++ b/cmd/podman/containers/rm.go
@@ -57,13 +57,12 @@ func rmFlags(flags *pflag.FlagSet) {
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.BoolVar(&rmOptions.Storage, "storage", false, "Remove container from storage library")
flags.BoolVarP(&rmOptions.Volumes, "volumes", "v", false, "Remove anonymous volumes associated with the container")
flags.StringArrayVarP(&rmOptions.CIDFiles, "cidfile", "", nil, "Read the container ID from the file")
- if registry.IsRemote() {
- _ = flags.MarkHidden("ignore")
- _ = flags.MarkHidden("cidfile")
+ if !registry.IsRemote() {
+ // This option is deprecated, but needs to still exists for backwards compatibility
+ flags.Bool("storage", false, "Remove container from storage library")
_ = flags.MarkHidden("storage")
}
}
@@ -97,12 +96,6 @@ func removeContainers(namesOrIDs []string, rmOptions entities.RmOptions, setExit
var (
errs utils.OutputErrors
)
- // Storage conflicts with --all/--latest/--volumes/--cidfile/--ignore
- if rmOptions.Storage {
- if rmOptions.All || rmOptions.Ignore || rmOptions.Latest || rmOptions.Volumes || rmOptions.CIDFiles != nil {
- return errors.Errorf("--storage conflicts with --volumes, --all, --latest, --ignore and --cidfile")
- }
- }
responses, err := registry.ContainerEngine().ContainerRm(context.Background(), namesOrIDs, rmOptions)
if err != nil {
if setExit {
diff --git a/docs/source/markdown/podman-rm.1.md b/docs/source/markdown/podman-rm.1.md
index e3e6740df..36904a128 100644
--- a/docs/source/markdown/podman-rm.1.md
+++ b/docs/source/markdown/podman-rm.1.md
@@ -43,13 +43,6 @@ to run containers such as CRI-O, the last started container could be from either
The latest option is not supported on the remote client.
-**--storage**
-
-Remove external containers from the storage library.
-This is only possible with containers that are not present in libpod can be seen by **podman ps --all --storage**).
-It is used to remove external containers from **podman build** and **buildah**, and orphan containers which were only partially removed by **podman rm**.
-The storage option conflicts with the **--all**, **--latest**, and **--volumes** options.
-
**--volumes**, **-v**
Remove anonymous volumes associated with the container. This does not include named volumes
@@ -96,7 +89,7 @@ $ podman rm -f --latest
**125** The command fails for any other reason
## SEE ALSO
-podman(1), podman-image-rm(1), podman-ps(1), podman-build(1)
+podman(1), podman-image-rm(1), podman-ps(1), podman-build(1), buildah(1), cri-o(1)
## HISTORY
August 2017, Originally compiled by Ryan Cole <rycole@redhat.com>
diff --git a/pkg/domain/entities/containers.go b/pkg/domain/entities/containers.go
index 7b272f01e..3b6dd106f 100644
--- a/pkg/domain/entities/containers.go
+++ b/pkg/domain/entities/containers.go
@@ -131,7 +131,6 @@ type RmOptions struct {
Force bool
Ignore bool
Latest bool
- Storage bool
Volumes bool
}
diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go
index d92911e0c..71b2fa8e2 100644
--- a/pkg/domain/infra/abi/containers.go
+++ b/pkg/domain/infra/abi/containers.go
@@ -273,16 +273,6 @@ func (ic *ContainerEngine) ContainerRestart(ctx context.Context, namesOrIds []st
func (ic *ContainerEngine) ContainerRm(ctx context.Context, namesOrIds []string, options entities.RmOptions) ([]*entities.RmReport, error) {
reports := []*entities.RmReport{}
- if options.Storage {
- for _, ctr := range namesOrIds {
- report := entities.RmReport{Id: ctr}
- if err := ic.Libpod.RemoveStorageContainer(ctr, options.Force); err != nil {
- report.Err = err
- }
- reports = append(reports, &report)
- }
- return reports, nil
- }
names := namesOrIds
for _, cidFile := range options.CIDFiles {
@@ -294,6 +284,22 @@ func (ic *ContainerEngine) ContainerRm(ctx context.Context, namesOrIds []string,
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 {
+ // remove container names that we successfully deleted
+ tmpNames = append(tmpNames, ctr)
+ } else {
+ reports = append(reports, &report)
+ }
+ }
+ if len(tmpNames) < len(names) {
+ names = tmpNames
+ }
+
ctrs, err := getContainersByContext(options.All, options.Latest, names, ic.Libpod)
if err != nil && !(options.Ignore && errors.Cause(err) == define.ErrNoSuchCtr) {
// Failed to get containers. If force is specified, get the containers ID
@@ -302,7 +308,7 @@ func (ic *ContainerEngine) ContainerRm(ctx context.Context, namesOrIds []string,
return nil, err
}
- for _, ctr := range namesOrIds {
+ for _, ctr := range names {
logrus.Debugf("Evicting container %q", ctr)
report := entities.RmReport{Id: ctr}
id, err := ic.Libpod.EvictContainer(ctx, ctr, options.Volumes)
diff --git a/test/apiv2/20-containers.at b/test/apiv2/20-containers.at
index 28289955a..d7e5bfee8 100644
--- a/test/apiv2/20-containers.at
+++ b/test/apiv2/20-containers.at
@@ -206,7 +206,7 @@ t POST containers/${cid_top}/stop "" 204
t DELETE containers/$cid 204
t DELETE containers/$cid_top 204
-# test the apiv2 create, should't ignore the ENV and WORKDIR from the image
+# test the apiv2 create, shouldn't ignore the ENV and WORKDIR from the image
t POST containers/create '"Image":"'$ENV_WORKDIR_IMG'","Env":["testKey1"]' 201 \
.Id~[0-9a-f]\\{64\\}
cid=$(jq -r '.Id' <<<"$output")
diff --git a/test/e2e/network_test.go b/test/e2e/network_test.go
index a15359ea3..cbfd72da6 100644
--- a/test/e2e/network_test.go
+++ b/test/e2e/network_test.go
@@ -28,7 +28,7 @@ func removeConf(confPath string) {
// generateNetworkConfig generates a cni config with a random name
// it returns the network name and the filepath
func generateNetworkConfig(p *PodmanTestIntegration) (string, string) {
- // generate a random name to preven conflicts with other tests
+ // generate a random name to prevent conflicts with other tests
name := "net" + stringid.GenerateNonCryptoID()
path := filepath.Join(p.CNIConfigDir, fmt.Sprintf("%s.conflist", name))
conf := fmt.Sprintf(`{
diff --git a/test/e2e/play_kube_test.go b/test/e2e/play_kube_test.go
index b7398a58a..3cff8c1fa 100644
--- a/test/e2e/play_kube_test.go
+++ b/test/e2e/play_kube_test.go
@@ -665,7 +665,7 @@ var _ = Describe("Podman generate kube", func() {
It("podman play kube test restartPolicy", func() {
// podName, set, expect
testSli := [][]string{
- {"testPod1", "", "always"}, // Default eqaul to always
+ {"testPod1", "", "always"}, // Default equal to always
{"testPod2", "Always", "always"},
{"testPod3", "OnFailure", "on-failure"},
{"testPod4", "Never", "no"},
diff --git a/test/e2e/rm_test.go b/test/e2e/rm_test.go
index 7eff8c6ed..524c07cc6 100644
--- a/test/e2e/rm_test.go
+++ b/test/e2e/rm_test.go
@@ -236,7 +236,6 @@ var _ = Describe("Podman rm", func() {
})
It("podman rm --ignore bogus container and a running container", func() {
-
session := podmanTest.RunTopContainer("test1")
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go
index cd32e5a77..e6bba9f67 100644
--- a/test/e2e/run_test.go
+++ b/test/e2e/run_test.go
@@ -394,7 +394,7 @@ USER bin`
})
It("podman run sysctl test", func() {
- SkipIfRootless("Network sysctls are not avalable root rootless")
+ SkipIfRootless("Network sysctls are not available root rootless")
session := podmanTest.Podman([]string{"run", "--rm", "--sysctl", "net.core.somaxconn=65535", ALPINE, "sysctl", "net.core.somaxconn"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))
diff --git a/test/system/055-rm.bats b/test/system/055-rm.bats
index c8475c3e9..7176ae4b8 100644
--- a/test/system/055-rm.bats
+++ b/test/system/055-rm.bats
@@ -33,6 +33,21 @@ load helpers
run_podman rm -f $cid
}
+@test "podman rm container from storage" {
+ if is_remote; then
+ skip "only applicable for local podman"
+ fi
+ rand=$(random_string 30)
+ run_podman create --name $rand $IMAGE /bin/true
+
+ # Create a container that podman does not know about
+ run buildah from $IMAGE
+ cid="$output"
+
+ # rm should succeed
+ run_podman rm $rand $cid
+}
+
# 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'