Repository: metal3-io/baremetal-operator
Pull request #933 (2021-07-07 14:09:05)

Do not disable iDRAC Redfish vendor interfaces
There is no harm in having them enabled, they're not used implicitly.
In the future we may start using them, e.g. to eject virtual CD.

Requires https://github.com/metal3-io/ironic-image/pull/284.


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/master/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"]} -->
dtantsur: /test-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/933#issuecomment-880512572):

>/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>
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/933#pullrequestreview-707085264" title="Approved">elfosardo</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/master/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: PR needs rebase.

<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.
</details>

Patch:
diff --git a/pkg/bmc/idrac_virtualmedia.go b/pkg/bmc/idrac_virtualmedia.go
index 82e32db1..d23c74a8 100644
--- a/pkg/bmc/idrac_virtualmedia.go
+++ b/pkg/bmc/idrac_virtualmedia.go
@@ -91,7 +91,8 @@ func (a *redfishiDracVirtualMediaAccessDetails) RAIDInterface() string {
 }
 
 func (a *redfishiDracVirtualMediaAccessDetails) VendorInterface() string {
-	return "no-vendor"
+	// NOTE(dtantsur): the idrac hardware type defaults to WSMAN vendor, we need to use the Redfish implementation.
+	return "idrac-redfish"
 }
 
 func (a *redfishiDracVirtualMediaAccessDetails) SupportsSecureBoot() bool {
diff --git a/pkg/bmc/redfish.go b/pkg/bmc/redfish.go
index a54673ff..d791d5e6 100644
--- a/pkg/bmc/redfish.go
+++ b/pkg/bmc/redfish.go
@@ -162,7 +162,8 @@ func (a *redfishiDracAccessDetails) RAIDInterface() string {
 }
 
 func (a *redfishiDracAccessDetails) VendorInterface() string {
-	return "no-vendor"
+	// NOTE(dtantsur): the idrac hardware type defaults to WSMAN vendor, we need to use the Redfish implementation.
+	return "idrac-redfish"
 }
 
 func (a *redfishiDracAccessDetails) BuildBIOSSettings(firmwareConfig *metal3v1alpha1.FirmwareConfig) (settings []map[string]string, err error) {


Test patch:
diff --git a/pkg/bmc/access_test.go b/pkg/bmc/access_test.go
index 77b46c61..f4df5c6b 100644
--- a/pkg/bmc/access_test.go
+++ b/pkg/bmc/access_test.go
@@ -456,7 +456,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			management: "",
 			power:      "",
 			raid:       "no-raid",
-			vendor:     "",
 		},
 
 		{
@@ -469,7 +468,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			management: "",
 			power:      "",
 			raid:       "no-raid",
-			vendor:     "",
 		},
 
 		{
@@ -482,7 +480,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			management: "",
 			power:      "",
 			raid:       "no-raid",
-			vendor:     "",
 		},
 
 		{
@@ -495,7 +492,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			management: "",
 			power:      "",
 			raid:       "irmc",
-			vendor:     "",
 		},
 
 		{
@@ -508,7 +504,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			management: "",
 			power:      "",
 			raid:       "no-raid",
-			vendor:     "",
 		},
 
 		{
@@ -521,7 +516,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			management: "",
 			power:      "",
 			raid:       "no-raid",
-			vendor:     "",
 		},
 
 		{
@@ -534,7 +528,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			management: "",
 			power:      "",
 			raid:       "no-raid",
-			vendor:     "",
 		},
 
 		{
@@ -547,7 +540,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			management: "",
 			power:      "",
 			raid:       "no-raid",
-			vendor:     "",
 		},
 
 		{
@@ -560,7 +552,7 @@ func TestStaticDriverInfo(t *testing.T) {
 			management: "idrac-redfish",
 			power:      "idrac-redfish",
 			raid:       "no-raid",
-			vendor:     "no-vendor",
+			vendor:     "idrac-redfish",
 		},
 
 		{
@@ -600,7 +592,7 @@ func TestStaticDriverInfo(t *testing.T) {
 			management: "idrac-redfish",
 			power:      "idrac-redfish",
 			raid:       "no-raid",
-			vendor:     "no-vendor",
+			vendor:     "idrac-redfish",
 		},
 
 		{
@@ -613,7 +605,7 @@ func TestStaticDriverInfo(t *testing.T) {
 			management: "idrac-redfish",
 			power:      "idrac-redfish",
 			raid:       "no-raid",
-			vendor:     "no-vendor",
+			vendor:     "idrac-redfish",
 		},
 
 		{
@@ -626,7 +618,7 @@ func TestStaticDriverInfo(t *testing.T) {
 			management: "idrac-redfish",
 			power:      "idrac-redfish",
 			raid:       "no-raid",
-			vendor:     "no-vendor",
+			vendor:     "idrac-redfish",
 		},
 
 		{
@@ -639,7 +631,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			management: "ibmc",
 			power:      "ibmc",
 			raid:       "no-raid",
-			vendor:     "",
 		},
 
 		{
@@ -652,7 +643,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			management: "",
 			power:      "",
 			raid:       "no-raid",
-			vendor:     "",
 		},
 
 		{
@@ -665,7 +655,6 @@ func TestStaticDriverInfo(t *testing.T) {
 			management: "",
 			power:      "",
 			raid:       "ilo5",
-			vendor:     "",
 		},
 	} {
 		t.Run(tc.Scenario, func(t *testing.T) {
@@ -687,6 +676,10 @@ func TestStaticDriverInfo(t *testing.T) {
 				t.Fatalf("Unexpected bios interface %q, expected %q",
 					acc.BIOSInterface(), tc.bios)
 			}
+			if acc.VendorInterface() != tc.vendor {
+				t.Fatalf("Unexpected vendor interface %q, expected %q",
+					acc.VendorInterface(), tc.vendor)
+			}
 		})
 	}
 }