fix(servers): 同名重建服务器时旧行从头开始，不再继承上一任的主人、别名、allowlist 和回收警告 (d807fd58) · Commits · FelisMC / Felis · GitLab

internal/api/api_test.go

+6 −0

Changes for internal/api/api_test.go: 6 added lines, 0 removed lines.
Original line number	Diff line number	Diff line
		@@ -878,6 +878,12 @@ func (f *fakeRepo) SeedServer(_ context.Context, name, subdomain string, _, _, _
		if bound, ok := f.aliases[subdomain]; ok && bound != name {
		return ErrConflict
		}
		// Like the SQL: an earlier server of this name gives up its other aliases.
		for sub, bound := range f.aliases {
		if bound == name && sub != subdomain {
		delete(f.aliases, sub)
		}
		}
		f.seeded[name] = true
		f.aliases[subdomain] = name
		return nil

internal/api/pgrepo.go

+25 −6

Changes for internal/api/pgrepo.go: 25 added lines, 6 removed lines.
Original line number	Diff line number	Diff line
		@@ -637,12 +637,20 @@ func (p *PGRepo) ServerOwners(ctx context.Context) (map[string]ServerOwnership,
		}

		// SeedServer inserts the business rows backing a newly created server (spec
		// §15): the servers row (owner_id left NULL — the server is created unowned and
		// claimed later, spec §9.3) and its subdomain alias. Both inserts are
		// ON CONFLICT DO NOTHING so a retried create is idempotent. The alias subdomain
		// is a PRIMARY KEY, so a no-op insert means it was already bound; we then
		// confirm it resolves to this server and return ErrConflict otherwise, letting
		// §15): the servers row (owner_id NULL — the server is created unowned and
		// claimed later, spec §9.3) and its subdomain alias. The create handler has
		// already found no server and no world volume of this name, so a row that is
		// here belongs to an earlier server of the same name: one removed with kubectl,
		// or a create whose CRD write failed. That row starts over, and the earlier
		// server's other aliases and allowlist go with it; nothing of its owner, claim,
		// activity clock or reaper warnings reaches the new server. A retried create
		// lands on the same fresh state. The alias subdomain is a PRIMARY KEY: bound to
		// another server, it rolls the whole seed back and returns ErrConflict, letting
		// the create handler answer 409 before it touches the CRD.
		//
		// Two creates of one name racing between the handler's cluster check and the
		// first CRD write can still leave the loser's alias in place of the winner's;
		// that window is the create path's documented non-transactional tradeoff.
		func (p *PGRepo) SeedServer(ctx context.Context, name, subdomain string, cpuMilli, memoryMB, storageMB int) error {
		tx, err := p.db.BeginTx(ctx, nil)
		if err != nil {
		@@ -651,9 +659,20 @@ func (p *PGRepo) SeedServer(ctx context.Context, name, subdomain string, cpuMill
		defer tx.Rollback() //nolint:errcheck // no-op after commit

		if _, err := tx.ExecContext(ctx,
		`INSERT INTO servers (name, cached_cpu_milli, cached_memory_mb, cached_storage_mb) VALUES ($1, $2, $3, $4) ON CONFLICT (name) DO UPDATE SET cached_cpu_milli = EXCLUDED.cached_cpu_milli, cached_memory_mb = EXCLUDED.cached_memory_mb, cached_storage_mb = EXCLUDED.cached_storage_mb`, name, cpuMilli, memoryMB, storageMB); err != nil {
		`INSERT INTO servers (name, cached_cpu_milli, cached_memory_mb, cached_storage_mb) VALUES ($1, $2, $3, $4)
		ON CONFLICT (name) DO UPDATE SET owner_id = NULL, claimed_at = NULL, last_active_at = now(),
		warned_3d_at = NULL, warned_1d_at = NULL, cached_phase = NULL, created_at = now(), deleted_at = NULL,
		cached_cpu_milli = EXCLUDED.cached_cpu_milli, cached_memory_mb = EXCLUDED.cached_memory_mb,
		cached_storage_mb = EXCLUDED.cached_storage_mb`,
		name, cpuMilli, memoryMB, storageMB); err != nil {
		return fmt.Errorf("seed server row: %w", err)
		}
		if _, err := tx.ExecContext(ctx, `DELETE FROM server_allowlist WHERE server_name = $1`, name); err != nil {
		return fmt.Errorf("clear an earlier allowlist: %w", err)
		}
		if _, err := tx.ExecContext(ctx, `DELETE FROM server_aliases WHERE server_name = $1`, name); err != nil {
		return fmt.Errorf("clear earlier aliases: %w", err)
		}
		if _, err := tx.ExecContext(ctx,
		`INSERT INTO server_aliases (subdomain, server_name) VALUES ($1, $2) ON CONFLICT DO NOTHING`,
		subdomain, name); err != nil {

internal/api/repo.go

+7 −4

Changes for internal/api/repo.go: 7 added lines, 4 removed lines.
Original line number	Diff line number	Diff line
		@@ -387,10 +387,13 @@ type Repo interface {
		BackupStoreBytes(ctx context.Context) (int64, error)
		// SeedServer inserts the business-layer rows for a newly created server (spec
		// §15): a servers row (owner_id NULL — claimed later, spec §9.3) and its
		// subdomain alias, both idempotent. The resource cache (cpuMilli, memoryMB,
		// storageMB) is seeded alongside so QuotaCheck can aggregate per-owner usage
		// without cross-system CRD reads. It returns ErrConflict if the subdomain is
		// already bound to a different server.
		// subdomain alias. A row left by an earlier server of the same name starts over:
		// no owner, claim, activity clock, reaper warnings, other aliases or allowlist
		// carry over, and a retried create lands on the same fresh state. The resource
		// cache (cpuMilli, memoryMB, storageMB) is seeded alongside so QuotaCheck can
		// aggregate per-owner usage without cross-system CRD reads. It returns
		// ErrConflict, and changes nothing, if the subdomain is already bound to a
		// different server.
		SeedServer(ctx context.Context, name, subdomain string, cpuMilli, memoryMB, storageMB int) error
		// UpdateServerResources updates the resource cache columns for a server
		// after a spec mutation (spec §7 PATCH), so the per-owner aggregate stays in

internal/pgint/pgint_test.go

+92 −0

Changes for internal/pgint/pgint_test.go: 92 added lines, 0 removed lines.
Original line number	Diff line number	Diff line
		@@ -626,6 +626,98 @@ func TestSetQuotasReplacesEveryCap(t *testing.T) {
		}
		}

		// A server removed with kubectl leaves its servers row, aliases and allowlist
		// behind, and the create handler lets its name be reused once the world volume is
		// gone too. SeedServer used to refresh only the resource cache, so the new server
		// came up owned by the earlier owner (and charged to their quota), reachable at
		// the earlier subdomain, with the earlier allowlist, reaper warnings and even the
		// earlier deleted_at. The new server must start clean.
		func TestSeedServerReusedNameStartsClean(t *testing.T) {
		ctx := context.Background()
		u := newUser(t, "user", "seedold")
		name, oldSub, oldSub2, newSub := "sd-"+suffix(t), "sdo-"+suffix(t), "sdp-"+suffix(t), "sdn-"+suffix(t)
		exec := func(q string, args ...any) {
		t.Helper()
		if _, err := db.ExecContext(ctx, q, args...); err != nil {
		t.Fatalf("%s: %v", q, err)
		}
		}
		if err := repo.SeedServer(ctx, name, oldSub, 1000, 2048, 10240); err != nil {
		t.Fatalf("seed the earlier server: %v", err)
		}
		past := time.Now().Add(-90 * 24 * time.Hour)
		exec(`UPDATE servers SET owner_id = $2, claimed_at = $3, last_active_at = $3, warned_3d_at = $3,
		warned_1d_at = $3, cached_phase = 'Running', created_at = $3, deleted_at = $3 WHERE name = $1`, name, u.ID, past)
		exec(`INSERT INTO server_aliases (subdomain, server_name) VALUES ($1, $2)`, oldSub2, name)
		exec(`INSERT INTO server_allowlist (server_name, mc_uuid) VALUES ($1, $2)`, name, testUUID(t))

		state := func() string {
		t.Helper()
		var owner, phase sql.NullString
		var claimed, w3, w1, deleted sql.NullTime
		var created, active time.Time
		var cpu, mem, stor, allow int
		var aliases string
		if err := db.QueryRowContext(ctx,
		`SELECT owner_id, claimed_at, warned_3d_at, warned_1d_at, cached_phase, deleted_at, created_at, last_active_at,
		cached_cpu_milli, cached_memory_mb, cached_storage_mb,
		(SELECT count(*) FROM server_allowlist WHERE server_name = s.name),
		(SELECT COALESCE(string_agg(subdomain, ',' ORDER BY subdomain), '') FROM server_aliases WHERE server_name = s.name)
		FROM servers s WHERE name = $1`, name).Scan(
		&owner, &claimed, &w3, &w1, &phase, &deleted, &created, &active, &cpu, &mem, &stor, &allow, &aliases); err != nil {
		t.Fatalf("read the servers row: %v", err)
		}
		recent := func(at time.Time) bool { return time.Since(at) < time.Hour }
		return fmt.Sprintf("owner=%v claimed=%v warned=%v/%v phase=%v deleted=%v fresh=%v/%v cache=%d/%d/%d allow=%d aliases=%s",
		owner.Valid, claimed.Valid, w3.Valid, w1.Valid, phase.Valid, deleted.Valid, recent(created), recent(active),
		cpu, mem, stor, allow, strings.ReplaceAll(strings.ReplaceAll(aliases, oldSub2, "old2"), oldSub, "old"))
		}
		earlier := "owner=true claimed=true warned=true/true phase=true deleted=true fresh=false/false cache=1000/2048/10240 allow=1 aliases=old,old2"
		if got := state(); got != earlier {
		t.Fatalf("setup: %s, want %s", got, earlier)
		}

		// The new subdomain is bound to another server: the seed changes nothing.
		other, otherSub := "sdz-"+suffix(t), "sdzs-"+suffix(t)
		if err := repo.SeedServer(ctx, other, otherSub, 100, 128, 1024); err != nil {
		t.Fatalf("seed the other server: %v", err)
		}
		if err := repo.SeedServer(ctx, name, otherSub, 2000, 4096, 20480); !errors.Is(err, api.ErrConflict) {
		t.Fatalf("seed onto a subdomain another server holds = %v, want ErrConflict", err)
		}
		if got := state(); got != earlier {
		t.Fatalf("a refused seed changed the row: %s, want %s", got, earlier)
		}

		clean := "owner=false claimed=false warned=false/false phase=false deleted=false fresh=true/true cache=2000/4096/20480 allow=0 aliases=" + newSub
		for i := 0; i < 2; i++ { // a retried create lands on the same state
		if err := repo.SeedServer(ctx, name, newSub, 2000, 4096, 20480); err != nil {
		t.Fatalf("seed the new server (try %d): %v", i+1, err)
		}
		if got := state(); got != clean {
		t.Fatalf("after seeding the new server (try %d): %s, want %s", i+1, got, clean)
		}
		}
		rec, err := repo.ServerByName(ctx, name)
		if err != nil \|\| rec.OwnerID != "" \|\| rec.Subdomain != newSub {
		t.Fatalf("ServerByName = %+v, %v; want the new server, unowned, at %s", rec, err, newSub)
		}
		for _, sub := range []string{oldSub, oldSub2} {
		var n int
		if err := db.QueryRowContext(ctx, `SELECT count(*) FROM server_aliases WHERE subdomain = $1`, sub).Scan(&n); err != nil \|\| n != 0 {
		t.Fatalf("the earlier subdomain %s still resolves (%d rows, %v)", sub, n, err)
		}
		}

		// The earlier server's own subdomain may come back with it.
		if err := repo.SeedServer(ctx, name, oldSub, 2000, 4096, 20480); err != nil {
		t.Fatalf("seed at the earlier subdomain: %v", err)
		}
		if got, want := state(), strings.Replace(clean, "aliases="+newSub, "aliases=old", 1); got != want {
		t.Fatalf("after seeding at the earlier subdomain: %s, want %s", got, want)
		}
		}

		// The fleet read needs every live server's claim state: the owner's id to tell
		// the caller's own servers apart, the display name (email, else username), and
		// the unclaimed rows too, since only those may be claimed. A soft-deleted row is