detached 66fbb46

fix(backup): honor ctx cancellation and selected database

commit
66fbb46be3655e5cbc401460f0f2f2ec7a3813d8
author
brice · · 2026-02-07 16:40 UTC
parent
511c20b
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
4 files · +104 -7
Unified
UnifiedSplit
M internal/usecase/backup/service.go +15 -6
@@ -86,7 +86,7 @@ func (s *Service) RunBackup(ctx context.Context, domainName, dbName string) (*do
8686 defer cancelExec()
8787 dumpPath := fmt.Sprintf("/tmp/gordon-backup-%d.bak", started.UnixNano())
8888
89- execResult, err := s.runtime.ExecInContainer(execCtx, db.ContainerID, []string{"sh", "-c", pgDumpToPathCommand(dumpPath)})
89+ execResult, err := s.runtime.ExecInContainer(execCtx, db.ContainerID, []string{"sh", "-c", pgDumpToPathCommand(dumpPath, db.Name)})
9090 if err != nil {
9191 if errors.Is(execCtx.Err(), context.DeadlineExceeded) {
9292 return nil, fmt.Errorf("pg_dump timed out after %s", backupExecTimeout)
@@ -164,8 +164,13 @@ func (s *Service) Status(ctx context.Context) ([]domain.BackupJob, error) {
164164 default:
165165 }
166166
167+ select {
168+ case sem <- struct{}{}:
169+ case <-ctx.Done():
170+ return nil, ctx.Err()
171+ }
172+
167173 wg.Add(1)
168- sem <- struct{}{}
169174 go func(idx int, name string) {
170175 defer wg.Done()
171176 defer func() { <-sem }()
@@ -226,12 +231,16 @@ func selectDatabase(dbs []domain.DBInfo, requested string) (domain.DBInfo, error
226231 return domain.DBInfo{}, fmt.Errorf("database %q not found for domain", requested)
227232 }
228233
229-func postgresDumpCommand() string {
230- return "pg_dump -Fc --dbname=\"${POSTGRES_DB:-postgres}\" --username=\"${POSTGRES_USER:-postgres}\""
234+func postgresDumpCommand(dbName string) string {
235+ return fmt.Sprintf("PGDATABASE=%s pg_dump -Fc --dbname=\"$PGDATABASE\" --username=\"${POSTGRES_USER:-postgres}\"", shellQuote(dbName))
236+}
237+
238+func pgDumpToPathCommand(path, dbName string) string {
239+ return fmt.Sprintf("%s > %q", postgresDumpCommand(dbName), path)
231240 }
232241
233-func pgDumpToPathCommand(path string) string {
234- return fmt.Sprintf("%s > %q", postgresDumpCommand(), path)
242+func shellQuote(value string) string {
243+ return "'" + strings.ReplaceAll(value, "'", "'\"'\"'") + "'"
235244 }
236245
237246 func (s *Service) cleanupDumpFile(containerID, dumpPath string) {
@@ -86,7 +86,7 @@ func (s *Service) RunBackup(ctx context.Context, domainName, dbName string) (*do
86 defer cancelExec()86 defer cancelExec()
87 dumpPath := fmt.Sprintf("/tmp/gordon-backup-%d.bak", started.UnixNano())87 dumpPath := fmt.Sprintf("/tmp/gordon-backup-%d.bak", started.UnixNano())
88 88
89- execResult, err := s.runtime.ExecInContainer(execCtx, db.ContainerID, []string{"sh", "-c", pgDumpToPathCommand(dumpPath)})89+ execResult, err := s.runtime.ExecInContainer(execCtx, db.ContainerID, []string{"sh", "-c", pgDumpToPathCommand(dumpPath, db.Name)})
90 if err != nil {90 if err != nil {
91 if errors.Is(execCtx.Err(), context.DeadlineExceeded) {91 if errors.Is(execCtx.Err(), context.DeadlineExceeded) {
92 return nil, fmt.Errorf("pg_dump timed out after %s", backupExecTimeout)92 return nil, fmt.Errorf("pg_dump timed out after %s", backupExecTimeout)
@@ -164,8 +164,13 @@ func (s *Service) Status(ctx context.Context) ([]domain.BackupJob, error) {
164 default:164 default:
165 }165 }
166 166
167+ select {
168+ case sem <- struct{}{}:
169+ case <-ctx.Done():
170+ return nil, ctx.Err()
171+ }
172+
167 wg.Add(1)173 wg.Add(1)
168- sem <- struct{}{}
169 go func(idx int, name string) {174 go func(idx int, name string) {
170 defer wg.Done()175 defer wg.Done()
171 defer func() { <-sem }()176 defer func() { <-sem }()
@@ -226,12 +231,16 @@ func selectDatabase(dbs []domain.DBInfo, requested string) (domain.DBInfo, error
226 return domain.DBInfo{}, fmt.Errorf("database %q not found for domain", requested)231 return domain.DBInfo{}, fmt.Errorf("database %q not found for domain", requested)
227 }232 }
228 233
229-func postgresDumpCommand() string {234+func postgresDumpCommand(dbName string) string {
230- return "pg_dump -Fc --dbname=\"${POSTGRES_DB:-postgres}\" --username=\"${POSTGRES_USER:-postgres}\""235+ return fmt.Sprintf("PGDATABASE=%s pg_dump -Fc --dbname=\"$PGDATABASE\" --username=\"${POSTGRES_USER:-postgres}\"", shellQuote(dbName))
236+}
237+
238+func pgDumpToPathCommand(path, dbName string) string {
239+ return fmt.Sprintf("%s > %q", postgresDumpCommand(dbName), path)
231 }240 }
232 241
233-func pgDumpToPathCommand(path string) string {242+func shellQuote(value string) string {
234- return fmt.Sprintf("%s > %q", postgresDumpCommand(), path)243+ return "'" + strings.ReplaceAll(value, "'", "'\"'\"'") + "'"
235 }244 }
236 245
237 func (s *Service) cleanupDumpFile(containerID, dumpPath string) {246 func (s *Service) cleanupDumpFile(containerID, dumpPath string) {
M internal/usecase/backup/service_test.go +58 -1
@@ -4,7 +4,9 @@ import (
44 "bytes"
55 "context"
66 "io"
7+ "sync/atomic"
78 "testing"
9+ "time"
810
911 inmocks "github.com/bnema/gordon/internal/boundaries/in/mocks"
1012 outiface "github.com/bnema/gordon/internal/boundaries/out"
@@ -47,7 +49,9 @@ func TestService_RunBackup_Postgres(t *testing.T) {
4749 if len(cmd) != 3 || cmd[0] != "sh" || cmd[1] != "-c" {
4850 return false
4951 }
50- return bytes.Contains([]byte(cmd[2]), []byte("pg_dump -Fc")) && bytes.Contains([]byte(cmd[2]), []byte(" > "))
52+ return bytes.Contains([]byte(cmd[2]), []byte("pg_dump -Fc")) &&
53+ bytes.Contains([]byte(cmd[2]), []byte("PGDATABASE='postgres'")) &&
54+ bytes.Contains([]byte(cmd[2]), []byte(" > "))
5155 })).Return(&outiface.ExecResult{ExitCode: 0, Stdout: []byte("backup-data")}, nil)
5256
5357 runtime.EXPECT().CopyFromContainer(mock.Anything, "db123", mock.MatchedBy(func(path string) bool {
@@ -95,3 +99,56 @@ func TestSelectDatabaseAutoSelectsOnlyDatabaseWhenUnspecified(t *testing.T) {
9599 require.NoError(t, err)
96100 assert.Equal(t, "postgres", db.Name)
97101 }
102+
103+func TestPostgresDumpCommandQuotesDatabaseName(t *testing.T) {
104+ cmd := postgresDumpCommand("customer data")
105+ assert.Contains(t, cmd, "PGDATABASE='customer data'")
106+}
107+
108+func TestServiceStatusReturnsWhenContextCancelledDuringSemaphoreAcquire(t *testing.T) {
109+ runtime := outmocks.NewMockContainerRuntime(t)
110+ storage := outmocks.NewMockBackupStorage(t)
111+ containerSvc := inmocks.NewMockContainerService(t)
112+
113+ containerSvc.EXPECT().List(mock.Anything).Return(map[string]*domain.Container{
114+ "a.example.com": {},
115+ "b.example.com": {},
116+ "c.example.com": {},
117+ "d.example.com": {},
118+ "e.example.com": {},
119+ })
120+
121+ unblock := make(chan struct{})
122+ defer close(unblock)
123+
124+ var started int32
125+ startedFour := make(chan struct{})
126+ storage.EXPECT().List(mock.Anything, mock.Anything, (*domain.BackupSchedule)(nil)).RunAndReturn(
127+ func(context.Context, string, *domain.BackupSchedule) ([]domain.BackupJob, error) {
128+ if atomic.AddInt32(&started, 1) == 4 {
129+ close(startedFour)
130+ }
131+ <-unblock
132+ return nil, nil
133+ },
134+ ).Times(4)
135+
136+ svc := NewService(runtime, storage, containerSvc, domain.BackupConfig{}, zerowrap.Default())
137+
138+ ctx, cancel := context.WithCancel(context.Background())
139+ errCh := make(chan error, 1)
140+ go func() {
141+ _, err := svc.Status(ctx)
142+ errCh <- err
143+ }()
144+
145+ <-startedFour
146+ cancel()
147+
148+ select {
149+ case err := <-errCh:
150+ require.ErrorIs(t, err, context.Canceled)
151+ case <-time.After(250 * time.Millisecond):
152+ t.Fatal("Status did not return after context cancellation while waiting for semaphore")
153+ }
154+}
@@ -4,7 +4,9 @@ import (
4 "bytes"4 "bytes"
5 "context"5 "context"
6 "io"6 "io"
7+ "sync/atomic"
7 "testing"8 "testing"
9+ "time"
8 10
9 inmocks "github.com/bnema/gordon/internal/boundaries/in/mocks"11 inmocks "github.com/bnema/gordon/internal/boundaries/in/mocks"
10 outiface "github.com/bnema/gordon/internal/boundaries/out"12 outiface "github.com/bnema/gordon/internal/boundaries/out"
@@ -47,7 +49,9 @@ func TestService_RunBackup_Postgres(t *testing.T) {
47 if len(cmd) != 3 || cmd[0] != "sh" || cmd[1] != "-c" {49 if len(cmd) != 3 || cmd[0] != "sh" || cmd[1] != "-c" {
48 return false50 return false
49 }51 }
50- return bytes.Contains([]byte(cmd[2]), []byte("pg_dump -Fc")) && bytes.Contains([]byte(cmd[2]), []byte(" > "))52+ return bytes.Contains([]byte(cmd[2]), []byte("pg_dump -Fc")) &&
53+ bytes.Contains([]byte(cmd[2]), []byte("PGDATABASE='postgres'")) &&
54+ bytes.Contains([]byte(cmd[2]), []byte(" > "))
51 })).Return(&outiface.ExecResult{ExitCode: 0, Stdout: []byte("backup-data")}, nil)55 })).Return(&outiface.ExecResult{ExitCode: 0, Stdout: []byte("backup-data")}, nil)
52 56
53 runtime.EXPECT().CopyFromContainer(mock.Anything, "db123", mock.MatchedBy(func(path string) bool {57 runtime.EXPECT().CopyFromContainer(mock.Anything, "db123", mock.MatchedBy(func(path string) bool {
@@ -95,3 +99,56 @@ func TestSelectDatabaseAutoSelectsOnlyDatabaseWhenUnspecified(t *testing.T) {
95 require.NoError(t, err)99 require.NoError(t, err)
96 assert.Equal(t, "postgres", db.Name)100 assert.Equal(t, "postgres", db.Name)
97 }101 }
102+
103+func TestPostgresDumpCommandQuotesDatabaseName(t *testing.T) {
104+ cmd := postgresDumpCommand("customer data")
105+ assert.Contains(t, cmd, "PGDATABASE='customer data'")
106+}
107+
108+func TestServiceStatusReturnsWhenContextCancelledDuringSemaphoreAcquire(t *testing.T) {
109+ runtime := outmocks.NewMockContainerRuntime(t)
110+ storage := outmocks.NewMockBackupStorage(t)
111+ containerSvc := inmocks.NewMockContainerService(t)
112+
113+ containerSvc.EXPECT().List(mock.Anything).Return(map[string]*domain.Container{
114+ "a.example.com": {},
115+ "b.example.com": {},
116+ "c.example.com": {},
117+ "d.example.com": {},
118+ "e.example.com": {},
119+ })
120+
121+ unblock := make(chan struct{})
122+ defer close(unblock)
123+
124+ var started int32
125+ startedFour := make(chan struct{})
126+ storage.EXPECT().List(mock.Anything, mock.Anything, (*domain.BackupSchedule)(nil)).RunAndReturn(
127+ func(context.Context, string, *domain.BackupSchedule) ([]domain.BackupJob, error) {
128+ if atomic.AddInt32(&started, 1) == 4 {
129+ close(startedFour)
130+ }
131+ <-unblock
132+ return nil, nil
133+ },
134+ ).Times(4)
135+
136+ svc := NewService(runtime, storage, containerSvc, domain.BackupConfig{}, zerowrap.Default())
137+
138+ ctx, cancel := context.WithCancel(context.Background())
139+ errCh := make(chan error, 1)
140+ go func() {
141+ _, err := svc.Status(ctx)
142+ errCh <- err
143+ }()
144+
145+ <-startedFour
146+ cancel()
147+
148+ select {
149+ case err := <-errCh:
150+ require.ErrorIs(t, err, context.Canceled)
151+ case <-time.After(250 * time.Millisecond):
152+ t.Fatal("Status did not return after context cancellation while waiting for semaphore")
153+ }
154+}
M internal/usecase/cron/scheduler.go +7 -0
@@ -121,6 +121,13 @@ func (s *Scheduler) Start(ctx context.Context) {
121121 return
122122 }
123123
124+ select {
125+ case <-s.stopCh:
126+ s.started.Store(false)
127+ return
128+ default:
129+ }
130+
124131 ticker := time.NewTicker(time.Minute)
125132 go func() {
126133 defer ticker.Stop()
@@ -121,6 +121,13 @@ func (s *Scheduler) Start(ctx context.Context) {
121 return121 return
122 }122 }
123 123
124+ select {
125+ case <-s.stopCh:
126+ s.started.Store(false)
127+ return
128+ default:
129+ }
130+
124 ticker := time.NewTicker(time.Minute)131 ticker := time.NewTicker(time.Minute)
125 go func() {132 go func() {
126 defer ticker.Stop()133 defer ticker.Stop()
M internal/usecase/cron/scheduler_test.go +24 -0
@@ -156,3 +156,27 @@ func TestSchedulerRunNowRecoversFromPanics(t *testing.T) {
156156 assert.False(t, entries[0].LastRun.IsZero())
157157 assert.False(t, entries[0].NextRun.IsZero())
158158 }
159+
160+func TestSchedulerStartNoOpWhenStopped(t *testing.T) {
161+ s := NewScheduler(zerowrap.Default())
162+ now := time.Date(2026, 2, 7, 12, 0, 0, 0, time.UTC)
163+ s.nowFn = func() time.Time { return now }
164+
165+ runs := 0
166+ err := s.Add("stopped-job", "stopped job", domain.CronSchedule{Preset: domain.ScheduleDaily}, func(context.Context) error {
167+ runs++
168+ return nil
169+ })
170+ require.NoError(t, err)
171+
172+ s.mu.Lock()
173+ s.entries["stopped-job"].nextRun = now.Add(-time.Minute)
174+ s.mu.Unlock()
175+
176+ s.Stop()
177+ s.Start(context.Background())
178+
179+ time.Sleep(50 * time.Millisecond)
180+ assert.Equal(t, 0, runs)
181+ assert.False(t, s.started.Load())
182+}
@@ -156,3 +156,27 @@ func TestSchedulerRunNowRecoversFromPanics(t *testing.T) {
156 assert.False(t, entries[0].LastRun.IsZero())156 assert.False(t, entries[0].LastRun.IsZero())
157 assert.False(t, entries[0].NextRun.IsZero())157 assert.False(t, entries[0].NextRun.IsZero())
158 }158 }
159+
160+func TestSchedulerStartNoOpWhenStopped(t *testing.T) {
161+ s := NewScheduler(zerowrap.Default())
162+ now := time.Date(2026, 2, 7, 12, 0, 0, 0, time.UTC)
163+ s.nowFn = func() time.Time { return now }
164+
165+ runs := 0
166+ err := s.Add("stopped-job", "stopped job", domain.CronSchedule{Preset: domain.ScheduleDaily}, func(context.Context) error {
167+ runs++
168+ return nil
169+ })
170+ require.NoError(t, err)
171+
172+ s.mu.Lock()
173+ s.entries["stopped-job"].nextRun = now.Add(-time.Minute)
174+ s.mu.Unlock()
175+
176+ s.Stop()
177+ s.Start(context.Background())
178+
179+ time.Sleep(50 * time.Millisecond)
180+ assert.Equal(t, 0, runs)
181+ assert.False(t, s.started.Load())
182+}

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 7

Tags 106

↑↓ moveenter openesc close