on stop timeout, kill then wait for services (#2169)

also improve on the handling of services not stopping
by starting them over

pkg/upgrade/upgrade.go

@@ -379,17 +379,27 @@ func (u *Upgrader) ensureRestarted(service ...string) error {
 
 	log.Debug().Strs("services", service).Msg("restarting services")
 	if err := u.zinit.StopMultiple(20*time.Second, service...); err != nil {
-		return err
+		// we log here so we don't leave the node in a bad state!
+		// by just trying to start as much services as we can
+		log.Error().Err(err).Msg("failed to stop all services")
 	}
 
 	for _, name := range service {
+		log.Info().Str("service", name).Msg("starting service")
 		if err := u.zinit.Forget(name); err != nil {
 			log.Warn().Err(err).Str("service", name).Msg("could not forget service")
 		}
 
 		if err := u.zinit.Monitor(name); err != nil && err != zinit.ErrAlreadyMonitored {
 			log.Error().Err(err).Str("service", name).Msg("could not monitor service")
 		}
+
+		// this has no effect if Monitor already worked with no issue
+		// but we do it anyway for services that could not be forgotten (did not stop)
+		// so we start them again
+		if err := u.zinit.Start(name); err != nil {
+			log.Error().Err(err).Str("service", name).Msg("could not start service")
+		}
 	}
 
 	return nil

pkg/zinit/commands.go

@@ -1,6 +1,7 @@
 package zinit
 
 import (
+	"context"
 	"fmt"
 	"os"
 	"os/exec"
@@ -496,8 +497,10 @@ func (c *Client) StopMultiple(timeout time.Duration, service ...string) error {
 		services[name] = struct{}{}
 	}
 
-	deadline := time.After(timeout)
+	ctx, cancel := context.WithTimeout(context.Background(), timeout)
+	defer cancel()
 
+	killCount := 0
 	for len(services) > 0 {
 		var stopped []string
 		for service := range services {
@@ -531,17 +534,20 @@ func (c *Client) StopMultiple(timeout time.Duration, service ...string) error {
 		}
 
 		select {
-		case <-deadline:
+		case <-ctx.Done():
 			for service := range services {
 				log.Warn().Str("service", service).Msg("service didn't stop in time. use SIGKILL")
 				if err := c.Kill(service, SIGKILL); err != nil {
 					log.Error().Err(err).Msgf("failed to send SIGKILL to service %s", service)
 				}
 			}
-			// after a kill we wait 1 second to make sure
-			// services are really dead before we move on
+			// we do kill -9 only 10 times before we give up
+			killCount += 1
+			if killCount == 10 {
+				return fmt.Errorf("not all services are dead in time")
+			}
+			// we wait 1 second between each kill
 			<-time.After(1 * time.Second)
-			return nil
 		case <-time.After(1 * time.Second):
 		}
 	}