Skip to content

Commit

Permalink
Cleanup CIDFile on podman-remote run --rm command
Browse files Browse the repository at this point in the history
Currently the CIDFile is not removed with podman --remote run --rm
if the client and server are on different machines.

[NO NEW TESTS NEEDED] i
There is currently a test for this that does not fail because the client
and server are on the same machine.

If we run these tests on a MAC or Windows platform, they would start
failing.

Fixes: containers#19420

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
  • Loading branch information
rhatdan committed Aug 1, 2023
1 parent 608f484 commit 851cd9c
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 13 deletions.
3 changes: 2 additions & 1 deletion docs/source/markdown/options/cidfile.write.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@
####> are applicable to all of those.
#### **--cidfile**=*file*

Write the container ID to *file*. The file is removed along with the container.
Write the container ID to *file*. The file is removed along with the container, except
when used with podman --remote run on detached containers.
1 change: 1 addition & 0 deletions docs/source/markdown/podman-ps.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ Valid placeholders for the Go template are listed below:
| **Placeholder** | **Description** |
|--------------------|----------------------------------------------|
| .AutoRemove | If true, containers are removed on exit |
| .CIDFile | Container ID File |
| .Command | Quoted command used |
| .Created | Creation time for container, Y-M-D H:M:S |
| .CreatedAt | Creation time for container (same as above) |
Expand Down
2 changes: 2 additions & 0 deletions pkg/domain/entities/container_ps.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ type ListContainer struct {
Created time.Time
// Human-readable container creation time.
CreatedAt string
// CIDFile specified at creation time.
CIDFile string
// If container has exited/stopped
Exited bool
// Time container exited
Expand Down
29 changes: 19 additions & 10 deletions pkg/domain/infra/tunnel/containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,12 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri
}
}
removeOptions := new(containers.RemoveOptions).WithVolumes(true).WithForce(false)
removeContainer := func(id string) {
removeContainer := func(id, CIDFile string) {
if CIDFile != "" {
if err := os.Remove(CIDFile); err != nil && !errors.Is(err, os.ErrNotExist) {
logrus.Warnf("Cleaning up CID file: %s", err)
}
}
reports, err := containers.Remove(ic.ClientCtx, id, removeOptions)
logIfRmError(id, err, reports)
}
Expand Down Expand Up @@ -722,7 +727,7 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri

if err != nil {
if ctr.AutoRemove {
removeContainer(ctr.ID)
removeContainer(ctr.ID, ctr.CIDFile)
}
report.ExitCode = define.ExitCode(report.Err)
report.Err = err
Expand All @@ -741,7 +746,7 @@ func (ic *ContainerEngine) ContainerStart(ctx context.Context, namesOrIds []stri
logrus.Errorf("Should restart: %v", shouldRestart)

if !shouldRestart && ctr.AutoRemove {
removeContainer(ctr.ID)
removeContainer(ctr.ID, ctr.CIDFile)
}
}()
}
Expand Down Expand Up @@ -827,7 +832,13 @@ func (ic *ContainerEngine) ContainerRun(ctx context.Context, opts entities.Conta
for _, w := range con.Warnings {
fmt.Fprintf(os.Stderr, "%s\n", w)
}
removeContainer := func(id string, force bool) error {
removeContainer := func(id, CIDFile string, force bool) error {
if CIDFile != "" {
if err := os.Remove(CIDFile); err != nil && !errors.Is(err, os.ErrNotExist) {
logrus.Warnf("Cleaning up CID file: %s", err)
}
}

removeOptions := new(containers.RemoveOptions).WithVolumes(true).WithForce(force)
reports, err := containers.Remove(ic.ClientCtx, id, removeOptions)
logIfRmError(id, err, reports)
Expand All @@ -837,7 +848,7 @@ func (ic *ContainerEngine) ContainerRun(ctx context.Context, opts entities.Conta
if opts.CIDFile != "" {
if err := util.CreateIDFile(opts.CIDFile, con.ID); err != nil {
// If you fail to create CIDFile then remove the container
_ = removeContainer(con.ID, true)
_ = removeContainer(con.ID, opts.CIDFile, true)
return nil, err
}
}
Expand All @@ -850,9 +861,7 @@ func (ic *ContainerEngine) ContainerRun(ctx context.Context, opts entities.Conta
if err != nil {
report.ExitCode = define.ExitCode(err)
if opts.Rm {
if rmErr := removeContainer(con.ID, true); rmErr != nil && !errors.Is(rmErr, define.ErrNoSuchCtr) {
logrus.Errorf("Container %s failed to be removed", con.ID)
}
_ = removeContainer(con.ID, opts.CIDFile, true)
}
}
return &report, err
Expand All @@ -873,7 +882,7 @@ func (ic *ContainerEngine) ContainerRun(ctx context.Context, opts entities.Conta

report.ExitCode = define.ExitCode(err)
if opts.Rm {
_ = removeContainer(con.ID, false)
_ = removeContainer(con.ID, opts.CIDFile, false)
}
return &report, err
}
Expand All @@ -889,7 +898,7 @@ func (ic *ContainerEngine) ContainerRun(ctx context.Context, opts entities.Conta
}

if !shouldRestart {
_ = removeContainer(con.ID, false)
_ = removeContainer(con.ID, opts.CIDFile, false)
}
}()
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/ps/ps.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,10 +237,11 @@ func ListContainerBatch(rt *libpod.Runtime, ctr *libpod.Container, opts entities

ps := entities.ListContainer{
AutoRemove: ctr.AutoRemove(),
CIDFile: conConfig.Spec.Annotations[define.InspectAnnotationCIDFile],
Command: conConfig.Command,
Created: conConfig.CreatedTime,
Exited: exited,
ExitCode: exitCode,
Exited: exited,
ExitedAt: exitedTime.Unix(),
ID: conConfig.ID,
Image: conConfig.RootfsImageName,
Expand All @@ -253,11 +254,11 @@ func ListContainerBatch(rt *libpod.Runtime, ctr *libpod.Container, opts entities
Pid: pid,
Pod: conConfig.Pod,
Ports: portMappings,
Restarts: restartCount,
Size: size,
StartedAt: startedTime.Unix(),
State: conState.String(),
Status: healthStatus,
Restarts: restartCount,
}
if opts.Pod && len(conConfig.Pod) > 0 {
podName, err := rt.GetPodName(conConfig.Pod)
Expand Down

0 comments on commit 851cd9c

Please sign in to comment.