Repository: metal3-io/baremetal-operator
Pull request #567 (2020-06-30 14:06:34)

pkg/bmc: refactor to support all schemes
All of the redfish BMC's should support `+http` or `+https` explicitly,
however it's not configured for any of the virtualmedia BMC's. This
refactories the registerFactory code to allow specifying schemes for a
BMC, and ensuring the schemed versions exist for every one.

This keeps it explicit, so we can say which BMC's support specifying a
scheme.


Patch:
diff --git a/pkg/bmc/access.go b/pkg/bmc/access.go
index f29d041a..98a90d2c 100644
--- a/pkg/bmc/access.go
+++ b/pkg/bmc/access.go
@@ -1,6 +1,7 @@
 package bmc
 
 import (
+	"fmt"
 	"net"
 	"net/url"
 	"strings"
@@ -16,8 +17,12 @@ var factories = map[string]AccessDetailsFactory{}
 
 // We could make this function public if we want to support
 // out-of-tree factories.
-func registerFactory(name string, factory AccessDetailsFactory) {
+func registerFactory(name string, factory AccessDetailsFactory, schemes []string) {
 	factories[name] = factory
+
+	for _, scheme := range schemes {
+		factories[fmt.Sprintf("%s+%s", name, scheme)] = factory
+	}
 }
 
 // AccessDetails contains the information about how to get to a BMC.
diff --git a/pkg/bmc/ibmc.go b/pkg/bmc/ibmc.go
index 12c41a7c..f726d56e 100644
--- a/pkg/bmc/ibmc.go
+++ b/pkg/bmc/ibmc.go
@@ -6,9 +6,7 @@ import (
 )
 
 func init() {
-	registerFactory("ibmc", newIbmcAccessDetails)
-	registerFactory("ibmc+http", newIbmcAccessDetails)
-	registerFactory("ibmc+https", newIbmcAccessDetails)
+	registerFactory("ibmc", newIbmcAccessDetails, []string{"http", "https"})
 }
 
 func newIbmcAccessDetails(parsedURL *url.URL, disableCertificateVerification bool) (AccessDetails, error) {
diff --git a/pkg/bmc/idrac.go b/pkg/bmc/idrac.go
index cb3a5c1f..943f005a 100644
--- a/pkg/bmc/idrac.go
+++ b/pkg/bmc/idrac.go
@@ -6,9 +6,7 @@ import (
 )
 
 func init() {
-	registerFactory("idrac", newIDRACAccessDetails)
-	registerFactory("idrac+http", newIDRACAccessDetails)
-	registerFactory("idrac+https", newIDRACAccessDetails)
+	registerFactory("idrac", newIDRACAccessDetails, []string{"http", "https"})
 }
 
 func newIDRACAccessDetails(parsedURL *url.URL, disableCertificateVerification bool) (AccessDetails, error) {
diff --git a/pkg/bmc/ipmi.go b/pkg/bmc/ipmi.go
index 46eac512..a65cea1d 100644
--- a/pkg/bmc/ipmi.go
+++ b/pkg/bmc/ipmi.go
@@ -5,8 +5,8 @@ import (
 )
 
 func init() {
-	registerFactory("ipmi", newIPMIAccessDetails)
-	registerFactory("libvirt", newIPMIAccessDetails)
+	registerFactory("ipmi", newIPMIAccessDetails, []string{})
+	registerFactory("libvirt", newIPMIAccessDetails, []string{})
 }
 
 func newIPMIAccessDetails(parsedURL *url.URL, disableCertificateVerification bool) (AccessDetails, error) {
diff --git a/pkg/bmc/irmc.go b/pkg/bmc/irmc.go
index 5b54ad50..cc7cfab7 100644
--- a/pkg/bmc/irmc.go
+++ b/pkg/bmc/irmc.go
@@ -5,7 +5,7 @@ import (
 )
 
 func init() {
-	registerFactory("irmc", newIRMCAccessDetails)
+	registerFactory("irmc", newIRMCAccessDetails, []string{})
 }
 
 func newIRMCAccessDetails(parsedURL *url.URL, disableCertificateVerification bool) (AccessDetails, error) {
diff --git a/pkg/bmc/redfish.go b/pkg/bmc/redfish.go
index c2e09313..1613e1f9 100644
--- a/pkg/bmc/redfish.go
+++ b/pkg/bmc/redfish.go
@@ -6,12 +6,11 @@ import (
 )
 
 func init() {
-	registerFactory("redfish", newRedfishAccessDetails)
-	registerFactory("redfish+http", newRedfishAccessDetails)
-	registerFactory("redfish+https", newRedfishAccessDetails)
-	registerFactory("redfish-virtualmedia", newRedfishVirtualMediaAccessDetails)
-	registerFactory("ilo5-virtualmedia", newRedfishVirtualMediaAccessDetails)
-	registerFactory("idrac-virtualmedia", newRedfishiDracVirtualMediaAccessDetails)
+	schemes := []string{"http", "https"}
+	registerFactory("redfish", newRedfishAccessDetails, schemes)
+	registerFactory("redfish-virtualmedia", newRedfishVirtualMediaAccessDetails, schemes)
+	registerFactory("ilo5-virtualmedia", newRedfishVirtualMediaAccessDetails, schemes)
+	registerFactory("idrac-virtualmedia", newRedfishiDracVirtualMediaAccessDetails, schemes)
 }
 
 func redfishDetails(parsedURL *url.URL, disableCertificateVerification bool) *redfishAccessDetails {


Test patch:
diff --git a/pkg/bmc/access_test.go b/pkg/bmc/access_test.go
index 1214010f..3fd32b4f 100644
--- a/pkg/bmc/access_test.go
+++ b/pkg/bmc/access_test.go
@@ -453,6 +453,30 @@ func TestStaticDriverInfo(t *testing.T) {
 			vendor:     "",
 		},
 
+		{
+			Scenario:   "redfish virtual media HTTP",
+			input:      "redfish-virtualmedia+http://192.168.122.1",
+			needsMac:   true,
+			driver:     "redfish",
+			boot:       "redfish-virtual-media",
+			management: "",
+			power:      "",
+			raid:       "",
+			vendor:     "",
+		},
+
+		{
+			Scenario:   "redfish virtual media HTTPS",
+			input:      "redfish-virtualmedia+https://192.168.122.1",
+			needsMac:   true,
+			driver:     "redfish",
+			boot:       "redfish-virtual-media",
+			management: "",
+			power:      "",
+			raid:       "",
+			vendor:     "",
+		},
+
 		{
 			Scenario: "ilo5 virtual media",
 			input:    "ilo5-virtualmedia://192.168.122.1",
@@ -461,6 +485,22 @@ func TestStaticDriverInfo(t *testing.T) {
 			boot:     "redfish-virtual-media",
 		},
 
+		{
+			Scenario: "ilo5 virtual media HTTP",
+			input:    "ilo5-virtualmedia+http://192.168.122.1",
+			needsMac: true,
+			driver:   "redfish",
+			boot:     "redfish-virtual-media",
+		},
+
+		{
+			Scenario: "ilo5 virtual media HTTPS",
+			input:    "ilo5-virtualmedia+https://192.168.122.1",
+			needsMac: true,
+			driver:   "redfish",
+			boot:     "redfish-virtual-media",
+		},
+
 		{
 			Scenario:   "idrac virtual media",
 			input:      "idrac-virtualmedia://192.168.122.1",
@@ -473,6 +513,30 @@ func TestStaticDriverInfo(t *testing.T) {
 			vendor:     "no-vendor",
 		},
 
+		{
+			Scenario:   "idrac virtual media HTTP",
+			input:      "idrac-virtualmedia+http://192.168.122.1",
+			needsMac:   true,
+			driver:     "idrac",
+			boot:       "idrac-redfish-virtual-media",
+			management: "idrac-redfish",
+			power:      "idrac-redfish",
+			raid:       "no-raid",
+			vendor:     "no-vendor",
+		},
+
+		{
+			Scenario:   "idrac virtual media HTTPS",
+			input:      "idrac-virtualmedia+https://192.168.122.1",
+			needsMac:   true,
+			driver:     "idrac",
+			boot:       "idrac-redfish-virtual-media",
+			management: "idrac-redfish",
+			power:      "idrac-redfish",
+			raid:       "no-raid",
+			vendor:     "no-vendor",
+		},
+
 		{
 			Scenario:   "ibmc",
 			input:      "ibmc://192.168.122.1:6233",