From 6893498267ae5f39004b857a98f6e88c8d83805d Mon Sep 17 00:00:00 2001 From: Dusty Mabe Date: Fri, 27 Mar 2026 18:04:05 +0000 Subject: [PATCH 1/6] mantle/platform: fold QemuMachineOptions into MachineOptions Maintaining separate QemuMachineOptions and MachineOptions structs led to complications: awkward two-level nesting at call sites, a shadowed Firmware field, and type assertions needed just to pass QEMU-specific options. Move the QEMU-only fields (HostForwardPorts, DisablePDeathSig, OverrideBackingFile, Nvme, Cex) into MachineOptions and delete the QemuMachineOptions type. Add EnsureNoQEMUOnlyOptions() so non-QEMU platforms (aws, azure, gcp, do, esx, openstack) reject these options early with a clear error. Keep NewMachineWithQemuOptions as a thin alias on qemu/qemuiso clusters for backward compatibility. Written-by: --- mantle/cmd/kola/spawn.go | 2 +- mantle/kola/tests/coretest/core.go | 2 +- mantle/kola/tests/ignition/kdump.go | 4 +-- mantle/kola/tests/ignition/luks.go | 8 +++--- mantle/kola/tests/misc/boot-mirror.go | 16 ++++------- mantle/kola/tests/misc/network.go | 6 ++-- mantle/kola/tests/ostree/sync.go | 4 +-- mantle/kola/tests/rhcos/upgrade.go | 2 +- mantle/platform/machine/aws/cluster.go | 3 ++ mantle/platform/machine/azure/cluster.go | 3 ++ mantle/platform/machine/do/cluster.go | 3 ++ mantle/platform/machine/esx/cluster.go | 3 ++ mantle/platform/machine/gcloud/cluster.go | 3 ++ mantle/platform/machine/openstack/cluster.go | 3 ++ mantle/platform/machine/qemu/cluster.go | 14 ++++----- mantle/platform/machine/qemuiso/cluster.go | 11 ++++--- mantle/platform/platform.go | 30 ++++++++++++++++++++ mantle/platform/qemu.go | 11 ------- 18 files changed, 79 insertions(+), 49 deletions(-) diff --git a/mantle/cmd/kola/spawn.go b/mantle/cmd/kola/spawn.go index c71964e065..4b83be0600 100644 --- a/mantle/cmd/kola/spawn.go +++ b/mantle/cmd/kola/spawn.go @@ -164,7 +164,7 @@ func runSpawn(cmd *cobra.Command, args []string) error { } // use qemu-specific interface only if needed if strings.HasPrefix(kolaPlatform, "qemu") && (spawnMachineOptions != "" || !spawnRemove) { - machineOpts := platform.QemuMachineOptions{ + machineOpts := platform.MachineOptions{ DisablePDeathSig: !spawnRemove, } if spawnMachineOptions != "" { diff --git a/mantle/kola/tests/coretest/core.go b/mantle/kola/tests/coretest/core.go index 4e27a89d10..066c91e64c 100644 --- a/mantle/kola/tests/coretest/core.go +++ b/mantle/kola/tests/coretest/core.go @@ -129,7 +129,7 @@ func runBasicTests(c cluster.TestCluster, firmware string, nvme bool) { var err error var m platform.Machine - options := platform.QemuMachineOptions{ + options := platform.MachineOptions{ Firmware: firmware, Nvme: nvme, } diff --git a/mantle/kola/tests/ignition/kdump.go b/mantle/kola/tests/ignition/kdump.go index 2429504ced..5d5ce61664 100644 --- a/mantle/kola/tests/ignition/kdump.go +++ b/mantle/kola/tests/ignition/kdump.go @@ -106,7 +106,7 @@ func setupSSHMachine(c cluster.TestCluster) SshServer { var address string var port string - options := platform.QemuMachineOptions{ + options := platform.MachineOptions{ HostForwardPorts: []platform.HostForwardPort{ {Service: "ssh", HostPort: 0, GuestPort: 22}, }, @@ -253,7 +253,7 @@ func setupNFSMachine(c cluster.TestCluster) NfsServer { var m platform.Machine var err error - options := platform.QemuMachineOptions{ + options := platform.MachineOptions{ HostForwardPorts: []platform.HostForwardPort{ {Service: "ssh", HostPort: 0, GuestPort: 22}, // Kdump NFS option does not allow a custom port diff --git a/mantle/kola/tests/ignition/luks.go b/mantle/kola/tests/ignition/luks.go index 2efb066678..813d6ef61e 100644 --- a/mantle/kola/tests/ignition/luks.go +++ b/mantle/kola/tests/ignition/luks.go @@ -74,7 +74,7 @@ func setupTangMachine(c cluster.TestCluster) ut.TangServer { var thumbprint []byte var tangAddress string - options := platform.QemuMachineOptions{ + options := platform.MachineOptions{ HostForwardPorts: []platform.HostForwardPort{ {Service: "ssh", HostPort: 0, GuestPort: 22}, {Service: "tang", HostPort: 0, GuestPort: 80}, @@ -234,10 +234,10 @@ func runCexTest(c cluster.TestCluster) { } }`) - opts := platform.QemuMachineOptions{ - Cex: true, + opts := platform.MachineOptions{ + Cex: true, + MinMemory: 8192, } - opts.MinMemory = 8192 switch pc := c.Cluster.(type) { case *qemu.Cluster: diff --git a/mantle/kola/tests/misc/boot-mirror.go b/mantle/kola/tests/misc/boot-mirror.go index e246ac3714..27ec960d96 100644 --- a/mantle/kola/tests/misc/boot-mirror.go +++ b/mantle/kola/tests/misc/boot-mirror.go @@ -94,11 +94,9 @@ func init() { func runBootMirrorTest(c cluster.TestCluster) { var m platform.Machine var err error - options := platform.QemuMachineOptions{ - MachineOptions: platform.MachineOptions{ - AdditionalDisks: []string{"5G", "5G"}, - MinMemory: 4096, - }, + options := platform.MachineOptions{ + AdditionalDisks: []string{"5G", "5G"}, + MinMemory: 4096, } // ppc64le uses 64K pages; see similar logic in harness.go and luks.go switch coreosarch.CurrentRpmArch() { @@ -146,11 +144,9 @@ func runBootMirrorTest(c cluster.TestCluster) { func runBootMirrorLUKSTest(c cluster.TestCluster) { var m platform.Machine var err error - options := platform.QemuMachineOptions{ - MachineOptions: platform.MachineOptions{ - AdditionalDisks: []string{"5G"}, - MinMemory: 4096, - }, + options := platform.MachineOptions{ + AdditionalDisks: []string{"5G"}, + MinMemory: 4096, } // ppc64le uses 64K pages; see similar logic in harness.go and luks.go switch coreosarch.CurrentRpmArch() { diff --git a/mantle/kola/tests/misc/network.go b/mantle/kola/tests/misc/network.go index cee6b8a57a..1aff5081ee 100644 --- a/mantle/kola/tests/misc/network.go +++ b/mantle/kola/tests/misc/network.go @@ -505,10 +505,8 @@ func setupMultipleNetworkTest(c cluster.TestCluster, primaryMac, secondaryMac st var m platform.Machine var err error - options := platform.QemuMachineOptions{ - MachineOptions: platform.MachineOptions{ - AdditionalNics: 2, - }, + options := platform.MachineOptions{ + AdditionalNics: 2, } // On s390x, multiple NICs are ordered by the CCW device number. Use classic ethX names to ensure consistent and ordered naming. if runtime.GOARCH == "s390x" { diff --git a/mantle/kola/tests/ostree/sync.go b/mantle/kola/tests/ostree/sync.go index 9253e08209..cc030d6366 100644 --- a/mantle/kola/tests/ostree/sync.go +++ b/mantle/kola/tests/ostree/sync.go @@ -96,13 +96,13 @@ func setupNFSMachine(c cluster.TestCluster) NfsServer { var err error var nfs_server string - options := platform.QemuMachineOptions{ + options := platform.MachineOptions{ HostForwardPorts: []platform.HostForwardPort{ {Service: "ssh", HostPort: 0, GuestPort: 22}, {Service: "nfs", HostPort: 2049, GuestPort: 2049}, }, + MinMemory: 2048, } - options.MinMemory = 2048 // start the machine switch c := c.Cluster.(type) { // These cases have to be separated because when put together to the same case statement diff --git a/mantle/kola/tests/rhcos/upgrade.go b/mantle/kola/tests/rhcos/upgrade.go index 8f435c3354..ea23946cb3 100644 --- a/mantle/kola/tests/rhcos/upgrade.go +++ b/mantle/kola/tests/rhcos/upgrade.go @@ -205,7 +205,7 @@ func rhcosUpgradeBasic(c cluster.TestCluster) { // no downgraded packages func rhcosUpgradeFromOcpRhcos(c cluster.TestCluster) { var m platform.Machine - options := platform.QemuMachineOptions{} + options := platform.MachineOptions{} ignition := conf.Ignition(`{ "ignition": { "version": "3.0.0" diff --git a/mantle/platform/machine/aws/cluster.go b/mantle/platform/machine/aws/cluster.go index 2fbf1e486e..889b4d452c 100644 --- a/mantle/platform/machine/aws/cluster.go +++ b/mantle/platform/machine/aws/cluster.go @@ -37,6 +37,9 @@ func (ac *cluster) NewMachine(userdata *conf.UserData) (platform.Machine, error) } func (ac *cluster) NewMachineWithOptions(userdata *conf.UserData, options platform.MachineOptions) (platform.Machine, error) { + if err := options.EnsureNoQEMUOnlyOptions("aws"); err != nil { + return nil, err + } if len(options.AdditionalDisks) > 0 { return nil, errors.New("platform aws does not yet support additional disks") } diff --git a/mantle/platform/machine/azure/cluster.go b/mantle/platform/machine/azure/cluster.go index ae97127497..19ccee9e39 100644 --- a/mantle/platform/machine/azure/cluster.go +++ b/mantle/platform/machine/azure/cluster.go @@ -46,6 +46,9 @@ func (ac *cluster) NewMachine(userdata *conf.UserData) (platform.Machine, error) } func (ac *cluster) NewMachineWithOptions(userdata *conf.UserData, options platform.MachineOptions) (platform.Machine, error) { + if err := options.EnsureNoQEMUOnlyOptions("azure"); err != nil { + return nil, err + } if options.MultiPathDisk { return nil, errors.New("platform azure does not support multipathed disks") } diff --git a/mantle/platform/machine/do/cluster.go b/mantle/platform/machine/do/cluster.go index f5808122d7..535c7097ca 100644 --- a/mantle/platform/machine/do/cluster.go +++ b/mantle/platform/machine/do/cluster.go @@ -37,6 +37,9 @@ func (dc *cluster) NewMachine(userdata *conf.UserData) (platform.Machine, error) } func (dc *cluster) NewMachineWithOptions(userdata *conf.UserData, options platform.MachineOptions) (platform.Machine, error) { + if err := options.EnsureNoQEMUOnlyOptions("do"); err != nil { + return nil, err + } if len(options.AdditionalDisks) > 0 { return nil, errors.New("platform do does not yet support additional disks") } diff --git a/mantle/platform/machine/esx/cluster.go b/mantle/platform/machine/esx/cluster.go index 16d372f963..1091091b9b 100644 --- a/mantle/platform/machine/esx/cluster.go +++ b/mantle/platform/machine/esx/cluster.go @@ -43,6 +43,9 @@ func (ec *cluster) NewMachine(userdata *platformConf.UserData) (platform.Machine } func (ec *cluster) NewMachineWithOptions(userdata *platformConf.UserData, options platform.MachineOptions) (platform.Machine, error) { + if err := options.EnsureNoQEMUOnlyOptions("esx"); err != nil { + return nil, err + } if len(options.AdditionalDisks) > 0 { return nil, errors.New("platform esx does not yet support additional disks") } diff --git a/mantle/platform/machine/gcloud/cluster.go b/mantle/platform/machine/gcloud/cluster.go index 3a6ee4d094..64b2620175 100644 --- a/mantle/platform/machine/gcloud/cluster.go +++ b/mantle/platform/machine/gcloud/cluster.go @@ -37,6 +37,9 @@ func (gc *cluster) NewMachine(userdata *conf.UserData) (platform.Machine, error) } func (gc *cluster) NewMachineWithOptions(userdata *conf.UserData, options platform.MachineOptions) (platform.Machine, error) { + if err := options.EnsureNoQEMUOnlyOptions("gcp"); err != nil { + return nil, err + } if options.MultiPathDisk { return nil, errors.New("platform gcp does not support multipathed disks") } diff --git a/mantle/platform/machine/openstack/cluster.go b/mantle/platform/machine/openstack/cluster.go index 3a634c2107..240e3eb95e 100644 --- a/mantle/platform/machine/openstack/cluster.go +++ b/mantle/platform/machine/openstack/cluster.go @@ -35,6 +35,9 @@ func (oc *cluster) NewMachine(userdata *conf.UserData) (platform.Machine, error) } func (oc *cluster) NewMachineWithOptions(userdata *conf.UserData, options platform.MachineOptions) (platform.Machine, error) { + if err := options.EnsureNoQEMUOnlyOptions("openstack"); err != nil { + return nil, err + } if len(options.AdditionalDisks) > 0 { return nil, errors.New("platform openstack does not yet support additional disks") } diff --git a/mantle/platform/machine/qemu/cluster.go b/mantle/platform/machine/qemu/cluster.go index 356587f525..6d66a81991 100644 --- a/mantle/platform/machine/qemu/cluster.go +++ b/mantle/platform/machine/qemu/cluster.go @@ -51,13 +51,6 @@ func (qc *Cluster) NewMachineWithOptions(userdata *conf.UserData, options platfo if options.InstanceType != "" { return nil, errors.New("platform qemu does not support changing instance types") } - return qc.NewMachineWithQemuOptions(userdata, platform.QemuMachineOptions{ - MachineOptions: options, - Firmware: options.Firmware, - }) -} - -func (qc *Cluster) NewMachineWithQemuOptions(userdata *conf.UserData, options platform.QemuMachineOptions) (platform.Machine, error) { id := uuid.New() dir := filepath.Join(qc.RuntimeConf().OutputDir, id) @@ -237,6 +230,13 @@ func (qc *Cluster) NewMachineWithQemuOptions(userdata *conf.UserData, options pl return qm, nil } +// NewMachineWithQemuOptions is a convenience alias for NewMachineWithOptions. +// Callers that previously used QemuMachineOptions can now pass MachineOptions +// directly — the QEMU-specific fields live there. +func (qc *Cluster) NewMachineWithQemuOptions(userdata *conf.UserData, options platform.MachineOptions) (platform.Machine, error) { + return qc.NewMachineWithOptions(userdata, options) +} + func (qc *Cluster) Destroy() { qc.tearingDown.Store(true) qc.BaseCluster.Destroy() diff --git a/mantle/platform/machine/qemuiso/cluster.go b/mantle/platform/machine/qemuiso/cluster.go index b32ae2b604..a3ca40c8d2 100644 --- a/mantle/platform/machine/qemuiso/cluster.go +++ b/mantle/platform/machine/qemuiso/cluster.go @@ -49,12 +49,6 @@ func (qc *Cluster) NewMachineWithOptions(userdata *conf.UserData, options platfo if options.MultiPathDisk { return nil, errors.New("platform qemu-iso does not support multipathed primary disks") } - return qc.NewMachineWithQemuOptions(userdata, platform.QemuMachineOptions{ - MachineOptions: options, - }) -} - -func (qc *Cluster) NewMachineWithQemuOptions(userdata *conf.UserData, options platform.QemuMachineOptions) (platform.Machine, error) { id := uuid.New() dir := filepath.Join(qc.RuntimeConf().OutputDir, id) @@ -165,6 +159,11 @@ func (qc *Cluster) NewMachineWithQemuOptions(userdata *conf.UserData, options pl return qm, nil } +// NewMachineWithQemuOptions is a convenience alias for NewMachineWithOptions. +func (qc *Cluster) NewMachineWithQemuOptions(userdata *conf.UserData, options platform.MachineOptions) (platform.Machine, error) { + return qc.NewMachineWithOptions(userdata, options) +} + func (qc *Cluster) Destroy() { qc.BaseCluster.Destroy() qc.flight.DelCluster(qc) diff --git a/mantle/platform/platform.go b/mantle/platform/platform.go index da2e0bb36d..8873dcca01 100644 --- a/mantle/platform/platform.go +++ b/mantle/platform/platform.go @@ -168,6 +168,36 @@ type MachineOptions struct { AppendFirstbootKernelArgs string InstanceType string Firmware string + + // Fields below are only supported on QEMU-based platforms. + // Non-QEMU platforms call EnsureNoQEMUOnlyOptions() to reject them. + HostForwardPorts []HostForwardPort + DisablePDeathSig bool + OverrideBackingFile string + Nvme bool + Cex bool +} + +// EnsureNoQEMUOnlyOptions returns an error if any QEMU-only options +// are set. Non-QEMU platforms should call this to reject unsupported +// options early. +func (m *MachineOptions) EnsureNoQEMUOnlyOptions(platformName string) error { + if len(m.HostForwardPorts) > 0 { + return fmt.Errorf("platform %s does not support host forward ports", platformName) + } + if m.DisablePDeathSig { + return fmt.Errorf("platform %s does not support DisablePDeathSig", platformName) + } + if m.OverrideBackingFile != "" { + return fmt.Errorf("platform %s does not support OverrideBackingFile", platformName) + } + if m.Nvme { + return fmt.Errorf("platform %s does not support NVMe", platformName) + } + if m.Cex { + return fmt.Errorf("platform %s does not support Cex", platformName) + } + return nil } // SystemdDropin is a userdata type agnostic struct representing a systemd dropin diff --git a/mantle/platform/qemu.go b/mantle/platform/qemu.go index 3bc0fd7cb7..916a3e3b2f 100644 --- a/mantle/platform/qemu.go +++ b/mantle/platform/qemu.go @@ -66,17 +66,6 @@ type HostForwardPort struct { GuestPort int } -// QemuMachineOptions is specialized MachineOption struct for QEMU. -type QemuMachineOptions struct { - MachineOptions - HostForwardPorts []HostForwardPort - DisablePDeathSig bool - OverrideBackingFile string - Firmware string - Nvme bool - Cex bool -} - // QEMUMachine represents a qemu instance. type QEMUMachine interface { // Embedding the Machine interface From a9df315aae69fe4c8feb98ba4b1957c97c5361b5 Mon Sep 17 00:00:00 2001 From: Dusty Mabe Date: Fri, 27 Mar 2026 18:09:48 +0000 Subject: [PATCH 2/6] mantle/platform: move more QEMU-only fields into EnsureNoQEMUOnlyOptions MultiPathDisk, PrimaryDisk, MinMemory, NumaNodes, AdditionalNics, AppendKernelArgs, AppendFirstbootKernelArgs, and Firmware are only implemented by QEMU-based platforms. Move them below the QEMU-only comment boundary and check them in EnsureNoQEMUOnlyOptions(), removing the now-redundant per-platform rejection checks in the non-QEMU cluster implementations. The remaining cross-platform fields are AdditionalDisks (qemu, qemuiso, azure, gcloud), MinDiskSize (qemu, aws), and InstanceType (azure). Written-by: --- mantle/platform/machine/aws/cluster.go | 17 -------- mantle/platform/machine/azure/cluster.go | 13 ------ mantle/platform/machine/do/cluster.go | 12 ----- mantle/platform/machine/esx/cluster.go | 12 ----- mantle/platform/machine/gcloud/cluster.go | 12 ----- mantle/platform/machine/openstack/cluster.go | 12 ----- mantle/platform/platform.go | 46 +++++++++++++++----- 7 files changed, 35 insertions(+), 89 deletions(-) diff --git a/mantle/platform/machine/aws/cluster.go b/mantle/platform/machine/aws/cluster.go index 889b4d452c..9ef5d29771 100644 --- a/mantle/platform/machine/aws/cluster.go +++ b/mantle/platform/machine/aws/cluster.go @@ -43,23 +43,6 @@ func (ac *cluster) NewMachineWithOptions(userdata *conf.UserData, options platfo if len(options.AdditionalDisks) > 0 { return nil, errors.New("platform aws does not yet support additional disks") } - - if options.MultiPathDisk { - return nil, errors.New("platform aws does not support multipathed disks") - } - - if options.AdditionalNics > 0 { - return nil, errors.New("platform aws does not support additional nics") - } - - if options.AppendKernelArgs != "" { - return nil, errors.New("platform aws does not support appending kernel arguments") - } - - if options.AppendFirstbootKernelArgs != "" { - return nil, errors.New("platform aws does not support appending firstboot kernel arguments") - } - if options.InstanceType != "" { return nil, errors.New("platform aws does not support changing instance types") } diff --git a/mantle/platform/machine/azure/cluster.go b/mantle/platform/machine/azure/cluster.go index 19ccee9e39..89a443d0d8 100644 --- a/mantle/platform/machine/azure/cluster.go +++ b/mantle/platform/machine/azure/cluster.go @@ -16,7 +16,6 @@ package azure import ( "crypto/rand" - "errors" "fmt" "os" "path/filepath" @@ -49,18 +48,6 @@ func (ac *cluster) NewMachineWithOptions(userdata *conf.UserData, options platfo if err := options.EnsureNoQEMUOnlyOptions("azure"); err != nil { return nil, err } - if options.MultiPathDisk { - return nil, errors.New("platform azure does not support multipathed disks") - } - if options.AdditionalNics > 0 { - return nil, errors.New("platform azure does not support additional nics") - } - if options.AppendKernelArgs != "" { - return nil, errors.New("platform azure does not support appending kernel arguments") - } - if options.AppendFirstbootKernelArgs != "" { - return nil, errors.New("platform azure does not support appending firstboot kernel arguments") - } conf, err := ac.RenderUserData(userdata, map[string]string{ "$private_ipv4": "${COREOS_AZURE_IPV4_DYNAMIC}", diff --git a/mantle/platform/machine/do/cluster.go b/mantle/platform/machine/do/cluster.go index 535c7097ca..e88aad2f1d 100644 --- a/mantle/platform/machine/do/cluster.go +++ b/mantle/platform/machine/do/cluster.go @@ -43,18 +43,6 @@ func (dc *cluster) NewMachineWithOptions(userdata *conf.UserData, options platfo if len(options.AdditionalDisks) > 0 { return nil, errors.New("platform do does not yet support additional disks") } - if options.MultiPathDisk { - return nil, errors.New("platform do does not support multipathed disks") - } - if options.AdditionalNics > 0 { - return nil, errors.New("platform do does not support additional nics") - } - if options.AppendKernelArgs != "" { - return nil, errors.New("platform do does not support appending kernel arguments") - } - if options.AppendFirstbootKernelArgs != "" { - return nil, errors.New("platform do does not support appending firstboot kernel arguments") - } if options.InstanceType != "" { return nil, errors.New("platform do does not support changing instance types") } diff --git a/mantle/platform/machine/esx/cluster.go b/mantle/platform/machine/esx/cluster.go index 1091091b9b..dc1008064c 100644 --- a/mantle/platform/machine/esx/cluster.go +++ b/mantle/platform/machine/esx/cluster.go @@ -49,18 +49,6 @@ func (ec *cluster) NewMachineWithOptions(userdata *platformConf.UserData, option if len(options.AdditionalDisks) > 0 { return nil, errors.New("platform esx does not yet support additional disks") } - if options.MultiPathDisk { - return nil, errors.New("platform esx does not support multipathed disks") - } - if options.AdditionalNics > 0 { - return nil, errors.New("platform esx does not support additional nics") - } - if options.AppendKernelArgs != "" { - return nil, errors.New("platform esx does not support appending kernel arguments") - } - if options.AppendFirstbootKernelArgs != "" { - return nil, errors.New("platform esx does not support appending firstboot kernel arguments") - } if options.InstanceType != "" { return nil, errors.New("platform esx does not support changing instance types") } diff --git a/mantle/platform/machine/gcloud/cluster.go b/mantle/platform/machine/gcloud/cluster.go index 64b2620175..8d46821618 100644 --- a/mantle/platform/machine/gcloud/cluster.go +++ b/mantle/platform/machine/gcloud/cluster.go @@ -40,18 +40,6 @@ func (gc *cluster) NewMachineWithOptions(userdata *conf.UserData, options platfo if err := options.EnsureNoQEMUOnlyOptions("gcp"); err != nil { return nil, err } - if options.MultiPathDisk { - return nil, errors.New("platform gcp does not support multipathed disks") - } - if options.AdditionalNics > 0 { - return nil, errors.New("platform gcp does not support additional nics") - } - if options.AppendKernelArgs != "" { - return nil, errors.New("platform gcp does not support appending kernel arguments") - } - if options.AppendFirstbootKernelArgs != "" { - return nil, errors.New("platform gcp does not support appending firstboot kernel arguments") - } if options.InstanceType != "" { return nil, errors.New("platform gcp does not support changing instance types") } diff --git a/mantle/platform/machine/openstack/cluster.go b/mantle/platform/machine/openstack/cluster.go index 240e3eb95e..a78b29a938 100644 --- a/mantle/platform/machine/openstack/cluster.go +++ b/mantle/platform/machine/openstack/cluster.go @@ -41,18 +41,6 @@ func (oc *cluster) NewMachineWithOptions(userdata *conf.UserData, options platfo if len(options.AdditionalDisks) > 0 { return nil, errors.New("platform openstack does not yet support additional disks") } - if options.MultiPathDisk { - return nil, errors.New("platform openstack does not support multipathed disks") - } - if options.AdditionalNics > 0 { - return nil, errors.New("platform openstack does not support additional nics") - } - if options.AppendKernelArgs != "" { - return nil, errors.New("platform openstack does not support appending kernel arguments") - } - if options.AppendFirstbootKernelArgs != "" { - return nil, errors.New("platform openstack does not support appending firstboot kernel arguments") - } if options.InstanceType != "" { return nil, errors.New("platform openstack does not support changing instance types") } diff --git a/mantle/platform/platform.go b/mantle/platform/platform.go index 8873dcca01..b43b032a09 100644 --- a/mantle/platform/platform.go +++ b/mantle/platform/platform.go @@ -157,31 +157,55 @@ type Flight interface { } type MachineOptions struct { + AdditionalDisks []string + MinDiskSize int + InstanceType string + + // Fields below are only supported on QEMU-based platforms. + // Non-QEMU platforms call EnsureNoQEMUOnlyOptions() to reject them. MultiPathDisk bool PrimaryDisk string - AdditionalDisks []string MinMemory int - MinDiskSize int NumaNodes bool AdditionalNics int AppendKernelArgs string AppendFirstbootKernelArgs string - InstanceType string Firmware string - - // Fields below are only supported on QEMU-based platforms. - // Non-QEMU platforms call EnsureNoQEMUOnlyOptions() to reject them. - HostForwardPorts []HostForwardPort - DisablePDeathSig bool - OverrideBackingFile string - Nvme bool - Cex bool + HostForwardPorts []HostForwardPort + DisablePDeathSig bool + OverrideBackingFile string + Nvme bool + Cex bool } // EnsureNoQEMUOnlyOptions returns an error if any QEMU-only options // are set. Non-QEMU platforms should call this to reject unsupported // options early. func (m *MachineOptions) EnsureNoQEMUOnlyOptions(platformName string) error { + if m.MultiPathDisk { + return fmt.Errorf("platform %s does not support multipathed disks", platformName) + } + if m.PrimaryDisk != "" { + return fmt.Errorf("platform %s does not support custom primary disks", platformName) + } + if m.MinMemory != 0 { + return fmt.Errorf("platform %s does not support setting minimum memory", platformName) + } + if m.NumaNodes { + return fmt.Errorf("platform %s does not support NUMA node simulation", platformName) + } + if m.AdditionalNics > 0 { + return fmt.Errorf("platform %s does not support additional NICs", platformName) + } + if m.AppendKernelArgs != "" { + return fmt.Errorf("platform %s does not support appending kernel arguments", platformName) + } + if m.AppendFirstbootKernelArgs != "" { + return fmt.Errorf("platform %s does not support appending firstboot kernel arguments", platformName) + } + if m.Firmware != "" { + return fmt.Errorf("platform %s does not support setting firmware", platformName) + } if len(m.HostForwardPorts) > 0 { return fmt.Errorf("platform %s does not support host forward ports", platformName) } From c75da99196715e6378e59445b91d1e9da701c38a Mon Sep 17 00:00:00 2001 From: Dusty Mabe Date: Fri, 27 Mar 2026 18:44:39 +0000 Subject: [PATCH 3/6] mantle/platform: delete NewMachineWithQemuOptions and update all callers Now that QemuMachineOptions has been folded into MachineOptions, NewMachineWithQemuOptions is just a trivial wrapper around NewMachineWithOptions. Delete it and update all callers to use NewMachineWithOptions through the Cluster interface directly. This removes the need for type assertions to *qemu.Cluster in tests that only used them to access NewMachineWithQemuOptions. Tests that still need the type assertion (luks tang setup, ostree sync, rhcos upgrade) keep it for platform-specific branching logic, but call NewMachineWithOptions through the interface rather than through the concrete type. Written-by: --- mantle/cmd/kola/spawn.go | 11 ++--------- mantle/kola/tests/coretest/core.go | 12 +---------- mantle/kola/tests/ignition/kdump.go | 23 ++-------------------- mantle/kola/tests/ignition/luks.go | 21 ++++++-------------- mantle/kola/tests/misc/boot-mirror.go | 5 ++--- mantle/kola/tests/misc/network.go | 12 +---------- mantle/kola/tests/ostree/sync.go | 10 +++------- mantle/kola/tests/rhcos/upgrade.go | 4 ++-- mantle/platform/machine/qemu/cluster.go | 7 ------- mantle/platform/machine/qemuiso/cluster.go | 5 ----- 10 files changed, 19 insertions(+), 91 deletions(-) diff --git a/mantle/cmd/kola/spawn.go b/mantle/cmd/kola/spawn.go index 4b83be0600..e7971f30b1 100644 --- a/mantle/cmd/kola/spawn.go +++ b/mantle/cmd/kola/spawn.go @@ -32,7 +32,6 @@ import ( "github.com/coreos/coreos-assembler/mantle/kola" "github.com/coreos/coreos-assembler/mantle/platform" "github.com/coreos/coreos-assembler/mantle/platform/conf" - "github.com/coreos/coreos-assembler/mantle/platform/machine/qemu" ) var ( @@ -162,7 +161,7 @@ func runSpawn(cmd *cobra.Command, args []string) error { if spawnVerbose { fmt.Println("Spawning machine...") } - // use qemu-specific interface only if needed + // use qemu-specific options only if needed if strings.HasPrefix(kolaPlatform, "qemu") && (spawnMachineOptions != "" || !spawnRemove) { machineOpts := platform.MachineOptions{ DisablePDeathSig: !spawnRemove, @@ -178,13 +177,7 @@ func runSpawn(cmd *cobra.Command, args []string) error { return errors.Wrapf(err, "Could not unmarshal machine options") } } - - switch qc := cluster.(type) { - case *qemu.Cluster: - mach, err = qc.NewMachineWithQemuOptions(userdata, machineOpts) - default: - plog.Fatalf("unreachable: qemu cluster %v unknown type", qc) - } + mach, err = cluster.NewMachineWithOptions(userdata, machineOpts) } else { mach, err = cluster.NewMachine(userdata) } diff --git a/mantle/kola/tests/coretest/core.go b/mantle/kola/tests/coretest/core.go index 066c91e64c..b2198ffca5 100644 --- a/mantle/kola/tests/coretest/core.go +++ b/mantle/kola/tests/coretest/core.go @@ -15,7 +15,6 @@ import ( "github.com/coreos/coreos-assembler/mantle/kola/cluster" "github.com/coreos/coreos-assembler/mantle/kola/register" "github.com/coreos/coreos-assembler/mantle/platform" - "github.com/coreos/coreos-assembler/mantle/platform/machine/qemu" ) const ( @@ -133,16 +132,7 @@ func runBasicTests(c cluster.TestCluster, firmware string, nvme bool) { Firmware: firmware, Nvme: nvme, } - switch pc := c.Cluster.(type) { - // These cases have to be separated because when put together to the same case statement - // the golang compiler no longer checks that the individual types in the case have the - // NewMachineWithQemuOptions function, but rather whether platform.Cluster - // does which fails - case *qemu.Cluster: - m, err = pc.NewMachineWithQemuOptions(nil, options) - default: - panic("Unsupported cluster type") - } + m, err = c.Cluster.NewMachineWithOptions(nil, options) if err != nil { c.Fatal(err) } diff --git a/mantle/kola/tests/ignition/kdump.go b/mantle/kola/tests/ignition/kdump.go index 5d5ce61664..f58352ee11 100644 --- a/mantle/kola/tests/ignition/kdump.go +++ b/mantle/kola/tests/ignition/kdump.go @@ -11,7 +11,6 @@ import ( "github.com/coreos/coreos-assembler/mantle/kola/register" "github.com/coreos/coreos-assembler/mantle/platform" "github.com/coreos/coreos-assembler/mantle/platform/conf" - "github.com/coreos/coreos-assembler/mantle/platform/machine/qemu" "github.com/coreos/coreos-assembler/mantle/util" ) @@ -146,16 +145,7 @@ func setupSSHMachine(c cluster.TestCluster) SshServer { }`, strings.TrimSpace(string(pubkeyBuf)))) // start the machine - switch c := c.Cluster.(type) { - // These cases have to be separated because when put together to the same case statement - // the golang compiler no longer checks that the individual types in the case have the - // NewMachineWithQemuOptions function, but rather whether platform.Cluster - // does which fails - case *qemu.Cluster: - m, err = c.NewMachineWithQemuOptions(ignition, options) - default: - panic("unreachable") - } + m, err = c.Cluster.NewMachineWithOptions(ignition, options) if err != nil { c.Fatal(err) } @@ -280,16 +270,7 @@ storage: - path: /var/nfs/crash`) // start the machine - switch c := c.Cluster.(type) { - // These cases have to be separated because when put together to the same case statement - // the golang compiler no longer checks that the individual types in the case have the - // NewMachineWithQemuOptions function, but rather whether platform.Cluster - // does which fails - case *qemu.Cluster: - m, err = c.NewMachineWithQemuOptions(nfs_server_butane, options) - default: - panic("unreachable") - } + m, err = c.Cluster.NewMachineWithOptions(nfs_server_butane, options) if err != nil { c.Fatal(err) } diff --git a/mantle/kola/tests/ignition/luks.go b/mantle/kola/tests/ignition/luks.go index 813d6ef61e..d601e7f1b0 100644 --- a/mantle/kola/tests/ignition/luks.go +++ b/mantle/kola/tests/ignition/luks.go @@ -87,20 +87,16 @@ func setupTangMachine(c cluster.TestCluster) ut.TangServer { } }`) - switch pc := c.Cluster.(type) { - // These cases have to be separated because when put together to the same case statement - // the golang compiler no longer checks that the individual types in the case have the - // NewMachineWithQemuOptions function, but rather whether platform.Cluster - // does which fails + switch c.Cluster.(type) { case *qemu.Cluster: - m, err = pc.NewMachineWithQemuOptions(ignition, options) + m, err = c.Cluster.NewMachineWithOptions(ignition, options) for _, hfp := range options.HostForwardPorts { if hfp.Service == "tang" { tangAddress = fmt.Sprintf("10.0.2.2:%d", hfp.HostPort) } } default: - m, err = pc.NewMachine(ignition) + m, err = c.Cluster.NewMachine(ignition) tangAddress = fmt.Sprintf("%s:80", m.PrivateIP()) } if err != nil { @@ -239,14 +235,9 @@ func runCexTest(c cluster.TestCluster) { MinMemory: 8192, } - switch pc := c.Cluster.(type) { - case *qemu.Cluster: - m, err = pc.NewMachineWithQemuOptions(ignition, opts) - if err != nil { - c.Fatalf("Unable to create test machine: %v", err) - } - default: - panic("Unsupported cluster type") + m, err = c.Cluster.NewMachineWithOptions(ignition, opts) + if err != nil { + c.Fatalf("Unable to create test machine: %v", err) } // copy over kolet into the machine diff --git a/mantle/kola/tests/misc/boot-mirror.go b/mantle/kola/tests/misc/boot-mirror.go index 27ec960d96..38a366ddd7 100644 --- a/mantle/kola/tests/misc/boot-mirror.go +++ b/mantle/kola/tests/misc/boot-mirror.go @@ -26,7 +26,6 @@ import ( "github.com/coreos/coreos-assembler/mantle/kola/tests/util" "github.com/coreos/coreos-assembler/mantle/platform" "github.com/coreos/coreos-assembler/mantle/platform/conf" - "github.com/coreos/coreos-assembler/mantle/platform/machine/qemu" ut "github.com/coreos/coreos-assembler/mantle/util" ) @@ -106,7 +105,7 @@ func runBootMirrorTest(c cluster.TestCluster) { // FIXME: for QEMU tests kola currently assumes the host CPU architecture // matches the one under test userdata := bootmirror.Subst("LAYOUT", coreosarch.CurrentRpmArch()) - m, err = c.Cluster.(*qemu.Cluster).NewMachineWithQemuOptions(userdata, options) + m, err = c.Cluster.NewMachineWithOptions(userdata, options) if err != nil { c.Fatal(err) } @@ -156,7 +155,7 @@ func runBootMirrorLUKSTest(c cluster.TestCluster) { // FIXME: for QEMU tests kola currently assumes the host CPU architecture // matches the one under test userdata := bootmirrorluks.Subst("LAYOUT", coreosarch.CurrentRpmArch()) - m, err = c.Cluster.(*qemu.Cluster).NewMachineWithQemuOptions(userdata, options) + m, err = c.Cluster.NewMachineWithOptions(userdata, options) if err != nil { c.Fatal(err) } diff --git a/mantle/kola/tests/misc/network.go b/mantle/kola/tests/misc/network.go index 1aff5081ee..0e9ab534f8 100644 --- a/mantle/kola/tests/misc/network.go +++ b/mantle/kola/tests/misc/network.go @@ -27,7 +27,6 @@ import ( "github.com/coreos/coreos-assembler/mantle/kola/register" "github.com/coreos/coreos-assembler/mantle/platform" "github.com/coreos/coreos-assembler/mantle/platform/conf" - "github.com/coreos/coreos-assembler/mantle/platform/machine/qemu" "github.com/coreos/coreos-assembler/mantle/util" ) @@ -537,16 +536,7 @@ func setupMultipleNetworkTest(c cluster.TestCluster, primaryMac, secondaryMac st } }`, base64.StdEncoding.EncodeToString([]byte(captureMacsScript)))) - switch pc := c.Cluster.(type) { - // These cases have to be separated because when put together to the same case statement - // the golang compiler no longer checks that the individual types in the case have the - // NewMachineWithQemuOptions function, but rather whether platform.Cluster - // does which fails - case *qemu.Cluster: - m, err = pc.NewMachineWithQemuOptions(userdata, options) - default: - panic("unreachable") - } + m, err = c.Cluster.NewMachineWithOptions(userdata, options) if err != nil { c.Fatal(err) } diff --git a/mantle/kola/tests/ostree/sync.go b/mantle/kola/tests/ostree/sync.go index cc030d6366..085afb9fa6 100644 --- a/mantle/kola/tests/ostree/sync.go +++ b/mantle/kola/tests/ostree/sync.go @@ -104,16 +104,12 @@ func setupNFSMachine(c cluster.TestCluster) NfsServer { MinMemory: 2048, } // start the machine - switch c := c.Cluster.(type) { - // These cases have to be separated because when put together to the same case statement - // the golang compiler no longer checks that the individual types in the case have the - // NewMachineWithQemuOptions function, but rather whether platform.Cluster - // does which fails + switch c.Cluster.(type) { case *qemu.Cluster: - m, err = c.NewMachineWithQemuOptions(nfs_server_butane, options) + m, err = c.Cluster.NewMachineWithOptions(nfs_server_butane, options) nfs_server = "10.0.2.2" default: - m, err = c.NewMachine(nfs_server_butane) + m, err = c.Cluster.NewMachine(nfs_server_butane) nfs_server = m.PrivateIP() } if err != nil { diff --git a/mantle/kola/tests/rhcos/upgrade.go b/mantle/kola/tests/rhcos/upgrade.go index ea23946cb3..ca5d895294 100644 --- a/mantle/kola/tests/rhcos/upgrade.go +++ b/mantle/kola/tests/rhcos/upgrade.go @@ -212,7 +212,7 @@ func rhcosUpgradeFromOcpRhcos(c cluster.TestCluster) { } }`) - switch pc := c.Cluster.(type) { + switch c.Cluster.(type) { case *qemu.Cluster: ostreeCommit := kola.CosaBuild.Meta.OstreeCommit temp := os.TempDir() @@ -228,7 +228,7 @@ func rhcosUpgradeFromOcpRhcos(c cluster.TestCluster) { defer os.Remove(rhcosQcow2) options.OverrideBackingFile = rhcosQcow2 - m, err = pc.NewMachineWithQemuOptions(ignition, options) + m, err = c.Cluster.NewMachineWithOptions(ignition, options) if err != nil { c.Fatal(err) } diff --git a/mantle/platform/machine/qemu/cluster.go b/mantle/platform/machine/qemu/cluster.go index 6d66a81991..40481c4ea1 100644 --- a/mantle/platform/machine/qemu/cluster.go +++ b/mantle/platform/machine/qemu/cluster.go @@ -230,13 +230,6 @@ func (qc *Cluster) NewMachineWithOptions(userdata *conf.UserData, options platfo return qm, nil } -// NewMachineWithQemuOptions is a convenience alias for NewMachineWithOptions. -// Callers that previously used QemuMachineOptions can now pass MachineOptions -// directly — the QEMU-specific fields live there. -func (qc *Cluster) NewMachineWithQemuOptions(userdata *conf.UserData, options platform.MachineOptions) (platform.Machine, error) { - return qc.NewMachineWithOptions(userdata, options) -} - func (qc *Cluster) Destroy() { qc.tearingDown.Store(true) qc.BaseCluster.Destroy() diff --git a/mantle/platform/machine/qemuiso/cluster.go b/mantle/platform/machine/qemuiso/cluster.go index a3ca40c8d2..f56a47bc5e 100644 --- a/mantle/platform/machine/qemuiso/cluster.go +++ b/mantle/platform/machine/qemuiso/cluster.go @@ -159,11 +159,6 @@ func (qc *Cluster) NewMachineWithOptions(userdata *conf.UserData, options platfo return qm, nil } -// NewMachineWithQemuOptions is a convenience alias for NewMachineWithOptions. -func (qc *Cluster) NewMachineWithQemuOptions(userdata *conf.UserData, options platform.MachineOptions) (platform.Machine, error) { - return qc.NewMachineWithOptions(userdata, options) -} - func (qc *Cluster) Destroy() { qc.BaseCluster.Destroy() qc.flight.DelCluster(qc) From 44d21ffbaedf5331e0f6122d1c61743499b94ac0 Mon Sep 17 00:00:00 2001 From: Dusty Mabe Date: Fri, 27 Mar 2026 18:45:12 +0000 Subject: [PATCH 4/6] mantle/cmd/kola: drop QEMU platform guard from kola spawn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, kola spawn only entered the DisablePDeathSig / spawnMachineOptions code path when the platform was "qemu". This guard existed because DisablePDeathSig and the machine options JSON file were QEMU-specific concepts that required a type assertion to *qemu.Cluster. Now that MachineOptions is the single unified type used by all platforms, and non-QEMU platforms call EnsureNoQEMUOnlyOptions() in their NewMachineWithOptions(), the guard is no longer necessary: - On QEMU: behavior is unchanged. - On non-QEMU with --remove=false: previously this silently ignored the user's intent (DisablePDeathSig has no meaning outside QEMU) and created the machine via NewMachine() with no options. Now it calls NewMachineWithOptions() which will reject DisablePDeathSig with a clear error, surfacing the fact that the flag combination is unsupported rather than silently doing the wrong thing. - On non-QEMU with --machine-options: same improvement — the user gets a clear error if the JSON contains QEMU-only fields. Written-by: --- mantle/cmd/kola/spawn.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mantle/cmd/kola/spawn.go b/mantle/cmd/kola/spawn.go index e7971f30b1..264cb76dae 100644 --- a/mantle/cmd/kola/spawn.go +++ b/mantle/cmd/kola/spawn.go @@ -161,8 +161,7 @@ func runSpawn(cmd *cobra.Command, args []string) error { if spawnVerbose { fmt.Println("Spawning machine...") } - // use qemu-specific options only if needed - if strings.HasPrefix(kolaPlatform, "qemu") && (spawnMachineOptions != "" || !spawnRemove) { + if spawnMachineOptions != "" || !spawnRemove { machineOpts := platform.MachineOptions{ DisablePDeathSig: !spawnRemove, } From 576ec1b7be8b323e73989ee6c0e4fd33e5117142 Mon Sep 17 00:00:00 2001 From: Dusty Mabe Date: Fri, 27 Mar 2026 19:00:56 +0000 Subject: [PATCH 5/6] mantle/kola/register: replace individual machine fields with MachineOptions The Test struct duplicated many fields from platform.MachineOptions (MultiPathDisk, AdditionalDisks, PrimaryDisk, MinMemory, MinDiskSize, NumaNodes, AdditionalNics, AppendKernelArgs, AppendFirstbootKernelArgs, InstanceType). The harness then manually copied each one into a MachineOptions before creating machines. Replace all of these with a single MachineOptions field on the Test struct. The harness now uses t.MachineOptions directly. Fields that are not MachineOptions (InjectContainer, ReservedMemoryCountMiB) stay on the Test struct. Update the two test files (multipath.go, network.go) that set these fields in their registrations, and update harness.go external test registration to populate the MachineOptions sub-struct. Written-by: --- mantle/kola/harness.go | 45 +++++++++------------ mantle/kola/register/register.go | 38 +++--------------- mantle/kola/tests/misc/multipath.go | 62 ++++++++++++++++------------- mantle/kola/tests/misc/network.go | 24 ++++++----- 4 files changed, 71 insertions(+), 98 deletions(-) diff --git a/mantle/kola/harness.go b/mantle/kola/harness.go index 8011f5546f..e0690853f0 100644 --- a/mantle/kola/harness.go +++ b/mantle/kola/harness.go @@ -1238,18 +1238,20 @@ ExecStart=%s DependencyDir: destDirs, Tags: []string{"external"}, - AdditionalDisks: targetMeta.AdditionalDisks, - PrimaryDisk: targetMeta.PrimaryDisk, - InjectContainer: targetMeta.InjectContainer, - MinMemory: targetMeta.MinMemory, - NumaNodes: targetMeta.NumaNodes, - MinDiskSize: targetMeta.MinDiskSize, - AdditionalNics: targetMeta.AdditionalNics, - AppendKernelArgs: targetMeta.AppendKernelArgs, - AppendFirstbootKernelArgs: targetMeta.AppendFirstbootKernelArgs, - InstanceType: targetMeta.InstanceType, - NonExclusive: !targetMeta.Exclusive, - Conflicts: targetMeta.Conflicts, + MachineOptions: platform.MachineOptions{ + AdditionalDisks: targetMeta.AdditionalDisks, + PrimaryDisk: targetMeta.PrimaryDisk, + MinMemory: targetMeta.MinMemory, + NumaNodes: targetMeta.NumaNodes, + MinDiskSize: targetMeta.MinDiskSize, + AdditionalNics: targetMeta.AdditionalNics, + AppendKernelArgs: targetMeta.AppendKernelArgs, + AppendFirstbootKernelArgs: targetMeta.AppendFirstbootKernelArgs, + InstanceType: targetMeta.InstanceType, + }, + InjectContainer: targetMeta.InjectContainer, + NonExclusive: !targetMeta.Exclusive, + Conflicts: targetMeta.Conflicts, Run: func(c cluster.TestCluster) { mach := c.Machines()[0] @@ -1620,7 +1622,7 @@ func makeNonExclusiveTest(bucket int, tests []*register.Test, flight platform.Fl if test.HasFlag(register.AllowConfigWarnings) { plog.Fatalf("Non-exclusive test %v cannot have AllowConfigWarnings flag", test.Name) } - if test.AppendKernelArgs != "" { + if test.MachineOptions.AppendKernelArgs != "" { plog.Fatalf("Non-exclusive test %v cannot have AppendKernelArgs", test.Name) } if !internetAccess && testRequiresInternet(test) { @@ -1782,8 +1784,8 @@ func getNeededMemoryMiB(t *register.Test) int { } } // If the test specifies MinMemory, use that. - if t.MinMemory != 0 { - return t.MinMemory + if t.MachineOptions.MinMemory != 0 { + return t.MachineOptions.MinMemory } // Fall back to architecture-specific defaults from the QEMU platform. return platform.DefaultMemoryMiB(Options.CosaBuildArch) @@ -1861,18 +1863,7 @@ func runTest(h *harness.H, t *register.Test, pltfrm string, flight platform.Flig if t.ClusterSize > 0 { var userdata *conf.UserData = t.UserData - options := platform.MachineOptions{ - MultiPathDisk: t.MultiPathDisk, - PrimaryDisk: t.PrimaryDisk, - AdditionalDisks: t.AdditionalDisks, - MinMemory: t.MinMemory, - MinDiskSize: t.MinDiskSize, - NumaNodes: t.NumaNodes, - AdditionalNics: t.AdditionalNics, - AppendKernelArgs: t.AppendKernelArgs, - AppendFirstbootKernelArgs: t.AppendFirstbootKernelArgs, - InstanceType: t.InstanceType, - } + options := t.MachineOptions if testSecureBoot(t) { options.Firmware = "uefi-secure" diff --git a/mantle/kola/register/register.go b/mantle/kola/register/register.go index ccb5d6ce26..6c5ade0c52 100644 --- a/mantle/kola/register/register.go +++ b/mantle/kola/register/register.go @@ -19,6 +19,7 @@ import ( "time" "github.com/coreos/coreos-assembler/mantle/kola/cluster" + "github.com/coreos/coreos-assembler/mantle/platform" "github.com/coreos/coreos-assembler/mantle/platform/conf" ) @@ -70,45 +71,20 @@ type Test struct { Timeout time.Duration // the duration for which a test will be allowed to run RequiredTag string // if specified, test is filtered by default unless tag is provided -- defaults to none Description string // test description - NumaNodes bool // simulate two NUMA nodes - // Whether the primary disk is multipathed. Deprecated in favour of PrimaryDisk. - MultiPathDisk bool - - // Sizes of additional empty disks to attach to the node, followed by - // comma-separated list of optional options (e.g. ["1G", - // "5G:mpath,foo,bar"]) -- defaults to none. - AdditionalDisks []string - - // Size of primary disk to attach to the node, followed by - // comma-separated list of optional options (e.g. "20G:mpath"]). - PrimaryDisk string + // MachineOptions contains options for machine creation (disks, memory, + // kernel args, etc.). The test harness passes these to + // NewMachineWithOptions when ClusterSize > 0. + MachineOptions platform.MachineOptions // InjectContainer will cause the ostree base image to be injected into the target InjectContainer bool - // Minimum amount of memory in MB required for test. - MinMemory int - // The artificially reserved memory count in MiB for the test. This is used // for budgeting memory usage for tests prior to the VMs starting up on the // QEMU platform. ReservedMemoryCountMiB int - // Minimum amount of primary disk in GB required for test. Deprecated in favour - // of PrimaryDisk. - MinDiskSize int - - // Additional amount of NICs required for test. - AdditionalNics int - - // Additional kernel arguments to append to the defaults. - AppendKernelArgs string - - // Additional first boot kernel arguments to append to the defaults. - AppendFirstbootKernelArgs string - - // ExternalTest is a path to a binary that will be uploaded ExternalTest string // DependencyDir is a path to directory that will be uploaded, normally used by external tests DependencyDir DepDirMap @@ -124,10 +100,6 @@ type Test struct { // Conflicts is non-empty iff nonexclusive is true // Contains the tests that conflict with this particular test Conflicts []string - - // If provided, this test will be run on the target instance type. - // This overrides the instance type set with `kola run` - InstanceType string } // Registered tests that run as part of `kola run` live here. Mapping of names diff --git a/mantle/kola/tests/misc/multipath.go b/mantle/kola/tests/misc/multipath.go index 088837e59a..cc643078c4 100644 --- a/mantle/kola/tests/misc/multipath.go +++ b/mantle/kola/tests/misc/multipath.go @@ -115,40 +115,48 @@ kernel_arguments: func init() { register.RegisterTest(®ister.Test{ - Name: "multipath.day1", - Description: "Verify that multipath can be configured day 1 through Ignition.", - Run: runMultipathDay1, - ClusterSize: 1, - Platforms: []string{"qemu"}, - UserData: mpath_on_boot_day1, - MultiPathDisk: true, + Name: "multipath.day1", + Description: "Verify that multipath can be configured day 1 through Ignition.", + Run: runMultipathDay1, + ClusterSize: 1, + Platforms: []string{"qemu"}, + UserData: mpath_on_boot_day1, + MachineOptions: platform.MachineOptions{ + MultiPathDisk: true, + }, }) register.RegisterTest(®ister.Test{ - Name: "multipath.day2", - Description: "Verify that multipath can be configured day 2 through Ignition.", - Run: runMultipathDay2, - ClusterSize: 1, - Platforms: []string{"qemu"}, - MultiPathDisk: true, + Name: "multipath.day2", + Description: "Verify that multipath can be configured day 2 through Ignition.", + Run: runMultipathDay2, + ClusterSize: 1, + Platforms: []string{"qemu"}, + MachineOptions: platform.MachineOptions{ + MultiPathDisk: true, + }, }) register.RegisterTest(®ister.Test{ - Name: "multipath.partition", - Description: "Verify that multipath can be configured for a partition.", - Run: runMultipathPartition, - ClusterSize: 1, - Platforms: []string{"qemu"}, - UserData: mpath_on_var_lib_containers, - AdditionalDisks: []string{"1G:mpath,wwn=1"}, + Name: "multipath.partition", + Description: "Verify that multipath can be configured for a partition.", + Run: runMultipathPartition, + ClusterSize: 1, + Platforms: []string{"qemu"}, + UserData: mpath_on_var_lib_containers, + MachineOptions: platform.MachineOptions{ + AdditionalDisks: []string{"1G:mpath,wwn=1"}, + }, }) // See https://issues.redhat.com/browse/OCPBUGS-56597 register.RegisterTest(®ister.Test{ - Name: "multipath.single-disk", - Description: "Verify that multipath can be reduced to one path", - Run: runMultipathReduceDisk, - ClusterSize: 1, - Platforms: []string{"qemu"}, - UserData: mpath_single_disk, - MultiPathDisk: true, + Name: "multipath.single-disk", + Description: "Verify that multipath can be reduced to one path", + Run: runMultipathReduceDisk, + ClusterSize: 1, + Platforms: []string{"qemu"}, + UserData: mpath_single_disk, + MachineOptions: platform.MachineOptions{ + MultiPathDisk: true, + }, }) } diff --git a/mantle/kola/tests/misc/network.go b/mantle/kola/tests/misc/network.go index 0e9ab534f8..f073f0a19b 100644 --- a/mantle/kola/tests/misc/network.go +++ b/mantle/kola/tests/misc/network.go @@ -67,17 +67,19 @@ func init() { kargs = "net.ifnames=0" } register.RegisterTest(®ister.Test{ - Run: InitInterfacesTest, - ClusterSize: 1, - Name: "rhcos.network.init-interfaces-test", - Description: "Verify init-interfaces script works in both fresh setup and reboot.", - Timeout: 40 * time.Minute, - Distros: []string{"rhcos"}, - Platforms: []string{"qemu"}, - RequiredTag: "openshift", - AdditionalNics: 2, - AppendKernelArgs: kargs, - UserData: userdata, + Run: InitInterfacesTest, + ClusterSize: 1, + Name: "rhcos.network.init-interfaces-test", + Description: "Verify init-interfaces script works in both fresh setup and reboot.", + Timeout: 40 * time.Minute, + Distros: []string{"rhcos"}, + Platforms: []string{"qemu"}, + RequiredTag: "openshift", + MachineOptions: platform.MachineOptions{ + AdditionalNics: 2, + AppendKernelArgs: kargs, + }, + UserData: userdata, }) } From eb2fc797a006b6372c191cad0874a145fca6e972 Mon Sep 17 00:00:00 2001 From: Dusty Mabe Date: Fri, 27 Mar 2026 19:03:03 +0000 Subject: [PATCH 6/6] mantle/kola/tests: let harness create machines for basic.uefi/nvme tests The basic.uefi, basic.uefi-secure, and basic.nvme tests previously set ClusterSize to 0 and manually created machines inside wrapper functions (uefiWithBasicTests, uefiSecureWithBasicTests, nvmeBasicTests) just to pass Firmware/Nvme in MachineOptions. They then manually called ScpKolet and LocalTests. Now that the Test struct carries MachineOptions directly, these tests can set ClusterSize to 1 with the desired MachineOptions and use LocalTests as their Run function. The harness handles machine creation, kolet upload, and native function dispatch automatically. This removes the three wrapper functions, runBasicTests, and the kola/cluster imports that were only needed for them. Written-by: --- mantle/kola/tests/coretest/core.go | 55 ++++++++---------------------- 1 file changed, 15 insertions(+), 40 deletions(-) diff --git a/mantle/kola/tests/coretest/core.go b/mantle/kola/tests/coretest/core.go index b2198ffca5..2d8a3592cf 100644 --- a/mantle/kola/tests/coretest/core.go +++ b/mantle/kola/tests/coretest/core.go @@ -11,8 +11,6 @@ import ( "github.com/pborman/uuid" - "github.com/coreos/coreos-assembler/mantle/kola" - "github.com/coreos/coreos-assembler/mantle/kola/cluster" "github.com/coreos/coreos-assembler/mantle/kola/register" "github.com/coreos/coreos-assembler/mantle/platform" ) @@ -63,31 +61,40 @@ func init() { register.RegisterTest(®ister.Test{ Name: "basic.uefi", Description: "Verify basic functionalities like SSH, systemd services, useradd, etc, with UEFI enabled", - Run: uefiWithBasicTests, + Run: LocalTests, Platforms: []string{"qemu"}, - ClusterSize: 0, + ClusterSize: 1, NativeFuncs: nativeFuncs, Architectures: []string{"x86_64", "aarch64"}, + MachineOptions: platform.MachineOptions{ + Firmware: uefi, + }, }) register.RegisterTest(®ister.Test{ Name: "basic.uefi-secure", Description: "Verify basic functionalities like SSH, systemd services, useradd, etc, with UEFI Secure Boot enabled", - Run: uefiSecureWithBasicTests, + Run: LocalTests, Platforms: []string{"qemu"}, - ClusterSize: 0, + ClusterSize: 1, NativeFuncs: nativeFuncs, Architectures: []string{"x86_64"}, + MachineOptions: platform.MachineOptions{ + Firmware: uefiSecure, + }, }) register.RegisterTest(®ister.Test{ Name: "basic.nvme", Description: "Verify basic functionalities like SSH, systemd services, useradd, etc, with nvme enabled", - Run: nvmeBasicTests, + Run: LocalTests, Platforms: []string{"qemu"}, - ClusterSize: 0, + ClusterSize: 1, NativeFuncs: nativeFuncs, // NVMe in theory is supported on all arches, but the way we test it seems to // only work on x86_64 and aarch64. Architectures: []string{"x86_64", "aarch64"}, + MachineOptions: platform.MachineOptions{ + Nvme: true, + }, }) register.RegisterTest(®ister.Test{ Name: "rootfs.uuid", @@ -112,38 +119,6 @@ func init() { }) } -func uefiWithBasicTests(c cluster.TestCluster) { - runBasicTests(c, uefi, false) -} - -func uefiSecureWithBasicTests(c cluster.TestCluster) { - runBasicTests(c, uefiSecure, false) -} - -func nvmeBasicTests(c cluster.TestCluster) { - runBasicTests(c, "", true) -} - -func runBasicTests(c cluster.TestCluster, firmware string, nvme bool) { - var err error - var m platform.Machine - - options := platform.MachineOptions{ - Firmware: firmware, - Nvme: nvme, - } - m, err = c.Cluster.NewMachineWithOptions(nil, options) - if err != nil { - c.Fatal(err) - } - - // copy over kolet into the machine - if err := kola.ScpKolet([]platform.Machine{m}); err != nil { - c.Fatal(err) - } - LocalTests(c) -} - func TestPortSsh() error { //t.Parallel() err := CheckPort("tcp", "127.0.0.1:22", PortTimeout)