bugfix: should initialize ContainerIO for all container

For now, when pouchd restarts, the restore function only re-connects to
existing running/pause containers with initializing container IO.
However, the stopped container also needs the container IO if the user
want to restart the container. For the case, we need to initialize
container IO for all existing container, not just for running/pause
containers.

Signed-off-by: Wei Fu <[email protected]>

daemon/mgr/container.go

@@ -256,19 +256,19 @@ func (mgr *ContainerManager) Restore(ctx context.Context) error {
 	containers, err := mgr.List(ctx,
 		&ContainerListOption{
 			All: true,
-			FilterFunc: func(c *Container) bool {
-				return (c.IsRunning() || c.IsPaused())
-			}})
+		},
+	)
 	if err != nil {
-		logrus.Errorf("failed to get container list when restore alive containers: %v", err)
+		logrus.Errorf("failed to get container list when restore containers: %v", err)
 		return errors.Wrap(err, "failed to get container list")
 	}
 
-	// start recover all alive containers
 	for _, c := range containers {
-		logrus.Debugf("Start recover container %s", c.Key())
 		id := c.Key()
-		// recover the running or paused container.
+
+		// NOTE: when pouch is restarting, we need to initialize
+		// container IO for the existing containers just in case that
+		// user tries to restart the stopped containers.
 		cntrio, err := mgr.initContainerIO(c)
 		if err != nil {
 			logrus.Errorf("failed to init container IO %s: %v", id, err)
@@ -280,6 +280,13 @@ func (mgr *ContainerManager) Restore(ctx context.Context) error {
 			return err
 		}
 
+		// recover the running or paused container.
+		if !(c.IsRunning() || c.IsPaused()) {
+			continue
+		}
+
+		logrus.Debugf("Start recover container %s", id)
+
 		// Start recover the container
 		err = mgr.Client.RecoverContainer(ctx, id, cntrio)
 		if err == nil {

test/z_cli_daemon_test.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"strconv"
 	"strings"
 	"syscall"
@@ -14,8 +15,8 @@ import (
 	"github.com/alibaba/pouch/test/command"
 	"github.com/alibaba/pouch/test/daemon"
 	"github.com/alibaba/pouch/test/environment"
-
 	"github.com/alibaba/pouch/test/util"
+
 	"github.com/go-check/check"
 	"github.com/gotestyourself/gotestyourself/icmd"
 )
@@ -470,6 +471,51 @@ func (suite *PouchDaemonSuite) TestDaemonWithMultiRuntimes(c *check.C) {
 	dcfg2.KillDaemon()
 }
 
+// TestRestartStoppedContainerAfterDaemonRestart is used to test the case that
+// when container is stopped and then pouchd restarts, the restore logic should
+// initialize the existing container IO settings even though they are not alive.
+func (suite *PouchDaemonSuite) TestRestartStoppedContainerAfterDaemonRestart(c *check.C) {
+	cfgFile := filepath.Join("/tmp", c.TestName())
+	c.Assert(CreateConfigFile(cfgFile, nil), check.IsNil)
+	defer os.RemoveAll(cfgFile)
+
+	cfg := daemon.NewConfig()
+	cfg.NewArgs("--config-file", cfgFile)
+	c.Assert(cfg.StartDaemon(), check.IsNil)
+
+	defer cfg.KillDaemon()
+
+	var (
+		cname = c.TestName()
+		msg   = "hello"
+	)
+
+	// pull image
+	RunWithSpecifiedDaemon(&cfg, "pull", busyboxImage).Assert(c, icmd.Success)
+
+	// run a container
+	res := RunWithSpecifiedDaemon(&cfg, "run", "--name", cname, busyboxImage, "echo", msg)
+	defer ensureContainerNotExist(&cfg, cname)
+
+	res.Assert(c, icmd.Success)
+	c.Assert(strings.TrimSpace(res.Combined()), check.Equals, msg)
+
+	// wait for it.
+	RunWithSpecifiedDaemon(&cfg, "wait", cname).Assert(c, icmd.Success)
+
+	// kill the daemon and make sure it has been killed
+	cfg.KillDaemon()
+	c.Assert(cfg.IsDaemonUp(), check.Equals, false)
+
+	// restart again
+	c.Assert(cfg.StartDaemon(), check.IsNil)
+
+	// start the container again
+	res = RunWithSpecifiedDaemon(&cfg, "start", "-a", cname)
+	res.Assert(c, icmd.Success)
+	c.Assert(strings.TrimSpace(res.Combined()), check.Equals, msg)
+}
+
 // TestUpdateDaemonWithLabels tests update daemon online with labels updated
 func (suite *PouchDaemonSuite) TestUpdateDaemonWithLabels(c *check.C) {
 	cfg := daemon.NewConfig()