mantle: rework handling of MachineOptions

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,9 +161,8 @@ func runSpawn(cmd *cobra.Command, args []string) error {
 		if spawnVerbose {
 			fmt.Println("Spawning machine...")
 		}
-		// use qemu-specific interface only if needed
-		if strings.HasPrefix(kolaPlatform, "qemu") && (spawnMachineOptions != "" || !spawnRemove) {
-			machineOpts := platform.QemuMachineOptions{
+		if spawnMachineOptions != "" || !spawnRemove {
+			machineOpts := platform.MachineOptions{
 				DisablePDeathSig: !spawnRemove,
 			}
 			if spawnMachineOptions != "" {
@@ -178,13 +176,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)
 		}

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"

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

mantle/kola/tests/coretest/core.go

@@ -11,11 +11,8 @@ 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"
-	"github.com/coreos/coreos-assembler/mantle/platform/machine/qemu"
 )
 
 const (
@@ -64,31 +61,40 @@ func init() {
 	register.RegisterTest(&register.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(&register.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(&register.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(&register.Test{
 		Name:        "rootfs.uuid",
@@ -113,47 +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.QemuMachineOptions{
-		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")
-	}
-	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)

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"
 )
 
@@ -106,7 +105,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},
 		},
@@ -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)
 	}
@@ -253,7 +243,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
@@ -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)
 	}