Repository: metal3-io/baremetal-operator
Pull request #1062 (2022-01-04 19:16:42)

Add physicalDisks and controller parameters to HardwareRAID
This patch adds the physical disks listing to the HardwareRAID
configuration as well as the ability to specify the RAID
controller to be used for configuration of RAID during
provisioning.

This work is based on the specification available here:
https://github.com/metal3-io/metal3-docs/pull/196

This work reimplements the work in #976


Patch:
diff --git a/apis/metal3.io/v1alpha1/baremetalhost_types.go b/apis/metal3.io/v1alpha1/baremetalhost_types.go
index 433a26f8..55d86798 100644
--- a/apis/metal3.io/v1alpha1/baremetalhost_types.go
+++ b/apis/metal3.io/v1alpha1/baremetalhost_types.go
@@ -254,6 +254,13 @@ type HardwareRAIDVolume struct {
 	// for the particular RAID level.
 	// +kubebuilder:validation:Minimum=1
 	NumberOfPhysicalDisks *int `json:"numberOfPhysicalDisks,omitempty"`
+
+	// The name of the RAID controller to use
+	Controller string `json:"controller,omitempty"`
+
+	// Optional list of physical disk names to be used for the Hardware RAID volumes. The disk names are interpreted
+	// by the Hardware RAID controller, and the format is hardware specific.
+	PhysicalDisks []string `json:"physicalDisks,omitempty"`
 }
 
 // SoftwareRAIDVolume defines the desired configuration of volume in software RAID
diff --git a/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go b/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go
index 59d01bae..6bb11382 100644
--- a/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go
+++ b/apis/metal3.io/v1alpha1/zz_generated.deepcopy.go
@@ -556,6 +556,11 @@ func (in *HardwareRAIDVolume) DeepCopyInto(out *HardwareRAIDVolume) {
 		*out = new(int)
 		**out = **in
 	}
+	if in.PhysicalDisks != nil {
+		in, out := &in.PhysicalDisks, &out.PhysicalDisks
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
 }
 
 // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HardwareRAIDVolume.
diff --git a/config/crd/bases/metal3.io_baremetalhosts.yaml b/config/crd/bases/metal3.io_baremetalhosts.yaml
index 6fe6077e..aa11c7fb 100644
--- a/config/crd/bases/metal3.io_baremetalhosts.yaml
+++ b/config/crd/bases/metal3.io_baremetalhosts.yaml
@@ -290,6 +290,9 @@ spec:
                       description: HardwareRAIDVolume defines the desired configuration
                         of volume in hardware RAID
                       properties:
+                        controller:
+                          description: The name of the RAID controller to use
+                          type: string
                         level:
                           description: 'RAID level for the logical disk. The following
                             levels are supported: 0;1;2;5;6;1+0;5+0;6+0.'
@@ -314,6 +317,14 @@ spec:
                             required for the particular RAID level.
                           minimum: 1
                           type: integer
+                        physicalDisks:
+                          description: Optional list of physical disk names to be
+                            used for the Hardware RAID volumes. The disk names are
+                            interpreted by the Hardware RAID controller, and the format
+                            is hardware specific.
+                          items:
+                            type: string
+                          type: array
                         rotational:
                           description: Select disks with only rotational or solid-state
                             storage
@@ -879,6 +890,9 @@ spec:
                           description: HardwareRAIDVolume defines the desired configuration
                             of volume in hardware RAID
                           properties:
+                            controller:
+                              description: The name of the RAID controller to use
+                              type: string
                             level:
                               description: 'RAID level for the logical disk. The following
                                 levels are supported: 0;1;2;5;6;1+0;5+0;6+0.'
@@ -903,6 +917,14 @@ spec:
                                 disks required for the particular RAID level.
                               minimum: 1
                               type: integer
+                            physicalDisks:
+                              description: Optional list of physical disk names to
+                                be used for the Hardware RAID volumes. The disk names
+                                are interpreted by the Hardware RAID controller, and
+                                the format is hardware specific.
+                              items:
+                                type: string
+                              type: array
                             rotational:
                               description: Select disks with only rotational or solid-state
                                 storage
