From 45a10c812e53a3b129815a97a975771565873690 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/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 + libpod/runtime_ctr.go | 10 ++++++ test/system/030-run.bats | 3 ++ test/system/200-pod.bats | 5 ++- 7 files changed, 47 insertions(+), 12 deletions(-) diff --git a/cmd/podman/pods/create.go b/cmd/podman/pods/create.go index 19daa3f8629c..9e4fb8500ab8 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 0aa64481dd10..041d411521b8 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 376dc5066170..1ced2862b842 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 abfa97f5bf08..093966b368b3 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/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index bb30078cb55c..7d286a4f87dd 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 3440508cd23a..5969ac2c5a16 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 8ece6e476fe5..464dff17a192 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 }