fb6afd77
test(agent): the lock and the fallback now have to happen
a73x 2026-08-23 11:11
Commit message
internal/agent/run/cli_test.go
| Old | New | ||
|---|---|---|---|
| @@ -10,6 +10,7 @@ import ( | |||
| 10 | "time" | 10 | "time" |
| 11 | 11 | ||
| 12 | "github.com/a73x/eitri/internal/agent/state" | 12 | "github.com/a73x/eitri/internal/agent/state" |
| 13 | "github.com/a73x/eitri/internal/agent/statelock" | ||
| 13 | "github.com/a73x/eitri/internal/joinblob" | 14 | "github.com/a73x/eitri/internal/joinblob" |
| 14 | "github.com/stretchr/testify/assert" | 15 | "github.com/stretchr/testify/assert" |
| 15 | "github.com/stretchr/testify/require" | 16 | "github.com/stretchr/testify/require" |
| @@ -202,13 +203,40 @@ func TestJoinInvalidBlob(t *testing.T) { | |||
| 202 | } | 203 | } |
| 203 | 204 | ||
| 204 | // TestServeNotEnrolled refuses to run before the host has enrolled, before any | 205 | // TestServeNotEnrolled refuses to run before the host has enrolled, before any |
| 205 | // network or signal handler is set up. | 206 | // network or signal handler is set up. One directory, as production always has |
| 207 | // it: the store and the config must name the same state dir or the test proves | ||
| 208 | // nothing about the pairing serve is actually given. | ||
| 206 | func TestServeNotEnrolled(t *testing.T) { | 209 | func TestServeNotEnrolled(t *testing.T) { |
| 207 | st, err := state.Open(t.TempDir()) | 210 | dir := t.TempDir() |
| 211 | st, err := state.Open(dir) | ||
| 212 | require.NoError(t, err) | ||
| 213 | err = serve(st, Config{StateDir: dir}) | ||
| 214 | require.Error(t, err) | ||
| 215 | assert.Contains(t, err.Error(), "not enrolled", "an unenrolled agent has no identity to drive") | ||
| 216 | } | ||
| 217 | |||
| 218 | // TestServeRefusesASecondAgentOnTheSameState pins the first thing serve does. | ||
| 219 | // Two agents on one state directory means two hypervisor managers driving the | ||
| 220 | // same VM records and the same disks, and the control plane keeps one sync | ||
| 221 | // session per host, so the second one silently displaces the first. The lock | ||
| 222 | // must be claimed before anything else — including the identity check, which is | ||
| 223 | // why an ENROLLED-looking failure here would be the wrong error. | ||
| 224 | func TestServeRefusesASecondAgentOnTheSameState(t *testing.T) { | ||
| 225 | dir := t.TempDir() | ||
| 226 | st, err := state.Open(dir) | ||
| 227 | require.NoError(t, err) | ||
| 228 | |||
| 229 | // Stand in for the agent that is already running on this identity. | ||
| 230 | lk, err := statelock.Acquire(dir) | ||
| 208 | require.NoError(t, err) | 231 | require.NoError(t, err) |
| 209 | err = serve(st, Config{StateDir: t.TempDir()}) | 232 | defer func() { _ = lk.Release() }() |
| 233 | |||
| 234 | err = serve(st, Config{StateDir: dir}) | ||
| 210 | require.Error(t, err) | 235 | require.Error(t, err) |
| 211 | assert.True(t, strings.Contains(err.Error(), "not enrolled"), "got %q", err.Error()) | 236 | assert.Contains(t, err.Error(), "already running", |
| 237 | "serve must claim the state directory before it touches anything: an unlocked second agent drives the same VMs and disks as the first") | ||
| 238 | assert.NotContains(t, err.Error(), "not enrolled", | ||
| 239 | "the lock is claimed FIRST — reaching the identity check means the ordering serve promises has been reversed") | ||
| 212 | } | 240 | } |
| 213 | 241 | ||
| 214 | // TestJoinPinsTheBlobsCertFingerprint proves the join blob is the sole trust | 242 | // TestJoinPinsTheBlobsCertFingerprint proves the join blob is the sole trust |
internal/agent/vfkit/vfkit_test.go
| Old | New | ||
|---|---|---|---|
| @@ -400,16 +400,52 @@ func TestShutdownAsksVfkitForAGracefulStop(t *testing.T) { | |||
| 400 | assert.JSONEq(t, `{"state":"Stop"}`, body) | 400 | assert.JSONEq(t, `{"state":"Stop"}`, body) |
| 401 | } | 401 | } |
| 402 | 402 | ||
| 403 | // TestShutdownFallsBackToSigtermWhenVfkitRefuses pins the fallback itself: a | ||
| 404 | // vfkit that answers 500 has not stopped the guest, so the signal must go to | ||
| 405 | // the process named by the pidfile. Without it an unhealthy vfkit means | ||
| 406 | // Shutdown quietly does nothing and the guest runs on until someone destroys | ||
| 407 | // it — a stop that reports success and leaves the VM up. | ||
| 403 | func TestShutdownFallsBackToSigtermWhenVfkitRefuses(t *testing.T) { | 408 | func TestShutdownFallsBackToSigtermWhenVfkitRefuses(t *testing.T) { |
| 404 | p := newTestProv(t, nil) | 409 | p := newTestProv(t, nil) |
| 405 | serveREST(t, p, "vm-1", func(w http.ResponseWriter, _ *http.Request) { | 410 | serveREST(t, p, "vm-1", func(w http.ResponseWriter, _ *http.Request) { |
| 406 | w.WriteHeader(http.StatusInternalServerError) | 411 | w.WriteHeader(http.StatusInternalServerError) |
| 407 | }) | 412 | }) |
| 408 | // A pid the signal cannot reach stands in for a VM whose process is gone: | 413 | writePidfile(t, p, "vm-1", "4321\n"+p.bootID()+"\n") |
| 409 | // the fallback must report success, not an error about a dead process. | 414 | var sent []struct { |
| 410 | require.NoError(t, os.WriteFile(p.pidPath("vm-1"), []byte("2147483647"), 0o600)) | 415 | pid int |
| 416 | sig syscall.Signal | ||
| 417 | } | ||
| 418 | p.signal = func(pid int, sig syscall.Signal) error { | ||
| 419 | sent = append(sent, struct { | ||
| 420 | pid int | ||
| 421 | sig syscall.Signal | ||
| 422 | }{pid, sig}) | ||
| 423 | return nil | ||
| 424 | } | ||
| 411 | 425 | ||
| 412 | assert.NoError(t, p.Shutdown(context.Background(), "vm-1")) | 426 | require.NoError(t, p.Shutdown(context.Background(), "vm-1")) |
| 427 | |||
| 428 | require.Len(t, sent, 1, "a refused stop must reach the process, exactly once") | ||
| 429 | assert.Equal(t, 4321, sent[0].pid, "the signal goes to the pid on the VM's own pidfile") | ||
| 430 | // SIGTERM, not SIGKILL: this is still the graceful stop, and the guest is | ||
| 431 | // owed the chance to flush. Cutting the power is Destroy's job. | ||
| 432 | assert.Equal(t, syscall.SIGTERM, sent[0].sig) | ||
| 433 | } | ||
| 434 | |||
| 435 | // TestShutdownTreatsAGoneProcessAsStopped pins the tolerance the fallback | ||
| 436 | // needs: a vfkit that has already exited leaves a stale pidfile, and the signal | ||
| 437 | // comes back ESRCH. That is the outcome Shutdown wanted — the guest is not | ||
| 438 | // running — so it must not be reported as a failure, or reconcile would retry a | ||
| 439 | // stop forever on a VM that stopped. | ||
| 440 | func TestShutdownTreatsAGoneProcessAsStopped(t *testing.T) { | ||
| 441 | p := newTestProv(t, nil) | ||
| 442 | serveREST(t, p, "vm-1", func(w http.ResponseWriter, _ *http.Request) { | ||
| 443 | w.WriteHeader(http.StatusInternalServerError) | ||
| 444 | }) | ||
| 445 | writePidfile(t, p, "vm-1", "4321\n"+p.bootID()+"\n") | ||
| 446 | p.signal = func(int, syscall.Signal) error { return syscall.ESRCH } | ||
| 447 | |||
| 448 | assert.NoError(t, p.Shutdown(context.Background(), "vm-1"), "a process that is already gone is a stop that already happened") | ||
| 413 | } | 449 | } |
| 414 | 450 | ||
| 415 | // TestShutdownTakesTheConsoleDownWithTheGuest pins the second half of the | 451 | // TestShutdownTakesTheConsoleDownWithTheGuest pins the second half of the |