diff --git a/config/render/capm3.yaml b/config/render/capm3.yaml
index e96be1dd..a9cb8131 100644
--- a/config/render/capm3.yaml
+++ b/config/render/capm3.yaml
@@ -288,6 +288,9 @@ spec:
                       description: HardwareRAIDVolume defines the desired configuration
                         of volume in hardware RAID
                       properties:
+                        controller:
+                          description: The name of the RAID controller to use
+                          type: string
                         level:
                           description: 'RAID level for the logical disk. The following
                             levels are supported: 0;1;2;5;6;1+0;5+0;6+0.'
@@ -312,6 +315,14 @@ spec:
                             required for the particular RAID level.
                           minimum: 1
                           type: integer
+                        physicalDisks:
+                          description: Optional list of physical disk names to be
+                            used for the Hardware RAID volumes. The disk names are
+                            interpreted by the Hardware RAID controller, and the format
+                            is hardware specific.
+                          items:
+                            type: string
+                          type: array
                         rotational:
                           description: Select disks with only rotational or solid-state
                             storage
@@ -877,6 +888,9 @@ spec:
                           description: HardwareRAIDVolume defines the desired configuration
                             of volume in hardware RAID
                           properties:
+                            controller:
+                              description: The name of the RAID controller to use
+                              type: string
                             level:
                               description: 'RAID level for the logical disk. The following
                                 levels are supported: 0;1;2;5;6;1+0;5+0;6+0.'
@@ -901,6 +915,14 @@ spec:
                                 disks required for the particular RAID level.
                               minimum: 1
                               type: integer
+                            physicalDisks:
+                              description: Optional list of physical disk names to
+                                be used for the Hardware RAID volumes. The disk names
+                                are interpreted by the Hardware RAID controller, and
+                                the format is hardware specific.
+                              items:
+                                type: string
+                              type: array
                             rotational:
                               description: Select disks with only rotational or solid-state
                                 storage
diff --git a/controllers/metal3.io/baremetalhost_controller.go b/controllers/metal3.io/baremetalhost_controller.go
index 870f3713..934bf25a 100644
--- a/controllers/metal3.io/baremetalhost_controller.go
+++ b/controllers/metal3.io/baremetalhost_controller.go
@@ -240,6 +240,7 @@ func (r *BareMetalHostReconciler) Reconcile(ctx context.Context, request ctrl.Re
 		return ctrl.Result{}, errors.Wrap(err, "failed to check services availability")
 	}
 	if !ready {
+		provisionerNotReady.Inc()
 		reqLogger.Info("provisioner is not ready", "RequeueAfter:", provisionerNotReadyRetryDelay)
 		return ctrl.Result{Requeue: true, RequeueAfter: provisionerNotReadyRetryDelay}, nil
 	}
diff --git a/controllers/metal3.io/metrics.go b/controllers/metal3.io/metrics.go
index 3dc7d854..b9d02197 100644
--- a/controllers/metal3.io/metrics.go
+++ b/controllers/metal3.io/metrics.go
@@ -118,6 +118,11 @@ var deleteWithoutDeprov = prometheus.NewCounter(prometheus.CounterOpts{
 	Help: "Number of times a host is deleted despite deprovisioning failing",
 })
 
+var provisionerNotReady = prometheus.NewCounter(prometheus.CounterOpts{
+	Name: "metal3_provisioner_not_ready_total",
+	Help: "Number of times a host is not provision ready",
+})
+
 func init() {
 	metrics.Registry.MustRegister(
 		reconcileCounters,
@@ -143,7 +148,8 @@ func init() {
 		stateChanges,
 		hostRegistrationRequired,
 		hostUnmanaged,
-		deleteWithoutDeprov)
+		deleteWithoutDeprov,
+		provisionerNotReady)
 }
 
 func hostMetricLabels(request ctrl.Request) prometheus.Labels {
diff --git a/docs/api.md b/docs/api.md
index 44f7b5c7..fd771de1 100644
--- a/docs/api.md
+++ b/docs/api.md
@@ -173,6 +173,10 @@ The sub-fields are:
   * *numberOfPhysicalDisks* -- Integer, number of physical disks to use for the
     logical disk. Defaults to minimum number of disks required for the
     particular RAID level.
+  * *physicalDisks* -- List of names of physical disks (Strings). This is an
+    optional field. If specified, the `controller` field must be specified too.
+  * *controller* -- String, Name of the RAID controller to be used in the
+     hardware RAID volume. Optional field.
   * *rotational* -- If true, select only rotational disks, if false - only
     solid-state and NVMe. Any disk types are used by default.
   * *sizeGibibytes* -- Size (Integer) of the logical disk to be created in GiB.
diff --git a/pkg/provisioner/ironic/raid.go b/pkg/provisioner/ironic/raid.go
index f9d59edc..69143126 100644
--- a/pkg/provisioner/ironic/raid.go
+++ b/pkg/provisioner/ironic/raid.go
@@ -99,11 +99,33 @@ func buildTargetHardwareRAIDCfg(volumes []metal3v1alpha1.HardwareRAIDVolume) (lo
 			}
 			nameCheckFlags[volume.Name] = index
 		}
