Skip to content

Commit 1252646

Browse files
committed
fix(vm): keep a stopped VM restorable after a transient restore failure
A hibernated (stopped) VM whose restore failed transiently was marked error, and ResolveForRestore refuses error-state VMs — the wake was permanently bricked until the lease reaped it. FailRestore now spares a stopped origin at the two sequence-level failure exits: nothing ran, record and snapshot are intact, and a retry converges. The run-dir- mutating steps (staged merge, direct populate) still quarantine unconditionally — a partial merge or clean-then-clone leaves mixed- vintage files that vm start must never boot. origin is the pre-kill state, since metering may flip the DB record to stopped after the kill.
1 parent 2c4b063 commit 1252646

4 files changed

Lines changed: 173 additions & 12 deletions

File tree

hypervisor/cloudhypervisor/restore.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,6 @@ func (ch *CloudHypervisor) terminateVMM(ctx context.Context, rec *hypervisor.VMR
4646
func (ch *CloudHypervisor) restoreAfterExtract(ctx context.Context, vmID string, vmCfg *types.VMConfig, rec *hypervisor.VMRecord, directBoot bool) (_ *types.VM, err error) {
4747
logger := log.WithFunc("cloudhypervisor.Restore")
4848

49-
defer func() {
50-
if err != nil {
51-
ch.MarkError(ctx, vmID)
52-
}
53-
}()
54-
5549
chConfigPath := filepath.Join(rec.RunDir, configJSONName)
5650
// rec may have trailing cidata absent from the snapshot (cloudimg post-first-boot); slice to sidecar length.
5751
meta, metaErr := hypervisor.LoadAndValidateMeta(rec.RunDir, ch.conf.RootDir, ch.conf.Config.RunDir)

hypervisor/firecracker/restore.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,6 @@ func (fc *Firecracker) terminateVMM(ctx context.Context, rec *hypervisor.VMRecor
5454
func (fc *Firecracker) restoreAfterExtract(ctx context.Context, vmID string, vmCfg *types.VMConfig, rec *hypervisor.VMRecord, cowPath string) (_ *types.VM, err error) {
5555
logger := log.WithFunc("firecracker.Restore")
5656

57-
defer func() {
58-
if err != nil {
59-
fc.MarkError(ctx, vmID)
60-
}
61-
}()
62-
6357
snapshotCOW := filepath.Join(rec.RunDir, cowFileName)
6458
if snapshotCOW != cowPath {
6559
if _, statErr := os.Stat(snapshotCOW); statErr == nil {

hypervisor/restore.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,17 @@ func (b *Backend) KillForRestore(ctx context.Context, vmID string, rec *VMRecord
2525
return nil
2626
}
2727

28+
// FailRestore marks the VM error after a post-kill restore failure; a stopped
29+
// origin is spared so hibernate wake stays retryable. Run-dir-mutating steps
30+
// (staged merge, direct populate) quarantine unconditionally at their own
31+
// site; origin is the pre-kill state — the DB may read stopped after the kill.
32+
func (b *Backend) FailRestore(ctx context.Context, vmID string, origin types.VMState) {
33+
if origin == types.VMStateStopped {
34+
return
35+
}
36+
b.MarkError(ctx, vmID)
37+
}
38+
2839
func (b *Backend) ResolveForRestore(ctx context.Context, vmRef string) (string, *VMRecord, error) {
2940
vmID, err := b.ResolveRef(ctx, vmRef)
3041
if err != nil {
@@ -101,6 +112,8 @@ func (b *Backend) RestoreSequence(ctx context.Context, vmRef string, spec Restor
101112
}
102113
}
103114
if mergeErr := MergeDirInto(stagingDir, rec.RunDir); mergeErr != nil {
115+
// A partial merge leaves mixed-vintage files in the run dir;
116+
// quarantine regardless of origin so vm start cannot boot them.
104117
b.MarkError(ctx, vmID)
105118
return fmt.Errorf("apply staged snapshot: %w", mergeErr)
106119
}
@@ -109,6 +122,7 @@ func (b *Backend) RestoreSequence(ctx context.Context, vmRef string, spec Restor
109122
return afterErr
110123
}
111124
if err := runWrapped(rec, spec.Wrap, inner); err != nil {
125+
b.FailRestore(ctx, vmID, rec.State)
112126
return nil, err
113127
}
114128
b.emitRestoreSuccess(ctx, result, oldShape, spec.SourceSnapshotID)
@@ -137,6 +151,8 @@ func (b *Backend) DirectRestoreSequence(ctx context.Context, vmRef string, spec
137151
var result *types.VM
138152
inner := func() error {
139153
if populateErr := spec.Populate(rec, spec.SrcDir); populateErr != nil {
154+
// Populate cleans then clones with no rollback; a partial run
155+
// dir must quarantine regardless of origin, like the merge.
140156
b.MarkError(ctx, vmID)
141157
return populateErr
142158
}
@@ -145,6 +161,7 @@ func (b *Backend) DirectRestoreSequence(ctx context.Context, vmRef string, spec
145161
return afterErr
146162
}
147163
if err := runWrapped(rec, spec.Wrap, inner); err != nil {
164+
b.FailRestore(ctx, vmID, rec.State)
148165
return nil, err
149166
}
150167
b.emitRestoreSuccess(ctx, result, oldShape, spec.SourceSnapshotID)

hypervisor/restore_test.go

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,168 @@
11
package hypervisor
22

33
import (
4+
"archive/tar"
5+
"bytes"
6+
"context"
7+
"errors"
8+
"io"
9+
"os"
10+
"path/filepath"
411
"strings"
512
"testing"
613

714
"github.com/cocoonstack/cocoon/types"
815
)
916

17+
func TestFailRestoreSparesStoppedOrigin(t *testing.T) {
18+
b, _ := newMeteringTestBackend(t)
19+
tests := []struct {
20+
name string
21+
origin types.VMState
22+
want types.VMState
23+
}{
24+
{"stopped origin stays stopped (wake retryable)", types.VMStateStopped, types.VMStateStopped},
25+
{"running origin marks error", types.VMStateRunning, types.VMStateError},
26+
}
27+
for _, tt := range tests {
28+
t.Run(tt.name, func(t *testing.T) {
29+
id := "vm-fail-" + string(tt.origin)
30+
seedVMRecord(t, b, id, 1, 512, 1024, true)
31+
if err := b.DB.Update(t.Context(), func(idx *VMIndex) error {
32+
idx.VMs[id].State = tt.origin
33+
return nil
34+
}); err != nil {
35+
t.Fatalf("seed state: %v", err)
36+
}
37+
38+
b.FailRestore(t.Context(), id, tt.origin)
39+
40+
rec, err := b.LoadRecord(t.Context(), id)
41+
if err != nil {
42+
t.Fatalf("load record: %v", err)
43+
}
44+
if rec.State != tt.want {
45+
t.Errorf("state %s, want %s", rec.State, tt.want)
46+
}
47+
})
48+
}
49+
}
50+
51+
func TestDirectRestoreFailureKeepsOriginContract(t *testing.T) {
52+
failAfterExtract := func(spec *DirectRestoreSpec) {
53+
spec.AfterExtract = func(context.Context, string, *types.VMConfig, *VMRecord) (*types.VM, error) {
54+
return nil, errors.New("launch boom")
55+
}
56+
}
57+
failPopulate := func(spec *DirectRestoreSpec) {
58+
spec.Populate = func(*VMRecord, string) error { return errors.New("populate boom") }
59+
}
60+
tests := []struct {
61+
name string
62+
origin types.VMState
63+
fail func(*DirectRestoreSpec)
64+
want types.VMState
65+
}{
66+
{"stopped origin survives a post-populate failure", types.VMStateStopped, failAfterExtract, types.VMStateStopped},
67+
{"running origin quarantines on a post-populate failure", types.VMStateRunning, failAfterExtract, types.VMStateError},
68+
{"partial populate quarantines even a stopped origin", types.VMStateStopped, failPopulate, types.VMStateError},
69+
}
70+
for _, tt := range tests {
71+
t.Run(tt.name, func(t *testing.T) {
72+
b, _ := newMeteringTestBackend(t)
73+
const id = "vm-direct-fail"
74+
seedVMRecord(t, b, id, 1, 512, 1024, true)
75+
if err := b.DB.Update(t.Context(), func(idx *VMIndex) error {
76+
idx.VMs[id].State = tt.origin
77+
return nil
78+
}); err != nil {
79+
t.Fatalf("seed state: %v", err)
80+
}
81+
82+
spec := DirectRestoreSpec{
83+
VMCfg: &types.VMConfig{Config: types.Config{CPU: 1, Memory: 512, Storage: 1024}},
84+
SrcDir: t.TempDir(),
85+
Preflight: func(string, *VMRecord) error { return nil },
86+
Kill: func(context.Context, string, *VMRecord) error { return nil },
87+
Populate: func(*VMRecord, string) error { return nil },
88+
}
89+
tt.fail(&spec)
90+
if _, err := b.DirectRestoreSequence(t.Context(), id, spec); err == nil {
91+
t.Fatal("expected restore failure")
92+
}
93+
rec, err := b.LoadRecord(t.Context(), id)
94+
if err != nil {
95+
t.Fatalf("load record: %v", err)
96+
}
97+
if rec.State != tt.want {
98+
t.Errorf("state %s, want %s", rec.State, tt.want)
99+
}
100+
})
101+
}
102+
}
103+
104+
func TestRestorePartialMergeQuarantinesEvenStoppedOrigin(t *testing.T) {
105+
b, _ := newMeteringTestBackend(t)
106+
const id = "vm-merge-fail"
107+
runDir := t.TempDir()
108+
seedVMRecord(t, b, id, 1, 512, 1024, true)
109+
if err := b.DB.Update(t.Context(), func(idx *VMIndex) error {
110+
idx.VMs[id].State = types.VMStateStopped
111+
idx.VMs[id].RunDir = runDir
112+
return nil
113+
}); err != nil {
114+
t.Fatalf("seed state: %v", err)
115+
}
116+
117+
// Staged "a" merges, then staged file "b" cannot rename over the run
118+
// dir's directory "b": the merge fails halfway through, which must
119+
// quarantine even a stopped origin.
120+
if err := os.MkdirAll(filepath.Join(runDir, "b"), 0o750); err != nil {
121+
t.Fatalf("setup: %v", err)
122+
}
123+
spec := RestoreSpec{
124+
VMCfg: &types.VMConfig{Config: types.Config{CPU: 1, Memory: 512, Storage: 1024}},
125+
Snapshot: tarWithFiles(t, "a", "b"),
126+
Preflight: func(string, *VMRecord) error { return nil },
127+
Kill: func(context.Context, string, *VMRecord) error { return nil },
128+
AfterExtract: func(context.Context, string, *types.VMConfig, *VMRecord) (*types.VM, error) {
129+
t.Fatal("AfterExtract must not run after a failed merge")
130+
return nil, nil
131+
},
132+
}
133+
if _, err := b.RestoreSequence(t.Context(), id, spec); err == nil {
134+
t.Fatal("expected merge failure")
135+
}
136+
rec, err := b.LoadRecord(t.Context(), id)
137+
if err != nil {
138+
t.Fatalf("load record: %v", err)
139+
}
140+
if rec.State != types.VMStateError {
141+
t.Errorf("state %s, want error (mixed run dir must quarantine)", rec.State)
142+
}
143+
if _, err := os.Stat(filepath.Join(runDir, "a")); err != nil {
144+
t.Errorf("merge was not partial: %v", err)
145+
}
146+
}
147+
148+
func tarWithFiles(t *testing.T, names ...string) io.Reader {
149+
t.Helper()
150+
var buf bytes.Buffer
151+
tw := tar.NewWriter(&buf)
152+
for _, name := range names {
153+
if err := tw.WriteHeader(&tar.Header{Name: name, Mode: 0o600, Size: 1}); err != nil {
154+
t.Fatalf("tar hdr %s: %v", name, err)
155+
}
156+
if _, err := tw.Write([]byte("y")); err != nil {
157+
t.Fatalf("tar body %s: %v", name, err)
158+
}
159+
}
160+
if err := tw.Close(); err != nil {
161+
t.Fatalf("tar close: %v", err)
162+
}
163+
return &buf
164+
}
165+
10166
func TestResolveForRestoreStates(t *testing.T) {
11167
b, _ := newMeteringTestBackend(t)
12168
tests := []struct {

0 commit comments

Comments
 (0)