From 3fee351c355788aed0d5f9e069f8b14ed7eb109a Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 3 Nov 2022 13:16:16 +0100 Subject: [PATCH] remove container/pod id file along with container/pod Remove the container/pod ID file along with the container/pod. It's primarily used in the context of systemd and are not useful nor needed once a container/pod has ceased to exist. Fixes: #16387 Signed-off-by: Valentin Rothberg --- cmd/podman/containers/rm.go | 3 ++ cmd/podman/pods/create.go | 3 +- cmd/podman/pods/rm.go | 35 ++++++++++++++----- docs/source/markdown/options/cidfile.write.md | 2 +- docs/source/markdown/podman-pod-rm.1.md.in | 1 + docs/source/markdown/podman-rm.1.md.in | 2 ++ libpod/runtime_ctr.go | 10 ++++++ test/system/030-run.bats | 3 ++ test/system/200-pod.bats | 5 ++- 9 files changed, 52 insertions(+), 12 deletions(-) diff --git a/cmd/podman/containers/rm.go b/cmd/podman/containers/rm.go index da5c24ab8c..41d2093321 100644 --- a/cmd/podman/containers/rm.go +++ b/cmd/podman/containers/rm.go @@ -110,6 +110,9 @@ func rm(cmd *cobra.Command, args []string) error { for _, cidFile := range rmCidFiles { content, err := os.ReadFile(cidFile) if err != nil { + if rmOptions.Ignore && errors.Is(err, os.ErrNotExist) { + continue + } return fmt.Errorf("reading CIDFile: %w", err) } id := strings.Split(string(content), "\n")[0] diff --git a/cmd/podman/pods/create.go b/cmd/podman/pods/create.go index 19daa3f862..9e4fb8500a 100644 --- a/cmd/podman/pods/create.go +++ b/cmd/podman/pods/create.go @@ -301,5 +301,6 @@ func replacePod(name string) error { Force: true, // stop and remove pod Ignore: true, // ignore if pod doesn't exist } - return removePods([]string{name}, rmOptions, false) + errs := removePods([]string{name}, rmOptions, false) + return errs.PrintErrors() } diff --git a/cmd/podman/pods/rm.go b/cmd/podman/pods/rm.go index 0aa64481dd..041d411521 100644 --- a/cmd/podman/pods/rm.go +++ b/cmd/podman/pods/rm.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "os" "strings" "github.com/containers/common/pkg/completion" @@ -72,23 +73,38 @@ func init() { } func rm(cmd *cobra.Command, args []string) error { - ids, err := specgenutil.ReadPodIDFiles(rmOptions.PodIDFiles) - if err != nil { - return err - } - args = append(args, ids...) + var errs utils.OutputErrors + if cmd.Flag("time").Changed { if !rmOptions.Force { return errors.New("--force option must be specified to use the --time option") } rmOptions.Timeout = &stopTimeout } - return removePods(args, rmOptions.PodRmOptions, true) + + errs = append(errs, removePods(args, rmOptions.PodRmOptions, true)...) + + for _, idFile := range rmOptions.PodIDFiles { + id, err := specgenutil.ReadPodIDFile(idFile) + if err != nil { + errs = append(errs, err) + continue + } + rmErrs := removePods([]string{id}, rmOptions.PodRmOptions, true) + errs = append(errs, rmErrs...) + if len(rmErrs) == 0 { + if err := os.Remove(idFile); err != nil { + errs = append(errs, err) + } + } + } + + return errs.PrintErrors() } // removePods removes the specified pods (names or IDs). Allows for sharing // pod-removal logic across commands. -func removePods(namesOrIDs []string, rmOptions entities.PodRmOptions, printIDs bool) error { +func removePods(namesOrIDs []string, rmOptions entities.PodRmOptions, printIDs bool) utils.OutputErrors { var errs utils.OutputErrors responses, err := registry.ContainerEngine().PodRm(context.Background(), namesOrIDs, rmOptions) @@ -97,7 +113,7 @@ func removePods(namesOrIDs []string, rmOptions entities.PodRmOptions, printIDs b return nil } setExitCode(err) - return err + return append(errs, err) } // in the cli, first we print out all the successful attempts @@ -114,8 +130,9 @@ func removePods(namesOrIDs []string, rmOptions entities.PodRmOptions, printIDs b errs = append(errs, r.Err) } } - return errs.PrintErrors() + return errs } + func setExitCode(err error) { if errors.Is(err, define.ErrNoSuchPod) || strings.Contains(err.Error(), define.ErrNoSuchPod.Error()) { registry.SetExitCode(1) diff --git a/docs/source/markdown/options/cidfile.write.md b/docs/source/markdown/options/cidfile.write.md index 376dc50661..1ced2862b8 100644 --- a/docs/source/markdown/options/cidfile.write.md +++ b/docs/source/markdown/options/cidfile.write.md @@ -4,4 +4,4 @@ ####> are applicable to all of those. #### **--cidfile**=*file* -Write the container ID to *file*. +Write the container ID to *file*. The file will be removed along with the container. diff --git a/docs/source/markdown/podman-pod-rm.1.md.in b/docs/source/markdown/podman-pod-rm.1.md.in index abfa97f5bf..093966b368 100644 --- a/docs/source/markdown/podman-pod-rm.1.md.in +++ b/docs/source/markdown/podman-pod-rm.1.md.in @@ -26,6 +26,7 @@ Stop running containers and delete all stopped containers before removal of pod. Instead of providing the pod name or ID, remove the last created pod. (This option is not available with the remote Podman client, including Mac and Windows (excluding WSL2) machines) @@option pod-id-file.pod +If specified, the pod-id-file will be removed along with the pod. @@option time diff --git a/docs/source/markdown/podman-rm.1.md.in b/docs/source/markdown/podman-rm.1.md.in index b3bff4934a..802096b7ab 100644 --- a/docs/source/markdown/podman-rm.1.md.in +++ b/docs/source/markdown/podman-rm.1.md.in @@ -57,6 +57,8 @@ In addition, forcing can be used to remove unusable containers, e.g. containers whose OCI runtime has become unavailable. @@option ignore +Further ignore when the specified `--cidfile` does not exist as it may have +already been removed along with the container. #### **--latest**, **-l** diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index bb30078cb5..7d286a4f87 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -777,6 +777,16 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force, remo } } + // Remove the container's CID file on container removal. + if cidFile, ok := c.config.Spec.Annotations[define.InspectAnnotationCIDFile]; ok { + if err := os.Remove(cidFile); err != nil { + if cleanupErr == nil { + cleanupErr = err + } else { + logrus.Errorf("Cleaning up CID file: %v", err) + } + } + } // Remove the container from the state if c.config.Pod != "" { // If we're removing the pod, the container will be evicted diff --git a/test/system/030-run.bats b/test/system/030-run.bats index 3440508cd2..5969ac2c5a 100644 --- a/test/system/030-run.bats +++ b/test/system/030-run.bats @@ -222,6 +222,9 @@ echo $rand | 0 | $rand # All OK. Kill container. run_podman rm -f $cid + if [[ -e $cidfile ]]; then + die "cidfile $cidfile should be removed along with container" + fi } @test "podman run docker-archive" { diff --git a/test/system/200-pod.bats b/test/system/200-pod.bats index 8ece6e476f..464dff17a1 100644 --- a/test/system/200-pod.bats +++ b/test/system/200-pod.bats @@ -314,7 +314,10 @@ EOF # Clean up run_podman rm $cid - run_podman pod rm -t 0 -f mypod + run_podman pod rm -t 0 -f --pod-id-file $pod_id_file + if [[ -e $pod_id_file ]]; then + die "pod-id-file $pod_id_file should be removed along with pod" + fi run_podman rmi $infra_image }