mantle/kola: copy basic tests into formal kola workflow

`cmd-kola` has non-conformed basic kola tests. These tests do not benefit from the supporting functionality around kola test's such as denylisting or other functionality. reduce `cmd-kola` and conform the basic kola tests. fixes #3418 co-authored-by: AdamOBrien <[email protected]>
coreos · Apr 14, 2024 · 2f44daa · 2f44daa
1 parent 5a1bd04
commit 2f44daa
Show file tree

Hide file tree

Showing 5 changed files with 94 additions and 45 deletions.
diff --git a/mantle/kola/harness.go b/mantle/kola/harness.go
@@ -1822,7 +1822,7 @@ func runTest(h *harness.H, t *register.Test, pltfrm string, flight platform.Flig
 
 	// drop kolet binary on machines
 	if t.ExternalTest != "" || t.NativeFuncs != nil {
-		if err := scpKolet(tcluster.Machines()); err != nil {
+		if err := ScpKolet(tcluster.Machines()); err != nil {
 			h.Fatal(err)
 		}
 	}
@@ -1865,8 +1865,8 @@ func runTest(h *harness.H, t *register.Test, pltfrm string, flight platform.Flig
 	t.Run(tcluster)
 }
 
-// scpKolet searches for a kolet binary and copies it to the machine.
-func scpKolet(machines []platform.Machine) error {
+// ScpKolet searches for a kolet binary and copies it to the machine.
+func ScpKolet(machines []platform.Machine) error {
 	mArch := Options.CosaBuildArch
 	exePath, err := os.Executable()
 	if err != nil {

diff --git a/mantle/kola/tests/coretest/core.go b/mantle/kola/tests/coretest/core.go
@@ -12,13 +12,19 @@ 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"
 	"github.com/coreos/coreos-assembler/mantle/util"
 )
 
 const (
 	DockerTimeout = time.Second * 60
 	PortTimeout   = time.Second * 3
+	uefi          = "uefi"
+	uefiSecure    = "uefi-secure"
+	bios          = "bios"
 )
 
 // RHCOS services we expect disabled/inactive
@@ -37,22 +43,48 @@ var offServices = []string{
 	"tcsd.service",
 }
 
+var nativeFuncs = map[string]register.NativeFuncWrap{
+	"PortSSH":        register.CreateNativeFuncWrap(TestPortSsh),
+	"DbusPerms":      register.CreateNativeFuncWrap(TestDbusPerms),
+	"ServicesActive": register.CreateNativeFuncWrap(TestServicesActive),
+	"ReadOnly":       register.CreateNativeFuncWrap(TestReadOnlyFs),
+	"Useradd":        register.CreateNativeFuncWrap(TestUseradd),
+	"MachineID":      register.CreateNativeFuncWrap(TestMachineID),
+	"RHCOSGrowpart":  register.CreateNativeFuncWrap(TestRHCOSGrowfs, []string{"fcos"}...),
+	"FCOSGrowpart":   register.CreateNativeFuncWrap(TestFCOSGrowfs, []string{"rhcos"}...),
+}
+
 func init() {
 	register.RegisterTest(&register.Test{
 		Name:        "basic",
 		Description: "Verify basic functionalities like SSH, systemd services, useradd, etc.",
 		Run:         LocalTests,
 		ClusterSize: 1,
-		NativeFuncs: map[string]register.NativeFuncWrap{
-			"PortSSH":        register.CreateNativeFuncWrap(TestPortSsh),
-			"DbusPerms":      register.CreateNativeFuncWrap(TestDbusPerms),
-			"ServicesActive": register.CreateNativeFuncWrap(TestServicesActive),
-			"ReadOnly":       register.CreateNativeFuncWrap(TestReadOnlyFs),
-			"Useradd":        register.CreateNativeFuncWrap(TestUseradd),
-			"MachineID":      register.CreateNativeFuncWrap(TestMachineID),
-			"RHCOSGrowpart":  register.CreateNativeFuncWrap(TestRHCOSGrowfs, []string{"fcos"}...),
-			"FCOSGrowpart":   register.CreateNativeFuncWrap(TestFCOSGrowfs, []string{"rhcos"}...),
-		},
+		NativeFuncs: nativeFuncs,
+	})
+	register.RegisterTest(&register.Test{
+		Name:        "basic.uefi",
+		Description: "Verify basic functionalities like SSH, systemd services, useradd, etc, with UEFI enabled",
+		Run:         uefiWithBasicTests,
+		Platforms:   []string{"qemu"},
+		ClusterSize: 0,
+		NativeFuncs: nativeFuncs,
+	})
+	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,
+		Platforms:   []string{"qemu"},
+		ClusterSize: 0,
+		NativeFuncs: nativeFuncs,
+	})
+	register.RegisterTest(&register.Test{
+		Name:        "basic.nvme",
+		Description: "Verify basic functionalities like SSH, systemd services, useradd, etc, with nvme enabled",
+		Run:         nvmeBasicTests,
+		Platforms:   []string{"qemu"},
+		ClusterSize: 0,
+		NativeFuncs: nativeFuncs,
 	})
 	// TODO: Enable DockerPing/DockerEcho once fixed
 	// TODO: Only enable PodmanPing on non qemu. Needs:
@@ -92,6 +124,47 @@ 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, bios, 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)

diff --git a/mantle/platform/machine/qemu/cluster.go b/mantle/platform/machine/qemu/cluster.go
@@ -141,7 +141,7 @@ func (qc *Cluster) NewMachineWithQemuOptions(userdata *conf.UserData, options pl
 	}
 
 	channel := "virtio"
-	if qc.flight.opts.Nvme {
+	if qc.flight.opts.Nvme || options.Nvme {
 		channel = "nvme"
 	}
 	sectorSize := 0
@@ -194,6 +194,9 @@ func (qc *Cluster) NewMachineWithQemuOptions(userdata *conf.UserData, options pl
 	if !qc.RuntimeConf().InternetAccess {
 		builder.RestrictNetworking = true
 	}
+	if options.Firmware != "" {
+		builder.Firmware = options.Firmware
+	}
 
 	inst, err := builder.Exec()
 	if err != nil {

diff --git a/mantle/platform/qemu.go b/mantle/platform/qemu.go
@@ -72,6 +72,8 @@ type QemuMachineOptions struct {
 	HostForwardPorts    []HostForwardPort
 	DisablePDeathSig    bool
 	OverrideBackingFile string
+	Firmware            string
+	Nvme                bool
 }
 
 // QEMUMachine represents a qemu instance.

diff --git a/src/cmd-kola b/src/cmd-kola
@@ -10,8 +10,6 @@ import sys
 # Just test these boot to start with.  In the future we should at least
 # do ostree upgrades with uefi etc.  But we don't really need the *full*
 # suite...if podman somehow broke with nvme or uefi I'd be amazed and impressed.
-BASIC_SCENARIOS = ["nvme=true", "firmware=uefi"]
-BASIC_SCENARIOS_SECURE_BOOT = ["firmware=uefi-secure"]
 arch = platform.machine()
 
 cosa_dir = os.path.dirname(os.path.abspath(__file__))
@@ -24,9 +22,7 @@ basearch = cmdlib.get_basearch()
 # Parse args and dispatch
 parser = argparse.ArgumentParser()
 parser.add_argument("--build", help="Build ID")
-parser.add_argument("--basic-qemu-scenarios", help="Run the basic test across uefi-secure,nvme etc.", action='store_true')
 parser.add_argument("--output-dir", help="Output directory")
-parser.add_argument("--skip-secure-boot", help="Use with '--basic-qemu-scenarios' to skip the Secure Boot tests", action='store_true')
 parser.add_argument("--upgrades", help="Run upgrade tests", action='store_true')
 parser.add_argument("subargs", help="Remaining arguments for kola", nargs='*',
                     default=[])
@@ -58,33 +54,8 @@ subargs = args.subargs or [default_cmd]
 kolaargs.extend(subargs)
 kolaargs.extend(unknown_args)
 
-if args.basic_qemu_scenarios:
-    if arch == "x86_64":
-        os.mkdir(outputdir)  # Create the toplevel output dir
-        for scenario in BASIC_SCENARIOS:
-            kolaargs.extend(['--output-dir',
-                             os.path.join(outputdir, scenario.replace('=', '-'))])
-            subargs = kolaargs + ['--qemu-' + scenario, 'basic']
-            print(subprocess.list2cmdline(subargs), flush=True)
-            subprocess.check_call(subargs)
-        if not args.skip_secure_boot:
-            for scenario in BASIC_SCENARIOS_SECURE_BOOT:
-                kolaargs.extend(['--output-dir',
-                                 os.path.join(outputdir, scenario.replace('=', '-'))])
-                # See https://issues.redhat.com/browse/COS-2000 - there's
-                # some bug with shim/grub2 that fails with secure boot on < ~1300MiB of RAM.
-                # But we're not going to block on that; real world OCP worker nodes are at least 16GiB etc.
-                subargs = kolaargs + ['--qemu-' + scenario, 'basic'] + ["--qemu-memory", "1536"]
-                print(subprocess.list2cmdline(subargs), flush=True)
-                subprocess.check_call(subargs)
-    else:
-        # Basic qemu scenarios using nvme and uefi
-        # are not supported on multi-arch
-        kolaargs.extend(['--output-dir', outputdir])
-        subargs = kolaargs + ['basic']
-        print(subprocess.list2cmdline(subargs), flush=True)
-        subprocess.check_call(subargs)
-elif args.upgrades:
+
+if args.upgrades:
     kolaargs.extend(['--output-dir', outputdir])
     if '--qemu-image-dir' not in unknown_args:
         os.makedirs('tmp/kola-qemu-cache', exist_ok=True)