fix: address CI failures and review feedback

This commit is contained in:
Harivansh Rathi 2026-04-11 02:08:57 +00:00
parent 15ad7e6632
commit 149bc2985a
4 changed files with 33 additions and 21 deletions

View file

@ -47,6 +47,7 @@ type Daemon struct {
reconfigureGuestIdentity func(context.Context, string, contracthost.MachineID, *contracthost.GuestConfig) error reconfigureGuestIdentity func(context.Context, string, contracthost.MachineID, *contracthost.GuestConfig) error
readGuestSSHPublicKey func(context.Context, string) (string, error) readGuestSSHPublicKey func(context.Context, string) (string, error)
syncGuestFilesystem func(context.Context, string) error syncGuestFilesystem func(context.Context, string) error
shutdownGuest func(context.Context, string) error
personalizeGuest func(context.Context, *model.MachineRecord, firecracker.MachineState) error personalizeGuest func(context.Context, *model.MachineRecord, firecracker.MachineState) error
locksMu sync.Mutex locksMu sync.Mutex
@ -94,6 +95,7 @@ func New(cfg appconfig.Config, store store.Store, runtime Runtime) (*Daemon, err
daemon.reconfigureGuestIdentity = daemon.reconfigureGuestIdentityOverSSH daemon.reconfigureGuestIdentity = daemon.reconfigureGuestIdentityOverSSH
daemon.readGuestSSHPublicKey = readGuestSSHPublicKey daemon.readGuestSSHPublicKey = readGuestSSHPublicKey
daemon.syncGuestFilesystem = daemon.syncGuestFilesystemOverSSH daemon.syncGuestFilesystem = daemon.syncGuestFilesystemOverSSH
daemon.shutdownGuest = daemon.issueGuestPoweroff
daemon.personalizeGuest = daemon.personalizeGuestConfig daemon.personalizeGuest = daemon.personalizeGuestConfig
if err := daemon.ensureBackendSSHKeyPair(); err != nil { if err := daemon.ensureBackendSSHKeyPair(); err != nil {
return nil, err return nil, err

View file

@ -28,6 +28,7 @@ type fakeRuntime struct {
lastSpec firecracker.MachineSpec lastSpec firecracker.MachineSpec
lastLoadSpec firecracker.SnapshotLoadSpec lastLoadSpec firecracker.SnapshotLoadSpec
mmdsWrites []any mmdsWrites []any
inspectOverride func(firecracker.MachineState) (*firecracker.MachineState, error)
} }
func (f *fakeRuntime) Boot(_ context.Context, spec firecracker.MachineSpec, _ []firecracker.NetworkAllocation) (*firecracker.MachineState, error) { func (f *fakeRuntime) Boot(_ context.Context, spec firecracker.MachineSpec, _ []firecracker.NetworkAllocation) (*firecracker.MachineState, error) {
@ -38,6 +39,9 @@ func (f *fakeRuntime) Boot(_ context.Context, spec firecracker.MachineSpec, _ []
} }
func (f *fakeRuntime) Inspect(state firecracker.MachineState) (*firecracker.MachineState, error) { func (f *fakeRuntime) Inspect(state firecracker.MachineState) (*firecracker.MachineState, error) {
if f.inspectOverride != nil {
return f.inspectOverride(state)
}
copy := state copy := state
return &copy, nil return &copy, nil
} }
@ -256,9 +260,15 @@ func TestStopMachineSyncsGuestFilesystemBeforeDelete(t *testing.T) {
t.Fatalf("create daemon: %v", err) t.Fatalf("create daemon: %v", err)
} }
var syncedHost string var shutdownHost string
hostDaemon.syncGuestFilesystem = func(_ context.Context, runtimeHost string) error { hostDaemon.shutdownGuest = func(_ context.Context, runtimeHost string) error {
syncedHost = runtimeHost shutdownHost = runtimeHost
// Simulate the VM exiting after poweroff by making Inspect return stopped.
runtime.inspectOverride = func(state firecracker.MachineState) (*firecracker.MachineState, error) {
state.Phase = firecracker.PhaseStopped
state.PID = 0
return &state, nil
}
return nil return nil
} }
@ -282,9 +292,10 @@ func TestStopMachineSyncsGuestFilesystemBeforeDelete(t *testing.T) {
t.Fatalf("stop machine: %v", err) t.Fatalf("stop machine: %v", err)
} }
if syncedHost != "172.16.0.2" { if shutdownHost != "172.16.0.2" {
t.Fatalf("sync host mismatch: got %q want %q", syncedHost, "172.16.0.2") t.Fatalf("shutdown host mismatch: got %q want %q", shutdownHost, "172.16.0.2")
} }
// runtime.Delete is always called to clean up TAP device and runtime dir.
if len(runtime.deleteCalls) != 1 { if len(runtime.deleteCalls) != 1 {
t.Fatalf("runtime delete call count mismatch: got %d want 1", len(runtime.deleteCalls)) t.Fatalf("runtime delete call count mismatch: got %d want 1", len(runtime.deleteCalls))
} }

View file

@ -498,15 +498,14 @@ func (d *Daemon) stopMachineRecord(ctx context.Context, record *model.MachineRec
d.stopMachineRelays(record.ID) d.stopMachineRelays(record.ID)
d.stopPublishedPortsForMachine(record.ID) d.stopPublishedPortsForMachine(record.ID)
cleanExit := false
if record.Phase == contracthost.MachinePhaseRunning && strings.TrimSpace(record.RuntimeHost) != "" { if record.Phase == contracthost.MachinePhaseRunning && strings.TrimSpace(record.RuntimeHost) != "" {
cleanExit = d.shutdownGuestClean(ctx, record) d.shutdownGuestClean(ctx, record)
} }
if !cleanExit { // Always call runtime.Delete: it cleans up the TAP device, runtime
// directory, and process (no-op if the process already exited).
if err := d.runtime.Delete(ctx, machineToRuntimeState(*record)); err != nil { if err := d.runtime.Delete(ctx, machineToRuntimeState(*record)); err != nil {
return err return err
} }
}
record.Phase = contracthost.MachinePhaseStopped record.Phase = contracthost.MachinePhaseStopped
record.Error = "" record.Error = ""
@ -525,7 +524,7 @@ func (d *Daemon) shutdownGuestClean(ctx context.Context, record *model.MachineRe
shutdownCtx, cancel := context.WithTimeout(ctx, defaultGuestStopTimeout) shutdownCtx, cancel := context.WithTimeout(ctx, defaultGuestStopTimeout)
defer cancel() defer cancel()
if err := d.issueGuestPoweroff(shutdownCtx, record.RuntimeHost); err != nil { if err := d.shutdownGuest(shutdownCtx, record.RuntimeHost); err != nil {
fmt.Fprintf(os.Stderr, "warning: guest poweroff for %q failed: %v\n", record.ID, err) fmt.Fprintf(os.Stderr, "warning: guest poweroff for %q failed: %v\n", record.ID, err)
return false return false
} }

View file

@ -608,8 +608,8 @@ func TestStopMachineContinuesWhenGuestSyncFails(t *testing.T) {
t.Fatalf("create daemon: %v", err) t.Fatalf("create daemon: %v", err)
} }
stubGuestSSHPublicKeyReader(hostDaemon) stubGuestSSHPublicKeyReader(hostDaemon)
hostDaemon.syncGuestFilesystem = func(context.Context, string) error { hostDaemon.shutdownGuest = func(context.Context, string) error {
return errors.New("guest sync failed") return errors.New("guest shutdown failed")
} }
now := time.Now().UTC() now := time.Now().UTC()