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 #23282

Signed-off-by: Paul Holzinger <[email protected]>
  • Loading branch information
Luap99 committed Jul 18, 2024
1 parent 22cf2b4 commit e1caf80
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 e1caf80

Please sign in to comment.