Repository: metal3-io/baremetal-operator
Pull request #522 (2020-05-13 21:52:45)

Arranging BMO & Ironic deployments with separate folders 
**What this PR does / why we need it:**
This PR breaks [metal3-io/baremetal-operator#481](https://github.com/metal3-io/baremetal-operator/pull/481) into two parts and this is a **first** part which introduces what is needed to be added in that PR. It structures `deploy` folder as simple as possible (only for BMO) and fits for operator-sdk while all ironic part is moved to its own folder.

Related to #540 
Follows the proposal in [metal3-docs/pull/91](https://github.com/metal3-io/metal3-docs/pull/91)
Documented in #541 


Patch:
diff --git a/deploy/crds/metal3.io_baremetalhosts_crd.yaml b/deploy/crds/metal3.io_baremetalhosts_crd.yaml
index 8f1ba483..adef06cc 100644
--- a/deploy/crds/metal3.io_baremetalhosts_crd.yaml
+++ b/deploy/crds/metal3.io_baremetalhosts_crd.yaml
@@ -203,51 +203,6 @@ spec:
             online:
               description: Should the server be online?
               type: boolean
-            rootDeviceHints:
-              description: Provide guidance about how to choose the device for the
-                image being provisioned.
-              properties:
-                deviceName:
-                  description: A Linux device name like "/dev/vda". The hint must
-                    match the actual value exactly.
-                  type: string
-                hctl:
-                  description: A SCSI bus address like 0:0:0:0. The hint must match
-                    the actual value exactly.
-                  type: string
-                minSizeGigabytes:
-                  description: The minimum size of the device in Gigabytes.
-                  minimum: 0
-                  type: integer
-                model:
-                  description: A vendor-specific device identifier. The hint can be
-                    a substring of the actual value.
-                  type: string
-                rotational:
-                  description: True if the device should use spinning media, false
-                    otherwise.
-                  type: boolean
-                serialNumber:
-                  description: Device serial number. The hint must match the actual
-                    value exactly.
-                  type: string
-                vendor:
-                  description: The name of the vendor or manufacturer of the device.
-                    The hint can be a substring of the actual value.
-                  type: string
-                wwn:
-                  description: Unique storage identifier. The hint must match the
-                    actual value exactly.
-                  type: string
-                wwnVendorExtension:
-                  description: Unique vendor storage identifier. The hint must match
-                    the actual value exactly.
-                  type: string
-                wwnWithExtension:
-                  description: Unique storage identifier with the vendor extension
-                    appended. The hint must match the actual value exactly.
-                  type: string
-              type: object
             taints:
               description: Taints is the full, authoritative list of taints to apply
                 to the corresponding Machine. This list will overwrite any modifications
@@ -617,50 +572,6 @@ spec:
                   - checksum
                   - url
                   type: object
-                rootDeviceHints:
-                  description: The RootDevicehints set by the user
-                  properties:
-                    deviceName:
-                      description: A Linux device name like "/dev/vda". The hint must
-                        match the actual value exactly.
-                      type: string
-                    hctl:
-                      description: A SCSI bus address like 0:0:0:0. The hint must
-                        match the actual value exactly.
-                      type: string
-                    minSizeGigabytes:
-                      description: The minimum size of the device in Gigabytes.
-                      minimum: 0
-                      type: integer
-                    model:
-                      description: A vendor-specific device identifier. The hint can
-                        be a substring of the actual value.
-                      type: string
-                    rotational:
-                      description: True if the device should use spinning media, false
-                        otherwise.
-                      type: boolean
-                    serialNumber:
-                      description: Device serial number. The hint must match the actual
-                        value exactly.
-                      type: string
-                    vendor:
-                      description: The name of the vendor or manufacturer of the device.
-                        The hint can be a substring of the actual value.
-                      type: string
-                    wwn:
-                      description: Unique storage identifier. The hint must match
-                        the actual value exactly.
-                      type: string
-                    wwnVendorExtension:
-                      description: Unique vendor storage identifier. The hint must
-                        match the actual value exactly.
-                      type: string
-                    wwnWithExtension:
-                      description: Unique storage identifier with the vendor extension
-                        appended. The hint must match the actual value exactly.
-                      type: string
-                  type: object
                 state:
                   description: An indiciator for what the provisioner is doing with
                     the host.
diff --git a/deploy/default/kustomization.yaml b/deploy/default/kustomization.yaml
index b1248090..bb463f36 100644
--- a/deploy/default/kustomization.yaml
+++ b/deploy/default/kustomization.yaml
@@ -5,13 +5,9 @@ resources:
 - ../namespace
 - ../rbac
 - ../crds
-- ../operator/ironic
+- ../operator
+- ../../ironic-deployment/default
 configMapGenerator:
 - envs:
   - ironic_bmo_configmap.env
-  name: ironic-bmo-configmap
-secretGenerator:
-- literals:
-  - password=changeme
-  name: mariadb-password
-  type: Opaque
+  name: ironic-bmo-configmap
\ No newline at end of file
diff --git a/deploy/operator/bmo.yaml b/deploy/operator/bmo.yaml
new file mode 100644
index 00000000..e96c4a1c
--- /dev/null
+++ b/deploy/operator/bmo.yaml
@@ -0,0 +1,32 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: metal3-baremetal-operator
+spec:
+  replicas: 1
+  selector:
+    matchLabels:
+      name: metal3-baremetal-operator
+  template:
+    metadata:
+      labels:
+        name: metal3-baremetal-operator
+    spec:
+      serviceAccountName: metal3-baremetal-operator
+      containers:
+        - name: baremetal-operator
+          image: quay.io/metal3-io/baremetal-operator
+          ports:
+          - containerPort: 60000
+            name: metrics
+          command:
+          - /baremetal-operator
+          imagePullPolicy: Always
+          env:
+            - name: POD_NAME
+              valueFrom:
+                fieldRef:
+                  fieldPath: metadata.name
+          envFrom:
+            - configMapRef:
+                name: ironic-bmo-configmap
\ No newline at end of file
diff --git a/deploy/operator/kustomization.yaml b/deploy/operator/kustomization.yaml
new file mode 100644
index 00000000..d7bb8f78
--- /dev/null
+++ b/deploy/operator/kustomization.yaml
@@ -0,0 +1,4 @@
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+resources:
+- bmo.yaml
\ No newline at end of file
diff --git a/docs/api.md b/docs/api.md
index 64e1ba03..c6663995 100644
--- a/docs/api.md
+++ b/docs/api.md
@@ -90,50 +90,6 @@ mainly, but not only, provisioning details.
 
 * *description* -- A human-provided string to help identify the host.
 
-* *hardwareProfile* -- **This field is deprecated. See rootDeviceHints instead.**
-  The name of the hardware profile to use. The following are the current
-  supported `hardwareProfile` settings and their corresponding root devices.
-
-  | **hardwareProfile** | **Root Device** |
-  |---------------------|-----------------|
-  | `unknown`           | /dev/sda        |
-  | `libvirt`           | /dev/vda        |
-  | `dell`              | HCTL: 0:0:0:0   |
-  | `dell-raid`         | HCTL: 0:2:0:0   |
-  | `openstack`         | /dev/vdb        |
-
-  **NOTE:** These are subject to change.
-
-* *rootDeviceHints* -- Guidance for how to choose the device to
-  receive the image being provisioned. The storage devices are
-  examined in the order they are discovered during inspection and the
-  hint values are compared to the inspected values. The first
-  discovered device that matches is used. Hints can be combined, and
-  if multiple hints are provided then a device must match all hints in
-  order to be selected.
-  * *deviceName* -- A string containing a Linux device name like
-    `/dev/vda`. The hint must match the actual value exactly.
-  * *hctl* -- A string containing a SCSI bus address like
-    `0:0:0:0`. The hint must match the actual value exactly.
-  * *model* -- A string containing a vendor-specific device
-    identifier. The hint can be a substring of the actual value.
-  * *vendor* -- A string containing the name of the vendor or
-    manufacturer of the device. The hint can be a substring of the
-    actual value.
-  * *serialNumber* -- A string contianing the device serial
-    number. The hint must match the actual value exactly.
-  * *minSizeGigabytes* -- An integer representing the minimum size of the
-    device in Gigabytes.
-  * *wwn* -- A string containing the unique storage identifier. The
-    hint must match the actual value exactly.
-  * *wwnWithExtension* -- A string containing the unique storage
-    identifier with the vendor extension appended. The hint must match
-    the actual value exactly.
-  * *wwnVendorExtension* -- A string containing the unique vendor
-    storage indentifier. The hint must match the actual value exactly.
-  * *rotational* -- A boolean indicating whether the device should be
-    a rotating disk (`true`) or not (`false`).
-
 ### BareMetalHost status
 
 Moving onto the next block, the *BareMetalHost's* *status* which represents
@@ -197,10 +153,8 @@ details, etc.
   * *systemVendor* -- Contains information about the host's *manufacturer*,
     the *productName* and *serialNumber*.
   * *ramMebibytes* -- The host's amount of memory in Mebibytes.
-
-* *hardwareProfile* -- **This field is deprecated. See rootDeviceHints instead.**
-  The name of the hardware profile that matches the
-  hardware discovered on the host based on the details saved
+* *hardwareProfile* -- The name of the hardware profile that matches the
+  hardware discovered on the host. These details are saved
   to the *Hardware* section. If the hardware does not match any
   known profile, the value `unknown` will be set on this field
   and is used by default. In practice, this only affects which device
@@ -219,7 +173,6 @@ details, etc.
 
 * *poweredOn* -- Boolean indicating whether the host is powered on.
   See *online* on the *BareMetalHost's* *Spec*.
-
 * *provisioning* -- Settings related to deploying an image to the host.
   * *state* -- The current state of any ongoing provisioning operation.
     The following are the currently supported ones:
@@ -240,11 +193,6 @@ details, etc.
       by an agent.
     * *power management error* -- An error was found while trying to power
       the host either on or off.
-  * *id* -- The unique identifier for the service in the underlying
-    provisioning tool.
-  * *image* -- The image most recently provisioned to the host.
-  * *rootDeviceHints* -- The root device selection instructions used
-    for the most recent provisioning operation.
 
 ### BareMetalHost Example
 
diff --git a/go.mod b/go.mod
index d404ca1a..f5dea03e 100644
--- a/go.mod
+++ b/go.mod
@@ -16,7 +16,6 @@ require (
 	github.com/prometheus/client_golang v1.5.1
 	github.com/securego/gosec v0.0.0-20200401082031-e946c8c39989 // indirect
 	github.com/spf13/cobra v0.0.6 // indirect
-	github.com/stretchr/testify v1.4.0
 	golang.org/x/crypto v0.0.0-20200302210943-78000ba7a073 // indirect
 	golang.org/x/lint v0.0.0-20200302205851-738671d3881b // indirect
 	golang.org/x/net v0.0.0-20200301022130-244492dfa37a // indirect
@@ -29,6 +28,7 @@ require (
 	k8s.io/utils v0.0.0-20200229041039-0a110f9eb7ab
 	sigs.k8s.io/controller-runtime v0.5.2
 	sigs.k8s.io/yaml v1.2.0
+
 )
 
 replace (
diff --git a/ironic-deployment/default/ironic_bmo_configmap.env b/ironic-deployment/default/ironic_bmo_configmap.env
new file mode 100644
index 00000000..938b3ee3
--- /dev/null
+++ b/ironic-deployment/default/ironic_bmo_configmap.env
@@ -0,0 +1,9 @@
+HTTP_PORT=6180
+PROVISIONING_INTERFACE=eth2
+DHCP_RANGE=172.22.0.10,172.22.0.100
+DEPLOY_KERNEL_URL=http://172.22.0.2:6180/images/ironic-python-agent.kernel
+DEPLOY_RAMDISK_URL=http://172.22.0.2:6180/images/ironic-python-agent.initramfs
+IRONIC_ENDPOINT=http://172.22.0.2:6385/v1/
+IRONIC_INSPECTOR_ENDPOINT=http://172.22.0.2:5050/v1/
+CACHEURL=http://172.22.0.1/images
+IRONIC_FAST_TRACK=false
\ No newline at end of file
diff --git a/ironic-deployment/default/kustomization.yaml b/ironic-deployment/default/kustomization.yaml
new file mode 100644
index 00000000..fb7c97cb
--- /dev/null
+++ b/ironic-deployment/default/kustomization.yaml
@@ -0,0 +1,14 @@
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+namespace: metal3
+resources:
+- ../ironic
+configMapGenerator:
+- envs:
+  - ironic_bmo_configmap.env
+  name: ironic-bmo-configmap
+secretGenerator:
+- literals:
+  - password=changeme
+  name: mariadb-password
+  type: Opaque
\ No newline at end of file
diff --git a/ironic-deployment/ironic/ironic.yaml b/ironic-deployment/ironic/ironic.yaml
new file mode 100644
index 00000000..037f8190
--- /dev/null
+++ b/ironic-deployment/ironic/ironic.yaml
@@ -0,0 +1,96 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: metal3-ironic
+spec:
+  replicas: 1
+  selector:
+    matchLabels:
+      name: metal3-ironic
+  template:
+    metadata:
+      labels:
+        name: metal3-ironic
+    spec:
+      hostNetwork: true
+      containers:
+        - name: ironic-dnsmasq
+          image: quay.io/metal3-io/ironic
+          imagePullPolicy: Always
+          securityContext:
+             capabilities:
+               add: ["NET_ADMIN"]
+          command:
+            - /bin/rundnsmasq
+          volumeMounts:
+            - mountPath: /shared
+              name: ironic-data-volume
+          envFrom:
+            - configMapRef:
+                name: ironic-bmo-configmap
+        - name: mariadb
+          image: quay.io/metal3-io/ironic
+          imagePullPolicy: Always
+          command:
+            - /bin/runmariadb
+          volumeMounts:
+            - mountPath: /shared
+              name: ironic-data-volume
+          env:
+            - name: MARIADB_PASSWORD
+              valueFrom:
+                secretKeyRef:
+                  name: mariadb-password
+                  key: password
+        - name: ironic-httpd
+          image: quay.io/metal3-io/ironic
+          imagePullPolicy: Always
+          securityContext:
+             capabilities:
+               add: ["NET_ADMIN"]
+          command:
+            - /bin/runhttpd
+          volumeMounts:
+            - mountPath: /shared
+              name: ironic-data-volume
+          envFrom:
+            - configMapRef:
+                name: ironic-bmo-configmap
+        - name: ironic
+          image: quay.io/metal3-io/ironic
+          imagePullPolicy: Always
+          command:
+            - /bin/runironic
+          volumeMounts:
+            - mountPath: /shared
+              name: ironic-data-volume
+          envFrom:
+            - configMapRef:
+                name: ironic-bmo-configmap
+          env:
+            - name: MARIADB_PASSWORD
+              valueFrom:
+                secretKeyRef:
+                  name: mariadb-password
+                  key: password
+        - name: ironic-inspector
+          image: quay.io/metal3-io/ironic-inspector
+          imagePullPolicy: Always
+          envFrom:
+            - configMapRef:
+                name: ironic-bmo-configmap
+      initContainers:
+        - name: ironic-ipa-downloader
+          image: quay.io/metal3-io/ironic-ipa-downloader
+          imagePullPolicy: Always
+          command:
+            - /usr/local/bin/get-resource.sh
+          envFrom:
+            - configMapRef:
+                name: ironic-bmo-configmap
+          volumeMounts:
+            - mountPath: /shared
+              name: ironic-data-volume
+      volumes:
+        - name: ironic-data-volume
+          emptyDir: {}
\ No newline at end of file
diff --git a/ironic-deployment/ironic/kustomization.yaml b/ironic-deployment/ironic/kustomization.yaml
new file mode 100644
index 00000000..01aaa615
--- /dev/null
+++ b/ironic-deployment/ironic/kustomization.yaml
@@ -0,0 +1,4 @@
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+resources:
+- ironic.yaml
\ No newline at end of file
diff --git a/ironic-deployment/keepalived/ironic_bmo_configmap.env b/ironic-deployment/keepalived/ironic_bmo_configmap.env
new file mode 100644
index 00000000..2ca15298
--- /dev/null
+++ b/ironic-deployment/keepalived/ironic_bmo_configmap.env
@@ -0,0 +1,11 @@
+HTTP_PORT=6180
+PROVISIONING_IP=172.22.0.2
+PROVISIONING_CIDR=24
+PROVISIONING_INTERFACE=ironicendpoint
+DHCP_RANGE=172.22.0.10,172.22.0.100
+DEPLOY_KERNEL_URL=http://172.22.0.2:6180/images/ironic-python-agent.kernel
+DEPLOY_RAMDISK_URL=http://172.22.0.2:6180/images/ironic-python-agent.initramfs
+IRONIC_ENDPOINT=http://172.22.0.2:6385/v1/
+IRONIC_INSPECTOR_ENDPOINT=http://172.22.0.2:5050/v1/
+CACHEURL=http://172.22.0.1/images
+IRONIC_FAST_TRACK=false
\ No newline at end of file
diff --git a/ironic-deployment/keepalived/keepalived_patch.yaml b/ironic-deployment/keepalived/keepalived_patch.yaml
new file mode 100644
index 00000000..e6a2e244
--- /dev/null
+++ b/ironic-deployment/keepalived/keepalived_patch.yaml
@@ -0,0 +1,17 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: metal3-ironic
+spec:
+  template:
+    spec:
+      containers:
+      # Change the value of image field below to your controller image URL
+      - image: quay.io/metal3-io/keepalived
+        name: ironic-endpoint-keepalived
+        securityContext:
+              capabilities:
+                add: ["NET_ADMIN"]
+        envFrom:
+          - configMapRef:
+              name: ironic-bmo-configmap
\ No newline at end of file
diff --git a/ironic-deployment/keepalived/kustomization.yaml b/ironic-deployment/keepalived/kustomization.yaml
new file mode 100644
index 00000000..e0df059c
--- /dev/null
+++ b/ironic-deployment/keepalived/kustomization.yaml
@@ -0,0 +1,17 @@
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+namespace: metal3
+resources:
+- ../../deploy/namespace
+- ../ironic
+configMapGenerator:
+- envs:
+  - ironic_bmo_configmap.env
+  name: ironic-bmo-configmap
+secretGenerator:
+- literals:
+  - password=changeme
+  name: mariadb-password
+  type: Opaque
+patchesStrategicMerge:
+- keepalived_patch.yaml
\ No newline at end of file
diff --git a/pkg/apis/metal3/v1alpha1/baremetalhost_types.go b/pkg/apis/metal3/v1alpha1/baremetalhost_types.go
index 945b7c8d..a4ffc717 100644
--- a/pkg/apis/metal3/v1alpha1/baremetalhost_types.go
+++ b/pkg/apis/metal3/v1alpha1/baremetalhost_types.go
@@ -30,49 +30,6 @@ const (
 	StatusAnnotation = "baremetalhost.metal3.io/status"
 )
 
-// RootDeviceHints holds the hints for specifying the storage location
-// for the root filesystem for the image.
-type RootDeviceHints struct {
-	// A Linux device name like "/dev/vda". The hint must match the
-	// actual value exactly.
-	DeviceName string `json:"deviceName,omitempty"`
-
-	// A SCSI bus address like 0:0:0:0. The hint must match the actual
-	// value exactly.
-	HCTL string `json:"hctl,omitempty"`
-
-	// A vendor-specific device identifier. The hint can be a
-	// substring of the actual value.
-	Model string `json:"model,omitempty"`
-
-	// The name of the vendor or manufacturer of the device. The hint
-	// can be a substring of the actual value.
-	Vendor string `json:"vendor,omitempty"`
-
-	// Device serial number. The hint must match the actual value
-	// exactly.
-	SerialNumber string `json:"serialNumber,omitempty"`
-
-	// The minimum size of the device in Gigabytes.
-	// +kubebuilder:validation:Minimum=0
-	MinSizeGigabytes int `json:"minSizeGigabytes,omitempty"`
-
-	// Unique storage identifier. The hint must match the actual value
-	// exactly.
-	WWN string `json:"wwn,omitempty"`
-
-	// Unique storage identifier with the vendor extension
-	// appended. The hint must match the actual value exactly.
-	WWNWithExtension string `json:"wwnWithExtension,omitempty"`
-
-	// Unique vendor storage identifier. The hint must match the
-	// actual value exactly.
-	WWNVendorExtension string `json:"wwnVendorExtension,omitempty"`
-
-	// True if the device should use spinning media, false otherwise.
-	Rotational *bool `json:"rotational,omitempty"`
-}
-
 // OperationalStatus represents the state of the host
 type OperationalStatus string
 
@@ -208,10 +165,6 @@ type BareMetalHostSpec struct {
 	// automatically determine the profile.
 	HardwareProfile string `json:"hardwareProfile,omitempty"`
 
-	// Provide guidance about how to choose the device for the image
-	// being provisioned.
-	RootDeviceHints *RootDeviceHints `json:"rootDeviceHints,omitempty"`
-
 	// Which MAC address will PXE boot? This is optional for some
 	// types, but required for libvirt VMs driven by vbmc.
 	// +kubebuilder:validation:Pattern=`[0-9a-fA-F]{2}(:[0-9a-fA-F]{2}){5}`
@@ -538,9 +491,6 @@ type ProvisionStatus struct {
 	// Image holds the details of the last image successfully
 	// provisioned to the host.
 	Image Image `json:"image,omitempty"`
-
-	// The RootDevicehints set by the user
-	RootDeviceHints *RootDeviceHints `json:"rootDeviceHints,omitempty"`
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
diff --git a/pkg/apis/metal3/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/metal3/v1alpha1/zz_generated.deepcopy.go
index 4c57aff7..dcd9c3ab 100644
--- a/pkg/apis/metal3/v1alpha1/zz_generated.deepcopy.go
+++ b/pkg/apis/metal3/v1alpha1/zz_generated.deepcopy.go
@@ -113,11 +113,6 @@ func (in *BareMetalHostSpec) DeepCopyInto(out *BareMetalHostSpec) {
 		}
 	}
 	out.BMC = in.BMC
-	if in.RootDeviceHints != nil {
-		in, out := &in.RootDeviceHints, &out.RootDeviceHints
-		*out = new(RootDeviceHints)
-		(*in).DeepCopyInto(*out)
-	}
 	if in.ConsumerRef != nil {
 		in, out := &in.ConsumerRef, &out.ConsumerRef
 		*out = new(v1.ObjectReference)
@@ -375,11 +370,6 @@ func (in *OperationMetric) DeepCopy() *OperationMetric {
 func (in *ProvisionStatus) DeepCopyInto(out *ProvisionStatus) {
 	*out = *in
 	in.Image.DeepCopyInto(&out.Image)
-	if in.RootDeviceHints != nil {
-		in, out := &in.RootDeviceHints, &out.RootDeviceHints
-		*out = new(RootDeviceHints)
-		(*in).DeepCopyInto(*out)
-	}
 	return
 }
 
@@ -393,27 +383,6 @@ func (in *ProvisionStatus) DeepCopy() *ProvisionStatus {
 	return out
 }
 
-// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
-func (in *RootDeviceHints) DeepCopyInto(out *RootDeviceHints) {
-	*out = *in
-	if in.Rotational != nil {
-		in, out := &in.Rotational, &out.Rotational
-		*out = new(bool)
-		**out = **in
-	}
-	return
-}
-
-// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RootDeviceHints.
-func (in *RootDeviceHints) DeepCopy() *RootDeviceHints {
-	if in == nil {
-		return nil
-	}
-	out := new(RootDeviceHints)
-	in.DeepCopyInto(out)
-	return out
-}
-
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *Storage) DeepCopyInto(out *Storage) {
 	*out = *in
diff --git a/pkg/controller/baremetalhost/baremetalhost_controller.go b/pkg/controller/baremetalhost/baremetalhost_controller.go
index 382bdae9..15aa32ef 100644
--- a/pkg/controller/baremetalhost/baremetalhost_controller.go
+++ b/pkg/controller/baremetalhost/baremetalhost_controller.go
@@ -566,7 +566,6 @@ func (r *ReconcileBareMetalHost) actionMatchProfile(prov provisioner.Provisioner
 		info.log.Info("updating hardware profile", "profile", hardwareProfile)
 		info.publishEvent("ProfileSet", fmt.Sprintf("Hardware profile set: %s", hardwareProfile))
 	}
-
 	info.host.ClearError()
 	return actionComplete{}
 }
@@ -616,13 +615,6 @@ func (r *ReconcileBareMetalHost) actionProvisioning(prov provisioner.Provisioner
 	return actionComplete{}
 }
 
-// clearHostProvisioningSettings removes the values related to
-// provisioning that do not trigger re-provisioning from the status
-// fields of a host.
-func clearHostProvisioningSettings(host *metal3v1alpha1.BareMetalHost) {
-	host.Status.Provisioning.RootDeviceHints = nil
-}
-
 func (r *ReconcileBareMetalHost) actionDeprovisioning(prov provisioner.Provisioner, info *reconcileInfo) actionResult {
 	info.log.Info("deprovisioning")
 
@@ -647,10 +639,9 @@ func (r *ReconcileBareMetalHost) actionDeprovisioning(prov provisioner.Provision
 		return actionContinueNoWrite{}
 	}
 
-	// After the provisioner is done, clear the provisioning settings
-	// so we transition to the next state.
+	// After the provisioner is done, clear the image settings so we
+	// transition to the next state.
 	info.host.Status.Provisioning.Image = metal3v1alpha1.Image{}
-	clearHostProvisioningSettings(info.host)
 
 	return actionComplete{}
 }
@@ -787,44 +778,12 @@ func (r *ReconcileBareMetalHost) actionManageReady(prov provisioner.Provisioner,
 	}
 
 	if info.host.NeedsProvisioning() {
-		// Ensure the root device hints we're going to use are stored.
-		dirty, err := saveHostProvisioningSettings(info.host)
-		if err != nil {
-			return actionError{errors.Wrap(err, "Could not save the host provisioning settings")}
-		}
-		if dirty {
-			info.log.Info("updating host provisioning settings")
-		}
 		info.host.ClearError()
 		return actionComplete{}
 	}
 	return r.manageHostPower(prov, info)
 }
 
-// saveHostProvisioningSettings copies the values related to
-// provisioning that do not trigger re-provisioning into the status
-// fields of the host.
-func saveHostProvisioningSettings(host *metal3v1alpha1.BareMetalHost) (dirty bool, err error) {
-
-	// Ensure the root device hints we're going to use are stored.
-	//
-	// If the user has provided explicit root device hints, they take
-	// precedence. Otherwise use the values from the hardware profile.
-	hintSource := host.Spec.RootDeviceHints
-	if hintSource == nil {
-		hwProf, err := hardware.GetProfile(host.HardwareProfile())
-		if err != nil {
-			return false, errors.Wrap(err, "Could not update root device hints")
-		}
-		hintSource = &hwProf.RootDeviceHints
-	}
-	if (hintSource != nil && host.Status.Provisioning.RootDeviceHints == nil) || *hintSource != *(host.Status.Provisioning.RootDeviceHints) {
-		host.Status.Provisioning.RootDeviceHints = hintSource
-		dirty = true
-	}
-	return
-}
-
 func (r *ReconcileBareMetalHost) saveHostStatus(host *metal3v1alpha1.BareMetalHost) error {
 	t := metav1.Now()
 	host.Status.LastUpdated = &t
diff --git a/pkg/hardware/profile.go b/pkg/hardware/profile.go
index 73c357ba..908a7747 100644
--- a/pkg/hardware/profile.go
+++ b/pkg/hardware/profile.go
@@ -2,8 +2,6 @@ package hardware
 
 import (
 	"fmt"
-
-	metal3v1alpha1 "github.com/metal3-io/baremetal-operator/pkg/apis/metal3/v1alpha1"
 )
 
 const (
@@ -19,7 +17,7 @@ type Profile struct {
 
 	// RootDeviceHints holds the suggestions for placing the storage
 	// for the root filesystem.
-	RootDeviceHints metal3v1alpha1.RootDeviceHints
+	RootDeviceHints RootDeviceHints
 
 	// RootGB is the size of the root volume in GB
 	RootGB int
@@ -31,12 +29,26 @@ type Profile struct {
 	CPUArch string
 }
 
+// RootDeviceHints holds the hints for specifying the storage location
+// for the root filesystem for the image.
+//
+// NOTE(dhellmann): Valid ironic hints are: "vendor,
+// wwn_vendor_extension, wwn_with_extension, by_path, serial, wwn,
+// size, rotational, name, hctl, model"
+type RootDeviceHints struct {
+	// A device name like "/dev/vda"
+	DeviceName string
+
+	// A SCSI bus address like 0:0:0:0
+	HCTL string
+}
+
 var profiles = make(map[string]Profile)
 
 func init() {
 	profiles[DefaultProfileName] = Profile{
 		Name: DefaultProfileName,
-		RootDeviceHints: metal3v1alpha1.RootDeviceHints{
+		RootDeviceHints: RootDeviceHints{
 			DeviceName: "/dev/sda",
 		},
 		RootGB:  10,
@@ -46,7 +58,7 @@ func init() {
 
 	profiles["libvirt"] = Profile{
 		Name: "libvirt",
-		RootDeviceHints: metal3v1alpha1.RootDeviceHints{
+		RootDeviceHints: RootDeviceHints{
 			DeviceName: "/dev/vda",
 		},
 		RootGB:  10,
@@ -56,7 +68,7 @@ func init() {
 
 	profiles["dell"] = Profile{
 		Name: "dell",
-		RootDeviceHints: metal3v1alpha1.RootDeviceHints{
+		RootDeviceHints: RootDeviceHints{
 			HCTL: "0:0:0:0",
 		},
 		RootGB:  10,
@@ -66,7 +78,7 @@ func init() {
 
 	profiles["dell-raid"] = Profile{
 		Name: "dell-raid",
-		RootDeviceHints: metal3v1alpha1.RootDeviceHints{
+		RootDeviceHints: RootDeviceHints{
 			HCTL: "0:2:0:0",
 		},
 		RootGB:  10,
@@ -76,7 +88,7 @@ func init() {
 
 	profiles["openstack"] = Profile{
 		Name: "openstack",
-		RootDeviceHints: metal3v1alpha1.RootDeviceHints{
+		RootDeviceHints: RootDeviceHints{
 			DeviceName: "/dev/vdb",
 		},
 		RootGB:  10,
diff --git a/pkg/provisioner/ironic/devicehints/devicehints.go b/pkg/provisioner/ironic/devicehints/devicehints.go
deleted file mode 100644
index f4f0cbd3..00000000
--- a/pkg/provisioner/ironic/devicehints/devicehints.go
+++ /dev/null
@@ -1,54 +0,0 @@
-package devicehints
-
-import (
-	"fmt"
-
-	metal3v1alpha1 "github.com/metal3-io/baremetal-operator/pkg/apis/metal3/v1alpha1"
-)
-
-// MakeHintMap converts a RootDeviceHints instance into a string map
-// suitable to pass to ironic.
-func MakeHintMap(source *metal3v1alpha1.RootDeviceHints) map[string]string {
-	hints := map[string]string{}
-
-	if source == nil {
-		return hints
-	}
-
-	if source.DeviceName != "" {
-		hints["name"] = fmt.Sprintf("s== %s", source.DeviceName)
-	}
-	if source.HCTL != "" {
-		hints["hctl"] = fmt.Sprintf("s== %s", source.HCTL)
-	}
-	if source.Model != "" {
-		hints["model"] = fmt.Sprintf("<in> %s", source.Model)
-	}
-	if source.Vendor != "" {
-		hints["vendor"] = fmt.Sprintf("<in> %s", source.Vendor)
-	}
-	if source.SerialNumber != "" {
-		hints["serial"] = fmt.Sprintf("s== %s", source.SerialNumber)
-	}
-	if source.MinSizeGigabytes != 0 {
-		hints["size"] = fmt.Sprintf(">= %d", source.MinSizeGigabytes)
-	}
-	if source.WWN != "" {
-		hints["wwn"] = fmt.Sprintf("s== %s", source.WWN)
-	}
-	if source.WWNWithExtension != "" {
-		hints["wwn_with_extension"] = fmt.Sprintf("s== %s", source.WWNWithExtension)
-	}
-	if source.WWNVendorExtension != "" {
-		hints["wwn_vendor_extension"] = fmt.Sprintf("s== %s", source.WWNVendorExtension)
-	}
-	switch {
-	case source.Rotational == nil:
-	case *source.Rotational == true:
-		hints["rotational"] = "true"
-	case *source.Rotational == false:
-		hints["rotational"] = "false"
-	}
-
-	return hints
-}
diff --git a/pkg/provisioner/ironic/ironic.go b/pkg/provisioner/ironic/ironic.go
index fab9139a..372600f7 100644
--- a/pkg/provisioner/ironic/ironic.go
+++ b/pkg/provisioner/ironic/ironic.go
@@ -24,7 +24,6 @@ import (
 	"github.com/metal3-io/baremetal-operator/pkg/bmc"
 	"github.com/metal3-io/baremetal-operator/pkg/hardware"
 	"github.com/metal3-io/baremetal-operator/pkg/provisioner"
-	"github.com/metal3-io/baremetal-operator/pkg/provisioner/ironic/devicehints"
 	"github.com/metal3-io/baremetal-operator/pkg/provisioner/ironic/hardwaredetails"
 )
 
@@ -561,7 +560,6 @@ func (p *ironicProvisioner) UpdateHardwareState() (result provisioner.Result, er
 func (p *ironicProvisioner) getUpdateOptsForNode(ironicNode *nodes.Node) (updates nodes.UpdateOpts, err error) {
 
 	hwProf, err := hardware.GetProfile(p.host.HardwareProfile())
-
 	if err != nil {
 		return updates, errors.Wrap(err,
 			fmt.Sprintf("Could not start provisioning with bad hardware profile %s",
@@ -675,12 +673,13 @@ func (p *ironicProvisioner) getUpdateOptsForNode(ironicNode *nodes.Node) (update
 		op = nodes.ReplaceOp
 		p.log.Info("updating root_device")
 	}
-
-	// hints
-	//
-	// If the user has provided explicit root device hints, they take
-	// precedence. Otherwise use the values from the hardware profile.
-	hints := devicehints.MakeHintMap(p.host.Status.Provisioning.RootDeviceHints)
+	hints := map[string]string{}
+	switch {
+	case hwProf.RootDeviceHints.DeviceName != "":
+		hints["name"] = hwProf.RootDeviceHints.DeviceName
+	case hwProf.RootDeviceHints.HCTL != "":
+		hints["hctl"] = hwProf.RootDeviceHints.HCTL
+	}
 	p.log.Info("using root device", "hints", hints)
 	updates = append(
 		updates,


Test patch:
diff --git a/pkg/controller/baremetalhost/baremetalhost_controller_test.go b/pkg/controller/baremetalhost/baremetalhost_controller_test.go
index 8fd7b8db..02211998 100644
--- a/pkg/controller/baremetalhost/baremetalhost_controller_test.go
+++ b/pkg/controller/baremetalhost/baremetalhost_controller_test.go
@@ -7,8 +7,6 @@ import (
 	"reflect"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
-
 	corev1 "k8s.io/api/core/v1"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -1039,111 +1037,3 @@ func TestDeleteHost(t *testing.T) {
 		})
 	}
 }
-
-// TestUpdateRootDeviceHints verifies that we apply the correct
-// precedence rules to the root device hints settings for a host.
-func TestUpdateRootDeviceHints(t *testing.T) {
-	rotational := true
-
-	testCases := []struct {
-		Scenario string
-		Host     metal3v1alpha1.BareMetalHost
-		Dirty    bool
-		Expected *metal3v1alpha1.RootDeviceHints
-	}{
-		{
-			Scenario: "override profile with explicit hints",
-			Host: metal3v1alpha1.BareMetalHost{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "myhost",
-					Namespace: "myns",
-					UID:       "27720611-e5d1-45d3-ba3a-222dcfaa4ca2",
-				},
-				Spec: metal3v1alpha1.BareMetalHostSpec{
-					HardwareProfile: "libvirt",
-					RootDeviceHints: &metal3v1alpha1.RootDeviceHints{
-						DeviceName:         "userd_devicename",
-						HCTL:               "1:2:3:4",
-						Model:              "userd_model",
-						Vendor:             "userd_vendor",
-						SerialNumber:       "userd_serial",
-						MinSizeGigabytes:   40,
-						WWN:                "userd_wwn",
-						WWNWithExtension:   "userd_with_extension",
-						WWNVendorExtension: "userd_vendor_extension",
-						Rotational:         &rotational,
-					},
-				},
-				Status: metal3v1alpha1.BareMetalHostStatus{
-					HardwareProfile: "libvirt",
-				},
-			},
-			Dirty: true,
-			Expected: &metal3v1alpha1.RootDeviceHints{
-				DeviceName:         "userd_devicename",
-				HCTL:               "1:2:3:4",
-				Model:              "userd_model",
-				Vendor:             "userd_vendor",
-				SerialNumber:       "userd_serial",
-				MinSizeGigabytes:   40,
-				WWN:                "userd_wwn",
-				WWNWithExtension:   "userd_with_extension",
-				WWNVendorExtension: "userd_vendor_extension",
-				Rotational:         &rotational,
-			},
-		},
-
-		{
-			Scenario: "use profile hints",
-			Host: metal3v1alpha1.BareMetalHost{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "myhost",
-					Namespace: "myns",
-					UID:       "27720611-e5d1-45d3-ba3a-222dcfaa4ca2",
-				},
-				Spec: metal3v1alpha1.BareMetalHostSpec{
-					HardwareProfile: "libvirt",
-				},
-				Status: metal3v1alpha1.BareMetalHostStatus{
-					HardwareProfile: "libvirt",
-				},
-			},
-			Dirty: true,
-			Expected: &metal3v1alpha1.RootDeviceHints{
-				DeviceName: "/dev/vda",
-			},
-		},
-
-		{
-			Scenario: "default profile hints",
-			Host: metal3v1alpha1.BareMetalHost{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "myhost",
-					Namespace: "myns",
-					UID:       "27720611-e5d1-45d3-ba3a-222dcfaa4ca2",
-				},
-				Spec: metal3v1alpha1.BareMetalHostSpec{
-					HardwareProfile: "unknown",
-				},
-				Status: metal3v1alpha1.BareMetalHostStatus{
-					HardwareProfile: "unknown",
-				},
-			},
-			Dirty: true,
-			Expected: &metal3v1alpha1.RootDeviceHints{
-				DeviceName: "/dev/sda",
-			},
-		},
-	}
-
-	for _, tc := range testCases {
-		t.Run(tc.Scenario, func(t *testing.T) {
-			dirty, err := saveHostProvisioningSettings(&tc.Host)
-			if err != nil {
-				t.Fatal(err)
-			}
-			assert.Equal(t, tc.Dirty, dirty, "dirty flag did not match")
-			assert.Equal(t, tc.Expected, tc.Host.Status.Provisioning.RootDeviceHints)
-		})
-	}
-}
diff --git a/pkg/provisioner/ironic/devicehints/devicehints_test.go b/pkg/provisioner/ironic/devicehints/devicehints_test.go
deleted file mode 100644
index 609f1344..00000000
--- a/pkg/provisioner/ironic/devicehints/devicehints_test.go
+++ /dev/null
@@ -1,157 +0,0 @@
-package devicehints
-
-import (
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-
-	metal3v1alpha1 "github.com/metal3-io/baremetal-operator/pkg/apis/metal3/v1alpha1"
-)
-
-func TestMakeHintMap(t *testing.T) {
-	addressableTrue := true
-	addressableFalse := false
-
-	for _, tc := range []struct {
-		Scenario string
-		Hints    metal3v1alpha1.RootDeviceHints
-		Expected map[string]string
-	}{
-		{
-			Scenario: "device-name",
-			Hints: metal3v1alpha1.RootDeviceHints{
-				DeviceName: "userd_devicename",
-			},
-			Expected: map[string]string{
-				"name": "s== userd_devicename",
-			},
-		},
-		{
-			Scenario: "hctl",
-			Hints: metal3v1alpha1.RootDeviceHints{
-				HCTL: "1:2:3:4",
-			},
-			Expected: map[string]string{
-				"hctl": "s== 1:2:3:4",
-			},
-		},
-		{
-			Scenario: "model",
-			Hints: metal3v1alpha1.RootDeviceHints{
-				Model: "userd_model",
-			},
-			Expected: map[string]string{
-				"model": "<in> userd_model",
-			},
-		},
-		{
-			Scenario: "vendor",
-			Hints: metal3v1alpha1.RootDeviceHints{
-				Vendor: "userd_vendor",
-			},
-			Expected: map[string]string{
-				"vendor": "<in> userd_vendor",
-			},
-		},
-		{
-			Scenario: "serial-number",
-			Hints: metal3v1alpha1.RootDeviceHints{
-				SerialNumber: "userd_serial",
-			},
-			Expected: map[string]string{
-				"serial": "s== userd_serial",
-			},
-		},
-		{
-			Scenario: "min-size",
-			Hints: metal3v1alpha1.RootDeviceHints{
-				MinSizeGigabytes: 40,
-			},
-			Expected: map[string]string{
-				"size": ">= 40",
-			},
-		},
-		{
-			Scenario: "wwn",
-			Hints: metal3v1alpha1.RootDeviceHints{
-				WWN: "userd_wwn",
-			},
-			Expected: map[string]string{
-				"wwn": "s== userd_wwn",
-			},
-		},
-		{
-			Scenario: "wwn-with-extension",
-			Hints: metal3v1alpha1.RootDeviceHints{
-				WWNWithExtension: "userd_with_extension",
-			},
-			Expected: map[string]string{
-				"wwn_with_extension": "s== userd_with_extension",
-			},
-		},
-		{
-			Scenario: "wwn-extension",
-			Hints: metal3v1alpha1.RootDeviceHints{
-				WWNVendorExtension: "userd_vendor_extension",
-			},
-			Expected: map[string]string{
-				"wwn_vendor_extension": "s== userd_vendor_extension",
-			},
-		},
-		{
-			Scenario: "rotational-true",
-			Hints: metal3v1alpha1.RootDeviceHints{
-				Rotational: &addressableTrue,
-			},
-			Expected: map[string]string{
-				"rotational": "true",
-			},
-		},
-		{
-			Scenario: "rotational-false",
-			Hints: metal3v1alpha1.RootDeviceHints{
-				Rotational: &addressableFalse,
-			},
-			Expected: map[string]string{
-				"rotational": "false",
-			},
-		},
-		{
-			Scenario: "everything-bagel",
-			Hints: metal3v1alpha1.RootDeviceHints{
-				DeviceName:         "userd_devicename",
-				HCTL:               "1:2:3:4",
-				Model:              "userd_model",
-				Vendor:             "userd_vendor",
-				SerialNumber:       "userd_serial",
-				MinSizeGigabytes:   40,
-				WWN:                "userd_wwn",
-				WWNWithExtension:   "userd_with_extension",
-				WWNVendorExtension: "userd_vendor_extension",
-				Rotational:         &addressableTrue,
-			},
-			Expected: map[string]string{
-				"name":                 "s== userd_devicename",
-				"hctl":                 "s== 1:2:3:4",
-				"model":                "<in> userd_model",
-				"vendor":               "<in> userd_vendor",
-				"serial":               "s== userd_serial",
-				"size":                 ">= 40",
-				"wwn":                  "s== userd_wwn",
-				"wwn_with_extension":   "s== userd_with_extension",
-				"wwn_vendor_extension": "s== userd_vendor_extension",
-				"rotational":           "true",
-			},
-		},
-		{
-			Scenario: "empty",
-			Hints:    metal3v1alpha1.RootDeviceHints{},
-			Expected: map[string]string{},
-		},
-	} {
-		t.Run(tc.Scenario, func(t *testing.T) {
-			actual := MakeHintMap(&tc.Hints)
-			assert.Equal(t, tc.Expected, actual, "hint map does not match")
-		})
-	}
-}
diff --git a/pkg/provisioner/ironic/ironic_test.go b/pkg/provisioner/ironic/ironic_test.go
index e26ac1ad..d172f9a2 100644
--- a/pkg/provisioner/ironic/ironic_test.go
+++ b/pkg/provisioner/ironic/ironic_test.go
@@ -1,11 +1,8 @@
 package ironic
 
 import (
-	"fmt"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
-
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
 
@@ -20,114 +17,6 @@ func init() {
 	logf.SetLogger(logf.ZapLogger(true))
 }
 
-func TestGetUpdateOptsForNodeWithRootHints(t *testing.T) {
-	rotational := true
-	host := &metal3v1alpha1.BareMetalHost{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "myhost",
-			Namespace: "myns",
-			UID:       "27720611-e5d1-45d3-ba3a-222dcfaa4ca2",
-		},
-		Spec: metal3v1alpha1.BareMetalHostSpec{
-			Image: &metal3v1alpha1.Image{
-				URL: "not-empty",
-			},
-			Online:          true,
-			HardwareProfile: "libvirt",
-			RootDeviceHints: &metal3v1alpha1.RootDeviceHints{
-				DeviceName:         "userd_devicename",
-				HCTL:               "1:2:3:4",
-				Model:              "userd_model",
-				Vendor:             "userd_vendor",
-				SerialNumber:       "userd_serial",
-				MinSizeGigabytes:   40,
-				WWN:                "userd_wwn",
-				WWNWithExtension:   "userd_with_extension",
-				WWNVendorExtension: "userd_vendor_extension",
-				Rotational:         &rotational,
-			},
-		},
-		Status: metal3v1alpha1.BareMetalHostStatus{
-			Provisioning: metal3v1alpha1.ProvisionStatus{
-				ID: "provisioning-id",
-				// Place the hints in the status field to pretend the
-				// controller has already reconciled partially.
-				RootDeviceHints: &metal3v1alpha1.RootDeviceHints{
-					DeviceName:         "userd_devicename",
-					HCTL:               "1:2:3:4",
-					Model:              "userd_model",
-					Vendor:             "userd_vendor",
-					SerialNumber:       "userd_serial",
-					MinSizeGigabytes:   40,
-					WWN:                "userd_wwn",
-					WWNWithExtension:   "userd_with_extension",
-					WWNVendorExtension: "userd_vendor_extension",
-					Rotational:         &rotational,
-				},
-			},
-			HardwareProfile: "libvirt",
-		},
-	}
-
-	eventPublisher := func(reason, message string) {}
-
-	prov, err := newProvisioner(host, bmc.Credentials{}, eventPublisher)
-	ironicNode := &nodes.Node{}
-
-	patches, err := prov.getUpdateOptsForNode(ironicNode)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	t.Logf("patches: %v", patches)
-
-	expected := []struct {
-		Path  string            // the node property path
-		Map   map[string]string // Expected roothdevicehint map
-		Value interface{}       // the value being passed to ironic (or value associated with the key)
-	}{
-		{
-			Path:  "/properties/root_device",
-			Value: "userdefined_devicename",
-			Map: map[string]string{
-				"name":                 "s== userd_devicename",
-				"hctl":                 "s== 1:2:3:4",
-				"model":                "<in> userd_model",
-				"vendor":               "<in> userd_vendor",
-				"serial":               "s== userd_serial",
-				"size":                 ">= 40",
-				"wwn":                  "s== userd_wwn",
-				"wwn_with_extension":   "s== userd_with_extension",
-				"wwn_vendor_extension": "s== userd_vendor_extension",
-				"rotational":           "true",
-			},
-		},
-	}
-
-	for _, e := range expected {
-		t.Run(e.Path, func(t *testing.T) {
-			t.Logf("expected: %v", e)
-			var update nodes.UpdateOperation
-			for _, patch := range patches {
-				update = patch.(nodes.UpdateOperation)
-				if update.Path == e.Path {
-					break
-				}
-			}
-			if update.Path != e.Path {
-				t.Errorf("did not find %q in updates", e.Path)
-				return
-			}
-			t.Logf("update: %v", update)
-			if e.Map != nil {
-				assert.Equal(t, e.Map, update.Value, fmt.Sprintf("%s does not match", e.Path))
-			} else {
-				assert.Equal(t, e.Value, update.Value, fmt.Sprintf("%s does not match", e.Path))
-			}
-		})
-	}
-}
-
 func TestGetUpdateOptsForNodeVirtual(t *testing.T) {
 	host := &metal3v1alpha1.BareMetalHost{
 		ObjectMeta: metav1.ObjectMeta{
@@ -164,6 +53,8 @@ func TestGetUpdateOptsForNodeVirtual(t *testing.T) {
 
 	t.Logf("patches: %v", patches)
 
+	var update nodes.UpdateOperation
+
 	expected := []struct {
 		Path  string      // the node property path
 		Key   string      // if value is a map, the key we care about
@@ -193,6 +84,11 @@ func TestGetUpdateOptsForNodeVirtual(t *testing.T) {
 			Path:  "/instance_info/root_gb",
 			Value: 10,
 		},
+		{
+			Path:  "/instance_info/root_device",
+			Value: "/dev/vda",
+			Key:   "name",
+		},
 		{
 			Path:  "/properties/cpu_arch",
 			Value: "x86_64",
@@ -203,23 +99,18 @@ func TestGetUpdateOptsForNodeVirtual(t *testing.T) {
 		},
 	}
 
-	for _, e := range expected {
-		t.Run(e.Path, func(t *testing.T) {
-			t.Logf("expected: %v", e)
-			var update nodes.UpdateOperation
-			for _, patch := range patches {
-				update = patch.(nodes.UpdateOperation)
-				if update.Path == e.Path {
-					break
-				}
+	for i, e := range expected {
+		update = patches[i].(nodes.UpdateOperation)
+		if e.Key != "" {
+			m := update.Value.(map[string]string)
+			if m[e.Key] != e.Value {
+				t.Errorf("expected %s=%q got %s=%q", e.Path, e.Value, update.Path, update.Value)
 			}
-			if update.Path != e.Path {
-				t.Errorf("did not find %q in updates", e.Path)
-				return
+		} else {
+			if update.Value != e.Value {
+				t.Errorf("expected %s=%q got %s=%q", e.Path, e.Value, update.Path, update.Value)
 			}
-			t.Logf("update: %v", update)
-			assert.Equal(t, e.Value, update.Value, fmt.Sprintf("%s does not match", e.Path))
-		})
+		}
 	}
 }
 
@@ -259,6 +150,8 @@ func TestGetUpdateOptsForNodeDell(t *testing.T) {
 
 	t.Logf("patches: %v", patches)
 
+	var update nodes.UpdateOperation
+
 	expected := []struct {
 		Path  string      // the node property path
 		Key   string      // if value is a map, the key we care about
@@ -284,6 +177,11 @@ func TestGetUpdateOptsForNodeDell(t *testing.T) {
 			Path:  "/instance_info/root_gb",
 			Value: 10,
 		},
+		{
+			Path:  "/instance_info/root_device",
+			Value: "0:0:0:0",
+			Key:   "hctl",
+		},
 		{
 			Path:  "/properties/cpu_arch",
 			Value: "x86_64",
@@ -294,22 +192,17 @@ func TestGetUpdateOptsForNodeDell(t *testing.T) {
 		},
 	}
 
-	for _, e := range expected {
-		t.Run(e.Path, func(t *testing.T) {
-			t.Logf("expected: %v", e)
-			var update nodes.UpdateOperation
-			for _, patch := range patches {
-				update = patch.(nodes.UpdateOperation)
-				if update.Path == e.Path {
-					break
-				}
+	for i, e := range expected {
+		update = patches[i].(nodes.UpdateOperation)
+		if e.Key != "" {
+			m := update.Value.(map[string]string)
+			if m[e.Key] != e.Value {
+				t.Errorf("expected %s=%q got %s=%q", e.Path, e.Value, update.Path, update.Value)
 			}
-			if update.Path != e.Path {
-				t.Errorf("did not find %q in updates", e.Path)
-				return
+		} else {
+			if update.Value != e.Value {
+				t.Errorf("expected %s=%q got %s=%q", e.Path, e.Value, update.Path, update.Value)
 			}
-			t.Logf("update: %v", update)
-			assert.Equal(t, e.Value, update.Value, fmt.Sprintf("%s does not match", e.Path))
-		})
+		}
 	}
 }