From 521807837ba60247005f78f0000cfa1b30585c21 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 2 May 2022 00:52:16 +0200 Subject: [PATCH] plugin: Executor.Signal() accept syscall.Signal This helps reducing some type-juggling / conversions further up the stack. Signed-off-by: Sebastiaan van Stijn --- plugin/executor/containerd/containerd.go | 4 +-- plugin/manager.go | 3 +- plugin/manager_linux.go | 37 ++++++++++++------------ plugin/manager_linux_test.go | 16 ++-------- 4 files changed, 25 insertions(+), 35 deletions(-) diff --git a/plugin/executor/containerd/containerd.go b/plugin/executor/containerd/containerd.go index 10186c6c91..92983056d0 100644 --- a/plugin/executor/containerd/containerd.go +++ b/plugin/executor/containerd/containerd.go @@ -113,8 +113,8 @@ func (e *Executor) IsRunning(id string) (bool, error) { } // Signal sends the specified signal to the container -func (e *Executor) Signal(id string, signal int) error { - return e.client.SignalProcess(context.Background(), id, libcontainerdtypes.InitProcessName, syscall.Signal(signal)) +func (e *Executor) Signal(id string, signal syscall.Signal) error { + return e.client.SignalProcess(context.Background(), id, libcontainerdtypes.InitProcessName, signal) } // ProcessEvent handles events from containerd diff --git a/plugin/manager.go b/plugin/manager.go index 3296ba4e77..0504362f3f 100644 --- a/plugin/manager.go +++ b/plugin/manager.go @@ -11,6 +11,7 @@ import ( "sort" "strings" "sync" + "syscall" "github.com/containerd/containerd/content" "github.com/containerd/containerd/content/local" @@ -37,7 +38,7 @@ type Executor interface { Create(id string, spec specs.Spec, stdout, stderr io.WriteCloser) error IsRunning(id string) (bool, error) Restore(id string, stdout, stderr io.WriteCloser) (alive bool, err error) - Signal(id string, signal int) error + Signal(id string, signal syscall.Signal) error } // EndpointResolver provides looking up registry endpoints for pulling. diff --git a/plugin/manager_linux.go b/plugin/manager_linux.go index 166abbb120..7daaf6af40 100644 --- a/plugin/manager_linux.go +++ b/plugin/manager_linux.go @@ -154,31 +154,30 @@ const shutdownTimeout = 10 * time.Second func shutdownPlugin(p *v2.Plugin, ec chan bool, executor Executor) { pluginID := p.GetID() - err := executor.Signal(pluginID, int(unix.SIGTERM)) - if err != nil { + if err := executor.Signal(pluginID, unix.SIGTERM); err != nil { logrus.Errorf("Sending SIGTERM to plugin failed with error: %v", err) - } else { + return + } - timeout := time.NewTimer(shutdownTimeout) - defer timeout.Stop() + timeout := time.NewTimer(shutdownTimeout) + defer timeout.Stop() + + select { + case <-ec: + logrus.Debug("Clean shutdown of plugin") + case <-timeout.C: + logrus.Debug("Force shutdown plugin") + if err := executor.Signal(pluginID, unix.SIGKILL); err != nil { + logrus.Errorf("Sending SIGKILL to plugin failed with error: %v", err) + } + + timeout.Reset(shutdownTimeout) select { case <-ec: - logrus.Debug("Clean shutdown of plugin") + logrus.Debug("SIGKILL plugin shutdown") case <-timeout.C: - logrus.Debug("Force shutdown plugin") - if err := executor.Signal(pluginID, int(unix.SIGKILL)); err != nil { - logrus.Errorf("Sending SIGKILL to plugin failed with error: %v", err) - } - - timeout.Reset(shutdownTimeout) - - select { - case <-ec: - logrus.Debug("SIGKILL plugin shutdown") - case <-timeout.C: - logrus.WithField("plugin", p.Name).Warn("Force shutdown plugin FAILED") - } + logrus.WithField("plugin", p.Name).Warn("Force shutdown plugin FAILED") } } } diff --git a/plugin/manager_linux_test.go b/plugin/manager_linux_test.go index 91f700beb6..6224cc018a 100644 --- a/plugin/manager_linux_test.go +++ b/plugin/manager_linux_test.go @@ -5,6 +5,7 @@ import ( "net" "os" "path/filepath" + "syscall" "testing" "github.com/docker/docker/api/types" @@ -87,24 +88,13 @@ func newTestPlugin(t *testing.T, name, cap, root string) *v2.Plugin { } type simpleExecutor struct { + Executor } func (e *simpleExecutor) Create(id string, spec specs.Spec, stdout, stderr io.WriteCloser) error { return errors.New("Create failed") } -func (e *simpleExecutor) Restore(id string, stdout, stderr io.WriteCloser) (bool, error) { - return false, nil -} - -func (e *simpleExecutor) IsRunning(id string) (bool, error) { - return false, nil -} - -func (e *simpleExecutor) Signal(id string, signal int) error { - return nil -} - func TestCreateFailed(t *testing.T) { root, err := os.MkdirTemp("", "test-create-failed") if err != nil { @@ -165,7 +155,7 @@ func (e *executorWithRunning) Restore(id string, stdout, stderr io.WriteCloser) return true, nil } -func (e *executorWithRunning) Signal(id string, signal int) error { +func (e *executorWithRunning) Signal(id string, signal syscall.Signal) error { ch := e.exitChans[id] ch <- struct{}{} <-ch