Repository: metal3-io/baremetal-operator
Pull request #1039 (2021-11-24 03:02:21)

make-bm-worker: add some options and fix some issues
* Improve the options of make-bm-worke so that it can generate more detailed yaml
* fix the position of disableCertificateVerification
* add some UT
* add the effective value of boot-mode argument
* fix consumer-namespace in template

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 assign **stbenjam** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @stbenjam` 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":["stbenjam"]} -->
metal3-io-bot: Hi @zhouhao3. 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>

hardys: /ok-to-test
/retitle Add some options to make-bm-worker
hardys: > Regular contributors should join the org to skip this step.

Hi @zhouhao3 - thanks for your PR, I can see you've been regularly contributing so I sent an invite to join the org, this means your PRs will not require the manual ok-to-test step in future :)
zhouhao3: /test-v1b1-centos-integration
/test-v1b1-integration 
zhouhao3: @fmuyassarov @andfasano @s3rj1k @dtantsur PTAL
andfasano: /lgtm
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 assign **zaneb** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @zaneb` 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":["zaneb"]} -->
metal3-io-bot: New changes are detected. LGTM label has been removed.
zhouhao3: /test-v1b1-integration
/test-v1b1-centos-integration
andfasano: /lgtm
metal3-io-bot: New changes are detected. LGTM label has been removed.
zhouhao3: /test-v1b1-integration
/test-v1b1-centos-integration
fmuyassarov: @zhouhao3 this looks good to me. Can we squash the commits before merging ?