+
+		// Check that controller field is specified if PhysicalDisks are used
+		if len(volume.PhysicalDisks) != 0 {
+			if volume.Controller == "" {
+				return nil, errors.Errorf("'controller' must be specified if 'physicalDisks' are used!")
+			}
+		}
+
+		// Check numberOfPhysicalDisks is same as len(physicalDisks)
+		if volume.NumberOfPhysicalDisks != nil && len(volume.PhysicalDisks) != 0 {
+			if *volume.NumberOfPhysicalDisks != len(volume.PhysicalDisks) {
+				return nil, errors.Errorf("the numberOfPhysicalDisks[%d] is not same as number of items in physicalDisks[%d]", *volume.NumberOfPhysicalDisks, len(volume.PhysicalDisks))
+			}
+		}
+		// Create the physicalDisks slice for logicalDisk struct
+		physicalDisks := make([]interface{}, len(volume.PhysicalDisks))
+		for i, physicalDisk := range volume.PhysicalDisks {
+			physicalDisks[i] = physicalDisk
+		}
+
 		// Build logicalDisk
 		logicalDisk = nodes.LogicalDisk{
-			SizeGB:     volume.SizeGibibytes,
-			RAIDLevel:  nodes.RAIDLevel(volume.Level),
-			VolumeName: volume.Name,
+			SizeGB:        volume.SizeGibibytes,
+			RAIDLevel:     nodes.RAIDLevel(volume.Level),
+			VolumeName:    volume.Name,
+			Controller:    volume.Controller,
+			PhysicalDisks: physicalDisks,
 		}
 		if volume.Rotational != nil {
 			if *volume.Rotational {


Test patch:
diff --git a/apis/metal3.io/v1alpha1/baremetalhost_types_test.go b/apis/metal3.io/v1alpha1/baremetalhost_types_test.go
index 8d499c86..5e1435a6 100644
--- a/apis/metal3.io/v1alpha1/baremetalhost_types_test.go
+++ b/apis/metal3.io/v1alpha1/baremetalhost_types_test.go
@@ -468,7 +468,7 @@ func TestBootMode(t *testing.T) {
 			Expected:  UEFI,
 		},
 		{
-			Scenario:  "legcy",
+			Scenario:  "legacy",
 			HostValue: Legacy,
 			Expected:  Legacy,
 		},
diff --git a/controllers/metal3.io/baremetalhost_controller_test.go b/controllers/metal3.io/baremetalhost_controller_test.go
index e5633a90..92d62c14 100644
--- a/controllers/metal3.io/baremetalhost_controller_test.go
+++ b/controllers/metal3.io/baremetalhost_controller_test.go
@@ -18,6 +18,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 
+	promutil "github.com/prometheus/client_golang/prometheus/testutil"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
@@ -1550,11 +1551,13 @@ func TestProvisionerIsReady(t *testing.T) {
 	fix := fixture.Fixture{BecomeReadyCounter: 5}
 	r := newTestReconcilerWithFixture(&fix, host)
 
+	assert.Equal(t, 0.0, promutil.ToFloat64(provisionerNotReady))
 	tryReconcile(t, r, host,
 		func(host *metal3v1alpha1.BareMetalHost, result reconcile.Result) bool {
 			return host.Status.Provisioning.State != metal3v1alpha1.StateNone
 		},
 	)
+	assert.Equal(t, 4.0, promutil.ToFloat64(provisionerNotReady))
 }
 
 func TestUpdateEventHandler(t *testing.T) {
diff --git a/pkg/provisioner/ironic/raid_test.go b/pkg/provisioner/ironic/raid_test.go
index 0c48e8ad..dbba02ea 100644
--- a/pkg/provisioner/ironic/raid_test.go
+++ b/pkg/provisioner/ironic/raid_test.go
@@ -1,6 +1,7 @@
 package ironic
 
 import (
+	"fmt"
 	"reflect"
 	"testing"
 
@@ -12,12 +13,49 @@ import (
 func TestBuildTargetRAIDCfg(t *testing.T) {
 	var TRUE = true
 	var FALSE = false
+	physicalDisks := make([]interface{}, 0)
+	physicalDisks2 := []string{
+		"Disk-1",
+		"Disk-2",
+	}
+	numberOfPhysicalDisks := 3
+
 	cases := []struct {
 		name          string
 		raid          *metal3v1alpha1.RAIDConfig
 		expected      []nodes.LogicalDisk
 		expectedError string
 	}{
+		{
+			name: "hardware raid, physicalDisks without controller",
+			raid: &metal3v1alpha1.RAIDConfig{
+				HardwareRAIDVolumes: []metal3v1alpha1.HardwareRAIDVolume{
+					{
+						Name:          "volume1",
+						Level:         "1",
+						Rotational:    &FALSE,
+						PhysicalDisks: physicalDisks2,
+					}, // end of RAID volume
+				}, // end of RAID volumes slice
+			}, // end of RAID config
+			expectedError: "'controller' must be specified if 'physicalDisks' are used!",
+		}, // end of test case
+		{
+			name: "hardware raid, len(physicalDisks) != numberOfPhysicalDisks",
+			raid: &metal3v1alpha1.RAIDConfig{
+				HardwareRAIDVolumes: []metal3v1alpha1.HardwareRAIDVolume{
+					{
+						Name:                  "volume1",
+						Level:                 "1",
+						Rotational:            &TRUE,
+						PhysicalDisks:         physicalDisks2,         // slice of 2 disks
+						NumberOfPhysicalDisks: &numberOfPhysicalDisks, // defined as 3 above
+						Controller:            "Controller-1",
+					}, // end of RAID volume
+				}, // end of RAID volumes slice
+			}, // end of RAID config
+			expectedError: fmt.Sprintf("the numberOfPhysicalDisks[%d] is not same as number of items in physicalDisks[%d]", numberOfPhysicalDisks, len(physicalDisks2)),
+		},
 		{
 			name: "hardware raid",
 			raid: &metal3v1alpha1.RAIDConfig{
@@ -44,14 +82,16 @@ func TestBuildTargetRAIDCfg(t *testing.T) {
 			},
 			expected: []nodes.LogicalDisk{
 				{
-					RAIDLevel:  "1",
-					VolumeName: "root",
-					DiskType:   nodes.SSD,
+					RAIDLevel:     "1",
+					VolumeName:    "root",
+					DiskType:      nodes.SSD,
+					PhysicalDisks: physicalDisks,
 				},
 				{
-					RAIDLevel:  "1",
-					DiskType:   nodes.HDD,
-					VolumeName: "v1",
+					RAIDLevel:     "1",
+					DiskType:      nodes.HDD,
+					VolumeName:    "v1",
+					PhysicalDisks: physicalDisks,
 				},
 			},
 		},
@@ -87,12 +127,14 @@ func TestBuildTargetRAIDCfg(t *testing.T) {
 			},
 			expected: []nodes.LogicalDisk{
 				{
-					RAIDLevel:  "1",
-					VolumeName: "",
+					RAIDLevel:     "1",
+					VolumeName:    "",
+					PhysicalDisks: physicalDisks,
 				},
 				{
-					RAIDLevel:  "1",
-					VolumeName: "",
+					RAIDLevel:     "1",
+					VolumeName:    "",
+					PhysicalDisks: physicalDisks,
 				},
 			},
 		},