Merge pull request metal3-io#2223 from Nordix/lentzi90/e2e-vbmctl-ref…

…actor

🌱 E2E vbmctl and ssh boot check refactor

hack/ci-e2e.sh

@@ -82,7 +82,7 @@ if [[ "${BMO_E2E_EMULATOR}" == "vbmc" ]]; then
   readarray -t BMCS < <(yq e -o=j -I=0 '.[]' "${E2E_BMCS_CONF_FILE}")
   for bmc in "${BMCS[@]}"; do
     address=$(echo "${bmc}" | jq -r '.address')
-    hostName=$(echo "${bmc}" | jq -r '.hostName')
+    hostName=$(echo "${bmc}" | jq -r '.name')
     vbmc_port="${address##*:}"
     "${REPO_ROOT}/tools/bmh_test/vm2vbmc.sh" "${hostName}" "${vbmc_port}" "${IP_ADDRESS}"
   done

test/e2e/bmc.go

@@ -9,28 +9,28 @@ import (
 	"gopkg.in/yaml.v2"
 )
 
-// BMC defines a BMH to use in the tests.
+// BMC defines connection details for a baseboard management controller
+// and other details needed for creating a virtual machine related to it.
 type BMC struct {
-	// BMC initial username
+	// User is the username for accessing the BMC.
 	User string `yaml:"user,omitempty"`
-	// BMC initial password
+	// Password is the password for accessing the BMC.
 	Password string `yaml:"password,omitempty"`
-	// BMC initial address
+	// Address of the BMC, e.g. "redfish-virtualmedia+http://192.168.222.1:8000/redfish/v1/Systems/bmo-e2e-1".
 	Address string `yaml:"address,omitempty"`
-	// BMC Mac address
+	// BootMacAddress is the MAC address of the VMs network interface.
 	BootMacAddress string `yaml:"bootMacAddress,omitempty"`
-	// The Hostname of the node, which will be read into BMH object
-	HostName string `yaml:"hostName,omitempty"`
-	// The IP address of the node
-	// Optional. Only needed if e2eConfig variable
-	// SSH_CHECK_PROVISIONED is true
+	// Name of the machine associated with this BMC.
+	Name string `yaml:"name,omitempty"`
+	// NetworkName is the name of the network that the new VM should be attached to
+	NetworkName string `yaml:"networkName,omitempty"`
+	// IPAddress is a reserved IP address for the VM.
+	// This will be paired with the MAC address in the DHCP configuration.
+	// Example: 192.168.222.122
 	IPAddress string `yaml:"ipAddress,omitempty"`
-	// Optional. Only needed if e2eConfig variable
-	// SSH_CHECK_PROVISIONED is true
-	SSHPort string `yaml:"sshPort,omitempty"`
 }
 
