Repository: metal3-io/baremetal-operator
Pull request #1063 (2022-01-05 11:39:18)

Fix software RAID
The current code treats software RAID similarly to hardware RAID and
expects the driver's RAIDInterface value to be "agent" to use it.
This is never the case, as software RAID is not driver-specific.

This change adjusts the procedure to set the expected RAIDInterface
before setting the target configuration. This makes software RAID work
with any values of the driver's RAIDInterface.

metal3-io-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please ask for approval from **dtantsur** after the PR has been reviewed.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=metal3-io%2Fbaremetal-operator).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/metal3-io/baremetal-operator/blob/main/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["dtantsur"]} -->
metal3-io-bot: @dtantsur: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
unit | 237c18764defe109f20f9c1acca9b4430d154aed | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1063/unit/1478692589967249408) | true | `/test unit`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1063). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:dtantsur).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @dtantsur: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
unit | 237c18764defe109f20f9c1acca9b4430d154aed | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1063/unit/1478692589967249408) | true | `/test unit`
golint | 237c18764defe109f20f9c1acca9b4430d154aed | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1063/golint/1478692590101467136) | true | `/test golint`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1063). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:dtantsur).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @dtantsur: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
unit | 237c18764defe109f20f9c1acca9b4430d154aed | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1063/unit/1478692589967249408) | true | `/test unit`
golint | 237c18764defe109f20f9c1acca9b4430d154aed | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1063/golint/1478692590101467136) | true | `/test golint`
gofmt | c563a2b3295e7b5b1f8ffb40fa29279abd2787a2 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1063/gofmt/1478709437341372416) | true | `/test gofmt`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1063). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:dtantsur).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @dtantsur: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
unit | 237c18764defe109f20f9c1acca9b4430d154aed | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1063/unit/1478692589967249408) | true | `/test unit`
golint | 237c18764defe109f20f9c1acca9b4430d154aed | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1063/golint/1478692590101467136) | true | `/test golint`
gofmt | c563a2b3295e7b5b1f8ffb40fa29279abd2787a2 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1063/gofmt/1478709437341372416) | true | `/test gofmt`
generate | c563a2b3295e7b5b1f8ffb40fa29279abd2787a2 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1063/generate/1478709437626585088) | true | `/test generate`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1063). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:dtantsur).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
dtantsur: /test-v1b1-centos-integration 
/test-v1b1-integration

I must admit I haven't tested this change, but it looks reasonable, or at least more reasonable than the code we have :)
dtantsur: /test-v1b1-integration
dtantsur: /test-v1b1-centos-integration
/test-v1b1-integration
metal3-io-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/metal3-io/baremetal-operator/pull/1063#pullrequestreview-858804761" title="Approved">bfournie</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please ask for approval from **dtantsur** after the PR has been reviewed.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=metal3-io%2Fbaremetal-operator).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/metal3-io/baremetal-operator/blob/main/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["dtantsur"]} -->
bfournie: /lgtm
zaneb: /cc @Hellcatlk 
Hellcatlk: lgtm, but I want to do some testing before merging,
But I'm very sorry, 1.31~2.6 is the Chinese New Year holiday, I can't start this work right now.
I will start this work immediately after the holidays. 
Hellcatlk: After testing, this patch works fine.
But Ironic doesn't seem to have successfully configured the software raid,
In the preparing state, we can see the following raid configuration in bmh:
```
[core@worker-2 ~]$ sudo lsblk
NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
loop0 7:0 0 15.4G 0 loop /run/ephemeral
loop1 7:1 0 882.7M 0 loop /sysroot
sda 8:0 0 372.6G 0 disk
└─sda1 8:1 0 372.1G 0 part
  └─md0 9:0 0 372G 0 raid1
sdb 8:16 0 372.6G 0 disk
└─sdb1 8:17 0 372.1G 0 part
  └─md0 9:0 0 372G 0 raid1
sdc 8:32 0 1.8T 0 disk
└─sdc1 8:33 0 1.8T 0 part
  └─md0 9:0 0 372G 0 raid1
sdd 8:48 0 1.8T 0 disk
└─sdd1 8:49 0 1.8T 0 part
  └─md0 9:0 0 372G 0 raid1
sde 8:64 0 1.8T 0 disk
└─sde1 8:65 0 1.8T 0 part
  └─md0 9:0 0 372G 0 raid1
```