Patch:
diff --git a/cmd/make-bm-worker/main.go b/cmd/make-bm-worker/main.go
index 3974a29e..6eb9b34a 100644
--- a/cmd/make-bm-worker/main.go
+++ b/cmd/make-bm-worker/main.go
@@ -16,12 +16,20 @@ func main() {
 	var disableCertificateVerification = flag.Bool("disableCertificateVerification", false, "will skip certificate validation when true")
 	var hardwareProfile = flag.String("hardwareprofile", "", "hardwareProfile to be used")
 	var macAddress = flag.String("boot-mac", "", "boot-mac for bootMACAddress")
-	var bootMode = flag.String("boot-mode", "", "boot-mode for host (UEFI or legacy)")
+	var bootMode = flag.String("boot-mode", "", "boot-mode for host (UEFI, UEFISecureBoot or legacy)")
 	var verbose = flag.Bool("v", false, "turn on verbose output")
 	var consumer = flag.String(
 		"consumer", "", "specify name of a related, existing, consumer to link")
 	var consumerNamespace = flag.String(
 		"consumer-namespace", "", "specify namespace of a related, existing, consumer to link")
+	var automatedCleaningMode = flag.String(
+		"automatedCleaningMode", "", "automatic cleaning mode for host (metadata or disabled)")
+	var imageURL = flag.String("image-url", "", "url for the image")
+	var imageChecksum = flag.String("image-checksum", "", "checksum for the image")
+	var imageChecksumType = flag.String(
+		"image-checksum-type", "", "checksum algorithm for the image (md5, sha256 or sha512)")
+	var imageFormat = flag.String(
+		"image-format", "", "format of the image (raw, qcow2, vdi, vmdk, or live-iso)")
 
 	flag.Parse()
 
@@ -43,8 +51,27 @@ func main() {
 		os.Exit(1)
 	}
 
-	if *bootMode != "" && *bootMode != "UEFI" && *bootMode != "legacy" {
-		fmt.Fprintf(os.Stderr, "Invalid boot mode %q, use \"UEFI\" or \"legacy\"\n", *bootMode)
+	if *bootMode != "" && *bootMode != "UEFI" && *bootMode != "UEFISecureBoot" && *bootMode != "legacy" {
+		fmt.Fprintf(os.Stderr, "Invalid boot mode %q, use \"UEFI\", \"UEFISecureBoot\" or \"legacy\"\n", *bootMode)
+		os.Exit(1)
+	}
+
+	if *automatedCleaningMode != "" && *automatedCleaningMode != "metadata" && *automatedCleaningMode != "disabled" {
+		fmt.Fprintf(os.Stderr, "Invalid automatic cleaning mode %q, use \"metadata\" or \"disabled\"\n", *automatedCleaningMode)
+		os.Exit(1)
+	}
+
+	switch *imageChecksumType {
+	case "", "md5", "sha256", "sha512":
+	default:
+		fmt.Fprintf(os.Stderr, "Invalid image checksum type %q, use \"md5\", \"sha256\" or \"sha512\"\n", *imageChecksumType)
+		os.Exit(1)
+	}
+
+	switch *imageFormat {
+	case "", "raw", "qcow2", "vdi", "vmdk", "live-iso":
+	default:
+		fmt.Fprintf(os.Stderr, "Invalid image format %q, use \"raw\", \"qcow2\", \"vdi\", \"vmdk\" or \"live-iso\"\n", *imageFormat)
 		os.Exit(1)
 	}
 
@@ -58,6 +85,11 @@ func main() {
 		BootMacAddress:                 *macAddress,
 		Consumer:                       strings.TrimSpace(*consumer),
 		ConsumerNamespace:              strings.TrimSpace(*consumerNamespace),
+		AutomatedCleaningMode:          *automatedCleaningMode,
+		ImageURL:                       *imageURL,
+		ImageChecksum:                  *imageChecksum,
+		ImageChecksumType:              *imageChecksumType,
+		ImageFormat:                    *imageFormat,
 	}
 	if bootMode != nil {
 		template.BootMode = *bootMode
diff --git a/cmd/make-bm-worker/templates/templates.go b/cmd/make-bm-worker/templates/templates.go
index 90d7bc83..c4bc6832 100644
--- a/cmd/make-bm-worker/templates/templates.go
+++ b/cmd/make-bm-worker/templates/templates.go
@@ -22,6 +22,9 @@ kind: BareMetalHost
 metadata:
   name: {{ .Name }}
 spec:
+{{- if .AutomatedCleaningMode }}
+  automatedCleaningMode: {{ .AutomatedCleaningMode }}
+{{- end }}
   online: true
 {{- if .HardwareProfile }}
   hardwareProfile: {{ .HardwareProfile }}
@@ -35,13 +38,28 @@ spec:
   bmc:
     address: {{ .BMCAddress }}
     credentialsName: {{ .Name }}-bmc-secret
+{{- if .DisableCertificateVerification }}
+    disableCertificateVerification: true
+{{- end}}
 {{- if .Consumer }}
   consumerRef:
     name: {{ .Consumer }}
+{{- if .ConsumerNamespace }}
     namespace: {{ .ConsumerNamespace }}
 {{- end }}
-{{- if .DisableCertificateVerification }}
-  disableCertificateVerification: true
+{{- end }}
+{{- if .ImageURL }}
+  image:
+{{- if .ImageChecksum }}
+    checksum: {{ .ImageChecksum}}
+{{- end}}
+{{- if .ImageChecksumType }}
+    checksumType: {{ .ImageChecksumType}}
+{{- end}}
+{{- if .ImageFormat }}
+    format: {{ .ImageFormat}}
+{{- end}}
+    url: {{ .ImageURL}}
 {{- end}}
 `
 
@@ -57,6 +75,11 @@ type Template struct {
 	BootMode                       string
 	Consumer                       string
 	ConsumerNamespace              string
+	AutomatedCleaningMode          string
+	ImageURL                       string
+	ImageChecksum                  string
+	ImageChecksumType              string
+	ImageFormat                    string
 }
 
 // EncodedUsername returns the username in the format needed to store


Test patch:
diff --git a/cmd/make-bm-worker/templates/templates_test.go b/cmd/make-bm-worker/templates/templates_test.go
index 92a6aa32..1546d646 100644
--- a/cmd/make-bm-worker/templates/templates_test.go
+++ b/cmd/make-bm-worker/templates/templates_test.go
@@ -134,3 +134,190 @@ spec:
 		t.Fail()
 	}
 }
+
+func TestWithAutomatedCleaningMode(t *testing.T) {
+	template := Template{
+		Name:                  "hostname",
+		BMCAddress:            "bmcAddress",
+		Username:              "username",
+		Password:              "password",
+		AutomatedCleaningMode: "metadata",
+	}
+	actual, _ := template.Render()
+	expected := `---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: hostname-bmc-secret
+type: Opaque
+data:
+  username: dXNlcm5hbWU=
+  password: cGFzc3dvcmQ=
+
+---
+apiVersion: metal3.io/v1alpha1
+kind: BareMetalHost
+metadata:
+  name: hostname
+spec:
+  automatedCleaningMode: metadata
+  online: true
+  bmc:
+    address: bmcAddress
+    credentialsName: hostname-bmc-secret
+`
+	if !compareStrings(t, expected, actual) {
+		t.Fail()
+	}
+}
+
+func TestWithImage(t *testing.T) {
+	template := Template{
+		Name:              "hostname",
+		BMCAddress:        "bmcAddress",
+		Username:          "username",
+		Password:          "password",
+		ImageURL:          "imageURL",
+		ImageChecksum:     "imageChecksum",
+		ImageChecksumType: "md5",
+		ImageFormat:       "raw",
+	}
+	actual, _ := template.Render()
+	expected := `---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: hostname-bmc-secret
+type: Opaque
+data:
+  username: dXNlcm5hbWU=
+  password: cGFzc3dvcmQ=
+
+---
+apiVersion: metal3.io/v1alpha1
+kind: BareMetalHost
+metadata:
+  name: hostname
+spec:
+  online: true
+  bmc:
+    address: bmcAddress
+    credentialsName: hostname-bmc-secret
+  image:
+    checksum: imageChecksum
+    checksumType: md5
+    format: raw
+    url: imageURL
+`
+	if !compareStrings(t, expected, actual) {
+		t.Fail()
+	}
+}
+
+func TestWithDisableCertificateVerification(t *testing.T) {
+	template := Template{
+		Name:                           "hostname",
+		BMCAddress:                     "bmcAddress",
+		Username:                       "username",
+		Password:                       "password",
+		DisableCertificateVerification: true,
+	}
+	actual, _ := template.Render()
+	expected := `---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: hostname-bmc-secret
+type: Opaque
+data:
+  username: dXNlcm5hbWU=
+  password: cGFzc3dvcmQ=
+
+---
+apiVersion: metal3.io/v1alpha1
+kind: BareMetalHost
+metadata:
+  name: hostname
+spec:
+  online: true
+  bmc:
+    address: bmcAddress
+    credentialsName: hostname-bmc-secret
+    disableCertificateVerification: true
+`
+	if !compareStrings(t, expected, actual) {
+		t.Fail()
+	}
+}
+
+func TestWithBootMacAddress(t *testing.T) {
+	template := Template{
+		Name:           "hostname",
+		BMCAddress:     "bmcAddress",
+		Username:       "username",
+		Password:       "password",
+		BootMacAddress: "boot-mac",
+	}
+	actual, _ := template.Render()
+	expected := `---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: hostname-bmc-secret
+type: Opaque
+data:
+  username: dXNlcm5hbWU=
+  password: cGFzc3dvcmQ=
+
+---
+apiVersion: metal3.io/v1alpha1
+kind: BareMetalHost
+metadata:
+  name: hostname
+spec:
+  online: true
+  bootMACAddress: boot-mac
+  bmc:
+    address: bmcAddress
+    credentialsName: hostname-bmc-secret
+`
+	if !compareStrings(t, expected, actual) {
+		t.Fail()
+	}
+}
+
+func TestWithBootMode(t *testing.T) {
+	template := Template{
+		Name:       "hostname",
+		BMCAddress: "bmcAddress",
+		Username:   "username",
+		Password:   "password",
+		BootMode:   "UEFI",
+	}
+	actual, _ := template.Render()
+	expected := `---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: hostname-bmc-secret
+type: Opaque
+data:
+  username: dXNlcm5hbWU=
+  password: cGFzc3dvcmQ=
+
+---
+apiVersion: metal3.io/v1alpha1
+kind: BareMetalHost
+metadata:
+  name: hostname
+spec:
+  online: true
+  bootMode: UEFI
+  bmc:
+    address: bmcAddress
+    credentialsName: hostname-bmc-secret
+`
+	if !compareStrings(t, expected, actual) {
+		t.Fail()
+	}
+}