slice 7 Phase 1: unified bring-up reconcile job (provision + guest-loss DR) (v0.8.0)

The shared front half of provision and guest-loss DR as a journaled reconcile job
(internal/reconcile/bringup.go), mirroring the restore-test's crash-safety but keeping
the guest on success and applying a scenario-specific identity policy. Agent-only; no
hub/wire change. Grounded by the slice-7 bring-up spike (commit 3342993): F1/F3/F4.

- RunBringUp: restore -> reset identity -> size -> attach mounts -> start link-up;
  verdict is liveness (waitRunning), success KEEPS the guest.
- identity policy: provision = fresh MAC (net0 sans hwaddr -> PVE regen) + hostname,
  host-side; machine-id/host-keys regenerate guest-side (systemd + baked golden unit).
  dr_guest_loss = preserve continuity (keep hostname; keep MAC unless KeepMAC=false).
- compensating rollback: mid-flight failure destroys the just-created guest
  (SameTxnCreated provenance, gated); new Rollback journal flag + Recover.recoverBringUp
  reap a half-built guest from a crash.
- F4: coalesced config PUT + bounded retry on the transient PVE config-lock 500 only.
- --selftest=bring-up (mode/archive/vmid/hostname/keep).
- configs/build-golden.sh: validated golden recipe incl. the F3 first-boot host-key unit.
- doc-03 §9 + identity-reset settled/implemented.

