Commit 644155a37d

644155a37deeb4a675e67f8ecac4514c82425de8

parent: 5cc5eff559

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-23 06:58 UTC

gitbayd: a restart ends open follows before the drain

httpd.Server.Stop and sshd.Server.Stop close a channel the follows run
under; the build page and the ssh session say why they ended.

Ref #251
cmd/gitbayd/main.go +6
@@ -325,6 +325,12 @@ func serveCmd() *cobra.Command {
325325 ln.Close()
326326 }
327327 }
328 // Follows run until a build ends; end them first so the drain
329 // waits only for work that finishes.
330 web.Stop()
331 if sshSrv != nil {
332 sshSrv.Stop()
333 }
328334 drain, cancel := context.WithTimeout(context.Background(), 30*time.Second)
329335 defer cancel()
330336 if err := hs.Shutdown(drain); err != nil {
e2e/buildfollow_test.go +25 −11
@@ -54,15 +54,10 @@ func (s *streamReader) waitFor(t *testing.T, want string) string {
5454 return s.buf.String()
5555}
5656
57// A running build is followed over ssh and on its page: output the runner
58// sends arrives while the build runs, and both end with the outcome.
59func TestBuildLogFollow(t *testing.T) {
60 t.Parallel()
61 inst := startInstance(t)
62 aliceKey := inst.newKey(t, "alice")
63 runnerKey := inst.newKey(t, "ci")
64 inst.admin(t, "admin", "user", "create", "alice", "--key", aliceKey+".pub")
65 inst.admin(t, "admin", "user", "create", "ci", "--key", runnerKey+".pub", "--admin")
57// queueBuild creates alice/app with one CI job and pushes it, which
58// queues build 1. No runner is attached, so it stays queued.
59func queueBuild(t *testing.T, inst *instance, aliceKey string) {
60 t.Helper()
6661 if _, _, code := inst.ssh(t, aliceKey, "", "repo", "create", "alice/app"); code != 0 {
6762 t.Fatal("repo create failed")
6863 }
@@ -76,8 +71,12 @@ func TestBuildLogFollow(t *testing.T) {
7671 mustGit(t, dir, env, "add", ".")
7772 mustGit(t, dir, env, "commit", "-q", "-m", "ci")
7873 mustGit(t, dir, env, "push", "-q", "origin", "main")
74}
7975
80 // Claim build 1 by hand, so the test decides when output arrives.
76// claimBuild claims the oldest pending build with an admin key, as a
77// runner would, and returns its id for runner log and runner done.
78func claimBuild(t *testing.T, inst *instance, runnerKey string) string {
79 t.Helper()
8180 out, errOut, code := inst.ssh(t, runnerKey, "", "runner", "next", "--json")
8281 if code != 0 {
8382 t.Fatalf("runner next: %s", errOut)
@@ -90,7 +89,22 @@ func TestBuildLogFollow(t *testing.T) {
9089 if err := json.Unmarshal([]byte(out), &claim); err != nil || claim.Data.ID == 0 {
9190 t.Fatalf("runner next output %q: %v", out, err)
9291 }
93 id := fmt.Sprint(claim.Data.ID)
92 return fmt.Sprint(claim.Data.ID)
93}
94
95// A running build is followed over ssh and on its page: output the runner
96// sends arrives while the build runs, and both end with the outcome.
97func TestBuildLogFollow(t *testing.T) {
98 t.Parallel()
99 inst := startInstance(t)
100 aliceKey := inst.newKey(t, "alice")
101 runnerKey := inst.newKey(t, "ci")
102 inst.admin(t, "admin", "user", "create", "alice", "--key", aliceKey+".pub")
103 inst.admin(t, "admin", "user", "create", "ci", "--key", runnerKey+".pub", "--admin")
104 queueBuild(t, inst, aliceKey)
105
106 // Claim build 1 by hand, so the test decides when output arrives.
107 id := claimBuild(t, inst, runnerKey)
94108
95109 cmd := inst.sshCmd(aliceKey, "build", "log", "alice/app", "1", "--follow")
96110 stdout, err := cmd.StdoutPipe()
e2e/shutdown_test.go +74
@@ -1,6 +1,7 @@
11package e2e
22
33import (
4 "errors"
45 "fmt"
56 "net/http"
67 "os/exec"
@@ -83,3 +84,76 @@ func TestShutdownClosesIdleConnections(t *testing.T) {
8384 t.Fatalf("shutdown took %s with only an idle connection open", took)
8485 }
8586}
87
88// A deploy restarts the daemon while someone follows a build. The follows
89// end at once with a message saying so, rather than holding the drain for
90// its full 30 s and then being cut off mid-page.
91func TestShutdownEndsFollows(t *testing.T) {
92 t.Parallel()
93 inst := startInstance(t)
94 aliceKey := inst.newKey(t, "alice")
95 runnerKey := inst.newKey(t, "ci")
96 inst.admin(t, "admin", "user", "create", "alice", "--key", aliceKey+".pub")
97 inst.admin(t, "admin", "user", "create", "ci", "--key", runnerKey+".pub", "--admin")
98 queueBuild(t, inst, aliceKey)
99 // A line in the log is how the test knows both follows are streaming
100 // before the signal; a follow still connecting is an idle connection,
101 // which shutdown closes without a word.
102 id := claimBuild(t, inst, runnerKey)
103 if _, errOut, code := inst.ssh(t, runnerKey, "started\n", "runner", "log", id); code != 0 {
104 t.Fatalf("runner log: %s", errOut)
105 }
106
107 cmd := inst.sshCmd(aliceKey, "build", "log", "alice/app", "1", "--follow")
108 stdout, err := cmd.StdoutPipe()
109 if err != nil {
110 t.Fatal(err)
111 }
112 var stderr strings.Builder
113 cmd.Stderr = &stderr
114 if err := cmd.Start(); err != nil {
115 t.Fatal(err)
116 }
117 defer func() {
118 if cmd.ProcessState == nil {
119 cmd.Process.Kill()
120 cmd.Wait()
121 }
122 }()
123
124 page, err := http.Get(inst.base() + "/alice/app/builds/1")
125 if err != nil {
126 t.Fatal(err)
127 }
128 defer page.Body.Close()
129 web := newStreamReader(page.Body)
130 web.waitFor(t, "started")
131 newStreamReader(stdout).waitFor(t, "started")
132
133 start := time.Now()
134 if err := inst.proc.Process.Signal(syscall.SIGTERM); err != nil {
135 t.Fatal(err)
136 }
137 web.waitFor(t, "gitbay is restarting; reload in a moment")
138 web.waitFor(t, "</html>")
139 var exit *exec.ExitError
140 if err := cmd.Wait(); !errors.As(err, &exit) || exit.ExitCode() != 1 {
141 t.Fatalf("follow ended with %v, want exit 1\n%s", err, stderr.String())
142 }
143 if !strings.Contains(stderr.String(), "gitbay is restarting") {
144 t.Errorf("follow stderr %q", stderr.String())
145 }
146 done := make(chan error, 1)
147 go func() { done <- inst.proc.Wait() }()
148 select {
149 case err := <-done:
150 if err != nil {
151 t.Fatalf("daemon did not exit cleanly: %v", err)
152 }
153 case <-time.After(20 * time.Second):
154 t.Fatal("daemon still running 20s after SIGTERM")
155 }
156 if took := time.Since(start); took > 5*time.Second {
157 t.Fatalf("shutdown took %s with two follows open", took)
158 }
159}
internal/httpd/api.go +1 −1
@@ -73,7 +73,7 @@ func (s *Server) apiCmd(w http.ResponseWriter, r *http.Request) {
7373 JSON: true,
7474 ViaAPI: true,
7575 ReadOnly: scope == "read",
76 Done: r.Context().Done(),
76 Done: s.until(r),
7777 }
7878 code := control.Dispatch(ctx, req.Argv)
7979
internal/httpd/apiread.go +1 −1
@@ -62,7 +62,7 @@ func (s *Server) apiRead(w http.ResponseWriter, r *http.Request) {
6262 JSON: true,
6363 ViaAPI: true,
6464 ReadOnly: true,
65 Done: r.Context().Done(),
65 Done: s.until(r),
6666 }
6767 code := control.Dispatch(ctx, argv)
6868
internal/httpd/builds.go +6 −1
@@ -353,11 +353,16 @@ func (s *Server) streamBuild(w http.ResponseWriter, r *http.Request, v buildView
353353
354354 path := v.Repo.Path()
355355 msg, code := s.runControlStream(viewer, []string{"build", "log", path, n, "--follow"},
356 htmlStream{w: w, rc: rc}, r.Context().Done())
356 htmlStream{w: w, rc: rc}, s.until(r))
357357 if r.Context().Err() != nil {
358358 // The client left; nothing more to write.
359359 return
360360 }
361 if code != protocol.ExitOK && s.stopped() {
362 io.WriteString(w, `</pre><p class="notice" role="status">gitbay is restarting; reload in a moment to pick the log up again.</p>`)
363 io.WriteString(w, tail)
364 return
365 }
361366 if code == protocol.ExitDenied {
362367 // The follow cap: the stored log once, and why it is not live.
363368 log, _, _ := s.runControl(viewer, []string{"build", "log", path, n})
internal/httpd/smart.go +35 −1
@@ -14,6 +14,7 @@ import (
1414 "os"
1515 "os/exec"
1616 "strings"
17 "sync"
1718
1819 "gitbay.org/gitbay/internal/config"
1920 "gitbay.org/gitbay/internal/control"
@@ -26,11 +27,44 @@ type Server struct {
2627 st *store.Store
2728 apiLimit *apiLimiter
2829 proxies []*net.IPNet // http.trusted_proxies, parsed once
30 stopping chan struct{} // closed by Stop
31 stopOnce sync.Once
2932}
3033
3134func New(cfg config.Config, st *store.Store) *Server {
3235 proxies, _ := cfg.HTTP.TrustedProxyNets() // validated at config load
33 return &Server{cfg: cfg, st: st, apiLimit: newAPILimiter(cfg.Limits.APIRate), proxies: proxies}
36 return &Server{cfg: cfg, st: st, apiLimit: newAPILimiter(cfg.Limits.APIRate), proxies: proxies,
37 stopping: make(chan struct{})}
38}
39
40// Stop ends the requests running a command that lasts until something
41// happens (build log --follow), so a shutdown drain waits only for work
42// that finishes. Other requests, git transport included, run on.
43func (s *Server) Stop() {
44 s.stopOnce.Do(func() { close(s.stopping) })
45}
46
47// until is closed when the request ends or the server stops, whichever
48// comes first: the Done a following command runs under.
49func (s *Server) until(r *http.Request) <-chan struct{} {
50 done := make(chan struct{})
51 go func() {
52 select {
53 case <-r.Context().Done():
54 case <-s.stopping:
55 }
56 close(done)
57 }()
58 return done
59}
60
61func (s *Server) stopped() bool {
62 select {
63 case <-s.stopping:
64 return true
65 default:
66 return false
67 }
3468}
3569
3670// receivePackRefusal exists only to fail legibly if a client POSTs without