diff --git a/cmd/minstrel/main.go b/cmd/minstrel/main.go index 27103396..c77c7e84 100644 --- a/cmd/minstrel/main.go +++ b/cmd/minstrel/main.go @@ -69,12 +69,32 @@ func main() { return } } - if err := run(); err != nil { + err := run() + if errors.Is(err, errRestart) { + err = restartInPlace() + } + if err != nil { fmt.Fprintf(os.Stderr, "minstrel: %v\n", err) os.Exit(1) } } +// errRestart is run's answer when the server stopped in order to start again: +// after a web restore, so the restored settings are read at boot. +var errRestart = errors.New("restart requested") + +// restartInPlace replaces this process with a fresh start of the same binary, +// arguments and environment. The process id stays the same, so a container +// keeps running without needing a restart policy. +func restartInPlace() error { + exe, err := os.Executable() + if err != nil { + return fmt.Errorf("restart: %w", err) + } + // Exec returns only when it failed. + return fmt.Errorf("restart: %w", syscall.Exec(exe, os.Args, os.Environ())) +} + // safetyNetScanInterval backstops the fsnotify watcher with a low-frequency // full delta walk, catching anything the watcher missed (inotify watch-limit // exhaustion, dropped events). Fixed — there is no operator configuration. @@ -485,6 +505,13 @@ func run() error { go backups.Loop(ctx) srv.Backups = backups srv.StreamSecret = cfg.StreamSecret + restart := make(chan struct{}, 1) + srv.Restart = func() { + select { + case restart <- struct{}{}: + default: + } + } httpServer := &http.Server{ Addr: cfg.Server.Address, Handler: srv.Router(), @@ -510,14 +537,20 @@ func run() error { close(errCh) }() + restarting := false select { case <-ctx.Done(): logger.Info("shutdown signal received") + case <-restart: + logger.Info("restarting to load the restored backup") + restarting = true case err := <-errCh: if err != nil { return fmt.Errorf("server: %w", err) } } + // Ends the background work (scans, workers, schedules) as a signal would. + stop() shutdownCtx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() @@ -526,6 +559,10 @@ func run() error { } // A backup still running is stopped and records that it was cut off. backups.Close() + if restarting { + logger.Info("minstrel stopped; starting again") + return errRestart + } logger.Info("minstrel stopped") return nil } diff --git a/docs/hosting.md b/docs/hosting.md index 20e25e65..2e553d8a 100644 --- a/docs/hosting.md +++ b/docs/hosting.md @@ -119,8 +119,9 @@ to the music as well. ### Restoring -A backup restores into a new Minstrel: an empty database, with no accounts -and no library yet. +A backup restores into a new Minstrel: one nobody has signed up to yet. If +it has already scanned the music, that's fine; the backup's library takes +the place of what the scan found. 1. **Put the music back first**, at the path the library had. The restored settings name the old library folders, and each track is found again by @@ -130,7 +131,9 @@ and no library yet. 2. **Load the backup**, either way: - In the web app, choose **Create account**, then **Restore a backup**. Pick a `minstrel-….tar` file from the backup's `database/` folder and - enter the setup token from the log (see *First account*). + enter the setup token from the log (see *First account*). Minstrel + restarts by itself once the backup is in, so it runs on the restored + settings. - Or, with the server stopped, run `docker compose run --rm minstrel restore /backups/database/minstrel-20261102T030000Z.tar`, with the file on a volume the container can read. It brings the database diff --git a/internal/api/api.go b/internal/api/api.go index e807c05a..688fa1a2 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -41,7 +41,7 @@ import ( // Mount attaches /api/* handlers to r. Public endpoints (login) are outside // RequireUser; everything else is gated by the middleware. The events writer // is shared with the Subsonic mount so /rest/scrobble feeds the same store. -func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playevents.Writer, recCfg config.RecommendationConfig, recSettings *recsettings.Service, requestSvc *requests.Service, quarantineSvc *quarantine.Service, tracksSvc *tracks.Service, playlistsSvc *playlists.Service, coverEnricher *coverart.Enricher, coverSettings *coverart.SettingsService, tagSettings *tags.SettingsService, scanner *library.Scanner, scanCfg library.RunScanConfig, dataDir string, sender mailer.Sender, bus *eventbus.Bus, playlistScheduler *playlists.Scheduler, streamSecret []byte, netSettings *netsettings.Service, reacqSettings *reacquisition.SettingsService, fpSettings *library.FingerprintSettingsService, loudSettings *library.LoudnessSettingsService, acoustIDLookup *library.AcoustIDLookupWorker, backups *backup.Service) { +func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playevents.Writer, recCfg config.RecommendationConfig, recSettings *recsettings.Service, requestSvc *requests.Service, quarantineSvc *quarantine.Service, tracksSvc *tracks.Service, playlistsSvc *playlists.Service, coverEnricher *coverart.Enricher, coverSettings *coverart.SettingsService, tagSettings *tags.SettingsService, scanner *library.Scanner, scanCfg library.RunScanConfig, dataDir string, sender mailer.Sender, bus *eventbus.Bus, playlistScheduler *playlists.Scheduler, streamSecret []byte, netSettings *netsettings.Service, reacqSettings *reacquisition.SettingsService, fpSettings *library.FingerprintSettingsService, loudSettings *library.LoudnessSettingsService, acoustIDLookup *library.AcoustIDLookupWorker, backups *backup.Service, restart func()) { rng := rand.New(rand.NewSource(rand.Int63())) setupToken, err := auth.NewSetupToken() if err != nil { @@ -76,6 +76,7 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev loudnessSettings: loudSettings, acoustIDLookup: acoustIDLookup, backups: backups, + restart: restart, acquisition: acquire.NewStore(pool), catalogue: cat, wanted: wanted.New(pool, cat, logger), @@ -439,6 +440,9 @@ type handlers struct { acoustIDLookup *library.AcoustIDLookupWorker // backups is the server's own backup: settings, status, test and run. backups *backup.Service + // restart stops the server and starts it again in place, so a restored + // backup's settings are read at boot. Nil in tests. + restart func() // acquisition is the configured indexers and download clients. acquisition *acquire.Store // catalogue is the release catalogue read from MusicBrainz, and diff --git a/internal/api/auth_restore.go b/internal/api/auth_restore.go index 8a8e39a2..f43e5ca5 100644 --- a/internal/api/auth_restore.go +++ b/internal/api/auth_restore.go @@ -1,14 +1,12 @@ package api import ( - "context" "errors" "net/http" "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" "git.fabledsword.com/bvandeusen/minstrel/internal/backup" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" - "git.fabledsword.com/bvandeusen/minstrel/internal/library" ) // SetupTokenHeader carries the setup token on a restore upload, whose body is @@ -19,20 +17,24 @@ type restoreResp struct { // Accounts is how many accounts came back; everyone signs in with the // password they had. Accounts int64 `json:"accounts"` - // ScanStarted says the library scan that refills what the backup leaves - // out is under way. - ScanStarted bool `json:"scan_started"` + // Restarting says the server restarts now to load the restored settings. + // The scan at its start finds each music file and fills in what the + // backup leaves out. + Restarting bool `json:"restarting"` } // handleRestore implements POST /api/auth/restore: a new server, with no // accounts yet, loads a backup file sent as the request body. It asks for the // setup token from the server log, as creating the first account does, since -// the backup's admins become this server's admins. Then it starts a library -// scan, which finds each file at its path and fills in what the backup left -// out. +// the backup's admins become this server's admins. A library the server's +// first scan found is replaced by the backup's. +// +// Then the server restarts. Settings are read once, at boot, so the restored +// ones (the library folders among them) take effect the same way they do +// after `minstrel restore`, and the startup scan finds each file at its path. // // 200 restored · 400 not_a_backup · 403 setup_token_invalid · -// 409 not_empty (accounts or a library already) · 409 newer_backup +// 409 not_empty (accounts already) · 409 newer_backup func (h *handlers) handleRestore(w http.ResponseWriter, r *http.Request) { ctx := r.Context() q := dbq.New(h.pool) @@ -63,7 +65,7 @@ func (h *handlers) handleRestore(w http.ResponseWriter, r *http.Request) { } switch err := backup.Restore(ctx, h.pool, contents); { case errors.Is(err, backup.ErrNotEmpty): - writeErr(w, apierror.Conflict("not_empty", "this server already has accounts or a library; a backup restores into a new one")) + writeErr(w, apierror.Conflict("not_empty", "this server already has accounts; a backup restores into a new one")) return case errors.Is(err, backup.ErrNewerSchema): writeErr(w, apierror.Conflict("newer_backup", "the backup was made by a newer Minstrel; update this server first")) @@ -82,13 +84,11 @@ func (h *handlers) handleRestore(w http.ResponseWriter, r *http.Request) { } h.logger.Info("restored a backup", "made_at", manifest.CreatedAt, "build", manifest.Build, "schema", manifest.Schema, "accounts", out.Accounts) - if h.scanner != nil { - started, _, err := library.TryStartScan(context.Background(), h.pool, h.scanner, h.coverart, - h.logger.With("source", "restore"), h.scanCfg) - if err != nil { - h.logger.Warn("restore: start the library scan", "err", err) - } - out.ScanStarted = started - } + out.Restarting = h.restart != nil writeJSON(w, http.StatusOK, out) + if h.restart != nil { + // The server drains this request before it stops, so the answer + // reaches the browser first. + h.restart() + } } diff --git a/internal/api/auth_restore_test.go b/internal/api/auth_restore_test.go index fa23be36..e70c366e 100644 --- a/internal/api/auth_restore_test.go +++ b/internal/api/auth_restore_test.go @@ -40,6 +40,8 @@ func TestRestore_ANewServerTakesABackupWithTheSetupToken(t *testing.T) { } h.setupToken = token h.requireSetupToken = true + restarts := 0 + h.restart = func() { restarts++ } restore := func(setupToken string, body []byte) *httptest.ResponseRecorder { req := httptest.NewRequest(http.MethodPost, "/api/auth/restore", bytes.NewReader(body)) req.Header.Set("Content-Type", "application/x-tar") @@ -65,14 +67,20 @@ func TestRestore_ANewServerTakesABackupWithTheSetupToken(t *testing.T) { if err := pool.QueryRow(ctx, "SELECT count(*) FROM users").Scan(&n); err != nil || n != 0 { t.Fatalf("a refused restore left %d accounts (%v)", n, err) } + if restarts != 0 { + t.Errorf("a refused restore restarted the server %d times", restarts) + } w := restore(token.Value(), file.Bytes()) if w.Code != http.StatusOK { t.Fatalf("restore: status = %d body = %s", w.Code, w.Body.String()) } var out restoreResp - if err := json.NewDecoder(w.Body).Decode(&out); err != nil || out.Accounts != 1 { - t.Errorf("restore answered %+v (%v), want one account", out, err) + if err := json.NewDecoder(w.Body).Decode(&out); err != nil || out.Accounts != 1 || !out.Restarting { + t.Errorf("restore answered %+v (%v), want one account and a restart", out, err) + } + if restarts != 1 { + t.Errorf("the restore restarted the server %d times, want once so the restored settings load", restarts) } var admin bool if err := pool.QueryRow(ctx, "SELECT is_admin FROM users WHERE username = $1", @@ -84,4 +92,7 @@ func TestRestore_ANewServerTakesABackupWithTheSetupToken(t *testing.T) { !strings.Contains(w.Body.String(), "not_empty") { t.Errorf("a second restore: status = %d body = %s, want 409 not_empty", w.Code, w.Body.String()) } + if restarts != 1 { + t.Errorf("a refused second restore restarted the server (%d restarts)", restarts) + } } diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go index fbb9164f..5727ad53 100644 --- a/internal/backup/backup_test.go +++ b/internal/backup/backup_test.go @@ -189,7 +189,16 @@ func TestARestoredInstanceHoldsWhatTheBackupHeld_Integration(t *testing.T) { t.Fatalf("restore from a newer schema: err = %v, want ErrNewerSchema", err) } - mustExec(t, pool, "TRUNCATE users, artists, library_changes CASCADE") + // A new server scans its music before anyone signs up, so it already has + // a library, here with a track the backup doesn't know. It still takes + // the backup: the backup's library replaces what the scan found, which the + // next scan finds again under the backup's ids. + mustExec(t, pool, "TRUNCATE users CASCADE") + scannedArtist := mustID(t, pool, `INSERT INTO artists (name, sort_name) VALUES ('Scanned', 'scanned') RETURNING id::text`) + scannedAlbum := mustID(t, pool, `INSERT INTO albums (title, sort_title, artist_id) VALUES ('Scanned', 'scanned', $1) RETURNING id::text`, scannedArtist) + scanned := mustID(t, pool, `INSERT INTO tracks (title, album_id, artist_id, duration_ms, file_path, file_size, file_format) + VALUES ('Scanned', $1, $2, 1000, '/music/b.flac', 10, 'flac') RETURNING id::text`, scannedAlbum, scannedArtist) + mustExec(t, pool, `INSERT INTO library_changes (entity_type, entity_id, op) VALUES ('track', $1, 'upsert')`, scanned) if err := backup.Restore(ctx, pool, first); err != nil { t.Fatalf("restore: %v", err) } diff --git a/internal/backup/restore.go b/internal/backup/restore.go index 31d191dd..46969de5 100644 --- a/internal/backup/restore.go +++ b/internal/backup/restore.go @@ -13,9 +13,8 @@ import ( ) var ( - // ErrNotEmpty is a restore into an instance that already has accounts or - // a library. - ErrNotEmpty = errors.New("backup: this Minstrel already has accounts or a library; a backup restores into an empty one") + // ErrNotEmpty is a restore into an instance that already has accounts. + ErrNotEmpty = errors.New("backup: this Minstrel already has accounts; a backup restores into a new one") // ErrNewerSchema is a backup written by a newer Minstrel than this one. ErrNewerSchema = errors.New("backup: the backup was written by a newer version of Minstrel; upgrade this one first") @@ -30,10 +29,12 @@ const batchRows = 500 // Restore loads a backup into an empty instance, in one transaction: either // all of it lands or none of it does. // -// The instance must have no accounts and no tracks. Its other rows in the -// tables the backup holds are replaced, which is what a fresh instance's -// seeded settings need. A table the backup doesn't hold, because it was -// written before that table existed, keeps what the instance has. +// The instance must have no accounts. Its rows in the tables the backup holds +// are replaced: a fresh instance's seeded settings, and the library its +// startup scan found before anyone signed up, which the scan after the +// restore finds again under the backup's ids. A table the backup doesn't +// hold, because it was written before that table existed, keeps what the +// instance has. // // A row's columns are those the backup and the table both have: an older // backup's rows take the defaults of columns added since, and a column dropped @@ -52,15 +53,15 @@ func Restore(ctx context.Context, pool *pgxpool.Pool, c Contents) error { if c.Schema > schema { return ErrNewerSchema } - // Taken before the emptiness check, so a first account or a scan starting - // at the same moment waits for the restore, then finds the data. - if _, err := tx.Exec(ctx, "LOCK TABLE users, tracks IN EXCLUSIVE MODE"); err != nil { + // Taken before the emptiness check, so a first account made at the same + // moment waits for the restore, then finds the data. A scan still running + // waits too, rather than adding to the library while it is replaced. + if _, err := tx.Exec(ctx, "LOCK TABLE users, artists, albums, tracks IN EXCLUSIVE MODE"); err != nil { return fmt.Errorf("backup: lock: %w", err) } var occupied bool - if err := tx.QueryRow(ctx, - "SELECT EXISTS (SELECT 1 FROM users) OR EXISTS (SELECT 1 FROM tracks)").Scan(&occupied); err != nil { - return fmt.Errorf("backup: check the instance is empty: %w", err) + if err := tx.QueryRow(ctx, "SELECT EXISTS (SELECT 1 FROM users)").Scan(&occupied); err != nil { + return fmt.Errorf("backup: check the instance is new: %w", err) } if occupied { return ErrNotEmpty diff --git a/internal/server/server.go b/internal/server/server.go index d452841c..c9d66873 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -104,6 +104,9 @@ type Server struct { // leave it nil leave the cookie path intact and reject all signed // tokens (HMAC of empty key matches nothing a client could mint). StreamSecret []byte + // Restart asks the process to stop and start again in place. A web + // restore calls it so the restored settings are read at boot. + Restart func() } func New(logger *slog.Logger, pool *pgxpool.Pool, scanner ScanTrigger, subCfg subsonic.Config, eventsCfg config.EventsConfig, recCfg config.RecommendationConfig, dataDir string, brandingCfg config.BrandingConfig, coverEnricher *coverart.Enricher, coverSettings *coverart.SettingsService, libraryScanner *library.Scanner, scanCfg library.RunScanConfig) *Server { @@ -208,7 +211,7 @@ func (s *Server) Router() http.Handler { if backups == nil { backups = backup.NewService(s.Pool, s.Logger, buildinfo.Version) } - api.Mount(r, s.Pool, s.Logger, writer, s.RecommendationCfg, recSettings, requestSvc, quarantineSvc, tracksSvc, playlistsSvc, s.CoverEnricher, s.CoverSettings, s.TagSettings, s.LibraryScanner, s.ScanCfg, s.DataDir, smtpSender, bus, s.PlaylistScheduler, s.StreamSecret, netSettings, reacqSettings, fpSettings, loudSettings, acoustIDLookup, backups) + api.Mount(r, s.Pool, s.Logger, writer, s.RecommendationCfg, recSettings, requestSvc, quarantineSvc, tracksSvc, playlistsSvc, s.CoverEnricher, s.CoverSettings, s.TagSettings, s.LibraryScanner, s.ScanCfg, s.DataDir, smtpSender, bus, s.PlaylistScheduler, s.StreamSecret, netSettings, reacqSettings, fpSettings, loudSettings, acoustIDLookup, backups, s.Restart) // /api/admin/scan is the only admin route owned by the server package // (it needs the Scanner). Register it as a single inline-middleware // route — using r.Route("/api/admin", ...) here would create a second diff --git a/web/src/lib/auth/store.svelte.ts b/web/src/lib/auth/store.svelte.ts index efb626a8..59351d3a 100644 --- a/web/src/lib/auth/store.svelte.ts +++ b/web/src/lib/auth/store.svelte.ts @@ -75,7 +75,7 @@ export async function register(opts: { void sendTimezoneIfStale(); } -export type RestoreResult = { accounts: number; scan_started: boolean }; +export type RestoreResult = { accounts: number; restarting: boolean }; /** * Loads a backup file into a new server, which has no accounts yet. The file diff --git a/web/src/lib/components/auth/RestoreForm.svelte b/web/src/lib/components/auth/RestoreForm.svelte index 646bb26b..fc39711e 100644 --- a/web/src/lib/components/auth/RestoreForm.svelte +++ b/web/src/lib/components/auth/RestoreForm.svelte @@ -22,7 +22,7 @@ case 'newer_backup': return 'That backup was made by a newer Minstrel. Update this server first, then restore it.'; case 'not_empty': - return 'This server already has accounts or a library, so it can’t take a backup.'; + return 'This server already has accounts, so it can’t take a backup.'; case 'too_large': return 'That file is larger than a backup can be.'; default: @@ -50,12 +50,13 @@ {#if done}

