Fix: Prevent OCI runtime directory remain

[sstosh]

This bug was introduced in #8906.

When we use podman rm/restart/stop/kill etc... command to
the container running with --rm, the OCI runtime directory
remains at /run/<runtime name> (root user) or
/run/user/<user id>/<runtime name> (rootless user).

This bug could cause other bugs.
For example, when we checkpoint the container running with
--rm (podman checkpoint --export) and restore it
(podman restore --import) with crun, error message
Error: OCI runtime error: crun: container '<container id>' already exists is outputted.
This error is caused by an attempt to restore the container with
the same container ID as the remaining OCI runtime's container ID.

Therefore, I fix that the cleanupRuntime() function runs to
remove the OCI runtime directory,
even if the container has already been removed by --rm option.

Signed-off-by: Toshiki Sonoda [email protected]

Does this PR introduce a user-facing change?

This fix prevents the OCI runtime directory from remaining. It also prevents other bugs occurring.

[mheon]

I would expect that the cleanup process which removes containers started with --rm would handle removal of directories associated with the OCI runtime; removing a container always calls cleanup() which always calls cleanupRuntime(). Can you elaborate more as to what you're seeing?

[sstosh]

I would expect that the cleanup process which removes containers started with --rm would handle removal of directories associated with the OCI runtime; removing a container always calls cleanup() which always calls cleanupRuntime(). Can you elaborate more as to what you're seeing?

@mheon
Because cleanupRuntime() does nothing when cleanup() is called for ContainerStateStopping containers.
As a result, OCI runtime directory dose not removed.

I think there are two plans to remove the OCI runtime directory with cleanupRuntime() call.

1. Ensure cleanupRuntime() to call even if the container state is ContainerStateStopping (commit: 81adde3).
2. Change the container state from ContainerStateStopping to ContainerStateStopped and call cleanupRuntime() again (commit: 3619f0b).

The 1st plan needs to change the container state from ContainerStateStopping when cleanupRuntime() called.
This causes the warning message output from https://github.com/containers/podman/blob/main/libpod/container_internal.go#L1336
when we remove the not --rm container.

The 2nd plan needs to change the container state to ContainerStateStoped before cleanupRuntime() called.
This plan does not output the warning message.

If we choose the 1st plan, we need to deal with the warning message.
Also test scripts need fix because exit code changes.

I think the 2nd plan is better because it does not output warnings
and does not need to fix test scripts more than the 1st plan.

[mheon]

The logic behind cleanup is correct; we absolutely should not attempt to clean up Stopping containers, as they are still running. The problem is that, somehow, the state transition from Stopping to Stopped is not detected (from what you've said, seemingly only in the --rm case - so most likely a part of the RemoveContainer()) function). The question would be how a container in the Stopping state got through this to the cleanupContainer() bits below. I can see two possibilities:
1, the stop() function for the container is returning early (it should pick up the state transition automatically if it runs to the end of the function).
2. The container is already in Stopping state when RemoveContainer() is called. The code block that calls stop() never occurs because of this.

Of these, 2 presently seems more likely. It seems that https://github.com/containers/podman/blob/main/libpod/runtime_ctr.go#L701 should also trigger for ContainerStateStopping (possibly with an extra check to see if the container is already stopped before calling c.stop()).

[giuseppe]

LGTM

saw the notes from Matt

[rhatdan]

/approve
/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan, sstosh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment