Merge pull request #46869 from vvoland/liverestore-fix-46308-24

[24.0 backport] liverestore: Don't remove `--rm` containers on restart

daemon/daemon.go

@@ -472,10 +472,13 @@ func (daemon *Daemon) restore() error {
 				restartContainers[c] = make(chan struct{})
 				mapLock.Unlock()
 			} else if c.HostConfig != nil && c.HostConfig.AutoRemove {
+				// Remove the container if live-restore is disabled or if the container has already exited.
+				if !daemon.configStore.LiveRestoreEnabled || !alive {
 					mapLock.Lock()
 					removeContainers[c.ID] = c
 					mapLock.Unlock()
 				}
+			}
 
 			c.Lock()
 			if c.RemovalInProgress {

integration/daemon/daemon_test.go

@@ -21,6 +21,7 @@ import (
 	"github.com/docker/docker/daemon/config"
 	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/integration/internal/container"
+	"github.com/docker/docker/integration/internal/process"
 	"github.com/docker/docker/pkg/stdcopy"
 	"github.com/docker/docker/testutil/daemon"
 	"gotest.tools/v3/assert"
@@ -370,6 +371,73 @@ func TestLiveRestore(t *testing.T) {
 	skip.If(t, runtime.GOOS == "windows", "cannot start multiple daemons on windows")
 
 	t.Run("volume references", testLiveRestoreVolumeReferences)
+	t.Run("autoremove", testLiveRestoreAutoRemove)
+}
+
+func testLiveRestoreAutoRemove(t *testing.T) {
+	skip.If(t, testEnv.IsRootless(), "restarted rootless daemon will have a new process namespace")
+
+	t.Parallel()
+	ctx := context.Background()
+
+	run := func(t *testing.T) (*daemon.Daemon, func(), string) {
+		d := daemon.New(t)
+		d.StartWithBusybox(t, "--live-restore", "--iptables=false")
+		t.Cleanup(func() {
+			d.Stop(t)
+			d.Cleanup(t)
+		})
+
+		tmpDir := t.TempDir()
+
+		apiClient := d.NewClientT(t)
+
+		cID := container.Run(ctx, t, apiClient,
+			container.WithBind(tmpDir, "/v"),
+			// Run until a 'stop' file is created.
+			container.WithCmd("sh", "-c", "while [ ! -f /v/stop ]; do sleep 0.1; done"),
+			container.WithAutoRemove)
+		t.Cleanup(func() { apiClient.ContainerRemove(ctx, cID, types.ContainerRemoveOptions{Force: true}) })
+		finishContainer := func() {
+			file, err := os.Create(filepath.Join(tmpDir, "stop"))
+			assert.NilError(t, err, "Failed to create 'stop' file")
+			file.Close()
+		}
+		return d, finishContainer, cID
+	}
+
+	t.Run("engine restart shouldnt kill alive containers", func(t *testing.T) {
+		d, finishContainer, cID := run(t)
+
+		d.Restart(t, "--live-restore", "--iptables=false")
+
+		apiClient := d.NewClientT(t)
+		_, err := apiClient.ContainerInspect(ctx, cID)
+		assert.NilError(t, err, "Container shouldn't be removed after engine restart")
+
+		finishContainer()
+
+		poll.WaitOn(t, container.IsRemoved(ctx, apiClient, cID))
+	})
+	t.Run("engine restart should remove containers that exited", func(t *testing.T) {
+		d, finishContainer, cID := run(t)
+
+		apiClient := d.NewClientT(t)
+
+		// Get PID of the container process.
+		inspect, err := apiClient.ContainerInspect(ctx, cID)
+		assert.NilError(t, err)
+		pid := inspect.State.Pid
+
+		d.Stop(t)
+
+		finishContainer()
+		poll.WaitOn(t, process.NotAlive(pid))
+
+		d.Start(t, "--live-restore", "--iptables=false")
+
+		poll.WaitOn(t, container.IsRemoved(ctx, apiClient, cID))
+	})
 }
 
 func testLiveRestoreVolumeReferences(t *testing.T) {

integration/internal/process/wait.go

@@ -0,0 +1,17 @@
+package process
+
+import (
+	procpkg "github.com/docker/docker/pkg/process"
+	"gotest.tools/v3/poll"
+)
+
+// NotAlive verifies the process doesn't exist (finished or never started).
+func NotAlive(pid int) func(log poll.LogT) poll.Result {
+	return func(log poll.LogT) poll.Result {
+		if !procpkg.Alive(pid) {
+			return poll.Success()
+		}
+
+		return poll.Continue("waiting for process to finish")
+	}
+}