runner: drain on SIGTERM !322

merged merged by cmc on 2026-09-07 06:47 UTC · krz/gitbay:runner-drain-179 into main

4 files changed, +100 −8

Layout: unified · split

.gitbay/wiki/Admin.org +8

	@@ -476,6 +476,14 @@ cannot call =newuidmap= and the runner refuses to start. That is a
476	considered trade, explained in the file and in the Threat-Model; if you	476	considered trade, explained in the file and in the Threat-Model; if you
477	run with =-isolation none=, set it back to =yes=.	477	run with =-isolation none=, set it back to =yes=.
478		478
		479	Restarting the runner is safe. On SIGTERM it stops claiming, finishes
		480	the build in flight, reports it, and exits; the drop-in's
		481	=TimeoutStopSec=50min= covers the longest build. So =make deploy-runner=
		482	waits for a running build rather than orphaning it, and a build's result
		483	is retried for half a minute if gitbayd is restarting at that moment. A
		484	second SIGTERM ends the runner at once, abandoning the build to the
		485	reaper.
		486
479	*Validate podman mode on a scratch repository before pointing the runner	487	*Validate podman mode on a scratch repository before pointing the runner
480	at real ones.* Every deploy that switched the whole instance to	488	at real ones.* Every deploy that switched the whole instance to
481	containers and failed took CI down with it. Instead: create a throwaway	489	containers and failed took CI down with it. Instead: create a throwaway

cmd/gitbay-runner/env_test.go +46

	@@ -4,6 +4,7 @@ import (
4	"os"	4	"os"
5	"strings"	5	"strings"
6	"testing"	6	"testing"
		7	"time"
7	)	8	)
8		9
9	// A step's environment is constructed, not inherited: repository content	10	// A step's environment is constructed, not inherited: repository content
	@@ -129,3 +130,48 @@ func TestLimitArgs(t *testing.T) {
129	t.Errorf("limitArgs = %v, want %v", got, want)	130	t.Errorf("limitArgs = %v, want %v", got, want)
130	}	131	}
131	}	132	}
		133
		134	// Closing stop drains: the build in flight finishes and is reported, and
		135	// no further build is claimed (#179).
		136	func TestServeDrainsOnStop(t *testing.T) {
		137	stop := make(chan struct{})
		138	started := make(chan struct{})
		139	release := make(chan struct{})
		140	calls := 0
		141	r := &runner{stepFn: func() (bool, error) {
		142	calls++
		143	if calls == 1 {
		144	close(started)
		145	<-release // the build is in flight while stop closes
		146	}
		147	return true, nil
		148	}}
		149	done := make(chan struct{})
		150	go func() { r.serve(1, false, time.Millisecond, stop); close(done) }()
		151	<-started
		152	close(stop)
		153	close(release)
		154	select {
		155	case <-done:
		156	case <-time.After(2 * time.Second):
		157	t.Fatal("serve did not return after the in-flight build finished")
		158	}
		159	if calls != 1 {
		160	t.Errorf("claimed %d builds after stop, want the one already in flight", calls-1)
		161	}
		162	}
		163
		164	// An idle worker leaves promptly on stop rather than sleeping out a poll.
		165	func TestServeStopsWhileIdle(t *testing.T) {
		166	stop := make(chan struct{})
		167	r := &runner{stepFn: func() (bool, error) { return false, nil }}
		168	done := make(chan struct{})
		169	go func() { r.serve(1, false, time.Hour, stop); close(done) }()
		170	time.Sleep(20 * time.Millisecond)
		171	close(stop)
		172	select {
		173	case <-done:
		174	case <-time.After(2 * time.Second):
		175	t.Fatal("idle worker did not stop")
		176	}
		177	}

cmd/gitbay-runner/main.go +41 −8

 	"log"
 	"os"
 	"os/exec"
+	"os/signal"
 	"path/filepath"
 	"strings"
 	"sync"
 	// memory and cpus cap one build's container; empty means no cap.
 	memory string
 	cpus   string
+	// stepFn is step, replaceable by tests.
+	stepFn func() (bool, error)
 	// repos limits which repositories this runner claims builds for. Empty
 	// means any, which is what a runner on the server itself wants; a runner
 	// somewhere that should not execute every repository's steps names them.
 	// claiming at once is already safe; the runner just never used that.
 	// Each build works in its own build-<id> directory, so they do not
 	// meet on disk either.
+	// A stop signal drains: no build is claimed after it, and each build
+	// already in flight runs to completion and is reported. The old
+	// behaviour was to die mid-build, which left the build "running" on
+	// the server with nothing executing (#179). The unit's
+	// TimeoutStopSec bounds the drain; a second signal ends it now.
+	stop := make(chan struct{})
+	go func() {
+		sigs := make(chan os.Signal, 2)
+		signal.Notify(sigs, syscall.SIGTERM, syscall.SIGINT)
+		<-sigs
+		log.Printf("draining: finishing builds in flight, claiming no more")
+		close(stop)
+		<-sigs
+		log.Printf("second signal: exiting without draining")
+		os.Exit(1)
+	}()
+	r.serve(n, *once, *poll, stop)
+}
+// serve runs n workers until stop closes. A worker checks stop only
+// between builds, so closing it never interrupts one.
+func (r *runner) serve(n int, once bool, poll time.Duration, stop <-chan struct{}) {
+	if r.stepFn == nil {
+		r.stepFn = r.step
+	}
 	var wg sync.WaitGroup
 	for i := 0; i < n; i++ {
 		wg.Add(1)
 		go func(i int) {
 			defer wg.Done()
-			// Spread the idle polls across the interval rather than
-			// having every worker wake together: n workers asking the
-			// same question in the same instant is n times the load for
-			// one answer.
 			if n > 1 {
-				time.Sleep(time.Duration(i) * *poll / time.Duration(n))
+				time.Sleep(time.Duration(i) * poll / time.Duration(n))
 			}
 			for {
-				ran, err := r.step()
+				select {
+				case <-stop:
+					return
+				default:
+				}
+				ran, err := r.stepFn()
 				if err != nil {
 					log.Printf("runner: %v", err)
 				}
-				if *once {
+				if once {
 					return
 				}
 				if !ran {
-					time.Sleep(*poll)
+					select {
+					case <-stop:
+						return
+					case <-time.After(poll):
+					}
 				}
 			}
 		}(i)

deploy/gitbay-runner.override.conf +5

 # start joins its namespaces — including a /tmp that no longer exists.
 # End it with the service.
 ExecStopPost=-/usr/bin/pkill -u ci-runner -x catatonit
+# On stop the runner drains: it claims nothing more and finishes the
+# build in flight, then exits. Give it long enough — the per-build limit
+# is 45m plus half a minute of report retries — before systemd kills it.
+# `make deploy-runner` therefore waits for a running build (#179).
+TimeoutStopSec=50min
 ExecStart=
 ExecStart=/usr/local/bin/gitbay-runner -remote git@127.0.0.1 -workdir /var/lib/gitbay-runner/work -poll 5s -timeout 45m -repos krz/gitbay,cmc/ci-smoke -isolation podman -image localhost/gitbay-ci:1 -cpus 3
 Nice=10