But in provisioned state, the software raid is gone:
```
[root@worker-2 core]# lsblk
NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
sda 8:0 0 372.6G 0 disk
├─sda1 8:1 0 1M 0 part
├─sda2 8:2 0 127M 0 part
├─sda3 8:3 0 384M 0 part /boot
└─sda4 8:4 0 372.1G 0 part /sysroot
sdb 8:16 0 372.6G 0 disk
sdc 8:32 0 1.8T 0 disk
sdd 8:48 0 1.8T 0 disk
sde 8:64 0 1.8T 0 disk
```

Following is the raid configuration:
```
spec:
  raid:
    softwareRAIDVolumes:
    - level: "1"
```

Is this reasonable? 
zhouhao3: Hi @zaneb @dtantsur, as @Hellcatlk replied above, can we merge this PR if that's normal?
dtantsur: @Hellcatlk you may need to check if `/dev/md0` is set as a root device hint. I think Ironic sets it, but maybe BMO overwrites it then.

In any case, I think it's a great improvement, and we can iterate further.
dtantsur: Otherwise, I'll need ironic and BMO logs to be sure.
Hellcatlk: > @Hellcatlk you may need to check if /dev/md0 is set as a root device hint. I think Ironic sets it, but maybe BMO overwrites it then.

@dtantsur We checked that the root device hint is `/dev/sda`.
```
Raid:
  Hardware RAID Volumes: <nil> 
  Software RAID Volumes:
    Level: 1 
Root Device Hints
  Device Name: /dev/sda
```

> In any case, I think it's a great improvement, and we can iterate further.