Deferred (stated): provisioning back half -> slice 8; host-loss DR + escrow consumption
and the BringUpSpec source (hub desired-state) -> slice 10.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-06-09 21:27:49 +02:00
parent 9f6753de0f
commit 57405c1a99
10 changed files with 1184 additions and 73 deletions
+393
View File
@@ -0,0 +1,393 @@
package reconcile
import (
"context"
"errors"
"fmt"
"strconv"
"strings"
"time"
"gitea.dooplex.hu/admin/felhom-agent/internal/proxmox"
)
// The unified bring-up reconcile job (doc 03 §9, slice 7 Phase 1). It is the shared FRONT
// HALF of provision and guest-loss DR: restore an archive → reset identity (scenario-specific)
// → size → attach mounts → start LINK-UP. It mirrors RunRestoreTest's crash-safety (journal the
// owning entry BEFORE any mutation; Recover reaps a leak via ListLXC; idempotent, fail-safe) but
// DIFFERS in two load-bearing ways: it KEEPS the guest on success (no teardown), and its
// identity policy is scenario-specific (provision = fresh identity; DR = preserve continuity).
//
// Grounded by documentation/tests/slice7-bringup-spike-findings.md (commit 3342993): F1 (restore
// preserves the archived MAC → provision MUST reset it), F3 (machine-id + SSH host keys
// regenerate guest-side on first boot from the clean golden + its baked unit — the agent does
// NOT touch guest internals here), F4 (a transient PVE config-lock 500 → bounded retry).
//
// Out of scope (deferred — see REPORT): the provisioning BACK HALF (controller deploy, bootstrap,
// per-guest token mint → slice 8); host-loss DR + escrow consumption → slice 10; the SOURCE of a
// BringUpSpec (hub desired-state) → slice 10 (this job takes the spec as input).
// BringUpMode selects the identity policy + archive semantics.
type BringUpMode string
const (
// ModeProvision: from the golden base — a NEW guest, so reset identity fully.
ModeProvision BringUpMode = "provision"
// ModeDRGuestLoss: from a customer backup — CONTINUE the customer's world, so preserve
// continuity identity (hostname, host keys, and by default MAC), reset only what collides.
ModeDRGuestLoss BringUpMode = "dr_guest_loss"
)
const bringUpKind = "bring_up"
// configLockMaxAttempts bounds the F4 config-lock retry. configLockBackoff is a package var so
// tests can shrink it (the production value gives PVE time to release its async config lock).
const configLockMaxAttempts = 5
var configLockBackoff = 1500 * time.Millisecond
// GuestMount is one additive mountpoint to attach as mpN. Defined minimally now; slice 10 wires
// the hub storage manifest into this (do NOT couple to a not-yet-existing hub desired-state type).
type GuestMount struct {
Storage string // PVE storage id (e.g. "local-lvm")
SizeGB int // new-volume size in GiB
MountPoint string // in-guest path (e.g. "/mnt/data")
}
// BringUpSpec is the input to one bring-up. The caller resolves it (the selftest, or slice-10
// hub desired-state); this job does not decide WHAT to provision.
type BringUpSpec struct {
Mode BringUpMode // provision | dr_guest_loss
Archive string // source volid (golden for provision; customer backup for DR)
VMID int // caller-provided target VMID (NOT the restore-test band / 9999)
RestoreStorage string // rootfs target storage
Hostname string // hostname to set (provision); ignored for DR (continuity)
Cores int // 0 = leave as restored
MemoryMB int // 0 = leave as restored
RootfsGrowGB int // optional grow-only rootfs resize (0 = skip)
Mounts []GuestMount // additive mpN mounts (slice 7 may pass empty/test)
KeepMAC bool // DR knob: keep the archived MAC (true) unless a source may be live
BootTimeout time.Duration // 0 → DefaultBootTimeout; bounds the link-up liveness wait
}
// BringUpResult is the outcome. It reuses the restore-test's WARNINGS surface
// (StartWarnings/WarningsRecognized) — the start step here is the same liveness-anchored boot.
type BringUpResult struct {
VMID int
AssignedMAC string // the guest's net0 MAC after identity reset (fresh for provision)
Hostname string
Pass bool
Verified string // "boot+running"
Err error
StartedAt time.Time
Duration time.Duration
StartWarnings []string
WarningsRecognized bool
}
// IntentForRollbackDestroy builds the benign compensating-rollback teardown intent for a guest
// the agent created earlier in THIS journaled bring-up transaction: ClassGuestDestroy made benign
// by SameTxnCreated provenance (classify.go) — a rollback, not data loss. Gate-authorized unsigned
// but genuinely in-path (wrong provenance → pending_signature). Distinct from the restore-test's
// IntentForScratchDestroy (AgentTaggedScratch) — different audit label, same destroy machinery.
func IntentForRollbackDestroy(hostID string, vmid int) Intent {
return Intent{
Class: ClassGuestDestroy,
HostID: hostID,
GuestID: strconv.Itoa(vmid),
VMID: vmid,
Provenance: Provenance{SameTxnCreated: true},
Source: SourceOneShotJob,
}
}
// RunBringUp runs one bring-up on the target VMID's queue lane (inherits §10 per-guest
// serialization). On success the guest is KEPT; on any mid-flight failure it is
// compensating-rolled-back (destroyed). The returned Err is the job verdict's error.
func (e *Engine) RunBringUp(ctx context.Context, spec BringUpSpec) BringUpResult {
now := time.Now().UTC()
res := BringUpResult{VMID: spec.VMID, Hostname: spec.Hostname, StartedAt: now}
if spec.Archive == "" || spec.RestoreStorage == "" {
res.Err = fmt.Errorf("reconcile: bring-up needs an archive and a restore storage")
return res
}
if spec.VMID <= 0 {
res.Err = fmt.Errorf("reconcile: bring-up needs a target VMID")
return res
}
// Fence the reserved bands: never provision over the standing scratch (9999) or the
// restore-test scratch band (990000990009).
if spec.VMID == 9999 || (spec.VMID >= 990000 && spec.VMID <= 990009) {
res.Err = fmt.Errorf("reconcile: bring-up VMID %d is reserved (9999 / restore-test scratch band)", spec.VMID)
return res
}
switch spec.Mode {
case ModeProvision, ModeDRGuestLoss:
default:
res.Err = fmt.Errorf("reconcile: bring-up unknown mode %q", spec.Mode)
return res
}
ch := e.queue.Submit(spec.VMID, func() error {
e.runBringUp(ctx, spec, &res)
return res.Err
})
<-ch
res.Duration = time.Since(now)
return res
}
// runBringUp is the journaled body (runs on the target VMID's queue lane).
func (e *Engine) runBringUp(ctx context.Context, spec BringUpSpec, res *BringUpResult) {
// PROVISION is to a NEW VMID. Restoring OVER an existing guest is ClassRestoreOverwrite
// (destructive, slice 10) — never this benign path. Refuse before journaling.
lxc, err := e.api.ListLXC(ctx)
if err != nil {
res.Err = fmt.Errorf("reconcile: bring-up list guests: %w", err)
return
}
for _, g := range lxc {
if g.VMID == spec.VMID {
res.Err = fmt.Errorf("reconcile: bring-up VMID %d already exists (restore-over-existing is a signed op)", spec.VMID)
return
}
}
base := JournalEntry{OpID: e.bringUpOpID(spec.VMID), VMID: spec.VMID, Kind: bringUpKind, Rollback: true}
// OWN the rollback BEFORE any mutation. From here a crash leaves an in-flight Rollback
// entry meaning "VMID may be a half-built guest → destroy it" (Recover.recoverBringUp).
e.append(withState(base, OpStarted))
// Compensating rollback on EVERY non-committed exit (defer): destroy the just-created
// guest. On success we set committed and KEEP it (the key difference from the restore-test).
committed := false
defer func() {
if committed {
return
}
e.rollbackBringUp(ctx, base)
}()
// 1. Restore archive → VMID (token-covered ClassCreate; keyctl preserved — phase3 + spike).
upid, err := e.api.RestoreLXC(ctx, proxmox.RestoreLXCOptions{
VMID: spec.VMID, Archive: spec.Archive, Storage: spec.RestoreStorage,
})
if err != nil {
res.Err = fmt.Errorf("reconcile: bring-up restore: %w", err)
return
}
e.append(withUPID(base, upid, OpTaskRunning))
if _, err := e.waitTask(ctx, upid, proxmox.WaitOptions{}); err != nil {
res.Err = fmt.Errorf("reconcile: bring-up restore task: %w", err)
return
}
// 2. Read as-restored config (the net0 the MAC handling keys off).
cfg, err := e.api.GuestConfig(ctx, spec.VMID)
if err != nil {
res.Err = fmt.Errorf("reconcile: bring-up read config: %w", err)
return
}
// 3+5 coalesced (F4): identity reset + cores/mem sizing + additive mounts in ONE config PUT,
// with the bounded retry that fires ONLY on the transient PVE config-lock 500.
if params := buildBringUpConfig(spec, cfg); len(params) > 0 {
if err := e.setConfigWithLockRetry(ctx, spec.VMID, params); err != nil {
res.Err = fmt.Errorf("reconcile: bring-up config: %w", err)
return
}
}
// 4. rootfs grow-only resize as its OWN call (F4: kept separate from the config PUT).
if spec.RootfsGrowGB > 0 {
rupid, err := e.api.ResizeLXC(ctx, spec.VMID, "rootfs", fmt.Sprintf("+%dG", spec.RootfsGrowGB))
if err != nil {
res.Err = fmt.Errorf("reconcile: bring-up resize: %w", err)
return
}
if _, err := e.waitTask(ctx, rupid, proxmox.WaitOptions{}); err != nil {
res.Err = fmt.Errorf("reconcile: bring-up resize task: %w", err)
return
}
}
// Capture the post-reset MAC for the result (fresh for provision; archived for DR keep).
if cfg2, err := e.api.GuestConfig(ctx, spec.VMID); err == nil {
res.AssignedMAC = net0MAC(cfg2)
}
// 6. Start LINK-UP (ClassStart). The VERDICT is liveness (waitRunning), NEVER the start
// exitstatus — same liveness-anchoring as the restore-test fix; AllowWarnings so a benign
// start advisory (systemd-nesting) is surfaced, not failed.
startUPID, err := e.api.Start(ctx, spec.VMID)
if err != nil {
res.Err = fmt.Errorf("reconcile: bring-up start: %w", err)
return
}
if startUPID != "" {
st, err := e.api.WaitTask(ctx, startUPID, proxmox.WaitOptions{AllowWarnings: true})
if err != nil {
res.Err = fmt.Errorf("reconcile: bring-up start task: %w", err)
return
}
if strings.HasPrefix(st.ExitStatus, "WARNINGS") {
tail, logErr := e.api.TaskLogTail(ctx, startUPID, 50)
if logErr != nil {
e.logger.Warn("bring-up: could not read start-task log for warnings", "vmid", spec.VMID, "err", logErr)
}
res.StartWarnings = extractWarningLines(tail)
res.WarningsRecognized = warningsRecognized(res.StartWarnings)
}
}
bootTO := spec.BootTimeout
if bootTO <= 0 {
bootTO = DefaultBootTimeout
}
if err := e.waitRunning(ctx, spec.VMID, bootTO); err != nil {
res.Err = err
return
}
// 7. Success — KEEP the guest; mark the owning entry terminal so Recover ignores it.
res.Pass = true
res.Verified = "boot+running"
committed = true
e.append(withState(base, OpSucceeded))
}
// rollbackBringUp destroys the just-created guest (benign ClassGuestDestroy via SameTxnCreated
// provenance) and records the owning entry terminal. Mirrors teardownScratch: ALWAYS attempts the
// destroy (idempotent — a restore-POST failure that created no guest just errors harmlessly and is
// left in-flight for Recover, which existence-checks). On any teardown failure it leaves the entry
// in-flight so Recover reaps the guest later — never force-destroys.
func (e *Engine) rollbackBringUp(ctx context.Context, base JournalEntry) {
tctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 2*time.Minute)
defer cancel()
dec := e.gate.Authorize(IntentForRollbackDestroy(e.hostID, base.VMID), nil)
if !dec.Allowed {
e.logger.Error("bring-up: rollback destroy refused by gate (unexpected); left for Recover",
"vmid", base.VMID, "reason", dec.Reason)
return
}
upid, err := e.api.DestroyLXC(tctx, base.VMID)
if err != nil {
e.logger.Error("bring-up: rollback destroy failed; left for Recover", "vmid", base.VMID, "err", err)
return
}
if _, err := e.waitTask(tctx, upid, proxmox.WaitOptions{}); err != nil {
e.logger.Error("bring-up: rollback destroy task failed; left for Recover", "vmid", base.VMID, "err", err)
return
}
e.append(withState(base, OpSucceeded))
e.logger.Warn("bring-up: rolled back (destroyed half-built guest)", "vmid", base.VMID)
}
// buildBringUpConfig assembles the coalesced config PUT params per the scenario-specific identity
// policy (doc 03 §9). Provision: fresh MAC (strip hwaddr → PVE regenerates, F1) + hostname.
// DR: preserve continuity — keep MAC (unless KeepMAC=false: a source may be live) and keep
// hostname (no force-reset). Both: cores/mem sizing + additive mpN mounts. machine-id and SSH
// host keys are NOT touched here — they regenerate guest-side on first boot (golden bake + the
// baked first-boot unit), keeping the agent's front half host-side-only.
func buildBringUpConfig(spec BringUpSpec, cfg proxmox.GuestConfig) map[string]string {
params := map[string]string{}
resetMAC := spec.Mode == ModeProvision || (spec.Mode == ModeDRGuestLoss && !spec.KeepMAC)
if resetMAC {
if net0, ok := cfg.Nets()["net0"]; ok && net0 != "" {
params["net0"] = withoutHwaddr(net0) // omit hwaddr → PVE generates a fresh MAC (F1)
}
}
if spec.Mode == ModeProvision && spec.Hostname != "" {
params["hostname"] = spec.Hostname
}
if spec.Cores > 0 {
params["cores"] = strconv.Itoa(spec.Cores)
}
if spec.MemoryMB > 0 {
params["memory"] = strconv.Itoa(spec.MemoryMB)
}
for i, m := range spec.Mounts {
params[fmt.Sprintf("mp%d", i)] = fmt.Sprintf("%s:%d,mp=%s", m.Storage, m.SizeGB, m.MountPoint)
}
return params
}
// setConfigWithLockRetry issues the coalesced config PUT, retrying ONLY the transient PVE
// config-lock 500 (F4) with bounded backoff. A non-lock error (any other 500 included) fails
// immediately — never retry a real error. The slice-4 per-guest serializer prevents cross-op
// contention; this covers PVE releasing its own async config lock within one job.
func (e *Engine) setConfigWithLockRetry(ctx context.Context, vmid int, params map[string]string) error {
var lastErr error
for attempt := 1; attempt <= configLockMaxAttempts; attempt++ {
if _, err := e.api.SetConfig(ctx, vmid, params); err == nil {
return nil
} else if !pveConfigLock(err) {
return err // real error — never retry
} else {
lastErr = err
e.logger.Warn("bring-up: transient PVE config-lock; retrying",
"vmid", vmid, "attempt", attempt, "max", configLockMaxAttempts, "err", err)
}
select {
case <-ctx.Done():
return ctx.Err()
case <-time.After(configLockBackoff):
}
}
return fmt.Errorf("reconcile: bring-up config still lock-contended after %d attempts: %w", configLockMaxAttempts, lastErr)
}
// pveConfigLock reports whether err is the transient PVE config-lock 500 (F4) — and ONLY that.
// The lock surfaces as a proxmox.APIError 500 whose body carries the lock signature.
func pveConfigLock(err error) bool {
var ae *proxmox.APIError
if !errors.As(err, &ae) || ae.StatusCode != 500 {
return false
}
b := strings.ToLower(ae.Body)
return strings.Contains(b, "can't lock file") || strings.Contains(b, "got timeout")
}
// waitTask waits a (possibly empty) UPID — "" is the clean synchronous path.
func (e *Engine) waitTask(ctx context.Context, upid string, opts proxmox.WaitOptions) (proxmox.TaskStatus, error) {
if upid == "" {
return proxmox.TaskStatus{}, nil
}
return e.api.WaitTask(ctx, upid, opts)
}
func (e *Engine) bringUpOpID(vmid int) string {
return "bring-up-" + strconv.Itoa(vmid) + "-" + nextSeq(&e.opSeq)
}
// withoutHwaddr strips the hwaddr token from a netN config string. PUTting a netN with NO hwaddr
// makes PVE generate a fresh MAC (slice-7 spike F1: a restore preserves the archived MAC, so a
// provision MUST strip it to avoid a fleet-wide MAC collision). Mirrors withLinkDown's approach.
func withoutHwaddr(netN string) string {
parts := strings.Split(netN, ",")
out := parts[:0]
for _, p := range parts {
if p == "" || strings.HasPrefix(p, "hwaddr=") {
continue
}
out = append(out, p)
}
return strings.Join(out, ",")
}
// net0MAC extracts the hwaddr from a guest's net0 config ("" if absent).
func net0MAC(cfg proxmox.GuestConfig) string {
net0, ok := cfg.Nets()["net0"]
if !ok {
return ""
}
for _, p := range strings.Split(net0, ",") {
if strings.HasPrefix(p, "hwaddr=") {
return strings.TrimPrefix(p, "hwaddr=")
}
}
return ""
}
+382
View File
@@ -0,0 +1,382 @@
package reconcile
import (
"context"
"errors"
"strings"
"testing"
"time"
"gitea.dooplex.hu/admin/felhom-agent/internal/proxmox"
)
// setParamsFor returns the params of the (last) SetConfig call against vmid, or nil.
func setParamsFor(api *fakeAPI, vmid int) map[string]string {
var out map[string]string
for _, s := range api.sets {
if s.vmid == vmid {
out = s.params
}
}
return out
}
func TestRunBringUp_ProvisionHappyPath(t *testing.T) {
const vmid = 8000
api := &fakeAPI{cfg: map[int]proxmox.GuestConfig{vmid: scratchCfg()}} // empty lxc → vmid free; running default
e, _, q := newEngine(t, api, EmptyProvider{})
defer q.Close()
res := e.RunBringUp(context.Background(), BringUpSpec{
Mode: ModeProvision, Archive: "local:backup/golden.tar.zst", VMID: vmid,
RestoreStorage: "local-lvm", Hostname: "felhom-prov-8000",
Cores: 2, MemoryMB: 2048, RootfsGrowGB: 2,
Mounts: []GuestMount{{Storage: "local-lvm", SizeGB: 1, MountPoint: "/mnt/data"}},
})
if res.Err != nil || !res.Pass {
t.Fatalf("provision must pass, got %+v", res)
}
if res.Verified != "boot+running" {
t.Errorf("verified = %q", res.Verified)
}
// restore issued to the target vmid.
if len(api.restores) != 1 || api.restores[0].VMID != vmid || api.restores[0].Archive != "local:backup/golden.tar.zst" {
t.Fatalf("restore not issued correctly: %+v", api.restores)
}
// identity: a fresh MAC (net0 set WITHOUT hwaddr) + hostname, coalesced with sizing+mount.
p := setParamsFor(api, vmid)
if p == nil {
t.Fatal("expected a coalesced config PUT")
}
if net0, ok := p["net0"]; !ok || strings.Contains(net0, "hwaddr=") {
t.Errorf("provision must reset MAC: net0 must be set WITHOUT hwaddr, got %q", net0)
}
if p["hostname"] != "felhom-prov-8000" {
t.Errorf("hostname not set: %q", p["hostname"])
}
if p["cores"] != "2" || p["memory"] != "2048" {
t.Errorf("sizing not coalesced: cores=%q memory=%q", p["cores"], p["memory"])
}
if p["mp0"] != "local-lvm:1,mp=/mnt/data" {
t.Errorf("mount not attached: mp0=%q", p["mp0"])
}
// rootfs grow is a SEPARATE call (F4).
if len(api.resizes) != 1 || api.resizes[0].vmid != vmid || api.resizes[0].size != "+2G" {
t.Errorf("rootfs grow not issued separately: %+v", api.resizes)
}
// started link-up.
if len(api.starts) != 1 || api.starts[0] != vmid {
t.Errorf("guest not started: %+v", api.starts)
}
// THE key difference from the restore-test: the guest is KEPT (no teardown).
if len(api.destroys) != 0 {
t.Fatalf("provision success must NOT destroy the guest: %+v", api.destroys)
}
}
func TestRunBringUp_CompensatingRollback(t *testing.T) {
const vmid = 8000
lockBackoffFast(t)
cases := []struct {
name string
setup func(*fakeAPI)
}{
{"restore error", func(a *fakeAPI) { a.restoreErr = errors.New("restore boom") }},
{"config real error", func(a *fakeAPI) {
a.setFunc = func(int, map[string]string) (string, error) {
return "", &proxmox.APIError{StatusCode: 500, Body: "some non-lock internal error"}
}
}},
{"start-task real error", func(a *fakeAPI) {
a.startUPID = "UPID:demo:start:8000:"
a.waitFunc = func(upid string) (proxmox.TaskStatus, error) {
if upid == "UPID:demo:start:8000:" {
return proxmox.TaskStatus{}, errors.New("start task failed")
}
return proxmox.TaskStatus{Status: "stopped", ExitStatus: "OK"}, nil
}
}},
{"waitRunning timeout", func(a *fakeAPI) {
a.status = map[int]proxmox.Guest{vmid: {VMID: vmid, Status: "stopped"}}
}},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
api := &fakeAPI{cfg: map[int]proxmox.GuestConfig{vmid: scratchCfg()}}
tc.setup(api)
e, j, q := newEngine(t, api, EmptyProvider{})
defer q.Close()
res := e.RunBringUp(context.Background(), BringUpSpec{
Mode: ModeProvision, Archive: "vol", VMID: vmid, RestoreStorage: "local-lvm",
Hostname: "h", BootTimeout: 40 * time.Millisecond,
})
if res.Pass || res.Err == nil {
t.Fatalf("must fail, got %+v", res)
}
// The adversarial point: the just-created guest was ACTUALLY destroyed.
if len(api.destroys) != 1 || api.destroys[0] != vmid {
t.Fatalf("compensating rollback must destroy the guest: destroys=%+v", api.destroys)
}
// And the owning entry is terminal (rollback complete) — not left in-flight.
if len(j.InFlight()) != 0 {
t.Errorf("owning entry must be terminal after rollback: %+v", j.InFlight())
}
})
}
}
func TestRunBringUp_DRPreservesContinuityIdentity(t *testing.T) {
const vmid = 8001
api := &fakeAPI{cfg: map[int]proxmox.GuestConfig{vmid: scratchCfg()}}
e, _, q := newEngine(t, api, EmptyProvider{})
defer q.Close()
res := e.RunBringUp(context.Background(), BringUpSpec{
Mode: ModeDRGuestLoss, Archive: "local:backup/customer.tar.zst", VMID: vmid,
RestoreStorage: "local-lvm", Hostname: "ignored-in-dr", KeepMAC: true,
Cores: 4, // a benign config PUT happens, so we can inspect it has NO identity resets
})
if res.Err != nil || !res.Pass {
t.Fatalf("dr bring-up must pass, got %+v", res)
}
p := setParamsFor(api, vmid)
// DR continuity: MAC kept (no net0 reset) and hostname NOT force-reset.
if p != nil {
if _, ok := p["net0"]; ok {
t.Errorf("dr+KeepMAC must NOT reset net0 (keep the archived MAC): %+v", p)
}
if _, ok := p["hostname"]; ok {
t.Errorf("dr must NOT force-reset hostname (continuity): %+v", p)
}
if p["cores"] != "4" {
t.Errorf("benign sizing should still apply: cores=%q", p["cores"])
}
}
// AssignedMAC reflects the kept archived MAC (from scratchCfg's net0).
if res.AssignedMAC != "AA:BB:CC:DD:EE:FF" {
t.Errorf("dr should keep the archived MAC, got %q", res.AssignedMAC)
}
if len(api.destroys) != 0 {
t.Errorf("dr success must not destroy: %+v", api.destroys)
}
// The agent performs NO guest-internal host-key op — there is no such API call; host keys
// are preserved (DR) or regenerated by the baked golden unit (provision).
}
func TestRunBringUp_DRResetMACWhenSourceMayBeLive(t *testing.T) {
const vmid = 8002
api := &fakeAPI{cfg: map[int]proxmox.GuestConfig{vmid: scratchCfg()}}
e, _, q := newEngine(t, api, EmptyProvider{})
defer q.Close()
res := e.RunBringUp(context.Background(), BringUpSpec{
Mode: ModeDRGuestLoss, Archive: "vol", VMID: vmid, RestoreStorage: "local-lvm",
KeepMAC: false, // a source guest may still be live → reset MAC even in DR
})
if res.Err != nil || !res.Pass {
t.Fatalf("got %+v", res)
}
p := setParamsFor(api, vmid)
if p == nil || strings.Contains(p["net0"], "hwaddr=") {
t.Errorf("dr with KeepMAC=false must reset MAC (net0 without hwaddr): %+v", p)
}
}
func TestRunBringUp_LivenessIsTheVerdict(t *testing.T) {
const vmid = 8000
const startUPID = "UPID:demo:start:8000:"
mkAPI := func() *fakeAPI {
return &fakeAPI{
cfg: map[int]proxmox.GuestConfig{vmid: scratchCfg()},
startUPID: startUPID,
waitFunc: func(upid string) (proxmox.TaskStatus, error) {
if upid == startUPID {
return proxmox.TaskStatus{Status: "stopped", ExitStatus: "WARNINGS: 1"}, nil
}
return proxmox.TaskStatus{Status: "stopped", ExitStatus: "OK"}, nil
},
logTailFunc: func(string) ([]string, error) {
return []string{"WARN: Systemd 257 detected. You may need to enable nesting."}, nil
},
}
}
// start exits WARNINGS + guest reaches running → PASS, warnings surfaced + recognized.
t.Run("warnings + running -> pass", func(t *testing.T) {
api := mkAPI()
e, _, q := newEngine(t, api, EmptyProvider{})
defer q.Close()
res := e.RunBringUp(context.Background(), BringUpSpec{
Mode: ModeProvision, Archive: "v", VMID: vmid, RestoreStorage: "s", Hostname: "h",
})
if !res.Pass || res.Err != nil {
t.Fatalf("warnings+running must pass: %+v", res)
}
if len(res.StartWarnings) != 1 || !res.WarningsRecognized {
t.Errorf("warnings must be surfaced+recognized: %+v recognized=%v", res.StartWarnings, res.WarningsRecognized)
}
if len(api.destroys) != 0 {
t.Errorf("a passed bring-up must not destroy: %+v", api.destroys)
}
})
// same warnings but guest NEVER reaches running → FAIL (verdict is liveness), guest destroyed.
t.Run("warnings + not-running -> fail", func(t *testing.T) {
api := mkAPI()
api.status = map[int]proxmox.Guest{vmid: {VMID: vmid, Status: "stopped"}}
e, _, q := newEngine(t, api, EmptyProvider{})
defer q.Close()
res := e.RunBringUp(context.Background(), BringUpSpec{
Mode: ModeProvision, Archive: "v", VMID: vmid, RestoreStorage: "s", Hostname: "h",
BootTimeout: 40 * time.Millisecond,
})
if res.Pass || res.Err == nil {
t.Fatalf("not-running must fail regardless of warnings: %+v", res)
}
if len(api.destroys) != 1 {
t.Errorf("a failed bring-up must roll back (destroy): %+v", api.destroys)
}
})
}
func TestRunBringUp_F4_ConfigLockRetry(t *testing.T) {
const vmid = 8000
lockBackoffFast(t)
t.Run("transient lock-500 then 200 -> retries and proceeds", func(t *testing.T) {
api := &fakeAPI{cfg: map[int]proxmox.GuestConfig{vmid: scratchCfg()}}
var calls int
api.setFunc = func(int, map[string]string) (string, error) {
calls++
if calls == 1 {
return "", &proxmox.APIError{StatusCode: 500, Body: "can't lock file '/run/lock/lxc/pve-config-8000.lock' - got timeout"}
}
return "", nil
}
e, _, q := newEngine(t, api, EmptyProvider{})
defer q.Close()
res := e.RunBringUp(context.Background(), BringUpSpec{
Mode: ModeProvision, Archive: "v", VMID: vmid, RestoreStorage: "s", Hostname: "h",
})
if res.Err != nil || !res.Pass {
t.Fatalf("lock-500 then 200 must succeed: %+v", res)
}
if calls < 2 {
t.Errorf("expected a retry on the transient lock-500, calls=%d", calls)
}
})
t.Run("non-lock 500 -> fails without retry", func(t *testing.T) {
api := &fakeAPI{cfg: map[int]proxmox.GuestConfig{vmid: scratchCfg()}}
var calls int
api.setFunc = func(int, map[string]string) (string, error) {
calls++
return "", &proxmox.APIError{StatusCode: 500, Body: "internal error: disk full"}
}
e, _, q := newEngine(t, api, EmptyProvider{})
defer q.Close()
res := e.RunBringUp(context.Background(), BringUpSpec{
Mode: ModeProvision, Archive: "v", VMID: vmid, RestoreStorage: "s", Hostname: "h",
})
if res.Pass || res.Err == nil {
t.Fatalf("a non-lock 500 must fail: %+v", res)
}
if calls != 1 {
t.Errorf("a real error must NOT be retried, calls=%d", calls)
}
})
}
func TestRunBringUp_JournalsOwningEntryBeforeRestore(t *testing.T) {
const vmid = 8000
api := &fakeAPI{cfg: map[int]proxmox.GuestConfig{vmid: scratchCfg()}}
e, j, q := newEngine(t, api, EmptyProvider{})
defer q.Close()
// At the moment RestoreLXC is called, the owning Rollback entry must already be in-flight —
// so a crash here is recoverable (Recover reaps the half-built guest).
var ownedAtRestore bool
api.restoreHook = func() {
for _, en := range j.InFlight() {
if en.VMID == vmid && en.Rollback {
ownedAtRestore = true
}
}
}
res := e.RunBringUp(context.Background(), BringUpSpec{
Mode: ModeProvision, Archive: "v", VMID: vmid, RestoreStorage: "s", Hostname: "h",
})
if res.Err != nil {
t.Fatalf("setup: %+v", res)
}
if !ownedAtRestore {
t.Fatal("the owning Rollback entry MUST be journaled before the restore (crash-safety)")
}
}
func TestRunBringUp_RejectsReservedAndExistingVMID(t *testing.T) {
e, _, q := newEngine(t, &fakeAPI{}, EmptyProvider{})
defer q.Close()
for _, id := range []int{9999, 990000, 990005} {
res := e.RunBringUp(context.Background(), BringUpSpec{Mode: ModeProvision, Archive: "v", VMID: id, RestoreStorage: "s"})
if res.Err == nil {
t.Errorf("VMID %d is reserved and must be refused", id)
}
}
// existing VMID → refuse (restore-over-existing is a signed op, not this benign path).
api := &fakeAPI{lxc: []proxmox.Guest{{VMID: 8000}}}
e2, _, q2 := newEngine(t, api, EmptyProvider{})
defer q2.Close()
res := e2.RunBringUp(context.Background(), BringUpSpec{Mode: ModeProvision, Archive: "v", VMID: 8000, RestoreStorage: "s"})
if res.Err == nil {
t.Error("bring-up over an existing guest must be refused")
}
if len(api.restores) != 0 || len(api.destroys) != 0 {
t.Error("a refused bring-up must not restore or destroy anything")
}
}
func TestRecover_HalfBuiltBringUpRolledBack(t *testing.T) {
const vmid = 8000
// The guest still exists at startup (agent crashed mid-bring-up) → Recover destroys it.
api := &fakeAPI{lxc: []proxmox.Guest{{VMID: vmid, Status: "running"}}}
e, j, _ := newEngine(t, api, EmptyProvider{})
if err := j.Append(JournalEntry{OpID: "bring-up-8000-1", VMID: vmid, Kind: bringUpKind, Rollback: true, State: OpTaskRunning, At: time.Now().UTC()}); err != nil {
t.Fatal(err)
}
res := e.Recover(context.Background())
if res.BringUpRolledBack != 1 {
t.Fatalf("half-built bring-up must be rolled back, got %+v", res)
}
if len(api.destroys) != 1 || api.destroys[0] != vmid {
t.Fatalf("DestroyLXC not called for the half-built guest: %+v", api.destroys)
}
if len(j.InFlight()) != 0 {
t.Errorf("resolved rollback entry must not be in-flight: %+v", j.InFlight())
}
}
func TestRecover_HalfBuiltBringUpAlreadyGone(t *testing.T) {
const vmid = 8000
// Crash after the restore POST failed (no guest) → idempotent clean, no destroy.
api := &fakeAPI{lxc: []proxmox.Guest{{VMID: 9001}}} // 8000 absent
e, j, _ := newEngine(t, api, EmptyProvider{})
j.Append(JournalEntry{OpID: "bring-up-8000-1", VMID: vmid, Kind: bringUpKind, Rollback: true, State: OpStarted, At: time.Now().UTC()})
res := e.Recover(context.Background())
if res.BringUpClean != 1 || len(api.destroys) != 0 {
t.Fatalf("already-gone bring-up must be clean with no destroy: res=%+v destroys=%+v", res, api.destroys)
}
if len(j.InFlight()) != 0 {
t.Errorf("entry must be resolved: %+v", j.InFlight())
}
}
// lockBackoffFast shrinks the F4 retry backoff for tests and restores it after.
func lockBackoffFast(t *testing.T) {
t.Helper()
prev := configLockBackoff
configLockBackoff = time.Millisecond
t.Cleanup(func() { configLockBackoff = prev })
}
+12
View File
@@ -31,6 +31,11 @@ type fakeAPI struct {
statusFunc func(upid string) (proxmox.TaskStatus, error)
// logTailFunc backs TaskLogTail (restore-test start-warning surfacing); default = empty.
logTailFunc func(upid string) ([]string, error)
// setFunc, when set, backs SetConfig (drives the F4 lock-500-then-200 test).
setFunc func(vmid int, params map[string]string) (string, error)
// restoreHook, when set, fires inside RestoreLXC (used to assert the owning journal entry
// is written BEFORE the restore — crash-safety ordering).
restoreHook func()
starts []int
stops []int
@@ -48,6 +53,9 @@ type resizeCall struct {
}
func (f *fakeAPI) RestoreLXC(_ context.Context, opts proxmox.RestoreLXCOptions) (string, error) {
if f.restoreHook != nil {
f.restoreHook()
}
f.mu.Lock()
f.restores = append(f.restores, opts)
f.mu.Unlock()
@@ -121,7 +129,11 @@ func (f *fakeAPI) Stop(_ context.Context, vmid int) (string, error) {
func (f *fakeAPI) SetConfig(_ context.Context, vmid int, params map[string]string) (string, error) {
f.mu.Lock()
f.sets = append(f.sets, setCall{vmid, params})
fn := f.setFunc
f.mu.Unlock()
if fn != nil {
return fn(vmid, params)
}
return f.setUPID, f.setErr
}
+9 -2
View File
@@ -50,8 +50,15 @@ type JournalEntry struct {
// scratch guest may exist and MUST be destroyed" — so Recover resolves it by ensuring
// VMID is gone (a benign teardown), NOT by re-checking any sub-task UPID. The entry is
// terminal only after teardown. See recover.go.
Scratch bool `json:"scratch,omitempty"`
At time.Time `json:"at"`
Scratch bool `json:"scratch,omitempty"`
// Rollback marks an entry that OWNS a guest the agent is CREATING in this journaled
// bring-up transaction (slice 7, doc 03 §9). While such an entry is in-flight, the
// invariant is "VMID may be a half-built guest and MUST be destroyed" (a compensating
// rollback) — so Recover resolves it by ensuring VMID is gone, NOT by re-checking the
// restore sub-task UPID (whose OK status would otherwise leave a half-provisioned guest).
// On SUCCESS the bring-up records this entry terminal and KEEPS the guest. See recover.go.
Rollback bool `json:"rollback,omitempty"`
At time.Time `json:"at"`
}
// Journal is the durable operation log + idempotency store. It mirrors
+79 -8
View File
@@ -44,6 +44,15 @@ func (e *Engine) Recover(ctx context.Context) RecoverResult {
continue
}
// Rollback entries (slice-7 bring-up) own a guest the agent was CREATING. An in-flight
// one means "VMID may be a half-built guest → destroy it" (compensating rollback) — same
// reason the Scratch path runs before the generic UPID path: the restore sub-task's OK
// status would otherwise mark the entry succeeded and leave a half-provisioned guest.
if entry.Rollback {
e.recoverBringUp(ctx, entry, &res)
continue
}
if entry.UPID == "" {
// POST never confirmed → abandon (fail-safe).
e.append(terminal(entry, OpFailed))
@@ -144,16 +153,77 @@ func (e *Engine) recoverScratch(ctx context.Context, entry JournalEntry, res *Re
"op_id", entry.OpID, "vmid", entry.VMID)
}
// recoverBringUp rolls back a half-built bring-up guest left in-flight by a mid-job crash
// (slice 7, doc 03 §9). Invariant: a Rollback entry in-flight at startup means "VMID may be a
// half-provisioned guest and MUST be destroyed" (compensating rollback — the guest is only kept
// when the bring-up reached its terminal OpSucceeded). Idempotent: already-gone records
// terminal-clean. Routes the destroy through the gate as benign ClassGuestDestroy (SameTxnCreated
// provenance) — the same audit-bearing path the in-job rollback uses.
func (e *Engine) recoverBringUp(ctx context.Context, entry JournalEntry, res *RecoverResult) {
lxc, err := e.api.ListLXC(ctx)
if err != nil {
res.Unresolved++
e.logger.Warn("recover: cannot list guests to resolve half-built bring-up; left in-flight",
"op_id", entry.OpID, "vmid", entry.VMID, "err", err)
return
}
exists := false
for _, g := range lxc {
if g.VMID == entry.VMID {
exists = true
break
}
}
if !exists {
// Already gone (e.g. the restore POST never created it) → no leak.
e.append(terminal(entry, OpSucceeded))
res.BringUpClean++
e.logger.Info("recover: half-built bring-up resolved; guest already gone",
"op_id", entry.OpID, "vmid", entry.VMID)
return
}
dec := e.gate.Authorize(IntentForRollbackDestroy(e.hostID, entry.VMID), nil)
if !dec.Allowed {
// Should be benign (SameTxnCreated); if not, fail-safe — do NOT force a destroy.
res.Unresolved++
e.logger.Error("recover: bring-up rollback refused by gate (unexpected); left in-flight",
"op_id", entry.OpID, "vmid", entry.VMID, "reason", dec.Reason)
return
}
upid, err := e.api.DestroyLXC(ctx, entry.VMID)
if err != nil {
res.Unresolved++
e.logger.Warn("recover: destroying half-built bring-up failed; left in-flight (will retry)",
"op_id", entry.OpID, "vmid", entry.VMID, "err", err)
return
}
if upid != "" {
if _, err := e.api.WaitTask(ctx, upid, proxmox.WaitOptions{}); err != nil {
res.Unresolved++
e.logger.Warn("recover: half-built bring-up destroy task failed; left in-flight (will retry)",
"op_id", entry.OpID, "vmid", entry.VMID, "err", err)
return
}
}
e.append(terminal(entry, OpSucceeded))
res.BringUpRolledBack++
e.logger.Warn("recover: rolled back half-built bring-up guest",
"op_id", entry.OpID, "vmid", entry.VMID)
}
// RecoverResult summarizes a startup recovery pass.
type RecoverResult struct {
Examined int
Resumed int // task found completed OK and recorded succeeded
Failed int // task found ended non-OK and recorded failed
RolledBack int // no task id → abandoned (fail-safe)
StillRunning int // task still executing → left in-flight
Unresolved int // task status unreadable → left in-flight
ScratchClean int // scratch entry resolved: guest already gone (no leak)
ScratchDestroyed int // scratch entry resolved: leaked guest destroyed
Examined int
Resumed int // task found completed OK and recorded succeeded
Failed int // task found ended non-OK and recorded failed
RolledBack int // no task id → abandoned (fail-safe)
StillRunning int // task still executing → left in-flight
Unresolved int // task status unreadable → left in-flight
ScratchClean int // scratch entry resolved: guest already gone (no leak)
ScratchDestroyed int // scratch entry resolved: leaked guest destroyed
BringUpClean int // bring-up rollback entry resolved: guest already gone (no leak)
BringUpRolledBack int // bring-up rollback entry resolved: half-built guest destroyed
}
// terminal builds a terminal journal record preserving the op's identity, with the
@@ -167,6 +237,7 @@ func terminal(e JournalEntry, state OpState) JournalEntry {
State: state,
IdempKey: e.IdempKey,
Scratch: e.Scratch,
Rollback: e.Rollback,
At: time.Now().UTC(),
}
}