Bail out after stop timeout

Previously, the shutdown code looped endlessly until the child process
finished, requesting graceful termination over and over again. Change
this to a single request-termination -> wait -> bail-out logic. This
is to ensure that k0s won't hang when the supervised processes can't be
terminated for whichever reason: the code will terminate, at least after
the timeout expired.

Use a buffered channel for the wait result, so that the goroutine
will be able to exit, even if nothing reads from the channel anymore.
Introduce fine-grained error reporting to differentiate shutdown
outcomes (graceful shutdown, forced kill, failure, and so on).

Signed-off-by: Tom Wieczorek <[email protected]>

Author: twz123

pkg/supervisor/supervisor.go

@@ -15,6 +15,7 @@ import (
 	"strconv"
 	"strings"
 	"sync"
+	"syscall"
 	"time"
 
 	"github.com/sirupsen/logrus"
@@ -55,7 +56,7 @@ const k0sManaged = "_K0S_MANAGED=yes"
 // processWaitQuit waits for a process to exit or a shut down signal
 // returns true if shutdown is requested
 func (s *Supervisor) processWaitQuit(ctx context.Context, cmd *exec.Cmd) bool {
-	waitresult := make(chan error)
+	waitresult := make(chan error, 1)
 	go func() {
 		waitresult <- cmd.Wait()
 	}()
@@ -64,29 +65,14 @@ func (s *Supervisor) processWaitQuit(ctx context.Context, cmd *exec.Cmd) bool {
 
 	select {
 	case <-ctx.Done():
-		for {
-			s.log.Debug("Requesting graceful termination")
-			if err := requestGracefulTermination(cmd.Process); err != nil {
-				if errors.Is(err, os.ErrProcessDone) {
-					s.log.Info("Failed to request graceful termination: process has already terminated")
-				} else {
-					s.log.WithError(err).Error("Failed to request graceful termination")
-				}
-			} else {
-				s.log.Info("Requested graceful termination")
-			}
-			select {
-			case <-time.After(s.TimeoutStop):
-				continue
-			case err := <-waitresult:
-				if err != nil {
-					s.log.WithError(err).Error("Failed to wait for process")
-				} else {
-					s.log.Info("Process exited: ", s.cmd.ProcessState)
-				}
-				return true
-			}
+		err := s.terminateSupervisedProcess(cmd, waitresult)
+		if err != nil {
+			s.log.WithError(err).Error("Error while terminating process")
+		} else {
+			s.log.Info("Process terminated successfully")
 		}
+		return true
+
 	case err := <-waitresult:
 		var exitErr *exec.ExitError
 		state := cmd.ProcessState
@@ -103,6 +89,62 @@ func (s *Supervisor) processWaitQuit(ctx context.Context, cmd *exec.Cmd) bool {
 	}
 }
 
+func (s *Supervisor) terminateSupervisedProcess(cmd *exec.Cmd, waitresult <-chan error) error {
+	s.log.Debug("Requesting graceful termination")
+	err := requestGracefulTermination(cmd.Process)
+	switch {
+	case err == nil:
+		// Termination request sent, wait for process to finish.
+		s.log.Debug("Awaiting graceful process termination for ", s.TimeoutStop)
+
+		select {
+		case err := <-waitresult:
+			var exitErr *exec.ExitError
+			switch {
+			case err == nil:
+				return nil
+			case errors.As(err, &exitErr):
+				if status, ok := exitErr.Sys().(syscall.WaitStatus); ok && status.Signal() == syscall.SIGTERM {
+					return errors.New("process terminated without handling SIGTERM")
+				}
+				return exitErr
+			default:
+				return fmt.Errorf("failed to wait for process: %w", err)
+			}
+
+		case <-time.After(s.TimeoutStop):
+			err = fmt.Errorf("timed out after %s while waiting for process to terminate", s.TimeoutStop)
+		}
+
+		return err
+
+	case errors.Is(err, os.ErrProcessDone):
+		// The process has finished even before the termination could be requested.
+		select {
+		case err = <-waitresult:
+			var exitErr *exec.ExitError
+			state := cmd.ProcessState
+			switch {
+			case errors.As(err, &exitErr):
+				state = exitErr.ProcessState
+				fallthrough
+			case err == nil:
+				err = errors.New(state.String())
+			default:
+				return fmt.Errorf("failed to wait for process: %s (%w)", state, err)
+			}
+		default:
+			err = errors.New("process state unavailable")
+		}
+
+		return fmt.Errorf("process terminated before graceful termination could be requested: %w", err)
+
+	default:
+		// Something else went wrong
+		return fmt.Errorf("failed to request graceful termination: %w", err)
+	}
+}
+
 // Supervise Starts supervising the given process
 func (s *Supervisor) Supervise() error {
 	s.startStopMutex.Lock()