Merge pull request #10349 from elfosardo/q35-efi-metal-support

METAL-1750: baremetal: use q35 machine type with EFI firmware for bootstrap VM

Author: openshift-merge-bot[bot]

pkg/infrastructure/baremetal/bootstrap.go

@@ -344,6 +344,22 @@ func getHostCapabilities(virConn *libvirt.Libvirt) (libvirtxml.Caps, error) {
 	return caps, nil
 }
 
+func configureDomainArch(dom *libvirtxml.Domain, arch string) {
+	dom.OS.Type.Arch = arch
+
+	switch arch {
+	case "x86_64":
+		dom.OS.Type.Machine = "q35"
+		dom.OS.Firmware = "efi"
+	case "aarch64":
+		// reference: https://libvirt.org/formatdomain.html#bios-bootloader
+		dom.OS.Firmware = "efi"
+		fallthrough
+	case "s390x", "ppc64le":
+		dom.Devices.Graphics = nil
+	}
+}
+
 func createBootstrapDomain(virConn *libvirt.Libvirt, config baremetalConfig, pool libvirt.StoragePool, liveCDVolume, scratchVolume libvirt.StorageVol) error {
 	bootstrapDom := newDomain(fmt.Sprintf("%s-bootstrap", config.ClusterID))
 
@@ -353,18 +369,7 @@ func createBootstrapDomain(virConn *libvirt.Libvirt, config baremetalConfig, poo
 	}
 
 	arch := capabilities.Host.CPU.Arch
-	bootstrapDom.OS.Type.Arch = arch
-
-	if arch == "aarch64" {
-		// for aarch64 speciffying this will automatically select the firmware and NVRAM file
-		// reference: https://libvirt.org/formatdomain.html#bios-bootloader
-		bootstrapDom.OS.Firmware = "efi"
-	}
-
-	// For aarch64, s390x, ppc64 and ppc64le spice is not supported
-	if arch == "aarch64" || arch == "s390x" || strings.HasPrefix(arch, "ppc64") {
-		bootstrapDom.Devices.Graphics = nil
-	}
+	configureDomainArch(&bootstrapDom, arch)
 
 	for _, bridge := range config.Bridges {
 		netIface := libvirtxml.DomainInterface{

pkg/infrastructure/baremetal/bootstrap_test.go

@@ -0,0 +1,97 @@
+package baremetal
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"libvirt.org/go/libvirtxml"
+)
+
+func TestNewDomain(t *testing.T) {
+	dom := newDomain("test-bootstrap")
+
+	assert.Equal(t, "test-bootstrap", dom.Name)
+	assert.Equal(t, "kvm", dom.Type)
+	assert.Equal(t, "hvm", dom.OS.Type.Type)
+	assert.Empty(t, dom.OS.Type.Arch, "arch should not be set by newDomain")
+	assert.Empty(t, dom.OS.Type.Machine, "machine should not be set by newDomain")
+	assert.Equal(t, uint(6), dom.Memory.Value)
+	assert.Equal(t, "GiB", dom.Memory.Unit)
+	assert.Equal(t, uint(4), dom.VCPU.Value)
+	assert.Equal(t, "host-passthrough", dom.CPU.Mode)
+	assert.NotEmpty(t, dom.Devices.RNGs, "RNG device should be configured")
+	assert.NotEmpty(t, dom.Devices.Graphics, "graphics should be configured")
+	assert.NotEmpty(t, dom.Devices.Consoles, "console should be configured")
+}
+
+func TestConfigureDomainArch(t *testing.T) {
+	tests := []struct {
+		name           string
+		arch           string
+		expectMachine  string
+		expectFirmware string
+		expectGraphics bool
+	}{
+		{
+			name:           "x86_64 sets q35 and efi",
+			arch:           "x86_64",
+			expectMachine:  "q35",
+			expectFirmware: "efi",
+			expectGraphics: true,
+		},
+		{
+			name:           "aarch64 sets efi and removes graphics",
+			arch:           "aarch64",
+			expectMachine:  "",
+			expectFirmware: "efi",
+			expectGraphics: false,
+		},
+		{
+			name:           "s390x removes graphics",
+			arch:           "s390x",
+			expectMachine:  "",
+			expectFirmware: "",
+			expectGraphics: false,
+		},
+		{
+			name:           "ppc64le removes graphics",
+			arch:           "ppc64le",
+			expectMachine:  "",
+			expectFirmware: "",
+			expectGraphics: false,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			dom := newDomain("test-bootstrap")
+			configureDomainArch(&dom, tc.arch)
+
+			assert.Equal(t, tc.arch, dom.OS.Type.Arch)
+			assert.Equal(t, tc.expectMachine, dom.OS.Type.Machine)
+			assert.Equal(t, tc.expectFirmware, dom.OS.Firmware)
+
+			if tc.expectGraphics {
+				assert.NotNil(t, dom.Devices.Graphics, "graphics should be present for %s", tc.arch)
+			} else {
+				assert.Nil(t, dom.Devices.Graphics, "graphics should be removed for %s", tc.arch)
+			}
+		})
+	}
+}
+
+func TestConfigureDomainArchPreservesOtherFields(t *testing.T) {
+	dom := newDomain("test-bootstrap")
+
+	dom.Devices.Interfaces = []libvirtxml.DomainInterface{
+		{
+			Model: &libvirtxml.DomainInterfaceModel{Type: "virtio"},
+		},
+	}
+
+	configureDomainArch(&dom, "x86_64")
+
+	assert.Len(t, dom.Devices.Interfaces, 1, "interfaces should not be modified")
+	assert.Equal(t, "hvm", dom.OS.Type.Type, "OS type should remain hvm")
+	assert.Equal(t, "kvm", dom.Type, "domain type should remain kvm")
+}