From 4a44cf1d4c8e540b67aaa3834291a964c6ab7524 Mon Sep 17 00:00:00 2001 From: Anusha Ragunathan Date: Fri, 22 Jul 2016 08:53:26 -0700 Subject: [PATCH] Handle plugin shutdown when liveRestore is set. When daemon has liveRestore set, daemon shutdown should not shutdown plugins. Fixes #24759 Signed-off-by: Anusha Ragunathan --- daemon/daemon.go | 5 +-- .../docker_cli_daemon_experimental_test.go | 36 +++++++++++++++++-- plugin/manager.go | 2 +- plugin/manager_linux.go | 10 +++++- 4 files changed, 47 insertions(+), 6 deletions(-) diff --git a/daemon/daemon.go b/daemon/daemon.go index 264d270e55..84aab2259f 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -651,6 +651,9 @@ func (daemon *Daemon) Shutdown() error { daemon.shutdown = true // Keep mounts and networking running on daemon shutdown if // we are to keep containers running and restore them. + + pluginShutdown() + if daemon.configStore.LiveRestore && daemon.containers != nil { // check if there are any running containers, if none we should do some cleanup if ls, err := daemon.Containers(&types.ContainerListOptions{}); len(ls) != 0 || err != nil { @@ -687,8 +690,6 @@ func (daemon *Daemon) Shutdown() error { } } - pluginShutdown() - if err := daemon.cleanupMounts(); err != nil { return err } diff --git a/integration-cli/docker_cli_daemon_experimental_test.go b/integration-cli/docker_cli_daemon_experimental_test.go index d6cc0f71ad..d57e93a71e 100644 --- a/integration-cli/docker_cli_daemon_experimental_test.go +++ b/integration-cli/docker_cli_daemon_experimental_test.go @@ -74,8 +74,9 @@ func (s *DockerDaemonSuite) TestDaemonRestartWithPluginDisabled(c *check.C) { c.Assert(out, checker.Contains, "false") } -// TestDaemonShutdownLiveRestoreWithPlugins leaves plugin running. -func (s *DockerDaemonSuite) TestDaemonShutdownLiveRestoreWithPlugins(c *check.C) { +// TestDaemonKillLiveRestoreWithPlugins SIGKILLs daemon started with --live-restore. +// Plugins should continue to run. +func (s *DockerDaemonSuite) TestDaemonKillLiveRestoreWithPlugins(c *check.C) { if err := s.d.Start("--live-restore"); err != nil { c.Fatalf("Could not start daemon: %v", err) } @@ -104,6 +105,37 @@ func (s *DockerDaemonSuite) TestDaemonShutdownLiveRestoreWithPlugins(c *check.C) } } +// TestDaemonShutdownLiveRestoreWithPlugins SIGTERMs daemon started with --live-restore. +// Plugins should continue to run. +func (s *DockerDaemonSuite) TestDaemonShutdownLiveRestoreWithPlugins(c *check.C) { + if err := s.d.Start("--live-restore"); err != nil { + c.Fatalf("Could not start daemon: %v", err) + } + if out, err := s.d.Cmd("plugin", "install", "--grant-all-permissions", pluginName); err != nil { + c.Fatalf("Could not install plugin: %v %s", err, out) + } + defer func() { + if err := s.d.Restart("--live-restore"); err != nil { + c.Fatalf("Could not restart daemon: %v", err) + } + if out, err := s.d.Cmd("plugin", "disable", pluginName); err != nil { + c.Fatalf("Could not disable plugin: %v %s", err, out) + } + if out, err := s.d.Cmd("plugin", "remove", pluginName); err != nil { + c.Fatalf("Could not remove plugin: %v %s", err, out) + } + }() + + if err := s.d.cmd.Process.Signal(os.Interrupt); err != nil { + c.Fatalf("Could not kill daemon: %v", err) + } + + cmd := exec.Command("pgrep", "-f", "plugin-no-remove") + if out, ec, err := runCommandWithOutput(cmd); ec != 0 { + c.Fatalf("Expected exit code '0', got %d err: %v output: %s ", ec, err, out) + } +} + // TestDaemonShutdownWithPlugins shuts down running plugins. func (s *DockerDaemonSuite) TestDaemonShutdownWithPlugins(c *check.C) { if err := s.d.Start(); err != nil { diff --git a/plugin/manager.go b/plugin/manager.go index be2f601671..faed7185e7 100644 --- a/plugin/manager.go +++ b/plugin/manager.go @@ -319,7 +319,7 @@ func (pm *Manager) init() error { if requiresManualRestore { // if liveRestore is not enabled, the plugin will be stopped now so we should enable it if err := pm.enable(p); err != nil { - logrus.Errorf("Error restoring plugin '%s': %s", p.Name(), err) + logrus.Errorf("Error enabling plugin '%s': %s", p.Name(), err) } } }(p) diff --git a/plugin/manager_linux.go b/plugin/manager_linux.go index eff6d38ee6..2ce7470405 100644 --- a/plugin/manager_linux.go +++ b/plugin/manager_linux.go @@ -135,12 +135,16 @@ func (pm *Manager) Shutdown() { pm.shutdown = true for _, p := range pm.plugins { + if pm.liveRestore && p.P.Active { + logrus.Debug("Plugin active when liveRestore is set, skipping shutdown") + continue + } if p.restartManager != nil { if err := p.restartManager.Cancel(); err != nil { logrus.Error(err) } } - if pm.containerdClient != nil { + if pm.containerdClient != nil && p.P.Active { p.exitChan = make(chan bool) err := pm.containerdClient.Signal(p.P.ID, int(syscall.SIGTERM)) if err != nil { @@ -157,6 +161,10 @@ func (pm *Manager) Shutdown() { } } close(p.exitChan) + pm.Lock() + p.P.Active = false + pm.save() + pm.Unlock() } if err := os.RemoveAll(p.runtimeSourcePath); err != nil { logrus.Errorf("Remove plugin runtime failed with error: %v", err)