From b6a131c8efe4992dafe0669fe592333929850194 Mon Sep 17 00:00:00 2001 From: Shreyansh Sancheti Date: Mon, 16 Mar 2026 12:31:21 +0530 Subject: [PATCH 1/2] parity: restructure LCOW HCS document parity tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback from PR #2629. Major changes: Source changes (internal/uvm/): - Export VerifyOptions, MakeLCOWDoc, MakeLCOWSecurityDoc as public functions so they can be called directly from the test package. - Remove MakeLCOWDocument composite wrapper — test assembles the pipeline itself using the exported primitives. - Add NewUtilityVMForDoc constructor in types.go. This is needed because MakeLCOWDoc takes a *UtilityVM parameter and reads its unexported fields (scsiControllerCount, vpmemMaxCount, etc). The test cannot set these fields directly from outside the package, so NewUtilityVMForDoc creates a minimal UtilityVM with only the fields needed for document generation. - Original source code comments preserved — only function names changed from lowercase to uppercase. Test changes (test/parity/vm/): - Moved from test/parity/ to test/parity/vm/ for future WCOW support. - Removed functional build tag — these are not functional tests. - Removed all normalization (nil-vs-empty, map sorting, owner zeroing). cmp.Diff handles maps natively. Real differences are not masked. - Merged pipeline helpers into hcs_document_creator_test.go. - Removed helpers_test.go — setupBootFiles and jsonToString moved inline. - Generic doc.go that covers both LCOW and future WCOW. - All test cases explicitly populate every field (CPU, memory, MMIO, QoS, CPUGroupID) so comparison always checks populated values, not defaults. - Use maps.Clone for annotation copying. - Use testing.Verbose() to gate debug logging. - Descriptive error messages throughout. All 8 tests pass (3 document parity + 5 field parity). Signed-off-by: Shreyansh Sancheti --- internal/uvm/create.go | 2 +- internal/uvm/create_lcow.go | 10 +- internal/uvm/create_wcow.go | 2 +- internal/uvm/types.go | 16 ++ test/parity/vm/doc.go | 13 ++ test/parity/vm/hcs_document_creator_test.go | 113 +++++++++++ test/parity/vm/lcow_doc_test.go | 203 ++++++++++++++++++++ 7 files changed, 352 insertions(+), 7 deletions(-) create mode 100644 test/parity/vm/doc.go create mode 100644 test/parity/vm/hcs_document_creator_test.go create mode 100644 test/parity/vm/lcow_doc_test.go diff --git a/internal/uvm/create.go b/internal/uvm/create.go index 45eadde81e..846982747b 100644 --- a/internal/uvm/create.go +++ b/internal/uvm/create.go @@ -160,7 +160,7 @@ func verifyWCOWBootFiles(bootFiles *WCOWBootFiles) error { } // Verifies that the final UVM options are correct and supported. -func verifyOptions(_ context.Context, options interface{}) error { +func VerifyOptions(_ context.Context, options interface{}) error { switch opts := options.(type) { case *OptionsLCOW: if opts.EnableDeferredCommit && !opts.AllowOvercommit { diff --git a/internal/uvm/create_lcow.go b/internal/uvm/create_lcow.go index 96595ae8c4..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 @@ -537,7 +537,7 @@ Example JSON document produced once the hcsschema.ComputeSytem returned by makeL */ // Make the ComputeSystem document object that will be serialized to json to be presented to the HCS api. -func makeLCOWDoc(ctx context.Context, opts *OptionsLCOW, uvm *UtilityVM) (_ *hcsschema.ComputeSystem, err error) { +func MakeLCOWDoc(ctx context.Context, opts *OptionsLCOW, uvm *UtilityVM) (_ *hcsschema.ComputeSystem, err error) { if logrus.IsLevelEnabled(logrus.TraceLevel) { log.G(ctx).WithField("options", log.Format(ctx, opts)).Trace("makeLCOWDoc") } @@ -931,14 +931,14 @@ func CreateLCOW(ctx context.Context, opts *OptionsLCOW) (_ *UtilityVM, err error uvm.scsiControllerCount = 4 } - if err = verifyOptions(ctx, opts); err != nil { + if err = VerifyOptions(ctx, opts); err != nil { return nil, errors.Wrap(err, errBadUVMOpts.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), @@ -946,7 +946,7 @@ func CreateLCOW(ctx context.Context, opts *OptionsLCOW) (_ *UtilityVM, err error }).Trace("create_lcow::CreateLCOW makeLCOWSecurityDoc result") } } else { - doc, err = makeLCOWDoc(ctx, opts, uvm) + doc, err = MakeLCOWDoc(ctx, opts, uvm) if logrus.IsLevelEnabled(logrus.TraceLevel) { log.G(ctx).WithFields(logrus.Fields{ "doc": log.Format(ctx, doc), diff --git a/internal/uvm/create_wcow.go b/internal/uvm/create_wcow.go index 72308cf82b..d0ee3b2355 100644 --- a/internal/uvm/create_wcow.go +++ b/internal/uvm/create_wcow.go @@ -586,7 +586,7 @@ func CreateWCOW(ctx context.Context, opts *OptionsWCOW) (_ *UtilityVM, err error } }() - if err := verifyOptions(ctx, opts); err != nil { + if err := VerifyOptions(ctx, opts); err != nil { return nil, errors.Wrap(err, errBadUVMOpts.Error()) } diff --git a/internal/uvm/types.go b/internal/uvm/types.go index 84d08c0f2b..0f29079149 100644 --- a/internal/uvm/types.go +++ b/internal/uvm/types.go @@ -152,6 +152,22 @@ func (uvm *UtilityVM) ScratchEncryptionEnabled() bool { return uvm.encryptScratch } +// NewUtilityVMForDoc creates a minimal UtilityVM with the fields needed by +// 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, + owner: owner, + operatingSystem: "linux", + scsiControllerCount: scsiControllerCount, + vpmemMaxCount: vpmemMaxCount, + vpmemMaxSizeBytes: vpmemMaxSizeBytes, + vpciDevices: make(map[VPCIDeviceID]*VPCIDevice), + vpmemMultiMapping: vpmemMultiMapping, + } +} + type WCOWBootFilesType uint8 const ( diff --git a/test/parity/vm/doc.go b/test/parity/vm/doc.go new file mode 100644 index 0000000000..346c3d6557 --- /dev/null +++ b/test/parity/vm/doc.go @@ -0,0 +1,13 @@ +//go:build windows + +// Package vm validates that the v2 VM document builders produce HCS +// ComputeSystem documents equivalent to the legacy shim pipelines. +// +// 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 vm diff --git a/test/parity/vm/hcs_document_creator_test.go b/test/parity/vm/hcs_document_creator_test.go new file mode 100644 index 0000000000..3c7f2f33b0 --- /dev/null +++ b/test/parity/vm/hcs_document_creator_test.go @@ -0,0 +1,113 @@ +//go:build windows + +package vm + +import ( + "context" + "encoding/json" + "fmt" + "os" + "path/filepath" + "testing" + + "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" + hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" + "github.com/Microsoft/hcsshim/internal/oci" + "github.com/Microsoft/hcsshim/internal/uvm" + "github.com/Microsoft/hcsshim/internal/vm/vmutils" + "github.com/Microsoft/hcsshim/osversion" + vm "github.com/Microsoft/hcsshim/sandbox-spec/vm/v2" +) + +// buildLegacyLCOWDocument creates the HCS document for an LCOW VM using the +// legacy shim pipeline. It runs the same sequence as createInternal → createPod +// → CreateLCOW: annotation processing, spec conversion, option verification, +// and document generation. +func buildLegacyLCOWDocument( + ctx context.Context, + spec specs.Spec, + shimOpts *runhcsopts.Options, + bundle string, +) (*hcsschema.ComputeSystem, *uvm.OptionsLCOW, error) { + // Step 1: Merge shim options into the OCI spec annotations. + spec = oci.UpdateSpecFromOptions(spec, shimOpts) + + // Step 2: Expand annotation groups (e.g., security toggles). + if err := oci.ProcessAnnotations(ctx, spec.Annotations); err != nil { + return nil, nil, fmt.Errorf("failed to expand OCI annotations: %w", err) + } + + // Step 3: Convert OCI spec + annotations into OptionsLCOW. + rawOpts, err := oci.SpecToUVMCreateOpts(ctx, &spec, "test-parity@vm", "test-owner") + if err != nil { + return nil, nil, fmt.Errorf("failed to convert OCI spec to UVM create options: %w", err) + } + opts := rawOpts.(*uvm.OptionsLCOW) + opts.BundleDirectory = bundle + + // Step 4: Verify options constraints (same as CreateLCOW). + if err := uvm.VerifyOptions(ctx, opts); err != nil { + return nil, nil, fmt.Errorf("option verification failed: %w", err) + } + + // Step 5: Build the temporary UtilityVM with fields that MakeLCOWDoc reads. + scsiCount := opts.SCSIControllerCount + if osversion.Build() >= osversion.RS5 && opts.VPMemDeviceCount == 0 { + scsiCount = 4 + } + tempUVM := uvm.NewUtilityVMForDoc( + opts.ID, opts.Owner, + scsiCount, opts.VPMemDeviceCount, opts.VPMemSizeBytes, + !opts.VPMemNoMultiMapping, + ) + + // Step 6: Generate the HCS document. + doc, err := uvm.MakeLCOWDoc(ctx, opts, tempUVM) + if err != nil { + return nil, nil, fmt.Errorf("failed to generate legacy LCOW HCS document: %w", err) + } + + return doc, opts, nil +} + +// buildV2LCOWDocument creates the HCS document and sandbox options from the +// provided VM spec and runhcs options using the v2 modular builder. +// The returned document can be used to create a VM directly via HCS. +func buildV2LCOWDocument( + ctx context.Context, + shimOpts *runhcsopts.Options, + spec *vm.Spec, + bundle string, +) (*hcsschema.ComputeSystem, *lcowbuilder.SandboxOptions, error) { + return lcowbuilder.BuildSandboxConfig(ctx, "test-owner", bundle, shimOpts, spec) +} + +// setupBootFiles creates a temporary directory containing the kernel and rootfs +// files that both document builders probe during boot configuration resolution. +func setupBootFiles(t *testing.T) string { + t.Helper() + dir := t.TempDir() + for _, name := range []string{ + vmutils.KernelFile, + vmutils.UncompressedKernelFile, + vmutils.InitrdFile, + vmutils.VhdFile, + } { + if err := os.WriteFile(filepath.Join(dir, name), []byte("test"), 0644); err != nil { + t.Fatalf("failed to create boot file %s: %v", name, err) + } + } + return dir +} + +// jsonToString serializes v to indented JSON for test log output. +func jsonToString(v interface{}) string { + b, err := json.MarshalIndent(v, "", " ") + if err != nil { + panic(err) + } + return string(b) +} diff --git a/test/parity/vm/lcow_doc_test.go b/test/parity/vm/lcow_doc_test.go new file mode 100644 index 0000000000..d502fee84d --- /dev/null +++ b/test/parity/vm/lcow_doc_test.go @@ -0,0 +1,203 @@ +//go:build windows + +package vm + +import ( + "context" + "maps" + "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" + shimannotations "github.com/Microsoft/hcsshim/pkg/annotations" + vm "github.com/Microsoft/hcsshim/sandbox-spec/vm/v2" +) + +// TestLCOWDocumentParity feeds identical annotations, devices, and shim options +// to both the legacy and v2 LCOW pipelines and verifies the resulting HCS +// ComputeSystem documents are structurally identical. +func TestLCOWDocumentParity(t *testing.T) { + bootDir := setupBootFiles(t) + + tests := []struct { + name string + annotations map[string]string + devices []specs.WindowsDevice + shimOpts func() *runhcsopts.Options + }{ + { + name: "CPU + memory + QoS + MMIO + CPUGroup", + annotations: map[string]string{ + shimannotations.ProcessorCount: "2", + shimannotations.ProcessorLimit: "50000", + shimannotations.ProcessorWeight: "500", + shimannotations.CPUGroupID: "test-cpu-group-id-123", + shimannotations.MemorySizeInMB: "2048", + shimannotations.AllowOvercommit: "true", + shimannotations.EnableColdDiscardHint: "true", + shimannotations.MemoryLowMMIOGapInMB: "256", + shimannotations.MemoryHighMMIOBaseInMB: "1024", + shimannotations.MemoryHighMMIOGapInMB: "512", + shimannotations.StorageQoSIopsMaximum: "5000", + shimannotations.StorageQoSBandwidthMaximum: "1000000", + }, + }, + { + name: "memory + MMIO + QoS (overcommit off)", + annotations: map[string]string{ + shimannotations.MemorySizeInMB: "4096", + shimannotations.AllowOvercommit: "false", + shimannotations.MemoryLowMMIOGapInMB: "256", + shimannotations.MemoryHighMMIOBaseInMB: "1024", + shimannotations.MemoryHighMMIOGapInMB: "512", + shimannotations.CPUGroupID: "test-cpu-group-id-456", + shimannotations.StorageQoSIopsMaximum: "3000", + shimannotations.StorageQoSBandwidthMaximum: "500000", + }, + }, + { + name: "shim options CPU/memory + annotation QoS", + shimOpts: func() *runhcsopts.Options { + return &runhcsopts.Options{ + SandboxPlatform: "linux/amd64", + BootFilesRootPath: bootDir, + VmProcessorCount: 2, + VmMemorySizeInMb: 2048, + } + }, + annotations: map[string]string{ + shimannotations.CPUGroupID: "test-cpu-group-id-789", + shimannotations.StorageQoSIopsMaximum: "5000", + shimannotations.StorageQoSBandwidthMaximum: "1000000", + }, + }, + } + + 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() + } + + // Legacy path: OCI spec with annotations and devices. + 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) + } + + // V2 path: vm.Spec with the same annotations and devices. + 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) + } + + 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) + } + }) + } +} + +// TestLCOWSandboxOptionsFieldParity verifies that configuration fields carried +// by the legacy OptionsLCOW have matching values in the v2 SandboxOptions when +// both pipelines receive the same default inputs. +func TestLCOWSandboxOptionsFieldParity(t *testing.T) { + bootDir := setupBootFiles(t) + + tests := []struct { + name string + annotations map[string]string + }{ + { + name: "default config", + }, + } + + 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, + } + + legacySpec := specs.Spec{ + Annotations: maps.Clone(tc.annotations), + Linux: &specs.Linux{}, + Windows: &specs.Windows{HyperV: &specs.WindowsHyperV{}}, + } + if legacySpec.Annotations == nil { + legacySpec.Annotations = map[string]string{} + } + _, 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)} + if v2Spec.Annotations == nil { + v2Spec.Annotations = map[string]string{} + } + _, sandboxOpts, err := buildV2LCOWDocument(ctx, shimOpts, v2Spec, bootDir) + if err != nil { + t.Fatalf("failed to build v2 LCOW document: %v", err) + } + + 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) + } + }) + } + }) + } +} From c24edacf7fd42f1fcc05b37264cb2b228d7c8ac0 Mon Sep 17 00:00:00 2001 From: Shreyansh Sancheti Date: Mon, 16 Mar 2026 16:23:15 +0530 Subject: [PATCH 2/2] parity: add LCOW document permutation tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add TestLCOWDocumentParityPermutations to exercise annotation and option combinations that trigger different document construction branches in the legacy and v2 LCOW pipelines. Each test sets all required fields so comparisons check real values rather than defaults. Permutation categories: - CPU partial combinations (count, limit, weight) - Memory (overcommit disabled, cold discard hint) - Boot mode (kernel direct + VHD rootfs) - Feature flags (scratch encryption, writable overlay) - Device interactions (VPMem disabled → 4 SCSI controllers) - Cross-group (physically backed + VPMem + encryption) - Shim option overrides (annotation CPU/memory priority) - Kernel args (VPCIEnabled, time sync, process dump, initrd boot) Gap tests document three known v2 builder differences: - No CPUGroupID: legacy nil vs v2 empty CpuGroup struct - No StorageQoS: legacy nil vs v2 empty StorageQoS struct - Initrd boot: legacy VPMem controller vs v2 nil VirtualPMem Gap tests use inverted assertions — they expect a diff and only fail if documents unexpectedly match, signaling the v2 bug was fixed. Also adds normalizeKernelCmdLine and isOnlyKernelCmdLineWhitespaceDiff helpers to handle a known legacy quirk where initrd+KernelDirect boot produces a leading space in kernel command lines that v2 correctly omits. Signed-off-by: Shreyansh Sancheti --- test/parity/vm/hcs_document_creator_test.go | 74 +++++ test/parity/vm/lcow_doc_test.go | 349 +++++++++++++++++++- 2 files changed, 420 insertions(+), 3 deletions(-) diff --git a/test/parity/vm/hcs_document_creator_test.go b/test/parity/vm/hcs_document_creator_test.go index 3c7f2f33b0..8a9c229a39 100644 --- a/test/parity/vm/hcs_document_creator_test.go +++ b/test/parity/vm/hcs_document_creator_test.go @@ -8,10 +8,13 @@ import ( "fmt" "os" "path/filepath" + "strings" "testing" "github.com/opencontainers/runtime-spec/specs-go" + "github.com/google/go-cmp/cmp" + runhcsopts "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options" lcowbuilder "github.com/Microsoft/hcsshim/internal/builder/vm/lcow" hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" @@ -111,3 +114,74 @@ 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. +// This is a known legacy quirk where initrd+KernelDirect boot produces a +// leading space that v2 correctly omits. +func isOnlyKernelCmdLineWhitespaceDiff(legacy, v2 *hcsschema.ComputeSystem) bool { + // Deep copy and normalize, then re-compare. + legacyCopy := *legacy + v2Copy := *v2 + // Shallow copy the VM and chipset to avoid mutating originals. + 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 d502fee84d..2fcdf4c717 100644 --- a/test/parity/vm/lcow_doc_test.go +++ b/test/parity/vm/lcow_doc_test.go @@ -5,12 +5,14 @@ package vm 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" + iannotations "github.com/Microsoft/hcsshim/internal/annotations" shimannotations "github.com/Microsoft/hcsshim/pkg/annotations" vm "github.com/Microsoft/hcsshim/sandbox-spec/vm/v2" ) @@ -124,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) + } } }) } @@ -201,3 +209,338 @@ func TestLCOWSandboxOptionsFieldParity(t *testing.T) { }) } } + +// 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) + + 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 + }{ + // --- 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 --- + + { + 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", + }, + }, + + // --- Cases that expose known differences between legacy and v2 --- + // These document real parity gaps for the v2 builder team to fix. + + { + // No CPUGroupID set — legacy produces CpuGroup=nil, + // v2 produces CpuGroup=&{Id:""} (unconditional init in topology.go). + name: "gap: no CPUGroupID (nil vs empty CpuGroup)", + annotations: map[string]string{ + shimannotations.ProcessorCount: "2", + shimannotations.MemorySizeInMB: "2048", + shimannotations.StorageQoSIopsMaximum: "1000", + shimannotations.StorageQoSBandwidthMaximum: "100000", + }, + expectedDiffField: "CpuGroup", + }, + { + // No QoS set — legacy produces StorageQoS=nil, + // v2 may produce StorageQoS=&{} depending on builder. + name: "gap: no StorageQoS (nil vs empty)", + annotations: map[string]string{ + shimannotations.ProcessorCount: "2", + shimannotations.MemorySizeInMB: "2048", + shimannotations.CPUGroupID: "no-qos-group", + }, + expectedDiffField: "StorageQoS", + }, + { + // 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) + } + } + }) + } +}