diff --git a/cmd/podman/containers/create.go b/cmd/podman/containers/create.go index f9e37178e9..2bd6733d8e 100644 --- a/cmd/podman/containers/create.go +++ b/cmd/podman/containers/create.go @@ -169,7 +169,7 @@ func create(cmd *cobra.Command, args []string) error { } if cliVals.CIDFile != "" { - if err := util.CreateCidFile(cliVals.CIDFile, report.Id); err != nil { + if err := util.CreateIDFile(cliVals.CIDFile, report.Id); err != nil { return err } } diff --git a/cmd/podman/pods/create.go b/cmd/podman/pods/create.go index fc2e078948..19daa3f862 100644 --- a/cmd/podman/pods/create.go +++ b/cmd/podman/pods/create.go @@ -19,7 +19,6 @@ import ( "github.com/containers/podman/v4/cmd/podman/utils" "github.com/containers/podman/v4/libpod/define" "github.com/containers/podman/v4/pkg/domain/entities" - "github.com/containers/podman/v4/pkg/errorhandling" "github.com/containers/podman/v4/pkg/specgen" "github.com/containers/podman/v4/pkg/specgenutil" "github.com/containers/podman/v4/pkg/util" @@ -104,7 +103,6 @@ func init() { func create(cmd *cobra.Command, args []string) error { var ( err error - podIDFD *os.File imageName string rawImageName string podName string @@ -188,18 +186,6 @@ func create(cmd *cobra.Command, args []string) error { createOptions.Name = podName } - if cmd.Flag("pod-id-file").Changed { - podIDFD, err = util.OpenExclusiveFile(podIDFile) - if err != nil && os.IsExist(err) { - return fmt.Errorf("pod id file exists. Ensure another pod is not using it or delete %s", podIDFile) - } - if err != nil { - return fmt.Errorf("opening pod-id-file %s", podIDFile) - } - defer errorhandling.CloseQuiet(podIDFD) - defer errorhandling.SyncQuiet(podIDFD) - } - if len(createOptions.Net.PublishPorts) > 0 { if !createOptions.Infra { return fmt.Errorf("you must have an infra container to publish port bindings to the host") @@ -299,7 +285,7 @@ func create(cmd *cobra.Command, args []string) error { } if len(podIDFile) > 0 { - if err = os.WriteFile(podIDFile, []byte(response.Id), 0644); err != nil { + if err := util.CreateIDFile(podIDFile, response.Id); err != nil { return fmt.Errorf("failed to write pod ID to file: %w", err) } } diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go index 4845749511..493b07c3f7 100644 --- a/pkg/domain/infra/abi/containers.go +++ b/pkg/domain/infra/abi/containers.go @@ -1120,7 +1120,7 @@ func (ic *ContainerEngine) ContainerRun(ctx context.Context, opts entities.Conta } if opts.CIDFile != "" { - if err := util.CreateCidFile(opts.CIDFile, ctr.ID()); err != nil { + if err := util.CreateIDFile(opts.CIDFile, ctr.ID()); err != nil { // If you fail to create CIDFile then remove the container _ = removeContainer(ctr, true) return nil, err diff --git a/pkg/domain/infra/tunnel/containers.go b/pkg/domain/infra/tunnel/containers.go index 0b573686fb..a49048ce6c 100644 --- a/pkg/domain/infra/tunnel/containers.go +++ b/pkg/domain/infra/tunnel/containers.go @@ -804,7 +804,7 @@ func (ic *ContainerEngine) ContainerRun(ctx context.Context, opts entities.Conta } if opts.CIDFile != "" { - if err := util.CreateCidFile(opts.CIDFile, con.ID); err != nil { + if err := util.CreateIDFile(opts.CIDFile, con.ID); err != nil { // If you fail to create CIDFile then remove the container _ = removeContainer(con.ID, true) return nil, err diff --git a/pkg/systemd/generate/containers.go b/pkg/systemd/generate/containers.go index ae2dac80c4..deb8ade924 100644 --- a/pkg/systemd/generate/containers.go +++ b/pkg/systemd/generate/containers.go @@ -42,10 +42,10 @@ type containerInfo struct { ExtraEnvs []string EnvVariable string AdditionalEnvVariables []string - ExecStartPre string ExecStart string TimeoutStartSec uint TimeoutStopSec uint + ExecStartPre string ExecStop string ExecStopPost string GenerateNoHeader bool @@ -94,9 +94,6 @@ StartLimitBurst={{{{.StartLimitBurst}}}} TimeoutStartSec={{{{.TimeoutStartSec}}}} {{{{- end}}}} TimeoutStopSec={{{{.TimeoutStopSec}}}} -{{{{- if .ExecStartPre}}}} -ExecStartPre={{{{.ExecStartPre}}}} -{{{{- end}}}} ExecStart={{{{.ExecStart}}}} {{{{- if .ExecStop}}}} ExecStop={{{{.ExecStop}}}} @@ -316,7 +313,6 @@ func executeContainerTemplate(info *containerInfo, options entities.GenerateSyst info.NotifyAccess = "all" info.PIDFile = "" info.ContainerIDFile = "%t/%n.ctr-id" - info.ExecStartPre = formatOptionsString("/bin/rm -f {{{{.ContainerIDFile}}}}") info.ExecStop = formatOptionsString("{{{{.Executable}}}} stop --ignore {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}} --cidfile={{{{.ContainerIDFile}}}}") info.ExecStopPost = formatOptionsString("{{{{.Executable}}}} rm -f --ignore {{{{if (ge .StopTimeout 0)}}}}-t {{{{.StopTimeout}}}}{{{{end}}}} --cidfile={{{{.ContainerIDFile}}}}") // The create command must at least have three arguments: diff --git a/pkg/systemd/generate/containers_test.go b/pkg/systemd/generate/containers_test.go index 28d9b22b10..e4e12d8129 100644 --- a/pkg/systemd/generate/containers_test.go +++ b/pkg/systemd/generate/containers_test.go @@ -282,8 +282,6 @@ RequiresMountsFor=/var/run/containers/storage Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=70 -ExecStartPre=/bin/rm \ - -f %t/%n.ctr-id ExecStart=/usr/bin/podman container run \ --cidfile=%t/%n.ctr-id \ --cgroups=no-conmon \ @@ -321,8 +319,6 @@ RequiresMountsFor=/var/run/containers/storage Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=70 -ExecStartPre=/bin/rm \ - -f %t/%n.ctr-id ExecStart=/usr/bin/podman container run \ --cidfile=%t/%n.ctr-id \ --cgroups=no-conmon \ @@ -360,8 +356,6 @@ RequiresMountsFor=/var/run/containers/storage Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=70 -ExecStartPre=/bin/rm \ - -f %t/%n.ctr-id ExecStart=/usr/bin/podman container run \ --cidfile=%t/%n.ctr-id \ --cgroups=no-conmon \ @@ -399,8 +393,6 @@ RequiresMountsFor=/var/run/containers/storage Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=70 -ExecStartPre=/bin/rm \ - -f %t/%n.ctr-id ExecStart=/usr/bin/podman run \ --cidfile=%t/%n.ctr-id \ --cgroups=no-conmon \ @@ -438,8 +430,6 @@ RequiresMountsFor=/var/run/containers/storage Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=70 -ExecStartPre=/bin/rm \ - -f %t/%n.ctr-id ExecStart=/usr/bin/podman run \ --cidfile=%t/%n.ctr-id \ --cgroups=no-conmon \ @@ -478,8 +468,6 @@ RequiresMountsFor=/var/run/containers/storage Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=70 -ExecStartPre=/bin/rm \ - -f %t/%n.ctr-id ExecStart=/usr/bin/podman run \ --cidfile=%t/%n.ctr-id \ --cgroups=no-conmon \ @@ -517,8 +505,6 @@ RequiresMountsFor=/var/run/containers/storage Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=70 -ExecStartPre=/bin/rm \ - -f %t/%n.ctr-id ExecStart=/usr/bin/podman run \ --cidfile=%t/%n.ctr-id \ --cgroups=no-conmon \ @@ -554,8 +540,6 @@ RequiresMountsFor=/var/run/containers/storage Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=102 -ExecStartPre=/bin/rm \ - -f %t/%n.ctr-id ExecStart=/usr/bin/podman run \ --cidfile=%t/%n.ctr-id \ --cgroups=no-conmon \ @@ -594,8 +578,6 @@ RequiresMountsFor=/var/run/containers/storage Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=102 -ExecStartPre=/bin/rm \ - -f %t/%n.ctr-id ExecStart=/usr/bin/podman run \ --cidfile=%t/%n.ctr-id \ --cgroups=no-conmon \ @@ -634,8 +616,6 @@ RequiresMountsFor=/var/run/containers/storage Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=102 -ExecStartPre=/bin/rm \ - -f %t/%n.ctr-id ExecStart=/usr/bin/podman \ --events-backend none \ --runroot /root run \ @@ -672,8 +652,6 @@ RequiresMountsFor=/var/run/containers/storage Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=70 -ExecStartPre=/bin/rm \ - -f %t/%n.ctr-id ExecStart=/usr/bin/podman container run \ --cidfile=%t/%n.ctr-id \ --cgroups=no-conmon \ @@ -708,8 +686,6 @@ RequiresMountsFor=/var/run/containers/storage Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=70 -ExecStartPre=/bin/rm \ - -f %t/%n.ctr-id ExecStart=/usr/bin/podman run \ --cidfile=%t/%n.ctr-id \ --cgroups=no-conmon \ @@ -748,8 +724,6 @@ RequiresMountsFor=/var/run/containers/storage Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=70 -ExecStartPre=/bin/rm \ - -f %t/%n.ctr-id ExecStart=/usr/bin/podman run \ --cidfile=%t/%n.ctr-id \ --cgroups=no-conmon \ @@ -787,8 +761,6 @@ RequiresMountsFor=/var/run/containers/storage Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=70 -ExecStartPre=/bin/rm \ - -f %t/%n.ctr-id ExecStart=/usr/bin/podman run \ --cidfile=%t/%n.ctr-id \ --cgroups=no-conmon \ @@ -827,8 +799,6 @@ RequiresMountsFor=/var/run/containers/storage Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=70 -ExecStartPre=/bin/rm \ - -f %t/%n.ctr-id ExecStart=/usr/bin/podman run \ --cidfile=%t/%n.ctr-id \ --cgroups=no-conmon \ @@ -870,8 +840,6 @@ Environment=PODMAN_SYSTEMD_UNIT=%n Environment=FOO=abc "BAR=my test" USER=%%a Restart=on-failure TimeoutStopSec=70 -ExecStartPre=/bin/rm \ - -f %t/%n.ctr-id ExecStart=/usr/bin/podman run \ --cidfile=%t/%n.ctr-id \ --cgroups=no-conmon \ @@ -940,8 +908,6 @@ Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure StartLimitBurst=42 TimeoutStopSec=70 -ExecStartPre=/bin/rm \ - -f %t/%n.ctr-id ExecStart=/usr/bin/podman run \ --cidfile=%t/%n.ctr-id \ --cgroups=no-conmon \ @@ -976,8 +942,6 @@ RequiresMountsFor=/var/run/containers/storage Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=70 -ExecStartPre=/bin/rm \ - -f %t/%n.ctr-id ExecStart=/usr/bin/podman run \ --cidfile=%t/%n.ctr-id \ --cgroups=no-conmon \ @@ -1014,8 +978,6 @@ Environment=PODMAN_SYSTEMD_UNIT=%n-%i Restart=on-failure StartLimitBurst=42 TimeoutStopSec=70 -ExecStartPre=/bin/rm \ - -f %t/%n.ctr-id ExecStart=/usr/bin/podman run \ --name=container-foo-%i \ --cidfile=%t/%n.ctr-id \ diff --git a/pkg/systemd/generate/pods.go b/pkg/systemd/generate/pods.go index 588bfb430e..bdc7c34d8f 100644 --- a/pkg/systemd/generate/pods.go +++ b/pkg/systemd/generate/pods.go @@ -61,10 +61,8 @@ type podInfo struct { PodCreateCommand string // EnvVariable is generate.EnvVariable and must not be set. EnvVariable string - // ExecStartPre1 of the unit. - ExecStartPre1 string - // ExecStartPre2 of the unit. - ExecStartPre2 string + // ExecStartPre of the unit. + ExecStartPre string // ExecStart of the unit. ExecStart string // TimeoutStopSec of the unit. @@ -115,11 +113,8 @@ Restart={{{{.RestartPolicy}}}} RestartSec={{{{.RestartSec}}}} {{{{- end}}}} TimeoutStopSec={{{{.TimeoutStopSec}}}} -{{{{- if .ExecStartPre1}}}} -ExecStartPre={{{{.ExecStartPre1}}}} -{{{{- end}}}} -{{{{- if .ExecStartPre2}}}} -ExecStartPre={{{{.ExecStartPre2}}}} +{{{{- if .ExecStartPre}}}} +ExecStartPre={{{{.ExecStartPre}}}} {{{{- end}}}} ExecStart={{{{.ExecStart}}}} ExecStop={{{{.ExecStop}}}} @@ -371,8 +366,7 @@ func executePodTemplate(info *podInfo, options entities.GenerateSystemdOptions) startCommand = append(startCommand, podCreateArgs...) startCommand = escapeSystemdArguments(startCommand) - info.ExecStartPre1 = formatOptionsString("/bin/rm -f {{{{.PIDFile}}}} {{{{.PodIDFile}}}}") - info.ExecStartPre2 = formatOptions(startCommand) + info.ExecStartPre = formatOptions(startCommand) info.ExecStart = formatOptionsString("{{{{.Executable}}}} {{{{if .RootFlags}}}}{{{{ .RootFlags}}}} {{{{end}}}}pod start --pod-id-file {{{{.PodIDFile}}}}") info.ExecStop = formatOptionsString("{{{{.Executable}}}} {{{{if .RootFlags}}}}{{{{ .RootFlags}}}} {{{{end}}}}pod stop --ignore --pod-id-file {{{{.PodIDFile}}}} {{{{if (ge .StopTimeout 0)}}}} -t {{{{.StopTimeout}}}}{{{{end}}}}") info.ExecStopPost = formatOptionsString("{{{{.Executable}}}} {{{{if .RootFlags}}}}{{{{ .RootFlags}}}} {{{{end}}}}pod rm --ignore -f --pod-id-file {{{{.PodIDFile}}}}") diff --git a/pkg/systemd/generate/pods_test.go b/pkg/systemd/generate/pods_test.go index c44ab111e1..3cde6eacb0 100644 --- a/pkg/systemd/generate/pods_test.go +++ b/pkg/systemd/generate/pods_test.go @@ -258,8 +258,6 @@ Before= Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=70 -ExecStartPre=/bin/rm \ - -f %t/pod-123abc.pid %t/pod-123abc.pod-id ExecStartPre=/usr/bin/podman pod create \ --infra-conmon-pidfile %t/pod-123abc.pid \ --pod-id-file %t/pod-123abc.pod-id \ @@ -326,8 +324,6 @@ Before=container-1.service container-2.service Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=70 -ExecStartPre=/bin/rm \ - -f %t/pod-123abc.pid %t/pod-123abc.pod-id ExecStartPre=/usr/bin/podman pod create \ --infra-conmon-pidfile %t/pod-123abc.pid \ --pod-id-file %t/pod-123abc.pod-id \ @@ -367,8 +363,6 @@ Before=container-1.service container-2.service Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=70 -ExecStartPre=/bin/rm \ - -f %t/pod-123abc.pid %t/pod-123abc.pod-id ExecStartPre=/usr/bin/podman \ --events-backend none \ --runroot /root pod create \ @@ -410,8 +404,6 @@ Before=container-1.service container-2.service Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=70 -ExecStartPre=/bin/rm \ - -f %t/pod-123abc.pid %t/pod-123abc.pod-id ExecStartPre=/usr/bin/podman pod create \ --infra-conmon-pidfile %t/pod-123abc.pid \ --pod-id-file %t/pod-123abc.pod-id \ @@ -451,8 +443,6 @@ Before=container-1.service container-2.service Environment=PODMAN_SYSTEMD_UNIT=%n Restart=on-failure TimeoutStopSec=70 -ExecStartPre=/bin/rm \ - -f %t/pod-123abc.pid %t/pod-123abc.pod-id ExecStartPre=/usr/bin/podman pod create \ --infra-conmon-pidfile %t/pod-123abc.pid \ --pod-id-file %t/pod-123abc.pod-id \ diff --git a/pkg/util/utils.go b/pkg/util/utils.go index 87e403986d..c62b9a0189 100644 --- a/pkg/util/utils.go +++ b/pkg/util/utils.go @@ -682,18 +682,15 @@ func DefaultContainerConfig() *config.Config { return containerConfig } -func CreateCidFile(cidfile string, id string) error { - cidFile, err := OpenExclusiveFile(cidfile) +func CreateIDFile(path string, id string) error { + idFile, err := os.Create(path) if err != nil { - if os.IsExist(err) { - return fmt.Errorf("container id file exists. Ensure another container is not using it or delete %s", cidfile) - } - return fmt.Errorf("opening cidfile %s", cidfile) + return fmt.Errorf("creating idfile: %w", err) } - if _, err = cidFile.WriteString(id); err != nil { - logrus.Error(err) + defer idFile.Close() + if _, err = idFile.WriteString(id); err != nil { + return fmt.Errorf("writing idfile: %w", err) } - cidFile.Close() return nil } diff --git a/test/e2e/generate_systemd_test.go b/test/e2e/generate_systemd_test.go index 01c0aefab0..bd3d01f90d 100644 --- a/test/e2e/generate_systemd_test.go +++ b/test/e2e/generate_systemd_test.go @@ -570,8 +570,6 @@ var _ = Describe("Podman generate systemd", func() { Expect(session.OutputToString()).To(ContainSubstring("--pod-id-file %t/pod-foo.pod-id")) Expect(session.OutputToString()).To(ContainSubstring("--exit-policy=stop")) Expect(session.OutputToString()).To(ContainSubstring("--name foo")) - Expect(session.OutputToString()).To(ContainSubstring("ExecStartPre=/bin/rm")) - Expect(session.OutputToString()).To(ContainSubstring("-f %t/pod-foo.pid %t/pod-foo.pod-id")) Expect(session.OutputToString()).To(ContainSubstring("pod stop")) Expect(session.OutputToString()).To(ContainSubstring("--ignore")) Expect(session.OutputToString()).To(ContainSubstring("--pod-id-file %t/pod-foo.pod-id")) diff --git a/test/system/030-run.bats b/test/system/030-run.bats index 8de1625b5e..3440508cd2 100644 --- a/test/system/030-run.bats +++ b/test/system/030-run.bats @@ -192,6 +192,10 @@ echo $rand | 0 | $rand pidfile=${PODMAN_TMPDIR}/pidfile cidfile=${PODMAN_TMPDIR}/cidfile + # Write random content to the cidfile to make sure its content is truncated + # on write. + echo "$(random_string 120)" > $cidfile + cname=$(random_string) run_podman run --name $cname \ --conmon-pidfile=$pidfile \ @@ -218,12 +222,6 @@ echo $rand | 0 | $rand # All OK. Kill container. run_podman rm -f $cid - - # Podman must not overwrite existing cid file. - # (overwriting conmon-pidfile is OK, so don't test that) - run_podman 125 run --cidfile=$cidfile $IMAGE true - is "$output" "Error: container id file exists. .* delete $cidfile" \ - "podman will not overwrite existing cidfile" } @test "podman run docker-archive" {