METAL-1756: Baremetal platform improvements

go.mod

@@ -53,7 +53,7 @@ require (
 	github.com/lestrrat/go-jsschema v0.0.0-20181205002244-5c81c58ffcc3
 	github.com/lithammer/dedent v1.1.0
 	github.com/mattn/go-sqlite3 v1.14.30
-	github.com/metal3-io/baremetal-operator/apis v0.11.0-alpha.0
+	github.com/metal3-io/baremetal-operator/apis v0.11.0
 	github.com/metallb/frr-k8s v0.0.15
 	github.com/microsoftgraph/msgraph-sdk-go v1.81.0
 	github.com/onsi/ginkgo/v2 v2.23.3

go.sum

@@ -727,8 +727,8 @@ github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWE
 github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y=
 github.com/mattn/go-sqlite3 v1.14.30 h1:bVreufq3EAIG1Quvws73du3/QgdeZ3myglJlrzSYYCY=
 github.com/mattn/go-sqlite3 v1.14.30/go.mod h1:Uh1q+B4BYcTPb+yiD3kU8Ct7aC0hY9fxUwlHK0RXw+Y=
-github.com/metal3-io/baremetal-operator/apis v0.11.0-alpha.0 h1:DMdk7yuxgLXGTL+HmUNHzm8WyZaZGkSFxxdXJjWq6IU=
-github.com/metal3-io/baremetal-operator/apis v0.11.0-alpha.0/go.mod h1:TB7T7qAumrfrY/TCSuHvKtpEOLGw9QjnG43ez3vX14g=
+github.com/metal3-io/baremetal-operator/apis v0.11.0 h1:sYxjnObegWnDyz028m5Rc6gVYxbSLlvjOAqo6Iq1vOE=
+github.com/metal3-io/baremetal-operator/apis v0.11.0/go.mod h1:T0v/wKeJeUfVFeTq1sihv76IZEE2Bn+hqKkh5kGMxA4=
 github.com/metallb/frr-k8s v0.0.15 h1:6M3UGhovX1EFoaSGjrRD7djUAx3w2I+g81FH8OVtHkM=
 github.com/metallb/frr-k8s v0.0.15/go.mod h1:TjrGoAf+v00hYGlI8jUdyDxY5udMAOs2GWwrvLWnA4E=
 github.com/microsoft/kiota-abstractions-go v1.9.3 h1:cqhbqro+VynJ7kObmo7850h3WN2SbvoyhypPn8uJ1SE=

test/extended/baremetal/OWNERS

@@ -1,5 +1,10 @@
 approvers:
   - andfasano
-  - ardaguclu
-  - bfournie
+  - dtantsur
   - elfosardo
+  - honza
+  - hroyrh
+  - iurygregory
+  - jacob-anders
+  - MahnoorAsghar
+  - tdomnesc

test/extended/baremetal/common.go

@@ -2,15 +2,15 @@ package baremetal
 
 import (
 	"context"
-	"fmt"
-	"strings"
 
+	metal3v1alpha1 "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
 	g "github.com/onsi/ginkgo/v2"
 	o "github.com/onsi/gomega"
 	configv1 "github.com/openshift/api/config/v1"
 	exutil "github.com/openshift/origin/test/extended/util"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/dynamic"
 	e2eskipper "k8s.io/kubernetes/test/e2e/framework/skipper"
@@ -53,12 +53,10 @@ func skipIfUnsupportedPlatformOrConfig(oc *exutil.CLI, dc dynamic.Interface) {
 		fallthrough
 	case configv1.NonePlatformType:
 		provisioningNetwork := getProvisioningNetwork(dc)
-		if provisioningNetwork != "Disabled" {
-			e2eskipper.Skipf("Unsupported config in supported platform detected")
-		} else if provisioningNetwork == "" {
+		if provisioningNetwork == "" {
 			e2eskipper.Skipf("Unable to read ProvisioningNetwork from Provisioning CR")
-		} else {
-			return
+		} else if provisioningNetwork != "Disabled" {
+			e2eskipper.Skipf("Unsupported config in supported platform detected")
 		}
 	default:
 		e2eskipper.Skipf("No supported platform detected")
@@ -98,73 +96,25 @@ func preprovisioningImagesClient(dc dynamic.Interface) dynamic.ResourceInterface
 	return ppiClient.Namespace("openshift-machine-api")
 }
 
-type FieldGetterFunc func(obj map[string]interface{}, fields ...string) (interface{}, bool, error)
-
-func expectField(object unstructured.Unstructured, resource string, nestedField string, fieldGetter FieldGetterFunc) o.Assertion {
-	fields := strings.Split(nestedField, ".")
-
-	value, found, err := fieldGetter(object.Object, fields...)
-	o.Expect(err).NotTo(o.HaveOccurred())
-	o.Expect(found).To(o.BeTrue(), fmt.Sprintf("`%s` field `%s` not found", resource, nestedField))
-	return o.Expect(value)
-}
-
-func expectStringField(object unstructured.Unstructured, resource string, nestedField string) o.Assertion {
-	return expectField(object, resource, nestedField, func(obj map[string]interface{}, fields ...string) (interface{}, bool, error) {
-		return unstructured.NestedString(obj, fields...)
-	})
-}
-
-func expectBoolField(object unstructured.Unstructured, resource string, nestedField string) o.Assertion {
-	return expectField(object, resource, nestedField, func(obj map[string]interface{}, fields ...string) (interface{}, bool, error) {
-		return unstructured.NestedBool(obj, fields...)
-	})
+func firmwareSchemaClient(dc dynamic.Interface, namespace string) dynamic.ResourceInterface {
+	fsClient := dc.Resource(schema.GroupVersionResource{Group: "metal3.io", Resource: "firmwareschemas", Version: "v1alpha1"})
+	return fsClient.Namespace(namespace)
 }
 
-func expectStringMapField(object unstructured.Unstructured, resource string, nestedField string) o.Assertion {
-	return expectField(object, resource, nestedField, func(obj map[string]interface{}, fields ...string) (interface{}, bool, error) {
-		return unstructured.NestedStringMap(obj, fields...)
-	})
+func provisioningGVR() schema.GroupVersionResource {
+	return schema.GroupVersionResource{Group: "metal3.io", Resource: "provisionings", Version: "v1alpha1"}
 }
 
-func expectSliceField(object unstructured.Unstructured, resource string, nestedField string) o.Assertion {
-	return expectField(object, resource, nestedField, func(obj map[string]interface{}, fields ...string) (interface{}, bool, error) {
-		return unstructured.NestedSlice(obj, fields...)
-	})
-}
-
-// Conditions are stored as a slice of maps, check that the type has the correct status
-func checkConditionStatus(hfs unstructured.Unstructured, condType string, condStatus string) {
-
-	conditions, _, err := unstructured.NestedSlice(hfs.Object, "status", "conditions")
+func toBMH(obj unstructured.Unstructured) metal3v1alpha1.BareMetalHost {
+	var bmh metal3v1alpha1.BareMetalHost
+	err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, &bmh)
 	o.Expect(err).NotTo(o.HaveOccurred())
-	o.Expect(conditions).ToNot(o.BeEmpty())
-
-	for _, c := range conditions {
-		condition, ok := c.(map[string]interface{})
-		o.Expect(ok).To(o.BeTrue())
-
-		t, ok := condition["type"]
-		o.Expect(ok).To(o.BeTrue())
-		if t == condType {
-			s, ok := condition["status"]
-			o.Expect(ok).To(o.BeTrue())
-			o.Expect(s).To(o.Equal(condStatus))
-		}
-	}
+	return bmh
 }
 
-func getField(object unstructured.Unstructured, resource string, nestedField string, fieldGetter FieldGetterFunc) string {
-	fields := strings.Split(nestedField, ".")
-
-	value, found, err := fieldGetter(object.Object, fields...)
+func toHFS(obj unstructured.Unstructured) metal3v1alpha1.HostFirmwareSettings {
+	var hfs metal3v1alpha1.HostFirmwareSettings
+	err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, &hfs)
 	o.Expect(err).NotTo(o.HaveOccurred())
-	o.Expect(found).To(o.BeTrue(), fmt.Sprintf("`%s` field `%s` not found", resource, nestedField))
-	return value.(string)
-}
-
-func getStringField(object unstructured.Unstructured, resource string, nestedField string) string {
-	return getField(object, resource, nestedField, func(obj map[string]interface{}, fields ...string) (interface{}, bool, error) {
-		return unstructured.NestedFieldNoCopy(obj, fields...)
-	})
+	return hfs
 }

test/extended/baremetal/high_availability.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"time"
 
+	metal3v1alpha1 "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
 	configv1 "github.com/openshift/api/config/v1"
 	clusteroperatorhelpers "github.com/openshift/library-go/pkg/config/clusteroperator/v1helpers"
 
@@ -29,7 +30,7 @@ const (
 	metal3Deployment         = "metal3"
 )
 
-var _ = g.Describe("[sig-installer][Feature:baremetal][Serial] Baremetal platform should ensure [apigroup:config.openshift.io]", func() {
+var _ = g.Describe("[sig-installer][Feature:baremetal][Serial][apigroup:metal3.io][apigroup:config.openshift.io] Baremetal platform should ensure", func() {
 	defer g.GinkgoRecover()
 
 	var (
@@ -155,8 +156,12 @@ func checkMetal3DeploymentHealthy(oc *exutil.CLI) {
 	o.Expect(hosts.Items).ToNot(o.BeEmpty())
 
 	for _, h := range hosts.Items {
-		expectStringField(h, "baremetalhost", "status.operationalStatus").To(o.BeEquivalentTo("OK"))
-		expectStringField(h, "baremetalhost", "status.provisioning.state").To(o.Or(o.BeEquivalentTo("provisioned"), o.BeEquivalentTo("externally provisioned")))
-		expectBoolField(h, "baremetalhost", "spec.online").To(o.BeTrue())
+		bmh := toBMH(h)
+		o.Expect(bmh.Status.OperationalStatus).To(o.Equal(metal3v1alpha1.OperationalStatusOK), "host %s", bmh.Name)
+		o.Expect(bmh.Status.Provisioning.State).To(o.Or(
+			o.Equal(metal3v1alpha1.StateProvisioned),
+			o.Equal(metal3v1alpha1.StateExternallyProvisioned),
+		), "host %s", bmh.Name)
+		o.Expect(bmh.Spec.Online).To(o.BeTrue(), "host %s", bmh.Name)
 	}
 }

test/extended/baremetal/hosts.go

@@ -4,17 +4,17 @@ import (
 	"context"
 	"fmt"
 
+	metal3v1alpha1 "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
 	g "github.com/onsi/ginkgo/v2"
 	o "github.com/onsi/gomega"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 	e2e "k8s.io/kubernetes/test/e2e/framework"
 
 	exutil "github.com/openshift/origin/test/extended/util"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-var _ = g.Describe("[sig-installer][Feature:baremetal] Baremetal/OpenStack/vSphere/None/AWS/Azure/GCP platforms", func() {
+var _ = g.Describe("[sig-installer][Feature:baremetal][apigroup:metal3.io] Baremetal/OpenStack/vSphere/None/AWS/Azure/GCP platforms", func() {
 	defer g.GinkgoRecover()
 
 	var (
@@ -37,7 +37,7 @@ var _ = g.Describe("[sig-installer][Feature:baremetal] Baremetal/OpenStack/vSphe
 	})
 })
 
-var _ = g.Describe("[sig-installer][Feature:baremetal] Baremetal platform should", func() {
+var _ = g.Describe("[sig-installer][Feature:baremetal][apigroup:metal3.io][apigroup:config.openshift.io] Baremetal platform should", func() {
 	defer g.GinkgoRecover()
 
 	oc := exutil.NewCLI("baremetal")
@@ -52,21 +52,23 @@ var _ = g.Describe("[sig-installer][Feature:baremetal] Baremetal platform should
 		dc := oc.AdminDynamicClient()
 		bmc := baremetalClient(dc)
 
-		// In TNF Deployments we expect baremetal installs to be detached.
-		expectedOperationalStatus := "OK"
+		expectedOperationalStatus := metal3v1alpha1.OperationalStatusOK
 		if isTNFDeployment {
-			expectedOperationalStatus = "detached"
+			expectedOperationalStatus = metal3v1alpha1.OperationalStatusDetached
 		}
 
 		hosts, err := bmc.List(context.Background(), metav1.ListOptions{})
 		o.Expect(err).NotTo(o.HaveOccurred())
 		o.Expect(hosts.Items).ToNot(o.BeEmpty())
 
 		for _, h := range hosts.Items {
-			expectStringField(h, "baremetalhost", "status.operationalStatus").To(o.BeEquivalentTo(expectedOperationalStatus))
-			expectStringField(h, "baremetalhost", "status.provisioning.state").To(o.Or(o.BeEquivalentTo("provisioned"), o.BeEquivalentTo("externally provisioned")))
-			expectBoolField(h, "baremetalhost", "spec.online").To(o.BeTrue())
-
+			bmh := toBMH(h)
+			o.Expect(bmh.Status.OperationalStatus).To(o.Equal(expectedOperationalStatus), "host %s", bmh.Name)
+			o.Expect(bmh.Status.Provisioning.State).To(o.Or(
+				o.Equal(metal3v1alpha1.StateProvisioned),
+				o.Equal(metal3v1alpha1.StateExternallyProvisioned),
+			), "host %s", bmh.Name)
+			o.Expect(bmh.Spec.Online).To(o.BeTrue(), "host %s", bmh.Name)
 		}
 	})
 
@@ -79,11 +81,10 @@ var _ = g.Describe("[sig-installer][Feature:baremetal] Baremetal platform should
 		o.Expect(err).NotTo(o.HaveOccurred())
 
 		for _, h := range hosts.Items {
-			state := getStringField(h, "baremetalhost", "status.provisioning.state")
-			if state != "externally provisioned" {
-				hostName := getStringField(h, "baremetalhost", "metadata.name")
-				_, err := ppiClient.Get(context.Background(), hostName, metav1.GetOptions{})
-				o.Expect(err).NotTo(o.HaveOccurred())
+			bmh := toBMH(h)
+			if bmh.Status.Provisioning.State != metal3v1alpha1.StateExternallyProvisioned {
+				_, err := ppiClient.Get(context.Background(), bmh.Name, metav1.GetOptions{})
+				o.Expect(err).NotTo(o.HaveOccurred(), "missing PreprovisioningImage for host %s", bmh.Name)
 			}
 		}
 	})
@@ -99,28 +100,33 @@ var _ = g.Describe("[sig-installer][Feature:baremetal] Baremetal platform should
 		hfsClient := hostfirmwaresettingsClient(dc)
 
 		for _, h := range hosts.Items {
-			hostName := getStringField(h, "baremetalhost", "metadata.name")
+			bmh := toBMH(h)
 
-			g.By(fmt.Sprintf("check that baremetalhost %s has a corresponding hostfirmwaresettings", hostName))
-			hfs, err := hfsClient.Get(context.Background(), hostName, metav1.GetOptions{})
+			g.By(fmt.Sprintf("check that baremetalhost %s has a corresponding hostfirmwaresettings", bmh.Name))
+			hfsUnstructured, err := hfsClient.Get(context.Background(), bmh.Name, metav1.GetOptions{})
 			o.Expect(err).NotTo(o.HaveOccurred())
-			o.Expect(hfs).NotTo(o.Equal(nil))
 
-			// Reenable this when fix to prevent settings with 0 entries is in BMO
-			// g.By("check that hostfirmwaresettings settings have been populated")
-			// expectStringMapField(*hfs, "hostfirmwaresettings", "status.settings").ToNot(o.BeEmpty())
+			hfs := toHFS(*hfsUnstructured)
+
+			g.By("check that hostfirmwaresettings settings have been populated")
+			o.Expect(hfs.Status.Settings).ToNot(o.BeEmpty())
 
 			g.By("check that hostfirmwaresettings conditions show resource is valid")
-			checkConditionStatus(*hfs, "Valid", "True")
+			o.Expect(hfs.Status.Conditions).ToNot(o.BeEmpty())
+			for _, cond := range hfs.Status.Conditions {
+				if cond.Type == string(metal3v1alpha1.FirmwareSettingsValid) {
+					o.Expect(cond.Status).To(o.Equal(metav1.ConditionTrue))
+				}
+			}
 
 			g.By("check that hostfirmwaresettings reference a schema")
-			refName := getStringField(*hfs, "hostfirmwaresettings", "status.schema.name")
-			refNS := getStringField(*hfs, "hostfirmwaresettings", "status.schema.namespace")
+			o.Expect(hfs.Status.FirmwareSchema).ToNot(o.BeNil())
+			o.Expect(hfs.Status.FirmwareSchema.Name).ToNot(o.BeEmpty())
 
-			schemaClient := dc.Resource(schema.GroupVersionResource{Group: "metal3.io", Resource: "firmwareschemas", Version: "v1alpha1"}).Namespace(refNS)
-			schema, err := schemaClient.Get(context.Background(), refName, metav1.GetOptions{})
+			fsClient := firmwareSchemaClient(dc, hfs.Status.FirmwareSchema.Namespace)
+			fs, err := fsClient.Get(context.Background(), hfs.Status.FirmwareSchema.Name, metav1.GetOptions{})
 			o.Expect(err).NotTo(o.HaveOccurred())
-			o.Expect(schema).NotTo(o.Equal(nil))
+			o.Expect(fs).NotTo(o.BeNil())
 		}
 	})
 
@@ -133,9 +139,9 @@ var _ = g.Describe("[sig-installer][Feature:baremetal] Baremetal platform should
 		o.Expect(hosts.Items).ToNot(o.BeEmpty())
 
 		host := hosts.Items[0]
-		expectStringField(host, "baremetalhost", "spec.bootMACAddress").ShouldNot(o.BeNil())
-		// Already verified that bootMACAddress exists
-		bootMACAddress, _, _ := unstructured.NestedString(host.Object, "spec", "bootMACAddress")
+		bmh := toBMH(host)
+		o.Expect(bmh.Spec.BootMACAddress).ToNot(o.BeEmpty())
+
 		testMACAddress := "11:11:11:11:11:11"
 
 		g.By("updating bootMACAddress which is not allowed")
@@ -148,14 +154,59 @@ var _ = g.Describe("[sig-installer][Feature:baremetal] Baremetal platform should
 		g.By("verify bootMACAddress is not updated")
 		h, err := bmc.Get(context.Background(), host.GetName(), metav1.GetOptions{})
 		o.Expect(err).NotTo(o.HaveOccurred())
-		check, _, _ := unstructured.NestedString(h.Object, "spec", "bootMACAddress")
-		o.Expect(check).To(o.Equal(bootMACAddress))
+		updated := toBMH(*h)
+		o.Expect(updated.Spec.BootMACAddress).To(o.Equal(bmh.Spec.BootMACAddress))
+	})
+
+	g.It("have a valid provisioning configuration", func() {
+		dc := oc.AdminDynamicClient()
+		provisioningClient := dc.Resource(provisioningGVR())
+
+		provisioning, err := provisioningClient.Get(context.Background(), "provisioning-configuration", metav1.GetOptions{})
+		o.Expect(err).NotTo(o.HaveOccurred())
+
+		g.By("checking provisioning network is set")
+		provisioningNetwork, found, err := unstructured.NestedString(provisioning.Object, "spec", "provisioningNetwork")
+		o.Expect(err).NotTo(o.HaveOccurred())
+		o.Expect(found).To(o.BeTrue(), "spec.provisioningNetwork not found")
+		o.Expect(provisioningNetwork).ToNot(o.BeEmpty())
+	})
+
+	g.It("have all metal3 pod containers running", func() {
+		c, err := e2e.LoadClientset()
+		o.Expect(err).ToNot(o.HaveOccurred())
+
+		pods, err := c.CoreV1().Pods("openshift-machine-api").List(context.Background(), metav1.ListOptions{
+			LabelSelector: "baremetal.openshift.io/cluster-baremetal-operator=metal3-state",
+		})
+		o.Expect(err).NotTo(o.HaveOccurred())
+		o.Expect(pods.Items).ToNot(o.BeEmpty())
+
+		for _, pod := range pods.Items {
+			g.By(fmt.Sprintf("checking containers in pod %s", pod.Name))
+			for _, cs := range pod.Status.ContainerStatuses {
+				o.Expect(cs.Ready).To(o.BeTrue(), fmt.Sprintf("container %s in pod %s is not ready", cs.Name, pod.Name))
+				o.Expect(cs.RestartCount).To(o.BeNumerically("<", 5), fmt.Sprintf("container %s in pod %s has restarted %d times", cs.Name, pod.Name, cs.RestartCount))
+			}
+		}
+	})
+
+	g.It("have a metal3-image-customization deployment", func() {
+		c, err := e2e.LoadClientset()
+		o.Expect(err).ToNot(o.HaveOccurred())
+
+		icc, err := c.AppsV1().Deployments("openshift-machine-api").Get(context.Background(), "metal3-image-customization", metav1.GetOptions{})
+		o.Expect(err).NotTo(o.HaveOccurred())
+		o.Expect(icc.Status.AvailableReplicas).To(o.BeEquivalentTo(1))
+
+		o.Expect(icc.Annotations).Should(o.HaveKey("baremetal.openshift.io/owned"))
+		o.Expect(icc.Labels).Should(o.HaveKeyWithValue("baremetal.openshift.io/cluster-baremetal-operator", "metal3-image-customization-service"))
 	})
 })
 
 // This block must be used for the serial tests. Any eventual extra worker deployed will be
 // automatically deleted during the AfterEach
-var _ = g.Describe("[sig-installer][Feature:baremetal][Serial] Baremetal platform should", func() {
+var _ = g.Describe("[sig-installer][Feature:baremetal][Serial][apigroup:metal3.io] Baremetal platform should", func() {
 	defer g.GinkgoRecover()
 
 	var (
@@ -186,8 +237,7 @@ var _ = g.Describe("[sig-installer][Feature:baremetal][Serial] Baremetal platfor
 		host = helper.WaitForProvisioningState(host, "available")
 
 		g.By("Check that hardware field in status is empty")
-		_, found, err := unstructured.NestedString(host.Object, "status", "hardware")
-		o.Expect(err).NotTo(o.HaveOccurred())
-		o.Expect(found).To(o.BeFalse())
+		bmh := toBMH(*host)
+		o.Expect(bmh.Status.HardwareDetails).To(o.BeNil())
 	})
 })