Agree.
metal3-io-bot: New changes are detected. LGTM label has been removed.
metal3-io-bot: @dtantsur: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
gofmt | cbba308e41b7aa216654a6d8cbcbdb5072bfbc01 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1063/gofmt/1494313379127365632) | true | `/test gofmt`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1063). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:dtantsur).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
metal3-io-bot: @dtantsur: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
gofmt | cbba308e41b7aa216654a6d8cbcbdb5072bfbc01 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1063/gofmt/1494313379127365632) | true | `/test gofmt`
generate | cbba308e41b7aa216654a6d8cbcbdb5072bfbc01 | [link](https://prow.apps.test.metal3.io/view/s3/prow-logs/pr-logs/pull/metal3-io_baremetal-operator/1063/generate/1494313379488075776) | true | `/test generate`

[Full PR test history](https://prow.apps.test.metal3.io/pr-history?org=metal3-io&repo=baremetal-operator&pr=1063). [Your PR dashboard](https://prow.apps.test.metal3.io/pr?query=is:pr+state:open+author:dtantsur).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
dtantsur: @Hellcatlk can you try removing `RootDeviceHints` from your BMH?
elfosardo: /test-v1b1-centos-integration
/test-v1b1-ubuntu-integration 
elfosardo: /lgtm
metal3-io-bot: @elfosardo: adding LGTM is restricted to approvers and reviewers in OWNERS files.

<details>

In response to [this](https://github.com/metal3-io/baremetal-operator/pull/1063#issuecomment-1049622857):

>/lgtm


Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>
Hellcatlk: @dtantsur Hi, did the [above test results](https://github.com/metal3-io/baremetal-operator/pull/1063#issuecomment-1044422540) meet your expectations? Is there anything else we need to do?

Patch:
diff --git a/docs/api.md b/docs/api.md
index fd771de1..fd0561ed 100644
--- a/docs/api.md
+++ b/docs/api.md
@@ -213,19 +213,19 @@ spec:
 
 **NOTE:** Currently the 'raid' field is only supported by ilo5/idrac/irmc.
 
-**NOTE:** Software RAID will always be deleted.
+**NOTE:** Do not try to simultaneously delete hardware RAID volumes and add
+software RAID volumes. Delete hardware RAID first, wait for the host to settle
+down, add software volumes afterwards.
 
 **NOTE:** If you got following error message:
 
 1. raid settings are defined, but the node's driver %s does not support RAID.
 2. node's driver %s does not support hardware RAID.
-3. node's driver %s does not support software RAID.
 
 You can solve it by:
 
 1. Keep raid field is nil.
 2. Keep hardwareRAIDVolumes field is nil.
-3. Keep softwareRAIDVolumes field is nil.
 
 If the error message you get isn't included the above, you may need to check
 whether the BM has a RAID controller and keep the raid field blank.
diff --git a/pkg/provisioner/ironic/raid.go b/pkg/provisioner/ironic/raid.go
index 69143126..bb5c1741 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)
+	targetRaidInterface, err := CheckRAIDInterface(raidInterface, data.TargetRAIDConfig, data.ActualRAIDConfig)
 	if err != nil {
 		return operationFailed(err.Error())
 	}
@@ -43,6 +43,13 @@ func setTargetRAIDCfg(p *ironicProvisioner, raidInterface string, ironicNode *no
 		p.log.Info("rootDeviceHints is used, the first volume of raid will not be set to root")
 	}
 
+	updater := updateOptsBuilder(p.debugLog)
+	updater.SetTopLevelOpt("raid_interface", targetRaidInterface, ironicNode.RAIDInterface)
+	success, result, err := p.tryUpdateNode(ironicNode, updater)
+	if !success {
+		return result, err
+	}
+
 	// Set target for RAID configuration steps
 	err = nodes.SetRAIDConfig(
 		p.client,
@@ -178,28 +185,24 @@ 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, actual)
 	if err != nil {
 		return nil, err
 	}
 
-	// No RAID
-	if raidInterface == noRAIDInterface {
-		return
-	}
-
 	// Software RAID
-	if raidInterface == softwareRAIDInterface {
+	if target != nil && target.SoftwareRAIDVolumes != nil {
 		// Ignore HardwareRAIDVolumes
-		if target != nil {
-			target.HardwareRAIDVolumes = nil
-		}
+		target.HardwareRAIDVolumes = nil
 		if actual != nil {
 			actual.HardwareRAIDVolumes = nil
 		}
 		if reflect.DeepEqual(target, actual) {
 			return
 		}
+		if len(target.SoftwareRAIDVolumes) == 0 && (actual == nil || len(actual.SoftwareRAIDVolumes) == 0) {
+			return
+		}
 
 		cleanSteps = append(
 			cleanSteps,
@@ -216,7 +219,7 @@ func BuildRAIDCleanSteps(raidInterface string, target *metal3v1alpha1.RAIDConfig
 		)
 
 		// If software raid configuration is empty, only need to clear old configuration
-		if target == nil || len(target.SoftwareRAIDVolumes) == 0 {
+		if len(target.SoftwareRAIDVolumes) == 0 {
 			return
 		}
 
@@ -233,7 +236,7 @@ func BuildRAIDCleanSteps(raidInterface string, target *metal3v1alpha1.RAIDConfig
 	// Hardware RAID
 	// If hardware RAID configuration is nil,
 	// keep old hardware RAID configuration
-	if target == nil || target.HardwareRAIDVolumes == nil {
+	if raidInterface == noRAIDInterface || target == nil || target.HardwareRAIDVolumes == nil {
 		return
 	}
 
@@ -273,20 +276,32 @@ func BuildRAIDCleanSteps(raidInterface string, target *metal3v1alpha1.RAIDConfig
 	return
 }
 
-func checkRAIDConfigure(raidInterface string, raid *metal3v1alpha1.RAIDConfig) error {
-	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)
-		}
-	case softwareRAIDInterface:
-		if raid != nil && len(raid.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 {
-			return fmt.Errorf("node's driver %s does not support software RAID", raidInterface)
-		}
+// CheckRAIDInterface checks the current RAID interface against the requested configuration
+func CheckRAIDInterface(raidInterface string, target, actual *metal3v1alpha1.RAIDConfig) (string, error) {
+	if target == nil {
+		return raidInterface, nil
+	}
+
+	if raidInterface == noRAIDInterface && len(target.HardwareRAIDVolumes) != 0 {
+		return "", fmt.Errorf("RAID settings are defined, but the node's driver %s does not support RAID", raidInterface)
+	}
+
+	// This is not a real case since no BMC driver defaults to agent, but we add it for consistency
+	if raidInterface == softwareRAIDInterface && len(target.HardwareRAIDVolumes) != 0 {
+		return "", fmt.Errorf("hardware RAID settings are defined, but the node's driver %s only supports software RAID", raidInterface)
 	}
-	return nil
+
+	// If software RAID is requested, change the RAID interface.
+	// FIXME(dtantsur): if a user tries to simultaneously remove hardware RAID volumes and add software RAID volumes, it will not work.
+	// The matter is complicated because Ironic cannot change the RAID interface in the middle of cleaning. We better just document it.
+	if len(target.SoftwareRAIDVolumes) != 0 {
+		return softwareRAIDInterface, nil
+	}
+
+	// If software RAID is being deleted, also change the interface.
+	if actual != nil && len(actual.SoftwareRAIDVolumes) != 0 {
+		return softwareRAIDInterface, nil
+	}
+
+	return raidInterface, nil
 }


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..079b73d6 100644
--- a/pkg/provisioner/ironic/raid_test.go
+++ b/pkg/provisioner/ironic/raid_test.go
@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/gophercloud/gophercloud/openstack/baremetal/v1/nodes"
+	"github.com/stretchr/testify/assert"
 
 	metal3v1alpha1 "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
 )
@@ -259,11 +260,6 @@ func TestBuildRAIDCleanSteps(t *testing.T) {
 						Level: "1",
 					},
 				},
-				SoftwareRAIDVolumes: []metal3v1alpha1.SoftwareRAIDVolume{
-					{
-						Level: "1",
-					},
-				},
 			},
 			expected: []nodes.CleanStep{
 				{
@@ -286,11 +282,6 @@ func TestBuildRAIDCleanSteps(t *testing.T) {
 						Level: "1",
 					},
 				},
-				SoftwareRAIDVolumes: []metal3v1alpha1.SoftwareRAIDVolume{
-					{
-						Level: "1",
-					},
-				},
 			},
 			actual: &metal3v1alpha1.RAIDConfig{
 				HardwareRAIDVolumes: []metal3v1alpha1.HardwareRAIDVolume{
@@ -365,6 +356,21 @@ func TestBuildRAIDCleanSteps(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:          "keep missing software RAID",
+			raidInterface: "agent",
+			target: &metal3v1alpha1.RAIDConfig{
+				SoftwareRAIDVolumes: []metal3v1alpha1.SoftwareRAIDVolume{},
+			},
+		},
+		{
+			name:          "keep empty software RAID",
+			raidInterface: "agent",
+			target: &metal3v1alpha1.RAIDConfig{
+				SoftwareRAIDVolumes: []metal3v1alpha1.SoftwareRAIDVolume{},
+			},
+			actual: &metal3v1alpha1.RAIDConfig{},
+		},
 		{
 			name:          "clear software RAID",
 			raidInterface: "agent",
@@ -406,9 +412,11 @@ func TestBuildRAIDCleanSteps(t *testing.T) {
 
 func TestCheckRAIDConfigure(t *testing.T) {
 	cases := []struct {
-		raidInterface string
-		RAID          *metal3v1alpha1.RAIDConfig
-		expectedError bool
+		raidInterface        string
+		RAID                 *metal3v1alpha1.RAIDConfig
+		expectedError        bool
+		expectedNewInterface string
+		currentRAID          *metal3v1alpha1.RAIDConfig
 	}{
 		{
 			raidInterface: "no-raid",
@@ -429,6 +437,17 @@ func TestCheckRAIDConfigure(t *testing.T) {
 			},
 			expectedError: true,
 		},
+		{
+			raidInterface: "no-raid",
+			RAID: &metal3v1alpha1.RAIDConfig{
+				SoftwareRAIDVolumes: []metal3v1alpha1.SoftwareRAIDVolume{
+					{
+						Level: "1",
+					},
+				},
+			},
+			expectedNewInterface: "agent",
+		},
 		{
 			raidInterface: "agent",
 		},
@@ -485,16 +504,37 @@ func TestCheckRAIDConfigure(t *testing.T) {
 					},
 				},
 			},
-			expectedError: true,
+			expectedNewInterface: "agent",
+		},
+		{
+			raidInterface: "hardware",
+			RAID: &metal3v1alpha1.RAIDConfig{
+				SoftwareRAIDVolumes: []metal3v1alpha1.SoftwareRAIDVolume{},
+			},
+			currentRAID: &metal3v1alpha1.RAIDConfig{
+				SoftwareRAIDVolumes: []metal3v1alpha1.SoftwareRAIDVolume{
+					{
+						Level: "1",
+					},
+				},
+			},
+			expectedNewInterface: "agent",
 		},
 	}
 
 	for _, c := range cases {
 		t.Run(c.raidInterface, func(t *testing.T) {
-			err := checkRAIDConfigure(c.raidInterface, c.RAID)
+			newInterface, err := CheckRAIDInterface(c.raidInterface, c.RAID, c.currentRAID)
 			if (err != nil) != c.expectedError {
 				t.Errorf("Got unexpected error: %v", err)
 			}
+			if !c.expectedError {
+				if c.expectedNewInterface != "" {
+					assert.Equal(t, c.expectedNewInterface, newInterface)
+				} else {
+					assert.Equal(t, c.raidInterface, newInterface)
+				}
+			}
 		})
 	}
 }