Repository: metal3-io/baremetal-operator
Pull request #595 (2020-07-16 12:35:51)

Adding support for hardware type iLO
This commit add support for iLO hardware type to
deploy ilo4 based systems using BMO.

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/595#" title="Author self-approved">ankit-kumar5</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **maelk**
You can assign the PR to them by writing `/assign @maelk` in a comment when ready.

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":["maelk"]} -->
metal3-io-bot: Hi @ankit-kumar5. Thanks for your PR.

I'm waiting for a [metal3-io](https://github.com/orgs/metal3-io/people) member to verify that this patch is reasonable to test. If it is, they should reply with `/ok-to-test` on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should [join the org](https://github.com/orgs/metal3-io/people) to skip this step.

Once the patch is verified, the new status will be reflected by the `ok-to-test` label.

I understand the commands that are listed [here](https://go.k8s.io/bot-commands?repo=metal3-io%2Fbaremetal-operator).

<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>

metal3-io-bot: @ankit-kumar5: The following test **failed**, say `/retest` to rerun them all:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
gofmt | ba6dc38e8a3a28b07c18c31b965fefcca2435a60 | [link](https://prow.apps.ci.metal3.io/view/gcs/metal3-prow-artifacts/pr-logs/pull/metal3-io_baremetal-operator/595/gofmt/1283760300687888386) | `/test gofmt`



<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: @ankit-kumar5: The following test **failed**, say `/retest` to rerun them all:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
gofmt | ba6dc38e8a3a28b07c18c31b965fefcca2435a60 | [link](https://prow.apps.ci.metal3.io/view/gcs/metal3-prow-artifacts/pr-logs/pull/metal3-io_baremetal-operator/595/gofmt/1283760300687888392) | `/test gofmt`



<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: @ankit-kumar5: The following test **failed**, say `/retest` to rerun them all:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
unit | e4b182c287978c12b78f111b0bea8e9cb42e89fc | [link](https://prow.apps.ci.metal3.io/view/gcs/metal3-prow-artifacts/pr-logs/pull/metal3-io_baremetal-operator/595/unit/1284815325795192835) | `/test unit`



<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: @ankit-kumar5: The following test **failed**, say `/retest` to rerun them all:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
unit | 50253ed90d4b7cb0508f6320b82d6fabe56f18fe | [link](https://prow.apps.ci.metal3.io/view/gcs/metal3-prow-artifacts/pr-logs/pull/metal3-io_baremetal-operator/595/unit/1284817590790656003) | `/test unit`



<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: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/metal3-io/baremetal-operator/pull/595#" title="Author self-approved">ankit-kumar5</a>*, *<a href="https://github.com/metal3-io/baremetal-operator/pull/595#pullrequestreview-488896349" title="Approved">zaneb</a>*

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

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

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

- ~~[OWNERS](https://github.com/metal3-io/baremetal-operator/blob/master/OWNERS)~~ [zaneb]

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":[]} -->

Patch:
diff --git a/docs/api.md b/docs/api.md
index 94c5e855..545ce541 100644
--- a/docs/api.md
+++ b/docs/api.md
@@ -40,6 +40,12 @@ communicate with them.
   * `irmc://<host>:<port>`, where `<port>` is optional if using the default.
 * HUAWEI ibmc
   * `ibmc://<host>:<port>` (or `ibmc+http://<host>:<port>` to disable TLS)
+* HPE iLO 4
+  * `ilo4://<host>:<port>` for iLO 4 based systems and the port is optional,
+    if using the default one (443).
+* HPE iLO 5
+  * `ilo5://<host>:<port>` for iLO 5 based systems and the port is optional,
+    if using the default one (443).
 * iLO 5 Redfish
   * `ilo5-redfish://` (or `ilo5-redfish+http://` to disable TLS), the hostname
     or IP address, and the path to the system ID are required,
diff --git a/pkg/bmc/ilo4.go b/pkg/bmc/ilo4.go
new file mode 100644
index 00000000..0dca7050
--- /dev/null
+++ b/pkg/bmc/ilo4.go
@@ -0,0 +1,91 @@
+// Copyright (c) 2016-2018 Hewlett Packard Enterprise Development LP
+
+package bmc
+
+import (
+	"net/url"
+)
+
+func init() {
+	registerFactory("ilo4", newILOAccessDetails, []string{"https"})
+}
+
+func newILOAccessDetails(parsedURL *url.URL, disableCertificateVerification bool) (AccessDetails, error) {
+	return &iLOAccessDetails{
+		bmcType:                        parsedURL.Scheme,
+		portNum:                        parsedURL.Port(),
+		hostname:                       parsedURL.Hostname(),
+		disableCertificateVerification: disableCertificateVerification,
+	}, nil
+}
+
+type iLOAccessDetails struct {
+	bmcType                        string
+	portNum                        string
+	hostname                       string
+	disableCertificateVerification bool
+}
+
+func (a *iLOAccessDetails) Type() string {
+	return a.bmcType
+}
+
+// NeedsMAC returns true when the host is going to need a separate
+// port created rather than having it discovered.
+func (a *iLOAccessDetails) NeedsMAC() bool {
+	// For the inspection to work, we need a MAC address
+	// https://github.com/metal3-io/baremetal-operator/pull/284#discussion_r317579040
+	return true
+}
+
+func (a *iLOAccessDetails) Driver() string {
+	return "ilo"
+}
+
+func (a *iLOAccessDetails) DisableCertificateVerification() bool {
+	return a.disableCertificateVerification
+}
+
+// DriverInfo returns a data structure to pass as the DriverInfo
+// parameter when creating a node in Ironic. The structure is
+// pre-populated with the access information, and the caller is
+// expected to add any other information that might be needed (such as
+// the kernel and ramdisk locations).
+func (a *iLOAccessDetails) DriverInfo(bmcCreds Credentials) map[string]interface{} {
+
+	result := map[string]interface{}{
+		"ilo_username": bmcCreds.Username,
+		"ilo_password": bmcCreds.Password,
+		"ilo_address":  a.hostname,
+	}
+
+	if a.disableCertificateVerification {
+		result["ilo_verify_ca"] = false
+	}
+
+	if a.portNum != "" {
+		result["client_port"] = a.portNum
+	}
+
+	return result
+}
+
+func (a *iLOAccessDetails) BootInterface() string {
+	return "ilo-ipxe"
+}
+
+func (a *iLOAccessDetails) ManagementInterface() string {
+	return ""
+}
+
+func (a *iLOAccessDetails) PowerInterface() string {
+	return ""
+}
+
+func (a *iLOAccessDetails) RAIDInterface() string {
+	return ""
+}
+
+func (a *iLOAccessDetails) VendorInterface() string {
+	return ""
+}
diff --git a/pkg/bmc/ilo5.go b/pkg/bmc/ilo5.go
new file mode 100644
index 00000000..5ba56ee0
--- /dev/null
+++ b/pkg/bmc/ilo5.go
@@ -0,0 +1,91 @@
+// Copyright (c) 2016-2018 Hewlett Packard Enterprise Development LP
+
+package bmc
+
+import (
+	"net/url"
+)
+
+func init() {
+	registerFactory("ilo5", newILO5AccessDetails, []string{"https"})
+}
+
+func newILO5AccessDetails(parsedURL *url.URL, disableCertificateVerification bool) (AccessDetails, error) {
+	return &iLO5AccessDetails{
+		bmcType:                        parsedURL.Scheme,
+		portNum:                        parsedURL.Port(),
+		hostname:                       parsedURL.Hostname(),
+		disableCertificateVerification: disableCertificateVerification,
+	}, nil
+}
+
+type iLO5AccessDetails struct {
+	bmcType                        string
+	portNum                        string
+	hostname                       string
+	disableCertificateVerification bool
+}
+
+func (a *iLO5AccessDetails) Type() string {
+	return a.bmcType
+}
+
+// NeedsMAC returns true when the host is going to need a separate
+// port created rather than having it discovered.
+func (a *iLO5AccessDetails) NeedsMAC() bool {
+	// For the inspection to work, we need a MAC address
+	// https://github.com/metal3-io/baremetal-operator/pull/284#discussion_r317579040
+	return true
+}
+
+func (a *iLO5AccessDetails) Driver() string {
+	return "ilo5"
+}
+
+func (a *iLO5AccessDetails) DisableCertificateVerification() bool {
+	return a.disableCertificateVerification
+}
+
+// DriverInfo returns a data structure to pass as the DriverInfo
+// parameter when creating a node in Ironic. The structure is
+// pre-populated with the access information, and the caller is
+// expected to add any other information that might be needed (such as
+// the kernel and ramdisk locations).
+func (a *iLO5AccessDetails) DriverInfo(bmcCreds Credentials) map[string]interface{} {
+
+	result := map[string]interface{}{
+		"ilo_username": bmcCreds.Username,
+		"ilo_password": bmcCreds.Password,
+		"ilo_address":  a.hostname,
+	}
+
+	if a.disableCertificateVerification {
+		result["ilo_verify_ca"] = false
+	}
+
+	if a.portNum != "" {
+		result["client_port"] = a.portNum
+	}
+
+	return result
+}
+
+func (a *iLO5AccessDetails) BootInterface() string {
+	return "ilo-ipxe"
+}
+
+func (a *iLO5AccessDetails) ManagementInterface() string {
+	return ""
+}
+
+func (a *iLO5AccessDetails) PowerInterface() string {
+	return ""
+}
+
+func (a *iLO5AccessDetails) RAIDInterface() string {
+	return "ilo5"
+}
+
+func (a *iLO5AccessDetails) VendorInterface() string {
+	return ""
+}


Test patch:
diff --git a/pkg/bmc/access_test.go b/pkg/bmc/access_test.go
index 3fd32b4f..7f1ef1cd 100644
--- a/pkg/bmc/access_test.go
+++ b/pkg/bmc/access_test.go
@@ -311,6 +311,66 @@ func TestParse(t *testing.T) {
 			Hostname: "192.168.122.1",
 			Path:     "",
 		},
+
+		{
+			Scenario: "ilo4 url",
+			Address:  "ilo4://192.168.122.1",
+			Type:     "ilo4",
+			Port:     "",
+			Host:     "192.168.122.1",
+			Hostname: "192.168.122.1",
+			Path:     "",
+		},
+
+		{
+			Scenario: "ilo4 url, ipv6",
+			Address:  "ilo4://[fe80::fc33:62ff:fe83:8a76]",
+			Type:     "ilo4",
+			Port:     "",
+			Host:     "fe80::fc33:62ff:fe83:8a76",
+			Hostname: "[fe80::fc33:62ff:fe83:8a76]",
+			Path:     "",
+		},
+
+		{
+			Scenario: "ilo4 url, no sep",
+			Address:  "ilo4:192.168.122.1",
+			Type:     "ilo4",
+			Port:     "",
+			Host:     "192.168.122.1",
+			Hostname: "192.168.122.1",
+			Path:     "",
+		},
+
+		{
+			Scenario: "ilo5 url",
+			Address:  "ilo5://192.168.122.1",
+			Type:     "ilo5",
+			Port:     "",
+			Host:     "192.168.122.1",
+			Hostname: "192.168.122.1",
+			Path:     "",
+		},
+
+		{
+			Scenario: "ilo5 url, ipv6",
+			Address:  "ilo5://[fe80::fc33:62ff:fe83:8a76]",
+			Type:     "ilo5",
+			Port:     "",
+			Host:     "fe80::fc33:62ff:fe83:8a76",
+			Hostname: "[fe80::fc33:62ff:fe83:8a76]",
+			Path:     "",
+		},
+
+		{
+			Scenario: "ilo5 url, no sep",
+			Address:  "ilo5:192.168.122.1",
+			Type:     "ilo5",
+			Port:     "",
+			Host:     "192.168.122.1",
+			Hostname: "192.168.122.1",
+			Path:     "",
+		},
 	} {
 		t.Run(tc.Scenario, func(t *testing.T) {
 			url, err := getParsedURL(tc.Address)
@@ -548,6 +608,30 @@ func TestStaticDriverInfo(t *testing.T) {
 			raid:       "no-raid",
 			vendor:     "",
 		},
+
+		{
+			Scenario:   "ilo4",
+			input:      "ilo4://192.168.122.1",
+			needsMac:   true,
+			driver:     "ilo",
+			boot:       "ilo-ipxe",
+			management: "",
+			power:      "",
+			raid:       "",
+			vendor:     "",
+		},
+
+		{
+			Scenario:   "ilo5",
+			input:      "ilo5://192.168.122.1",
+			needsMac:   true,
+			driver:     "ilo5",
+			boot:       "ilo-ipxe",
+			management: "",
+			power:      "",
+			raid:       "ilo5",
+			vendor:     "",
+		},
 	} {
 		t.Run(tc.Scenario, func(t *testing.T) {
 			acc, err := NewAccessDetails(tc.input, false)
@@ -879,6 +963,98 @@ func TestDriverInfo(t *testing.T) {
 				"ibmc_verify_ca": false,
 			},
 		},
+
+		{
+			Scenario: "ilo4",
+			input:    "ilo4://192.168.122.1",
+			expects: map[string]interface{}{
+				"ilo_address":   "192.168.122.1",
+				"ilo_password":  "",
+				"ilo_username":  "",
+				"ilo_verify_ca": false,
+			},
+		},
+
+		{
+			Scenario: "ilo4 port",
+			input:    "ilo4://192.168.122.1:8080",
+			expects: map[string]interface{}{
+				"ilo_address":   "192.168.122.1",
+				"client_port":   "8080",
+				"ilo_password":  "",
+				"ilo_username":  "",
+				"ilo_verify_ca": false,
+			},
+		},
+
+		{
+			Scenario: "ilo4 ipv6",
+			input:    "ilo4://[fe80::fc33:62ff:fe83:8a76]",
+			expects: map[string]interface{}{
+				"ilo_address":   "fe80::fc33:62ff:fe83:8a76",
+				"ilo_password":  "",
+				"ilo_username":  "",
+				"ilo_verify_ca": false,
+			},
+		},
+
+		{
+			Scenario: "ilo4 ipv6 port",
+			input:    "ilo4://[fe80::fc33:62ff:fe83:8a76]:8080",
+			expects: map[string]interface{}{
+				"ilo_address":   "fe80::fc33:62ff:fe83:8a76",
+				"client_port":   "8080",
+				"ilo_password":  "",
+				"ilo_username":  "",
+				"ilo_verify_ca": false,
+			},
+		},
+
+		{
+			Scenario: "ilo5",
+			input:    "ilo5://192.168.122.1",
+			expects: map[string]interface{}{
+				"ilo_address":   "192.168.122.1",
+				"ilo_password":  "",
+				"ilo_username":  "",
+				"ilo_verify_ca": false,
+			},
+		},
+
+		{
+			Scenario: "ilo5 port",
+			input:    "ilo5://192.168.122.1:8080",
+			expects: map[string]interface{}{
+				"ilo_address":   "192.168.122.1",
+				"client_port":   "8080",
+				"ilo_password":  "",
+				"ilo_username":  "",
+				"ilo_verify_ca": false,
+			},
+		},
+
+		{
+			Scenario: "ilo5 ipv6",
+			input:    "ilo5://[fe80::fc33:62ff:fe83:8a76]",
+			expects: map[string]interface{}{
+				"ilo_address":   "fe80::fc33:62ff:fe83:8a76",
+				"ilo_password":  "",
+				"ilo_username":  "",
+				"ilo_verify_ca": false,
+			},
+		},
+
+		{
+			Scenario: "ilo5 ipv6 port",
+			input:    "ilo5://[fe80::fc33:62ff:fe83:8a76]:8080",
+			expects: map[string]interface{}{
+				"ilo_address":   "fe80::fc33:62ff:fe83:8a76",
+				"client_port":   "8080",
+				"ilo_password":  "",
+				"ilo_username":  "",
+				"ilo_verify_ca": false,
+			},
+		},
 	} {
 		t.Run(tc.Scenario, func(t *testing.T) {
 			acc, err := NewAccessDetails(tc.input, true)