fix: Gitter properly shutdown (#4618)

Actually recreate the gitter, and gracefully shutdown, as well as send
the shutdown signal to subprocesses to avoid locking.

Author: another-rex

deployment/clouddeploy/gke-workers/base/gitter.yaml

@@ -5,10 +5,7 @@ metadata:
 spec:
   replicas: 1
   strategy:
-    type: RollingUpdate
-    rollingUpdate:
-      maxUnavailable: 1
-      maxSurge: 1
+    type: Recreate
   selector:
     matchLabels:
       name: gitter
@@ -17,6 +14,7 @@ spec:
       labels:
         name: gitter
     spec:
+      terminationGracePeriodSeconds: 60
       tolerations:
       - key: workloadType
         operator: Equal

go/cmd/gitter/gitter.go

@@ -15,9 +15,11 @@ import (
 	"net/http"
 	"os"
 	"os/exec"
+	"os/signal"
 	"path"
 	"regexp"
 	"strings"
+	"syscall"
 	"time"
 
 	"github.com/google/osv.dev/go/logger"
@@ -36,6 +38,39 @@ var (
 	fetchTimeout    time.Duration
 )
 
+const shutdownTimeout = 10 * time.Second
+
+// runCmd executes a command with context cancellation handled by sending SIGINT.
+// It logs cancellation errors separately as requested.
+func runCmd(ctx context.Context, dir string, env []string, name string, args ...string) error {
+	cmd := exec.CommandContext(ctx, name, args...)
+	if dir != "" {
+		cmd.Dir = dir
+	}
+	if len(env) > 0 {
+		cmd.Env = append(os.Environ(), env...)
+	}
+	// Use SIGINT instead of SIGKILL for graceful shutdown of subprocesses
+	cmd.Cancel = func() error {
+		return cmd.Process.Signal(syscall.SIGINT)
+	}
+	// Ensure it eventually dies if it ignores SIGINT
+	cmd.WaitDelay = shutdownTimeout / 2
+
+	out, err := cmd.CombinedOutput()
+	if err != nil {
+		if ctx.Err() != nil {
+			// Log separately if cancelled
+			logger.Warn("Command cancelled", slog.String("cmd", name), slog.Any("err", ctx.Err()))
+			return fmt.Errorf("command %s cancelled: %w", name, ctx.Err())
+		}
+
+		return fmt.Errorf("command %s failed: %w, output: %s", name, err, out)
+	}
+
+	return nil
+}
+
 func isLocalRequest(r *http.Request) bool {
 	host, _, err := net.SplitHostPort(r.RemoteAddr)
 	if err != nil {
@@ -82,32 +117,31 @@ func fetchBlob(ctx context.Context, url string) ([]byte, error) {
 		logger.Info("Fetching git blob", slog.String("url", url), slog.Duration("sinceAccessTime", time.Since(accessTime)))
 		if _, err := os.Stat(path.Join(repoPath, ".git")); os.IsNotExist(err) {
 			// Clone
-			cmd := exec.Command("git", "clone", "--", url, repoPath)
-			cmd.Env = append(cmd.Env, "GIT_TERMINAL_PROMPT=0")
-			if out, err := cmd.CombinedOutput(); err != nil {
-				return nil, fmt.Errorf("git clone failed: %w, output: %s", err, out)
+			err := runCmd(ctx, "", []string{"GIT_TERMINAL_PROMPT=0"}, "git", "clone", "--", url, repoPath)
+			if err != nil {
+				return nil, fmt.Errorf("git clone failed: %w", err)
 			}
 		} else {
 			// Fetch/Pull - implementing simple git pull for now, might need reset --hard if we want exact mirrors
 			// For a generic "get latest", pull is usually sufficient if we treat it as read-only.
 			// Ideally safely: git fetch origin && git reset --hard origin/HEAD
-			cmd := exec.Command("git", "-C", repoPath, "fetch", "origin")
-			if out, err := cmd.CombinedOutput(); err != nil {
-				return nil, fmt.Errorf("git fetch failed: %w, output: %s", err, out)
+			err := runCmd(ctx, repoPath, nil, "git", "fetch", "origin")
+			if err != nil {
+				return nil, fmt.Errorf("git fetch failed: %w", err)
 			}
-			cmd = exec.Command("git", "-C", repoPath, "reset", "--hard", "origin/HEAD")
-			if out, err := cmd.CombinedOutput(); err != nil {
-				return nil, fmt.Errorf("git reset failed: %w, output: %s", err, out)
+			err = runCmd(ctx, repoPath, nil, "git", "reset", "--hard", "origin/HEAD")
+			if err != nil {
+				return nil, fmt.Errorf("git reset failed: %w", err)
 			}
 		}
 
 		logger.Info("Archiving git blob", slog.String("url", url))
 		// Archive
 		// tar --zstd -cf <archivePath> -C "<gitStorePath>/<repoDirName>" .
 		// using -C to archive the relative path so it unzips nicely
-		cmd := exec.Command("tar", "--zstd", "-cf", archivePath, "-C", path.Join(gitStorePath, repoDirName), ".")
-		if out, err := cmd.CombinedOutput(); err != nil {
-			return nil, fmt.Errorf("tar zstd failed: %w, output: %s", err, out)
+		err := runCmd(ctx, "", nil, "tar", "--zstd", "-cf", archivePath, "-C", path.Join(gitStorePath, repoDirName), ".")
+		if err != nil {
+			return nil, fmt.Errorf("tar zstd failed: %w", err)
 		}
 
 		updateLastFetch(url)
@@ -147,18 +181,57 @@ func main() {
 
 	loadMap()
 
+	// Create a context that listens for the interrupt signal from the OS.
+	ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM)
+	defer stop()
+
 	http.HandleFunc(getGitEndpoint, gitHandler)
 
 	logger.Info("Gitter starting and listening", slog.Int("port", *port))
 
+	// --- Server Shutdown Protocol ---
+	// This is what happens when a kubernetes send a SIGTERM signal:
+	// 1. Kubernetes sends SIGTERM to the process
+	// 2. The process receives the signal and prints "Shutting down gracefully..."
+	// 3. The process calls server.Shutdown(ctx) to close incoming connections, and wait for timeout.
+	// 4. The context within each request will be automatically cancelled (does not wait for timeout).
+	// 5. Any subprocesses will be sent SIGINT, with a timeout / 2 duration before SIGKILL.
+	// 6. The server waits for the timeout to finish processing all requests.
+	// 7. We save the lastFetch map to disk.
+	// 8. The process exits
+
 	server := &http.Server{
 		Addr:              fmt.Sprintf(":%d", *port),
 		ReadHeaderTimeout: 3 * time.Second,
+		BaseContext: func(_ net.Listener) context.Context {
+			// Return the context tied to the termination signal.
+			return ctx
+		},
 	}
-	if err := server.ListenAndServe(); err != nil {
-		logger.Error("Gitter failed to start", slog.Int("port", *port), slog.Any("error", err))
-		os.Exit(1)
+
+	go func() {
+		if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed {
+			logger.Error("Gitter failed to start", slog.Int("port", *port), slog.Any("error", err))
+		}
+	}()
+
+	// Listen for the interrupt signal.
+	<-ctx.Done()
+
+	// Restore default behavior on the interrupt signal and notify user of shutdown.
+	stop()
+	logger.Info("Shutting down gracefully, press Ctrl+C again to force")
+
+	// The context is used to inform the server it has 5 seconds to finish
+	// the request it is currently handling
+	ctx, cancel := context.WithTimeout(context.Background(), shutdownTimeout)
+	defer cancel()
+	if err := server.Shutdown(ctx); err != nil {
+		logger.Error("Server forced to shutdown", slog.Any("error", err))
 	}
+
+	saveMap()
+	logger.Info("Server exiting")
 }
 
 func gitHandler(w http.ResponseWriter, r *http.Request) {
@@ -203,4 +276,6 @@ func gitHandler(w http.ResponseWriter, r *http.Request) {
 
 		return
 	}
+
+	logger.Info("Request completed successfully", slog.String("url", url))
 }