From 4d1c0356ed70c81457ca1f8d1f21ee08b61adfc3 Mon Sep 17 00:00:00 2001 From: Shreyansh Sancheti Date: Fri, 20 Mar 2026 12:31:23 +0530 Subject: [PATCH] parity: add LCOW document permutation tests Restructure parity test framework and add permutation-based tests for LCOW HCS document creation comparing legacy and v2 builder outputs. Signed-off-by: Shreyansh Sancheti Signed-off-by: Shreyansh Sancheti --- internal/uvm/create_lcow.go | 4 +- internal/uvm/types.go | 4 +- test/parity/vm/doc.go | 12 +- .../vm/hcs_lcow_document_creator_test.go | 69 ++++ test/parity/vm/lcow_doc_test.go | 377 ++++++++++++++++-- 5 files changed, 434 insertions(+), 32 deletions(-) diff --git a/internal/uvm/create_lcow.go b/internal/uvm/create_lcow.go index 521e272543..93a6c4a080 100644 --- a/internal/uvm/create_lcow.go +++ b/internal/uvm/create_lcow.go @@ -458,7 +458,7 @@ func makeLCOWVMGSDoc(ctx context.Context, opts *OptionsLCOW, uvm *UtilityVM) (_ // This is done prior to json seriaisation and sending to the HCS layer to actually do the work of creating the VM. // Many details are quite different (see the typical JSON examples), in particular it boots from a VMGS file // which contains both the kernel and initrd as well as kernel boot options. -func makeLCOWSecurityDoc(ctx context.Context, opts *OptionsLCOW, uvm *UtilityVM) (_ *hcsschema.ComputeSystem, err error) { +func MakeLCOWSecurityDoc(ctx context.Context, opts *OptionsLCOW, uvm *UtilityVM) (_ *hcsschema.ComputeSystem, err error) { doc, vmgsErr := makeLCOWVMGSDoc(ctx, opts, uvm) if vmgsErr != nil { return nil, vmgsErr @@ -938,7 +938,7 @@ func CreateLCOW(ctx context.Context, opts *OptionsLCOW) (_ *UtilityVM, err error // HCS config for SNP isolated vm is quite different to the usual case var doc *hcsschema.ComputeSystem if opts.SecurityPolicyEnabled { - doc, err = makeLCOWSecurityDoc(ctx, opts, uvm) + doc, err = MakeLCOWSecurityDoc(ctx, opts, uvm) if logrus.IsLevelEnabled(logrus.TraceLevel) { log.G(ctx).WithFields(logrus.Fields{ "doc": log.Format(ctx, doc), diff --git a/internal/uvm/types.go b/internal/uvm/types.go index 2edc099701..897ff64b7c 100644 --- a/internal/uvm/types.go +++ b/internal/uvm/types.go @@ -154,8 +154,8 @@ func (uvm *UtilityVM) ScratchEncryptionEnabled() bool { } // NewUtilityVMForDoc creates a minimal UtilityVM with the fields needed by -// MakeLCOWDoc for HCS document generation in parity tests. -// UtilityVM fields are unexported, so this constructor must live in the uvm package. +// MakeLCOWDoc and MakeLCOWSecurityDoc for HCS document generation. This is +// not a runnable VM — it exists only for parity testing. func NewUtilityVMForDoc(id, owner string, scsiControllerCount, vpmemMaxCount uint32, vpmemMaxSizeBytes uint64, vpmemMultiMapping bool) *UtilityVM { return &UtilityVM{ id: id, diff --git a/test/parity/vm/doc.go b/test/parity/vm/doc.go index 7e35e75615..e5e340f4c4 100644 --- a/test/parity/vm/doc.go +++ b/test/parity/vm/doc.go @@ -1,7 +1,13 @@ //go:build windows -// Package vmparity validates that the v2 modular VM document builders produce -// HCS ComputeSystem documents equivalent to the legacy shim pipelines. +// Package vm validates that the v2 VM document builders produce HCS +// ComputeSystem documents equivalent to the legacy shim pipelines. // -// Currently covers LCOW; WCOW parity will be added in a future PR. +// Currently covers LCOW parity between: +// - Legacy: OCI spec → oci.UpdateSpecFromOptions → oci.ProcessAnnotations → +// oci.SpecToUVMCreateOpts → uvm.MakeLCOWDoc → *hcsschema.ComputeSystem +// - V2: vm.Spec + runhcsopts.Options → lcow.BuildSandboxConfig → +// *hcsschema.ComputeSystem + *SandboxOptions +// +// WCOW parity will be added in a future PR. package vmparity diff --git a/test/parity/vm/hcs_lcow_document_creator_test.go b/test/parity/vm/hcs_lcow_document_creator_test.go index 03be8c6797..a20c025afd 100644 --- a/test/parity/vm/hcs_lcow_document_creator_test.go +++ b/test/parity/vm/hcs_lcow_document_creator_test.go @@ -8,8 +8,10 @@ import ( "fmt" "os" "path/filepath" + "strings" "testing" + "github.com/google/go-cmp/cmp" "github.com/opencontainers/runtime-spec/specs-go" runhcsopts "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options" @@ -111,3 +113,70 @@ func jsonToString(v interface{}) string { } return string(b) } + +// normalizeKernelCmdLine trims leading/trailing whitespace from the kernel +// command line in the document. The legacy builder has a minor quirk that +// produces a leading space for initrd+KernelDirect boot. The v2 builder +// does not. Since HCS trims whitespace from kernel args, this difference +// is harmless and we normalize it away. +func normalizeKernelCmdLine(doc *hcsschema.ComputeSystem) { + if doc == nil || doc.VirtualMachine == nil || doc.VirtualMachine.Chipset == nil { + return + } + if kd := doc.VirtualMachine.Chipset.LinuxKernelDirect; kd != nil { + kd.KernelCmdLine = strings.TrimSpace(kd.KernelCmdLine) + } + if uefi := doc.VirtualMachine.Chipset.Uefi; uefi != nil && uefi.BootThis != nil { + uefi.BootThis.OptionalData = strings.TrimSpace(uefi.BootThis.OptionalData) + } +} + +// isOnlyKernelCmdLineWhitespaceDiff returns true if the only difference between +// two documents is leading/trailing whitespace in the kernel command line. +func isOnlyKernelCmdLineWhitespaceDiff(legacy, v2 *hcsschema.ComputeSystem) bool { + legacyCopy := *legacy + v2Copy := *v2 + if legacyCopy.VirtualMachine != nil { + vmCopy := *legacyCopy.VirtualMachine + legacyCopy.VirtualMachine = &vmCopy + if vmCopy.Chipset != nil { + chipCopy := *vmCopy.Chipset + legacyCopy.VirtualMachine.Chipset = &chipCopy + if chipCopy.LinuxKernelDirect != nil { + lkdCopy := *chipCopy.LinuxKernelDirect + legacyCopy.VirtualMachine.Chipset.LinuxKernelDirect = &lkdCopy + } + if chipCopy.Uefi != nil { + uefiCopy := *chipCopy.Uefi + legacyCopy.VirtualMachine.Chipset.Uefi = &uefiCopy + if uefiCopy.BootThis != nil { + btCopy := *uefiCopy.BootThis + legacyCopy.VirtualMachine.Chipset.Uefi.BootThis = &btCopy + } + } + } + } + if v2Copy.VirtualMachine != nil { + vmCopy := *v2Copy.VirtualMachine + v2Copy.VirtualMachine = &vmCopy + if vmCopy.Chipset != nil { + chipCopy := *vmCopy.Chipset + v2Copy.VirtualMachine.Chipset = &chipCopy + if chipCopy.LinuxKernelDirect != nil { + lkdCopy := *chipCopy.LinuxKernelDirect + v2Copy.VirtualMachine.Chipset.LinuxKernelDirect = &lkdCopy + } + if chipCopy.Uefi != nil { + uefiCopy := *chipCopy.Uefi + v2Copy.VirtualMachine.Chipset.Uefi = &uefiCopy + if uefiCopy.BootThis != nil { + btCopy := *uefiCopy.BootThis + v2Copy.VirtualMachine.Chipset.Uefi.BootThis = &btCopy + } + } + } + } + normalizeKernelCmdLine(&legacyCopy) + normalizeKernelCmdLine(&v2Copy) + return cmp.Diff(&legacyCopy, &v2Copy) == "" +} diff --git a/test/parity/vm/lcow_doc_test.go b/test/parity/vm/lcow_doc_test.go index 5d45b9c8b8..f701c13654 100644 --- a/test/parity/vm/lcow_doc_test.go +++ b/test/parity/vm/lcow_doc_test.go @@ -5,14 +5,14 @@ package vmparity import ( "context" "maps" + "strings" "testing" "github.com/google/go-cmp/cmp" "github.com/opencontainers/runtime-spec/specs-go" runhcsopts "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options" - lcowbuilder "github.com/Microsoft/hcsshim/internal/builder/vm/lcow" - "github.com/Microsoft/hcsshim/internal/uvm" + iannotations "github.com/Microsoft/hcsshim/internal/annotations" shimannotations "github.com/Microsoft/hcsshim/pkg/annotations" vm "github.com/Microsoft/hcsshim/sandbox-spec/vm/v2" ) @@ -126,9 +126,15 @@ func TestLCOWDocumentParity(t *testing.T) { } if diff := cmp.Diff(legacyDoc, v2Doc); diff != "" { - t.Logf("Legacy document:\n%s", jsonToString(legacyDoc)) - t.Logf("V2 document:\n%s", jsonToString(v2Doc)) - t.Errorf("LCOW HCS document mismatch (-legacy +v2):\n%s", diff) + // Check if the only difference is the legacy kernel cmdline + // leading space quirk. If so, warn instead of failing. + if isOnlyKernelCmdLineWhitespaceDiff(legacyDoc, v2Doc) { + t.Logf("WARNING: kernel cmdline has leading whitespace difference (legacy quirk): %s", diff) + } else { + t.Logf("Legacy document:\n%s", jsonToString(legacyDoc)) + t.Logf("V2 document:\n%s", jsonToString(v2Doc)) + t.Errorf("LCOW HCS document mismatch (-legacy +v2):\n%s", diff) + } } }) } @@ -181,33 +187,354 @@ func TestLCOWSandboxOptionsFieldParity(t *testing.T) { t.Fatalf("failed to build v2 LCOW document: %v", err) } - checkSandboxOptionsParity(t, legacyOpts, sandboxOpts) + checks := []struct { + name string + legacy interface{} + v2 interface{} + }{ + {"NoWritableFileShares", legacyOpts.NoWritableFileShares, sandboxOpts.NoWritableFileShares}, + {"EnableScratchEncryption", legacyOpts.EnableScratchEncryption, sandboxOpts.EnableScratchEncryption}, + {"PolicyBasedRouting", legacyOpts.PolicyBasedRouting, sandboxOpts.PolicyBasedRouting}, + {"FullyPhysicallyBacked", legacyOpts.FullyPhysicallyBacked, sandboxOpts.FullyPhysicallyBacked}, + {"VPMEMMultiMapping", !legacyOpts.VPMemNoMultiMapping, sandboxOpts.VPMEMMultiMapping}, + } + + for _, c := range checks { + t.Run(c.name, func(t *testing.T) { + if c.legacy != c.v2 { + t.Errorf("sandbox option %s mismatch: legacy=%v, v2=%v", c.name, c.legacy, c.v2) + } + }) + } }) } } -// checkSandboxOptionsParity verifies that each configuration field in the legacy -// OptionsLCOW matches its v2 SandboxOptions counterpart. Extracted as a helper -// for extensibility across test cases. -func checkSandboxOptionsParity(t *testing.T, legacyOpts *uvm.OptionsLCOW, sandboxOpts *lcowbuilder.SandboxOptions) { - t.Helper() +// TestLCOWDocumentParityPermutations exercises annotation and option combinations +// that trigger different document construction branches. Each test populates all +// fields it depends on so the comparison checks real values, not defaults. +func TestLCOWDocumentParityPermutations(t *testing.T) { + bootDir := setupBootFiles(t) - checks := []struct { - name string - legacy interface{} - v2 interface{} + tests := []struct { + name string + annotations map[string]string + devices []specs.WindowsDevice + shimOpts func() *runhcsopts.Options + expectedDiffField string // for gap tests: the HCS field path expected in the diff }{ - {"NoWritableFileShares", legacyOpts.NoWritableFileShares, sandboxOpts.NoWritableFileShares}, - {"EnableScratchEncryption", legacyOpts.EnableScratchEncryption, sandboxOpts.EnableScratchEncryption}, - {"PolicyBasedRouting", legacyOpts.PolicyBasedRouting, sandboxOpts.PolicyBasedRouting}, - {"FullyPhysicallyBacked", legacyOpts.FullyPhysicallyBacked, sandboxOpts.FullyPhysicallyBacked}, - {"VPMEMMultiMapping", !legacyOpts.VPMemNoMultiMapping, sandboxOpts.VPMEMMultiMapping}, - } + // --- CPU partial combinations --- + + { + name: "CPU: count only", + annotations: map[string]string{ + shimannotations.ProcessorCount: "2", + shimannotations.CPUGroupID: "cpu-only-group", + shimannotations.StorageQoSIopsMaximum: "1000", + shimannotations.StorageQoSBandwidthMaximum: "100000", + }, + }, + { + name: "CPU: limit only", + annotations: map[string]string{ + shimannotations.ProcessorLimit: "50000", + shimannotations.CPUGroupID: "limit-only-group", + shimannotations.StorageQoSIopsMaximum: "1000", + shimannotations.StorageQoSBandwidthMaximum: "100000", + }, + }, + { + name: "CPU: weight only", + annotations: map[string]string{ + shimannotations.ProcessorWeight: "500", + shimannotations.CPUGroupID: "weight-only-group", + shimannotations.StorageQoSIopsMaximum: "1000", + shimannotations.StorageQoSBandwidthMaximum: "100000", + }, + }, + + // --- Memory partial combinations --- + + { + name: "memory: overcommit disabled", + annotations: map[string]string{ + shimannotations.MemorySizeInMB: "2048", + shimannotations.AllowOvercommit: "false", + shimannotations.CPUGroupID: "mem-nocommit-group", + shimannotations.StorageQoSIopsMaximum: "1000", + shimannotations.StorageQoSBandwidthMaximum: "100000", + }, + }, + { + name: "memory: cold discard hint", + annotations: map[string]string{ + shimannotations.MemorySizeInMB: "1024", + shimannotations.EnableColdDiscardHint: "true", + shimannotations.CPUGroupID: "cold-discard-group", + shimannotations.StorageQoSIopsMaximum: "1000", + shimannotations.StorageQoSBandwidthMaximum: "100000", + }, + }, + + // --- Boot mode interactions --- + + { + name: "boot: kernel direct + VHD rootfs", + annotations: map[string]string{ + shimannotations.KernelDirectBoot: "true", + shimannotations.PreferredRootFSType: "vhd", + shimannotations.CPUGroupID: "vhd-boot-group", + shimannotations.StorageQoSIopsMaximum: "1000", + shimannotations.StorageQoSBandwidthMaximum: "100000", + }, + }, + + // --- Feature flag combinations --- + + { + name: "feature: scratch encryption + disable writable shares", + annotations: map[string]string{ + shimannotations.LCOWEncryptedScratchDisk: "true", + shimannotations.DisableWritableFileShares: "true", + shimannotations.CPUGroupID: "scratch-enc-group", + shimannotations.StorageQoSIopsMaximum: "1000", + shimannotations.StorageQoSBandwidthMaximum: "100000", + }, + }, + { + name: "feature: writable overlay dirs (VHD rootfs)", + annotations: map[string]string{ + shimannotations.PreferredRootFSType: "vhd", + shimannotations.KernelDirectBoot: "true", + iannotations.WritableOverlayDirs: "true", + shimannotations.CPUGroupID: "overlay-group", + shimannotations.StorageQoSIopsMaximum: "1000", + shimannotations.StorageQoSBandwidthMaximum: "100000", + }, + }, + + // --- Device interactions --- + + { + name: "VPMem disabled (4 SCSI controllers)", + annotations: map[string]string{ + shimannotations.VPMemCount: "0", + shimannotations.CPUGroupID: "no-vpmem-group", + shimannotations.StorageQoSIopsMaximum: "1000", + shimannotations.StorageQoSBandwidthMaximum: "100000", + }, + }, + + // --- Cross-group interactions --- + + { + name: "cross: physically backed + VPMem disabled + scratch encryption", + annotations: map[string]string{ + shimannotations.FullyPhysicallyBacked: "true", + shimannotations.VPMemCount: "0", + shimannotations.LCOWEncryptedScratchDisk: "true", + shimannotations.MemorySizeInMB: "4096", + shimannotations.CPUGroupID: "phys-backed-group", + shimannotations.StorageQoSIopsMaximum: "5000", + shimannotations.StorageQoSBandwidthMaximum: "1000000", + }, + }, + { + name: "cross: CPU + memory + MMIO + QoS + cold discard + VHD boot", + annotations: map[string]string{ + shimannotations.ProcessorCount: "2", + shimannotations.ProcessorLimit: "80000", + shimannotations.ProcessorWeight: "300", + shimannotations.CPUGroupID: "full-combo-group", + shimannotations.MemorySizeInMB: "4096", + shimannotations.AllowOvercommit: "true", + shimannotations.EnableColdDiscardHint: "true", + shimannotations.MemoryLowMMIOGapInMB: "512", + shimannotations.MemoryHighMMIOBaseInMB: "2048", + shimannotations.MemoryHighMMIOGapInMB: "1024", + shimannotations.StorageQoSIopsMaximum: "10000", + shimannotations.StorageQoSBandwidthMaximum: "2000000", + shimannotations.KernelDirectBoot: "true", + shimannotations.PreferredRootFSType: "vhd", + }, + }, + + // --- Shim options override vs annotation priority --- + + { + name: "override: annotation CPU overrides shim option CPU", + shimOpts: func() *runhcsopts.Options { + return &runhcsopts.Options{ + SandboxPlatform: "linux/amd64", + BootFilesRootPath: bootDir, + VmProcessorCount: 1, + } + }, + annotations: map[string]string{ + shimannotations.ProcessorCount: "2", + shimannotations.CPUGroupID: "override-cpu-group", + shimannotations.StorageQoSIopsMaximum: "1000", + shimannotations.StorageQoSBandwidthMaximum: "100000", + }, + }, + { + name: "override: annotation memory overrides shim option memory", + shimOpts: func() *runhcsopts.Options { + return &runhcsopts.Options{ + SandboxPlatform: "linux/amd64", + BootFilesRootPath: bootDir, + VmMemorySizeInMb: 1024, + } + }, + annotations: map[string]string{ + shimannotations.MemorySizeInMB: "4096", + shimannotations.CPUGroupID: "override-mem-group", + shimannotations.StorageQoSIopsMaximum: "1000", + shimannotations.StorageQoSBandwidthMaximum: "100000", + }, + }, + // --- Kernel args combinations --- - for _, c := range checks { - t.Run(c.name, func(t *testing.T) { - if c.legacy != c.v2 { - t.Errorf("sandbox option %s mismatch: legacy=%v, v2=%v", c.name, c.legacy, c.v2) + { + name: "kernel args: VPCIEnabled + custom boot options", + annotations: map[string]string{ + shimannotations.VPCIEnabled: "true", + shimannotations.KernelBootOptions: "loglevel=7 debug", + shimannotations.CPUGroupID: "vpci-boot-group", + shimannotations.StorageQoSIopsMaximum: "1000", + shimannotations.StorageQoSBandwidthMaximum: "100000", + }, + }, + { + name: "kernel args: disable time sync + process dump + writable overlay (VHD)", + annotations: map[string]string{ + shimannotations.KernelDirectBoot: "true", + shimannotations.PreferredRootFSType: "vhd", + shimannotations.DisableLCOWTimeSyncService: "true", + shimannotations.ContainerProcessDumpLocation: "/tmp/dumps", + iannotations.WritableOverlayDirs: "true", + shimannotations.CPUGroupID: "kargs-combo-group", + shimannotations.StorageQoSIopsMaximum: "1000", + shimannotations.StorageQoSBandwidthMaximum: "100000", + }, + }, + { + name: "kernel args: initrd boot (kernel cmdline whitespace warning)", + annotations: map[string]string{ + shimannotations.PreferredRootFSType: "initrd", + shimannotations.KernelDirectBoot: "true", + shimannotations.CPUGroupID: "initrd-kargs-group", + shimannotations.StorageQoSIopsMaximum: "1000", + shimannotations.StorageQoSBandwidthMaximum: "100000", + }, + }, + { + // Previously a gap (CpuGroup nil vs empty) — fixed in v2 builder. + name: "no CPUGroupID with QoS annotations", + annotations: map[string]string{ + shimannotations.ProcessorCount: "2", + shimannotations.MemorySizeInMB: "2048", + shimannotations.StorageQoSIopsMaximum: "1000", + shimannotations.StorageQoSBandwidthMaximum: "100000", + }, + }, + { + // Previously a gap (StorageQoS nil vs empty) — fixed in v2 builder. + name: "no StorageQoS with CPUGroupID", + annotations: map[string]string{ + shimannotations.ProcessorCount: "2", + shimannotations.MemorySizeInMB: "2048", + shimannotations.CPUGroupID: "no-qos-group", + }, + }, + + // --- Cases that expose known differences between legacy and v2 --- + + { + // Initrd preferred — legacy allocates VPMem controller with no + // devices, v2 sets VirtualPMem=nil. + name: "gap: initrd boot (VPMem nil vs empty controller)", + annotations: map[string]string{ + shimannotations.PreferredRootFSType: "initrd", + shimannotations.KernelDirectBoot: "true", + shimannotations.CPUGroupID: "initrd-group", + shimannotations.StorageQoSIopsMaximum: "1000", + shimannotations.StorageQoSBandwidthMaximum: "100000", + }, + expectedDiffField: "VirtualPMem", + }} + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + ctx := context.Background() + + shimOpts := &runhcsopts.Options{ + SandboxPlatform: "linux/amd64", + BootFilesRootPath: bootDir, + } + if tc.shimOpts != nil { + shimOpts = tc.shimOpts() + } + + legacySpec := specs.Spec{ + Annotations: maps.Clone(tc.annotations), + Linux: &specs.Linux{}, + Windows: &specs.Windows{ + HyperV: &specs.WindowsHyperV{}, + Devices: tc.devices, + }, + } + if legacySpec.Annotations == nil { + legacySpec.Annotations = map[string]string{} + } + legacyDoc, legacyOpts, err := buildLegacyLCOWDocument(ctx, legacySpec, shimOpts, bootDir) + if err != nil { + t.Fatalf("failed to build legacy LCOW document: %v", err) + } + + v2Spec := &vm.Spec{ + Annotations: maps.Clone(tc.annotations), + Devices: tc.devices, + } + if v2Spec.Annotations == nil { + v2Spec.Annotations = map[string]string{} + } + v2Doc, sandboxOpts, err := buildV2LCOWDocument(ctx, shimOpts, v2Spec, bootDir) + if err != nil { + t.Fatalf("failed to build v2 LCOW document: %v", err) + } + + if testing.Verbose() { + t.Logf("Legacy options: %+v", legacyOpts) + t.Logf("V2 sandbox options: %+v", sandboxOpts) + } + + diff := cmp.Diff(legacyDoc, v2Doc) + + // Gap tests document known v2 builder bugs. They expect a + // diff and only fail if the documents unexpectedly match, + // signaling the bug was fixed and the gap test should be + // removed. + if strings.HasPrefix(tc.name, "gap:") { + if diff == "" { + t.Errorf("gap test unexpectedly passed: v2 builder bug may be fixed; remove from gaps") + } else if tc.expectedDiffField != "" && !strings.Contains(diff, tc.expectedDiffField) { + t.Errorf("gap test diff does not contain expected field %q (unexpected regression?):\n%s", tc.expectedDiffField, diff) + } else { + t.Logf("expected gap diff on field %q (-legacy +v2):\n%s", tc.expectedDiffField, diff) + } + return + } + + if diff != "" { + if isOnlyKernelCmdLineWhitespaceDiff(legacyDoc, v2Doc) { + t.Logf("WARNING: kernel cmdline has leading whitespace difference (legacy quirk): %s", diff) + } else { + t.Logf("Legacy document:\n%s", jsonToString(legacyDoc)) + t.Logf("V2 document:\n%s", jsonToString(v2Doc)) + t.Errorf("LCOW HCS document mismatch (-legacy +v2):\n%s", diff) + } } }) }