-func LoadBMCConfig(configPath string) (*[]BMC, error) {
+func LoadBMCConfig(configPath string) ([]BMC, error) {
 	configData, err := os.ReadFile(configPath) //#nosec
 	var bmcs []BMC
 	if err != nil {
@@ -39,5 +39,5 @@ func LoadBMCConfig(configPath string) (*[]BMC, error) {
 	if err := yaml.Unmarshal(configData, &bmcs); err != nil {
 		return nil, err
 	}
-	return &bmcs, nil
+	return bmcs, nil
 }

test/e2e/common.go

@@ -292,8 +292,15 @@ echo "%s" >> /root/.ssh/authorized_keys`, sshPubKeyData)
 // PerformSSHBootCheck performs an SSH check to verify the node's boot source.
 // The `expectedBootMode` parameter should be "disk" or "memory".
 // The `auth` parameter is an ssh.AuthMethod for authentication.
-func PerformSSHBootCheck(e2eConfig *Config, expectedBootMode string, auth ssh.AuthMethod, sshAddress string) {
+func PerformSSHBootCheck(e2eConfig *Config, expectedBootMode string, ipAddress string) {
 	user := e2eConfig.GetVariable("SSH_USERNAME")
+	keyPath := e2eConfig.GetVariable("SSH_PRIV_KEY")
+	key, err := os.ReadFile(keyPath)
+	Expect(err).NotTo(HaveOccurred(), "unable to read private key")
+	signer, err := ssh.ParsePrivateKey(key)
+	Expect(err).NotTo(HaveOccurred(), "unable to parse private key")
+	auth := ssh.PublicKeys(signer)
+	sshAddress := fmt.Sprintf("%s:%s", ipAddress, e2eConfig.GetVariable("SSH_PORT"))
 
 	client := EstablishSSHConnection(e2eConfig, auth, user, sshAddress)
 	defer func() {

test/e2e/config/bmcs-fixture.yaml

@@ -1,16 +1,11 @@
-# For fixture config, only the `hostName` field matters. The number of BMCs,
-# however, needs to be equal or larger than `GINKGO_NODES`.
+# For fixture it doesn't matter much what we put here. The number of BMCs,
+# however, needs to be equal or larger than `GINKGO_NODES` or we will
+# run out of BMCs to test on.
 - user: admin
   password: password
   address: "redfish+http://192.168.222.1:8000/redfish/v1/Systems/bmo-e2e-0"
   bootMacAddress: "00:60:2f:31:81:01"
-  hostName: ""
-  ipAddress: "192.168.222.122"
-  sshPort: "22"
 - user: admin
   password: password
   address: "redfish+http://192.168.222.1:8000/redfish/v1/Systems/bmo-e2e-1"
   bootMacAddress: "00:60:2f:31:81:02"
-  hostName: ""
-  ipAddress: "192.168.222.123"
-  sshPort: "22"

test/e2e/config/bmcs-ipmi.yaml

@@ -2,13 +2,11 @@
   password: password
   address: "ipmi://192.168.222.1:16230"
   bootMacAddress: "00:60:2f:31:81:01"
-  hostName: "bmo-e2e-0"
+  name: "bmo-e2e-0"
   ipAddress: "192.168.222.122"
-  sshPort: "22"
 - user: admin
   password: password
   address: "ipmi://192.168.222.1:16231"
   bootMacAddress: "00:60:2f:31:81:02"
-  hostName: "bmo-e2e-1"
+  name: "bmo-e2e-1"
   ipAddress: "192.168.222.123"
-  sshPort: "22"

test/e2e/config/bmcs-redfish-virtualmedia.yaml

@@ -2,13 +2,11 @@
   password: password
   address: "redfish-virtualmedia+http://192.168.222.1:8000/redfish/v1/Systems/bmo-e2e-0"
   bootMacAddress: "00:60:2f:31:81:01"
-  hostName: "bmo-e2e-0"
+  name: "bmo-e2e-0"
   ipAddress: "192.168.222.122"
-  sshPort: "22"
 - user: admin
   password: password
   address: "redfish-virtualmedia+http://192.168.222.1:8000/redfish/v1/Systems/bmo-e2e-1"
   bootMacAddress: "00:60:2f:31:81:02"
-  hostName: "bmo-e2e-1"
+  name: "bmo-e2e-1"
   ipAddress: "192.168.222.123"
-  sshPort: "22"

test/e2e/config/bmcs-redfish.yaml

@@ -2,13 +2,11 @@
   password: password
   address: "redfish+http://192.168.222.1:8000/redfish/v1/Systems/bmo-e2e-0"
   bootMacAddress: "00:60:2f:31:81:01"
-  hostName: "bmo-e2e-0"
+  name: "bmo-e2e-0"
   ipAddress: "192.168.222.122"
-  sshPort: "22"
 - user: admin
   password: password
   address: "redfish+http://192.168.222.1:8000/redfish/v1/Systems/bmo-e2e-1"
   bootMacAddress: "00:60:2f:31:81:02"
-  hostName: "bmo-e2e-1"
+  name: "bmo-e2e-1"
   ipAddress: "192.168.222.123"
-  sshPort: "22"

test/e2e/config/ironic.yaml

@@ -41,6 +41,7 @@ variables:
   CERT_MANAGER_VERSION: "v1.13.1"
   SSH_CHECK_PROVISIONED: "true"
   SSH_USERNAME: "root"
+  SSH_PORT: "22"
   SSH_PRIV_KEY: "./images/ssh_testkey"
   SSH_PUB_KEY: "./images/ssh_testkey.pub"
   FETCH_IRONIC_NODES: "true"

test/e2e/e2e_suite_test.go

@@ -306,7 +306,7 @@ var _ = SynchronizedBeforeSuite(func() []byte {
 	e2eConfig = LoadE2EConfig(configPath)
 	bmcs, err := LoadBMCConfig(bmcConfigPath)
 	Expect(err).ToNot(HaveOccurred(), "Failed to read the bmcs config file")
-	bmc = (*bmcs)[GinkgoParallelProcess()-1]
+	bmc = (bmcs)[GinkgoParallelProcess()-1]
 	clusterProxy = framework.NewClusterProxy("bmo-e2e", kubeconfigPath, scheme)
 })
 

test/e2e/live_iso_test.go

@@ -6,14 +6,12 @@ package e2e
 import (
 	"context"
 	"fmt"
-	"os"
 	"path"
 
 	metal3api "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
 	metal3bmc "github.com/metal3-io/baremetal-operator/pkg/hardwareutils/bmc"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
-	"golang.org/x/crypto/ssh"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/pointer"
@@ -105,13 +103,7 @@ var _ = Describe("Live-ISO", Label("required", "live-iso"), func() {
 		// The ssh check is not possible in all situations (e.g. fixture) so it can be skipped
 		if e2eConfig.GetVariable("SSH_CHECK_PROVISIONED") == "true" {
 			By("Verifying the node booted from live ISO image")
-			keyPath := e2eConfig.GetVariable("SSH_PRIV_KEY")
-			key, err := os.ReadFile(keyPath)
-			Expect(err).NotTo(HaveOccurred(), "unable to read private key")
-			signer, err := ssh.ParsePrivateKey(key)
-			Expect(err).NotTo(HaveOccurred(), "unable to parse private key")
-			auth := ssh.PublicKeys(signer)
-			PerformSSHBootCheck(e2eConfig, "memory", auth, fmt.Sprintf("%s:%s", bmc.IPAddress, bmc.SSHPort))
+			PerformSSHBootCheck(e2eConfig, "memory", bmc.IPAddress)
 		} else {
 			Logf("WARNING: Skipping SSH check since SSH_CHECK_PROVISIONED != true")
 		}

test/e2e/provisioning_and_annotation_test.go

@@ -7,14 +7,12 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"os"
 	"path"
 	"time"
 
 	metal3api "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
-	"golang.org/x/crypto/ssh"
 	corev1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -121,15 +119,7 @@ var _ = Describe("Provision, detach, recreate from status and deprovision", Labe
 			// The ssh check is not possible in all situations (e.g. fixture) so it can be skipped
 			if e2eConfig.GetVariable("SSH_CHECK_PROVISIONED") == "true" {
 				By("Verifying the node booting from disk")
-				keyPath := e2eConfig.GetVariable("SSH_PRIV_KEY")
-				key, err := os.ReadFile(keyPath)
-				Expect(err).NotTo(HaveOccurred(), "unable to read private key")
-
-				signer, err := ssh.ParsePrivateKey(key)
-				Expect(err).NotTo(HaveOccurred(), "unable to parse private key")
-
-				auth := ssh.PublicKeys(signer)
-				PerformSSHBootCheck(e2eConfig, "disk", auth, fmt.Sprintf("%s:%s", bmc.IPAddress, bmc.SSHPort))
+				PerformSSHBootCheck(e2eConfig, "disk", bmc.IPAddress)
 			} else {
 				Logf("WARNING: Skipping SSH check since SSH_CHECK_PROVISIONED != true")
 			}

test/e2e/re_inspection_test.go

@@ -105,7 +105,8 @@ var _ = Describe("Re-Inspection", Label("required", "re-inspection"), func() {
 		By("checking that the hardware details are corrected after re-inspection")
 		key = types.NamespacedName{Namespace: bmh.Namespace, Name: bmh.Name}
 		Expect(clusterProxy.GetClient().Get(ctx, key, &bmh)).To(Succeed())
-		Expect(bmh.Status.HardwareDetails.Hostname).To(Equal(bmc.HostName))
+		// TODO(lentzi90): Hostname should not be determined or configured through BMC
+		Expect(bmh.Status.HardwareDetails.Hostname).To(Equal(bmc.Name))
 	})
 
 	AfterEach(func() {