pkg/containerutil: update restart.StatusLabel to Stopped when stopping container - #5156
pkg/containerutil: update restart.StatusLabel to Stopped when stopping container#5156VedantMadane wants to merge 3 commits into
Conversation
…g container (containerd#5153) When a container with a restart policy (such as --restart=always) is explicitly stopped or killed via nerdctl stop / nerdctl kill, update the containerd.io/restart.status label to containerd.Stopped so that containerd's restart monitor does not automatically restart the explicitly stopped container. Signed-off-by: Vedant Madane <6527493+VedantMadane@users.noreply.github.com>
|
Can we have an integration test? |
…ll (containerd#5153) Add TestRunRestartAlwaysStop and TestRunRestartAlwaysKill to verify that containers created with --restart=always transition containerd.io/restart.status to stopped and remain exited after being stopped or killed. Signed-off-by: Vedant Madane <6527493+VedantMadane@users.noreply.github.com>
|
Added integration tests in \cmd/nerdctl/container/container_run_restart_linux_test.go\ (\TestRunRestartAlwaysStop\ and \TestRunRestartAlwaysKill) to verify that containers created with --restart=always\ transition \containerd.io/restart.status\ to \stopped\ and remain in \exited\ status when explicitly stopped or killed. |
|
but if containerd daemon is restarted we should let containerd auto restart the container even the container is stoped by nerdctl when restart policy is always. |
| if err := containerutil.UpdateExplicitlyStoppedLabel(ctx, container, true); err != nil { | ||
| return err | ||
| } | ||
| if l, err := container.Labels(ctx); err == nil { |
There was a problem hiding this comment.
if restart policy is always we should skip to set it because if containerd daemon is restarted we should let containerd start the container.
|
|
||
| func killContainer(ctx context.Context, container containerd.Container, signal syscall.Signal) (err error) { | ||
| defer func() { | ||
| if err != nil { |
There was a problem hiding this comment.
if failed to kill it we should restore the label in defer.
|
UpdateExplicitlyStoppedLabel and UpdateStatusLabel I think we can combine them to reduce once container.Update logic. |
On containerd v1.7, inspect can report State.Status as empty once the task is gone even though Running is false and restart.status is stopped. Assert not-running / not-restarting plus the restart status label instead of requiring Status == exited. Signed-off-by: Vedant Madane <6527493+VedantMadane@users.noreply.github.com>
3 participants
Description
Fixes #5153.
When running a container with --restart=always\ or --restart=unless-stopped, containerd's restart monitor daemon uses the \containerd.io/restart.status\ label to determine whether an exited container should be automatically restarted (when the label value is
unning).
Previously, while \containerutil.Start\ updated the
estart.StatusLabel\ to \containerd.Running\ on startup, \containerutil.Stop\ and \pkg/cmd/container.killContainer\ only updated \containerd.io/restart.explicitly-stopped\ without transitioning \containerd.io/restart.status\ to \containerd.Stopped. This caused containerd's restart monitor to still observe the status as
unning\ and restart the container after an explicit
erdctl stop\ or
erdctl kill.
This PR ensures \containerutil.Stop\ and \killContainer\ update
estart.StatusLabel\ to \containerd.Stopped\ when a container with a restart policy is stopped or killed, aligning with Docker and containerd restart behavior.
Checklist