From a020dfe4e501eec0d0ccc821ce35223c5ac75460 Mon Sep 17 00:00:00 2001 From: tonic Date: Mon, 13 Jul 2026 17:41:12 +0800 Subject: [PATCH 1/2] fix(restore): materialize base images and pass --pull on the restore clone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guard added for fresh clones (862c5be6a806bf7a92f8debbc00126da6a0daaa6, #30) covers only the default clone branch of bringUpVM. The restore path (resolveWakeSource -> cloneFromHibernate, used by both same-node wake and the operator's cross-node migration restore) pulls just the :hibernate snapshot and invokes the clone without --pull, so restoring on a node whose local store lacks the snapshot's base cloudimg fails resolving the overlay's backing file — for BOTH base ref shapes: http(s) refs never get core's auto-pull (no --pull flag), and bare OCI refs (e.g. simular/win11:25h2-20260705-1) are ones core refuses to fetch at all. Today every internal-cocoon node happens to carry the fleet cloudimgs, which is the only reason migrate/wake works — verified on staging 2026-07-13 while testing cross-node migration: the restored VM's qcow2 chains directly onto /var/lib/cocoon/cloudimg/blobs/.qcow2 and nothing on the restore path would repopulate it. New or cross-region nodes hit this immediately. resolveWakeSource now returns the snapshot metadata it already looks up (inspecting the import after a registry pull, mirroring ensureSnapshot); cloneFromHibernate applies the same digest-deduped ensureRunImage guard as the fresh-clone path for OCI-ref bases and passes Pull so core fetches missing http(s) bases. --- provider/cocoon/create.go | 4 +- provider/cocoon/restore_test.go | 101 ++++++++++++++++++++++++++++++++ provider/cocoon/update.go | 40 +++++++++---- provider/cocoon/update_test.go | 7 ++- 4 files changed, 136 insertions(+), 16 deletions(-) diff --git a/provider/cocoon/create.go b/provider/cocoon/create.go index 4b93892..2de4845 100644 --- a/provider/cocoon/create.go +++ b/provider/cocoon/create.go @@ -149,11 +149,11 @@ func (p *Provider) bringUpVM(ctx context.Context, pod *corev1.Pod, spec meta.VMS } switch { case meta.ReadRestoreFromHibernate(pod): - sourceName, err := p.resolveWakeSource(ctx, spec.VMName) + sourceName, snapshot, err := p.resolveWakeSource(ctx, spec.VMName) if err != nil { return nil, "", err } - v, err := p.cloneFromHibernate(ctx, spec, sourceName) + v, err := p.cloneFromHibernate(ctx, spec, sourceName, snapshot) if err != nil { return nil, "", err } diff --git a/provider/cocoon/restore_test.go b/provider/cocoon/restore_test.go index 00dc306..f9f3977 100644 --- a/provider/cocoon/restore_test.go +++ b/provider/cocoon/restore_test.go @@ -57,3 +57,104 @@ func TestBringUpVMRestoreFromHibernate(t *testing.T) { }) } } + +func TestBringUpVMRestoreEnsuresOCIRefBaseImage(t *testing.T) { + const vmName = "vk-ns-demo-0" + rt := &fakeRuntime{ + snapshots: map[string]*vm.Snapshot{ + vmName: { + Name: vmName, + Image: "simular/win11:25h2-20260705-1", // bare OCI ref: cocoon's --pull refuses these + }, + }, + } + p := newTestProvider(t) + p.Runtime = rt + p.Probes = probes.NewManager(t.Context()) + + pod := newPodWithSpec(meta.VMSpec{ + VMName: vmName, + Backend: string(cocoonv1.BackendCloudHypervisor), + OS: string(cocoonv1.OSWindows), + }) + pod.Annotations[meta.AnnotationRestoreFromHibernate] = "true" + spec := meta.ParseVMSpec(pod) + + if _, _, err := p.bringUpVM(t.Context(), pod, spec); err != nil { + t.Fatalf("bringUpVM: %v", err) + } + if rt.cloned == nil { + t.Fatal("restore must clone from the hibernate snapshot, Clone was never called") + } + if len(rt.ensuredImages) != 1 || rt.ensuredImages[0].image != "simular/win11:25h2-20260705-1" { + t.Fatalf("OCI-ref base image was not ensured before restore, got %#v", rt.ensuredImages) + } +} + +func TestBringUpVMRestoreSkipsEnsureWhenDigestPresent(t *testing.T) { + const vmName = "vk-ns-demo-0" + rt := &fakeRuntime{ + snapshots: map[string]*vm.Snapshot{ + vmName: { + Name: vmName, + Image: "simular/win11:25h2-20260705-1", + ImageDigest: "sha256:142ab794", + }, + }, + imagesPresent: map[string]bool{"sha256:142ab794": true}, // same bytes under another name + } + p := newTestProvider(t) + p.Runtime = rt + p.Probes = probes.NewManager(t.Context()) + + pod := newPodWithSpec(meta.VMSpec{ + VMName: vmName, + Backend: string(cocoonv1.BackendCloudHypervisor), + OS: string(cocoonv1.OSWindows), + }) + pod.Annotations[meta.AnnotationRestoreFromHibernate] = "true" + spec := meta.ParseVMSpec(pod) + + if _, _, err := p.bringUpVM(t.Context(), pod, spec); err != nil { + t.Fatalf("bringUpVM: %v", err) + } + if len(rt.ensuredImages) != 0 { + t.Fatalf("EnsureImage should be skipped when the digest is already local, got %#v", rt.ensuredImages) + } +} + +func TestBringUpVMRestorePullsHTTPBase(t *testing.T) { + const vmName = "vk-ns-demo-0" + rt := &fakeRuntime{ + snapshots: map[string]*vm.Snapshot{ + vmName: { + Name: vmName, + Image: "https://epoch.simular.cloud/dl/simular/win11/25h2-20260608", + }, + }, + } + p := newTestProvider(t) + p.Runtime = rt + p.Probes = probes.NewManager(t.Context()) + + pod := newPodWithSpec(meta.VMSpec{ + VMName: vmName, + Backend: string(cocoonv1.BackendCloudHypervisor), + OS: string(cocoonv1.OSWindows), + }) + pod.Annotations[meta.AnnotationRestoreFromHibernate] = "true" + spec := meta.ParseVMSpec(pod) + + if _, _, err := p.bringUpVM(t.Context(), pod, spec); err != nil { + t.Fatalf("bringUpVM: %v", err) + } + if rt.cloned == nil { + t.Fatal("Runtime.Clone was not called") + } + if !rt.cloned.Pull { + t.Error("restore clone must pass --pull so core fetches a missing http(s) base") + } + if len(rt.ensuredImages) != 0 { + t.Errorf("EnsureImage should not run for an http(s) base (core --pull handles it), got %#v", rt.ensuredImages) + } +} diff --git a/provider/cocoon/update.go b/provider/cocoon/update.go index 67786fa..4ef112f 100644 --- a/provider/cocoon/update.go +++ b/provider/cocoon/update.go @@ -165,14 +165,14 @@ func (p *Provider) wake(ctx context.Context, pod *corev1.Pod) error { return nil } p.markLifecycleState(ctx, pod, meta.LifecycleStateCreating, "") - sourceName, err := p.resolveWakeSource(ctx, spec.VMName) + sourceName, snapshot, err := p.resolveWakeSource(ctx, spec.VMName) if err != nil { metrics.WakeTotal.WithLabelValues("failed").Inc() p.failOp(ctx, pod, "WakePullFailed", "update", err) return err } cloneStart := time.Now() - v, err := p.cloneFromHibernate(ctx, spec, sourceName) + v, err := p.cloneFromHibernate(ctx, spec, sourceName, snapshot) if err != nil { metrics.WakeTotal.WithLabelValues("failed").Inc() p.failOp(ctx, pod, "WakeCloneFailed", "update", err) @@ -191,8 +191,16 @@ func (p *Provider) wake(ctx context.Context, pod *corev1.Pod) error { // source. CH+Windows hibernate snapshots are captured NIC-less, so the clone // hot-adds a fresh NIC that Windows enumerates as new hardware. The local import // copy (cross-node pull) is dropped whether the clone succeeds or fails. -func (p *Provider) cloneFromHibernate(ctx context.Context, spec meta.VMSpec, sourceName string) (*vm.VM, error) { +func (p *Provider) cloneFromHibernate(ctx context.Context, spec meta.VMSpec, sourceName string, snapshot *vm.Snapshot) (*vm.VM, error) { defer p.cleanupWakeImport(spec.VMName, sourceName) + // Same guard as the fresh-clone path: `vm clone --pull` fetches only + // http(s) bases, so an OCI-ref base absent from the local store must be + // materialized here or the restore fails resolving the backing file. + if snapshot != nil && snapshot.Image != "" && !isHTTPURL(snapshot.Image) && !p.imagePresent(ctx, snapshot.ImageDigest) { + if _, imgErr := p.ensureRunImage(ctx, snapshot.Image, false); imgErr != nil { + return nil, fmt.Errorf("ensure restore base image %s: %w", snapshot.Image, imgErr) + } + } opts := vm.CloneOptions{ From: sourceName, To: spec.VMName, @@ -200,6 +208,9 @@ func (p *Provider) cloneFromHibernate(ctx context.Context, spec meta.VMSpec, sou Backend: spec.Backend, NoDirectIO: spec.NoDirectIO, OnDemand: useOnDemandClone(spec.OS), + // http(s) bases ride core's --pull (same as the fresh-clone path); + // OCI-ref bases were materialized above. + Pull: snapshot != nil && snapshot.Image != "", } if shouldDropNICBeforeHibernate(spec) { opts.NICs = ptr.To(1) @@ -298,27 +309,32 @@ func (p *Provider) waitForFreshIP(ctx context.Context, namespace, name string) b } } -// resolveWakeSource returns the local snapshot when present, else pulls. -func (p *Provider) resolveWakeSource(ctx context.Context, vmName string) (string, error) { - _, err := p.Runtime.Snapshot(ctx, vmName) +// resolveWakeSource returns the clone source name and its snapshot metadata — +// the local snapshot when present, else pulled from the registry. +func (p *Provider) resolveWakeSource(ctx context.Context, vmName string) (string, *vm.Snapshot, error) { + snapshot, err := p.Runtime.Snapshot(ctx, vmName) if err == nil { - return vmName, nil + return vmName, snapshot, nil } if !errors.Is(err, vm.ErrSnapshotNotFound) { - return "", fmt.Errorf("inspect local snapshot %s: %w", vmName, err) + return "", nil, fmt.Errorf("inspect local snapshot %s: %w", vmName, err) } if p.Puller == nil { - return "", fmt.Errorf("wake %s: no local snapshot and no puller configured", vmName) + return "", nil, fmt.Errorf("wake %s: no local snapshot and no puller configured", vmName) } importName := vmName + hibernateImportSuffix pullStart := time.Now() - if err := p.Puller.PullSnapshot(ctx, vmName, meta.HibernateSnapshotTag, importName); err != nil { + if pullErr := p.Puller.PullSnapshot(ctx, vmName, meta.HibernateSnapshotTag, importName); pullErr != nil { metrics.SnapshotPullTotal.WithLabelValues("failed").Inc() - return "", fmt.Errorf("pull hibernation snapshot %s: %w", vmName, err) + return "", nil, fmt.Errorf("pull hibernation snapshot %s: %w", vmName, pullErr) } metrics.SnapshotPullDuration.Observe(time.Since(pullStart).Seconds()) metrics.SnapshotPullTotal.WithLabelValues("ok").Inc() - return importName, nil + snapshot, err = p.Runtime.Snapshot(ctx, importName) + if err != nil { + return "", nil, fmt.Errorf("inspect imported snapshot %s: %w", importName, err) + } + return importName, snapshot, nil } // cleanupWakeImport drops the cross-node import; same-node keeps the diff --git a/provider/cocoon/update_test.go b/provider/cocoon/update_test.go index 319d885..7380d82 100644 --- a/provider/cocoon/update_test.go +++ b/provider/cocoon/update_test.go @@ -269,13 +269,16 @@ func TestResolveWakeSourceUsesLocalSnapshot(t *testing.T) { p := newTestProvider(t) p.Runtime = rt - got, err := p.resolveWakeSource(t.Context(), "vk-ns-demo-0") + got, snapshot, err := p.resolveWakeSource(t.Context(), "vk-ns-demo-0") if err != nil { t.Fatalf("resolveWakeSource: %v", err) } if got != "vk-ns-demo-0" { t.Errorf("source = %q, want vk-ns-demo-0 (local snapshot)", got) } + if snapshot == nil || snapshot.Name != "vk-ns-demo-0" { + t.Errorf("snapshot metadata = %+v, want the local snapshot", snapshot) + } } func TestResolveWakeSourceErrorsWhenLocalMissingAndNoPuller(t *testing.T) { @@ -283,7 +286,7 @@ func TestResolveWakeSourceErrorsWhenLocalMissingAndNoPuller(t *testing.T) { p := newTestProvider(t) p.Runtime = rt - if _, err := p.resolveWakeSource(t.Context(), "vk-ns-demo-0"); err == nil { + if _, _, err := p.resolveWakeSource(t.Context(), "vk-ns-demo-0"); err == nil { t.Fatal("expected error when local snapshot is missing and no Puller is set") } } From fe65646f2e399f8452a065b0e90567f8aa6d0b05 Mon Sep 17 00:00:00 2001 From: CMGS Date: Tue, 14 Jul 2026 21:31:33 +0800 Subject: [PATCH 2/2] test(restore): pin restore-clone --pull and digest-keyed presence check - assert cloned.Pull on the OCI-ref and digest-present restore tests so the fresh-clone mirror (--pull for materialized OCI bases) can't silently regress to Pull=isHTTPURL(image) - assert imageInspectCalls so the digest-present skip proves presence was resolved by digest via Image(), not a non-empty-digest shortcut - drop the redundant Pull-field comment; the guard comment above already carries the WHY, matching create.go's fresh-clone path --- provider/cocoon/restore_test.go | 9 +++++++++ provider/cocoon/update.go | 4 +--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/provider/cocoon/restore_test.go b/provider/cocoon/restore_test.go index f9f3977..c473537 100644 --- a/provider/cocoon/restore_test.go +++ b/provider/cocoon/restore_test.go @@ -89,6 +89,9 @@ func TestBringUpVMRestoreEnsuresOCIRefBaseImage(t *testing.T) { if len(rt.ensuredImages) != 1 || rt.ensuredImages[0].image != "simular/win11:25h2-20260705-1" { t.Fatalf("OCI-ref base image was not ensured before restore, got %#v", rt.ensuredImages) } + if !rt.cloned.Pull { + t.Error("materialized OCI-ref base still needs --pull so core resolves the backing file by digest") + } } func TestBringUpVMRestoreSkipsEnsureWhenDigestPresent(t *testing.T) { @@ -121,6 +124,12 @@ func TestBringUpVMRestoreSkipsEnsureWhenDigestPresent(t *testing.T) { if len(rt.ensuredImages) != 0 { t.Fatalf("EnsureImage should be skipped when the digest is already local, got %#v", rt.ensuredImages) } + if len(rt.imageInspectCalls) != 1 || rt.imageInspectCalls[0] != "sha256:142ab794" { + t.Fatalf("presence must be resolved by digest via Image(), got %#v", rt.imageInspectCalls) + } + if rt.cloned == nil || !rt.cloned.Pull { + t.Error("restore clone must pass --pull when the base is present") + } } func TestBringUpVMRestorePullsHTTPBase(t *testing.T) { diff --git a/provider/cocoon/update.go b/provider/cocoon/update.go index 4ef112f..9e5b91d 100644 --- a/provider/cocoon/update.go +++ b/provider/cocoon/update.go @@ -208,9 +208,7 @@ func (p *Provider) cloneFromHibernate(ctx context.Context, spec meta.VMSpec, sou Backend: spec.Backend, NoDirectIO: spec.NoDirectIO, OnDemand: useOnDemandClone(spec.OS), - // http(s) bases ride core's --pull (same as the fresh-clone path); - // OCI-ref bases were materialized above. - Pull: snapshot != nil && snapshot.Image != "", + Pull: snapshot != nil && snapshot.Image != "", } if shouldDropNICBeforeHibernate(spec) { opts.NICs = ptr.To(1)