- Restored {plural(done.accounts, 'account', 'accounts')}. Sign in with the password you had. + Restored {plural(done.accounts, 'account', 'accounts')}. + {done.restarting + ? 'Minstrel is restarting to load them; in a few seconds, sign in with the password you had.' + : 'Sign in with the password you had.'}

- {done.scan_started - ? 'The library scan has started: it finds each music file and fetches the artwork and tags again.' - : 'Start a library scan under Admin › Library once you’re signed in.'} + The library scan finds each music file and fetches the artwork and tags again.

Sign in
diff --git a/web/src/routes/register/register.test.ts b/web/src/routes/register/register.test.ts index 19f0c29c..d04f3783 100644 --- a/web/src/routes/register/register.test.ts +++ b/web/src/routes/register/register.test.ts @@ -166,7 +166,7 @@ describe('/register page', () => { test('a new server restores a backup with the setup token, then sends you to sign in', async () => { (getSetupStatus as ReturnType).mockResolvedValue({ setup_required: true }); - (restoreBackup as ReturnType).mockResolvedValue({ accounts: 2, scan_started: true }); + (restoreBackup as ReturnType).mockResolvedValue({ accounts: 2, restarting: true }); render(RegisterPage); await fireEvent.click(await screen.findByRole('button', { name: /restore a backup/i })); expect(screen.getByRole('heading', { name: /restore a backup/i })).toBeInTheDocument(); @@ -179,8 +179,8 @@ describe('/register page', () => { await waitFor(() => expect(restoreBackup).toHaveBeenCalledWith(file, '0123abcd')); const done = await screen.findByTestId('restore-done'); - expect(done.textContent).toMatch(/Restored 2 accounts\. Sign in with the password you had\./); - expect(done.textContent).toMatch(/library scan has started/); + expect(done.textContent).toMatch(/Restored 2 accounts\.\s+Minstrel is restarting to load them; in a few seconds, sign in with the password you had\./); + expect(done.textContent).toMatch(/library scan finds each music file/); expect(within(done).getByRole('link', { name: /^sign in$/i })).toHaveAttribute('href', '/login'); });