Repository: metal3-io/baremetal-operator
Pull request #1094 (2022-03-08 09:40:27)

Expose CheckRAIDInterface from the RAID module
We need to use it in the terraform provider as well:
https://github.com/openshift-metal3/terraform-provider-ironic/pull/53.

Also remove no-op RAID testing bits from the access tests.



Patch:
diff --git a/pkg/provisioner/ironic/raid.go b/pkg/provisioner/ironic/raid.go
index 69143126..83ee1383 100644
--- a/pkg/provisioner/ironic/raid.go
+++ b/pkg/provisioner/ironic/raid.go
@@ -19,7 +19,7 @@ const (
 
 // setTargetRAIDCfg set the RAID settings to the ironic Node for RAID configuration steps
 func setTargetRAIDCfg(p *ironicProvisioner, raidInterface string, ironicNode *nodes.Node, data provisioner.PrepareData) (provisioner.Result, error) {
-	err := checkRAIDConfigure(raidInterface, data.TargetRAIDConfig)
+	err := CheckRAIDInterface(raidInterface, data.TargetRAIDConfig)
 	if err != nil {
 		return operationFailed(err.Error())
 	}
@@ -178,7 +178,7 @@ func buildTargetSoftwareRAIDCfg(volumes []metal3v1alpha1.SoftwareRAIDVolume) (lo
 
 // BuildRAIDCleanSteps build the clean steps for RAID configuration from BaremetalHost spec
 func BuildRAIDCleanSteps(raidInterface string, target *metal3v1alpha1.RAIDConfig, actual *metal3v1alpha1.RAIDConfig) (cleanSteps []nodes.CleanStep, err error) {
-	err = checkRAIDConfigure(raidInterface, target)
+	err = CheckRAIDInterface(raidInterface, target)
 	if err != nil {
 		return nil, err
 	}
@@ -273,18 +273,21 @@ func BuildRAIDCleanSteps(raidInterface string, target *metal3v1alpha1.RAIDConfig
 	return
 }
 
-func checkRAIDConfigure(raidInterface string, raid *metal3v1alpha1.RAIDConfig) error {
+// CheckRAIDInterface checks the current RAID interface against the requested configuration
+func CheckRAIDInterface(raidInterface string, target *metal3v1alpha1.RAIDConfig) error {
+	// FIXME(dtantsur): the software RAID logic is completely broken: no driver sets raidInterface to "agent".
+	// This value must be used when software RAID volumes are requested or deleted.
 	switch raidInterface {
 	case noRAIDInterface:
-		if raid != nil && (len(raid.HardwareRAIDVolumes) != 0 || len(raid.SoftwareRAIDVolumes) != 0) {
-			return fmt.Errorf("raid settings are defined, but the node's driver %s does not support RAID", raidInterface)
+		if target != nil && (len(target.HardwareRAIDVolumes) != 0 || len(target.SoftwareRAIDVolumes) != 0) {
+			return fmt.Errorf("target settings are defined, but the node's driver %s does not support RAID", raidInterface)
 		}
 	case softwareRAIDInterface:
-		if raid != nil && len(raid.HardwareRAIDVolumes) != 0 {
+		if target != nil && len(target.HardwareRAIDVolumes) != 0 {
 			return fmt.Errorf("node's driver %s does not support hardware RAID", raidInterface)
 		}
 	default:
-		if raid != nil && len(raid.HardwareRAIDVolumes) == 0 && len(raid.SoftwareRAIDVolumes) != 0 {
+		if target != nil && len(target.HardwareRAIDVolumes) == 0 && len(target.SoftwareRAIDVolumes) != 0 {
 			return fmt.Errorf("node's driver %s does not support software RAID", raidInterface)
 		}
 	}


Test patch:
diff --git a/pkg/hardwareutils/bmc/access_test.go b/pkg/hardwareutils/bmc/access_test.go
index 2619c2d1..85bcb6aa 100644
--- a/pkg/hardwareutils/bmc/access_test.go
+++ b/pkg/hardwareutils/bmc/access_test.go
@@ -434,7 +434,6 @@ func TestStaticDriverInfo(t *testing.T) {
 		boot       string
 		management string
 		power      string
-		raid       string
 		vendor     string
 	}{
 		{
@@ -446,7 +445,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			boot:       "ipxe",
 			management: "",
 			power:      "",
-			raid:       "no-raid",
 		},
 
 		{
@@ -458,7 +456,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			boot:       "ipxe",
 			management: "",
 			power:      "",
-			raid:       "no-raid",
 		},
 
 		{
@@ -470,7 +467,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			boot:       "ipxe",
 			management: "",
 			power:      "",
-			raid:       "no-raid",
 		},
 
 		{
@@ -482,7 +478,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			boot:       "ipxe",
 			management: "",
 			power:      "",
-			raid:       "irmc",
 		},
 
 		{
@@ -494,7 +489,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			boot:       "ipxe",
 			management: "",
 			power:      "",
-			raid:       "no-raid",
 		},
 
 		{
@@ -506,7 +500,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			boot:       "redfish-virtual-media",
 			management: "",
 			power:      "",
-			raid:       "no-raid",
 		},
 
 		{
@@ -518,7 +511,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			boot:       "redfish-virtual-media",
 			management: "",
 			power:      "",
-			raid:       "no-raid",
 		},
 
 		{
@@ -530,7 +522,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			boot:       "redfish-virtual-media",
 			management: "",
 			power:      "",
-			raid:       "no-raid",
 		},
 
 		{
@@ -542,7 +533,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			boot:       "ipxe",
 			management: "idrac-redfish",
 			power:      "idrac-redfish",
-			raid:       "no-raid",
 			vendor:     "idrac-redfish",
 		},
 
@@ -582,7 +572,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			boot:       "idrac-redfish-virtual-media",
 			management: "idrac-redfish",
 			power:      "idrac-redfish",
-			raid:       "no-raid",
 			vendor:     "idrac-redfish",
 		},
 
@@ -595,7 +584,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			boot:       "idrac-redfish-virtual-media",
 			management: "idrac-redfish",
 			power:      "idrac-redfish",
-			raid:       "no-raid",
 			vendor:     "idrac-redfish",
 		},
 
@@ -608,7 +596,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			boot:       "idrac-redfish-virtual-media",
 			management: "idrac-redfish",
 			power:      "idrac-redfish",
-			raid:       "no-raid",
 			vendor:     "idrac-redfish",
 		},
 
@@ -621,7 +608,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			boot:       "pxe",
 			management: "ibmc",
 			power:      "ibmc",
-			raid:       "no-raid",
 		},
 
 		{
@@ -633,7 +619,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			boot:       "ilo-ipxe",
 			management: "",
 			power:      "",
-			raid:       "no-raid",
 		},
 
 		{
@@ -645,7 +630,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			boot:       "ilo-virtual-media",
 			management: "",
 			power:      "",
-			raid:       "no-raid",
 		},
 
 		{
@@ -657,7 +641,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			boot:       "ilo-ipxe",
 			management: "",
 			power:      "",
-			raid:       "ilo5",
 		},
 	} {
 		t.Run(tc.Scenario, func(t *testing.T) {
diff --git a/pkg/provisioner/ironic/raid_test.go b/pkg/provisioner/ironic/raid_test.go
index dbba02ea..b27e9ed6 100644
--- a/pkg/provisioner/ironic/raid_test.go
+++ b/pkg/provisioner/ironic/raid_test.go
@@ -491,7 +491,7 @@ func TestCheckRAIDConfigure(t *testing.T) {
 
 	for _, c := range cases {
 		t.Run(c.raidInterface, func(t *testing.T) {
-			err := checkRAIDConfigure(c.raidInterface, c.RAID)
+			err := CheckRAIDInterface(c.raidInterface, c.RAID)
 			if (err != nil) != c.expectedError {
 				t.Errorf("Got unexpected error: %v", err)
 			}