Skip to content

Commit

Permalink
podman ps: fix racy pod name query
Browse files Browse the repository at this point in the history
The pod name was queried without holding the container lock, thus it was
possible that the pod was deleted in the meantime and podman just failed
with "no such pod" as the errors.Is() check matched the wrong error.

Move it into the locked code this should prevent anyone from removing
the pod while the container is part of it. Also fix the returned error,
there is no reason to special case one specific error just wrap any error
here so callers at least know where it happened. However this is not
good enough because the batch doesn't update the state which means it
see everything before the container was locked. In this case it might be
possible the ctr and pod was already removed so let the caller skip both
ctr and pod removed errors.

Fixes containers#23282

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
  • Loading branch information
Luap99 authored and edsantiago committed Jul 18, 2024
1 parent 600483e commit ebaf9eb
Showing 1 changed file with 12 additions and 15 deletions.
27 changes: 12 additions & 15 deletions pkg/ps/ps.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ func GetContainerLists(runtime *libpod.Runtime, options entities.ContainerListOp
for _, con := range cons {
listCon, err := ListContainerBatch(runtime, con, options)
switch {
case errors.Is(err, define.ErrNoSuchCtr):
// ignore both no ctr and no such pod errors as it means the ctr is gone now
case errors.Is(err, define.ErrNoSuchCtr), errors.Is(err, define.ErrNoSuchPod):
continue
case err != nil:
return nil, err
Expand Down Expand Up @@ -148,6 +149,7 @@ func ListContainerBatch(rt *libpod.Runtime, ctr *libpod.Container, opts entities
networks []string
healthStatus string
restartCount uint
podName string
)

batchErr := ctr.Batch(func(c *libpod.Container) error {
Expand Down Expand Up @@ -201,10 +203,6 @@ func ListContainerBatch(rt *libpod.Runtime, ctr *libpod.Container, opts entities
return err
}

if !opts.Size && !opts.Namespace {
return nil
}

if opts.Namespace {
ctrPID := strconv.Itoa(pid)
cgroup, _ = getNamespaceInfo(filepath.Join("/proc", ctrPID, "ns", "cgroup"))
Expand All @@ -231,6 +229,14 @@ func ListContainerBatch(rt *libpod.Runtime, ctr *libpod.Container, opts entities
size.RootFsSize = rootFsSize
size.RwSize = rwSize
}

if opts.Pod && len(conConfig.Pod) > 0 {
podName, err = rt.GetPodName(conConfig.Pod)
if err != nil {
return fmt.Errorf("could not find container %s pod (id %s) in state: %w", conConfig.ID, conConfig.Pod, err)
}
}

return nil
})
if batchErr != nil {
Expand All @@ -256,23 +262,14 @@ func ListContainerBatch(rt *libpod.Runtime, ctr *libpod.Container, opts entities
Networks: networks,
Pid: pid,
Pod: conConfig.Pod,
PodName: podName,
Ports: portMappings,
Restarts: restartCount,
Size: size,
StartedAt: startedTime.Unix(),
State: conState.String(),
Status: healthStatus,
}
if opts.Pod && len(conConfig.Pod) > 0 {
podName, err := rt.GetPodName(conConfig.Pod)
if err != nil {
if errors.Is(err, define.ErrNoSuchCtr) {
return entities.ListContainer{}, fmt.Errorf("could not find container %s pod (id %s) in state: %w", conConfig.ID, conConfig.Pod, define.ErrNoSuchPod)
}
return entities.ListContainer{}, err
}
ps.PodName = podName
}

if opts.Namespace {
ps.Namespaces = entities.ListContainerNamespaces{
Expand Down

0 comments on commit ebaf9eb

Please sign in to comment.