Files
artifactapi/internal/api/v2/errorpaths_test.go
T
unkinben e24c35f534 feat: background syncer for github_rpm remotes (#108)
## Why

Builds on #107 (merged), which derives `github_rpm` RPM metadata lazily on the
client request path, single-flighted per replica. Two problems remain: the
derive still happens per replica, so across a multi-replica deployment the same
releases are scanned and re-derived N times, multiplying GitHub queries; and a
cold cache blocks the first request on a full derive. GitHub's rate limits are
low (~60/hr unauthenticated, ~5000/hr authenticated), so this needs a single
coordinated syncer with a shared rate limit and conditional requests.

## How

- Add a single per-process background syncer (started at boot, cleanly stopped
  on shutdown) that owns a deduped/coalescing work queue, a worker pool, and one
  global token-bucket rate limiter (`golang.org/x/time/rate`) bound onto the
  github provider so every GitHub call (releases list + each ranged asset GET)
  acquires a token first.
- Re-check each `github_rpm` remote for new/changed releases on its existing
  `mutable_ttl` cadence; derive only new/changed assets incrementally and prune
  assets that disappear upstream. Repodata is served from primed DB rows.
- Prime metadata in the background on remote creation; the create call returns
  immediately.
- Send the stored releases-list `ETag` as `If-None-Match`; a `304` derives
  nothing and is not counted against GitHub's rate limit, so an unchanged repo
  is nearly free.
- Coordinate replicas through a `github_rpm_sync_state` row (`last_synced_at`,
  `etag`, `sync_lease_owner`, `sync_lease_expires`): a periodic scan runs only
  for the replica that atomically claims the lease, bounding total GitHub load
  to ~once per `mutable_ttl` regardless of replica count; the ETag is shared
  through the same row.
- Keep the request path fast: serve current cache, enqueue a prime on an empty
  cache, and return a bounded wait then a retryable `503` rather than blocking
  on a cold derive.
- Add `GITHUB_SYNC_RATE` / `GITHUB_SYNC_BURST` / `GITHUB_SYNC_WORKERS` /
  `GITHUB_SYNC_POLL_INTERVAL` config with conservative defaults (1 req/s, burst
  5, 3 workers, 60s tick) and document the syncer in the README.

## Tests

- Unit (httptest, Range/ETag-aware fixture): `304` releases response derives
  nothing; incremental derive fetches only the newly added asset; the shared
  limiter caps request rate; work-queue enqueues coalesce to one job; prime
  enqueues a job; a held lease stops a second replica from scanning; cold-start
  serves `503` while warm cache serves `200`.
- DB integration (testcontainers postgres): the real lease SQL — one holder at a
  time, recency gate blocks a too-soon periodic re-claim, prime (freshness 0)
  bypasses recency but respects a live lease.
- Docker e2e re-run: `dnf install dotvault` works; prime-on-create derives in the
  background at ~1 req/s (global limiter); `dnf makecache` served fast from the
  priming cache (no cold block); clean shutdown mid-scan, no panics.

## Notes

- Reuses `mutable_ttl` as the check interval (no new per-remote field), per brief.

Reviewed-on: #108
Co-authored-by: Ben Vincent <ben@unkin.net>
Co-committed-by: Ben Vincent <ben@unkin.net>
2026-08-10 21:31:24 +10:00

131 lines
3.5 KiB
Go

package v2
import (
"context"
"io"
"net/http"
"net/http/httptest"
"os"
"strings"
"testing"
"git.unkin.net/unkin/artifactapi/internal/database"
"git.unkin.net/unkin/artifactapi/internal/testsupport"
)
var testDSN string
func TestMain(m *testing.M) {
ctx := context.Background()
dsn, terminate, err := testsupport.StartPostgres(ctx)
if err != nil {
os.Exit(m.Run())
}
testDSN = dsn
code := m.Run()
terminate()
if code != 0 {
os.Exit(code)
}
}
// closedDB returns a DB whose pool has been closed, so every query fails —
// used to drive the handlers' error branches.
func closedDB(t *testing.T) *database.DB {
t.Helper()
if testDSN == "" {
t.Skip("Docker unavailable")
}
db, err := database.New(testDSN)
if err != nil {
t.Fatalf("new db: %v", err)
}
db.Close()
return db
}
func do(t *testing.T, h http.Handler, method, path, body string) int {
t.Helper()
var r io.Reader
if body != "" {
r = strings.NewReader(body)
}
req := httptest.NewRequest(method, path, r)
w := httptest.NewRecorder()
h.ServeHTTP(w, req)
return w.Code
}
func TestRemotesErrorPaths(t *testing.T) {
h := NewRemotesHandler(closedDB(t), nil).Routes()
if c := do(t, h, "GET", "/", ""); c != 500 {
t.Errorf("list with dead db = %d, want 500", c)
}
if c := do(t, h, "POST", "/", `{"name":"x","package_type":"generic","repo_type":"remote","base_url":"https://x"}`); c != 500 {
t.Errorf("create with dead db = %d, want 500", c)
}
if c := do(t, h, "PUT", "/x", `{"package_type":"generic","base_url":"https://x"}`); c != 500 {
t.Errorf("update with dead db = %d, want 500", c)
}
if c := do(t, h, "GET", "/x", ""); c != 404 {
t.Errorf("get missing = %d, want 404", c)
}
if c := do(t, h, "DELETE", "/x", ""); c != 500 {
t.Errorf("delete with dead db = %d, want 500", c)
}
// Bad request bodies never reach the db.
if c := do(t, h, "POST", "/", `not json`); c != 400 {
t.Errorf("invalid json = %d, want 400", c)
}
}
func TestVirtualsErrorPaths(t *testing.T) {
h := NewVirtualsHandler(closedDB(t)).Routes()
if c := do(t, h, "GET", "/", ""); c != 500 {
t.Errorf("list = %d, want 500", c)
}
if c := do(t, h, "GET", "/x", ""); c != 404 {
t.Errorf("get missing = %d, want 404", c)
}
if c := do(t, h, "POST", "/", `{"name":"v","package_type":"helm","members":["a"]}`); c != 500 {
t.Errorf("create = %d, want 500", c)
}
if c := do(t, h, "PUT", "/v", `{"package_type":"helm","members":["a"]}`); c != 500 {
t.Errorf("update = %d, want 500", c)
}
if c := do(t, h, "DELETE", "/v", ""); c != 500 {
t.Errorf("delete = %d, want 500", c)
}
}
func TestStatsErrorPaths(t *testing.T) {
h := NewStatsHandler(closedDB(t)).Routes()
for _, p := range []string{"/", "/top-remotes", "/top-files-by-hits", "/top-files-by-bandwidth"} {
if c := do(t, h, "GET", p, ""); c != 500 {
t.Errorf("stats %s = %d, want 500", p, c)
}
}
}
func TestLocalErrorPaths(t *testing.T) {
h := NewLocalHandler(closedDB(t), nil).Routes()
// GetRemote fails on the closed db -> not found.
if c := do(t, h, "PUT", "/x/files/a.bin", "data"); c != 404 {
t.Errorf("upload unknown repo = %d, want 404", c)
}
// download / remove hit the db and 500.
if c := do(t, h, "GET", "/x/files/a.bin", ""); c != 500 {
t.Errorf("download = %d, want 500", c)
}
if c := do(t, h, "DELETE", "/x/files/a.bin", ""); c != 500 {
t.Errorf("remove = %d, want 500", c)
}
}
func TestLocalHandlerDBAccessor(t *testing.T) {
db := closedDB(t)
if NewLocalHandler(db, nil).DB() != db {
t.Error("DB() should return the handler's database")
}
}