add root device hints to BareMetalHost

Add a new field spec.rootDeviceHints to the API with sub-fields for
different types of hints based on what Ironic supports today.

Add a new field status.rootDeviceHints to record the values used when
provisioning the host.

Change the Provisioner API to return the new public struct with the
more detailed device hints so that the values in the hard-coded
profiles can be stored directly in the new status field.

If the user does not provide explicit hints, use the values from the
profile.

Implements https://github.com/metal3-io/metal3-docs/blob/master/design/user-defined-root-device-hints.md

Co-Authored-By: Mika Koskimaki <mika.koskimaki@est.tech>
Co-Authored-By: Doug Hellmann <dhellmann@redhat.com>
Signed-off-by: Doug Hellmann <dhellmann@redhat.com>

deploy/crds/metal3.io_baremetalhosts_crd.yaml

@@ -203,6 +203,43 @@ spec:
             online:
               description: Should the server be online?
               type: boolean
+            rootDeviceHints:
+              description: Provide guidance about how to find the device for the image
+                being provisioned.
+              properties:
+                deviceName:
+                  description: A Linux device name like "/dev/vda"
+                  type: string
+                hctl:
+                  description: A SCSI bus address like 0:0:0:0
+                  type: string
+                model:
+                  description: A vendor-specific device identifier
+                  type: string
+                rotational:
+                  description: Device rotational type
+                  type: boolean
+                serialNumber:
+                  description: Device serial number
+                  type: string
+                sizeGigabytes:
+                  description: Size of the device in Gigabytes
+                  type: integer
+                vendor:
+                  description: The name of the vendor or manufacturer of the device.
+                    Must match the results found through inspection.
+                  type: string
+                wwn:
+                  description: Unique storage identifier
+                  type: string
+                wwnVendorExtension:
+                  description: Unique vendor storage identifier
+                  type: string
+                wwnWithExtension:
+                  description: Unique storage identifier with the vendor extension
+                    appended
+                  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
@@ -572,6 +609,42 @@ spec:
                   - checksum
                   - url
                   type: object
+                rootDeviceHints:
+                  description: The RootDevicehints set by the user
+                  properties:
+                    deviceName:
+                      description: A Linux device name like "/dev/vda"
+                      type: string
+                    hctl:
+                      description: A SCSI bus address like 0:0:0:0
+                      type: string
+                    model:
+                      description: A vendor-specific device identifier
+                      type: string
+                    rotational:
+                      description: Device rotational type
+                      type: boolean
+                    serialNumber:
+                      description: Device serial number
+                      type: string
+                    sizeGigabytes:
+                      description: Size of the device in Gigabytes
+                      type: integer
+                    vendor:
+                      description: The name of the vendor or manufacturer of the device.
+                        Must match the results found through inspection.
+                      type: string
+                    wwn:
+                      description: Unique storage identifier
+                      type: string
+                    wwnVendorExtension:
+                      description: Unique vendor storage identifier
+                      type: string
+                    wwnWithExtension:
+                      description: Unique storage identifier with the vendor extension
+                        appended
+                      type: string
+                  type: object
                 state:
                   description: An indiciator for what the provisioner is doing with
                     the host.

go.mod

@@ -16,6 +16,7 @@ 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
@@ -28,7 +29,6 @@ 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 (

pkg/apis/metal3/v1alpha1/baremetalhost_types.go

@@ -30,6 +30,41 @@ 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"
+	DeviceName string `json:"deviceName,omitempty"`
+
+	// A SCSI bus address like 0:0:0:0
+	HCTL string `json:"hctl,omitempty"`
+
+	// A vendor-specific device identifier
+	Model string `json:"model,omitempty"`
+
+	// The name of the vendor or manufacturer of the device. Must
+	// match the results found through inspection.
+	Vendor string `json:"vendor,omitempty"`
+
+	// Device serial number
+	SerialNumber string `json:"serialNumber,omitempty"`
+
+	// Size of the device in Gigabytes
+	SizeGigabytes int `json:"sizeGigabytes,omitempty"`
+
+	// Unique storage identifier
+	WWN string `json:"wwn,omitempty"`
+
+	// Unique storage identifier with the vendor extension appended
+	WWNWithExtension string `json:"wwnWithExtension,omitempty"`
+
+	// Unique vendor storage identifier
+	WWNVendorExtension string `json:"wwnVendorExtension,omitempty"`
+
+	// Device rotational type
+	Rotational *bool `json:"rotational,omitempty"`
+}
+
 // OperationalStatus represents the state of the host
 type OperationalStatus string
 
@@ -165,6 +200,10 @@ type BareMetalHostSpec struct {
 	// automatically determine the profile.
 	HardwareProfile string `json:"hardwareProfile,omitempty"`
 
+	// Provide guidance about how to find 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}`
@@ -491,6 +530,9 @@ 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

pkg/controller/baremetalhost/baremetalhost_controller.go

@@ -566,10 +566,31 @@ 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{}
 }
 
+func updateRootDeviceHintsForHost(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
+		return true, nil
+	}
+	return false, nil
+}
+
 // Start/continue provisioning if we need to.
 func (r *ReconcileBareMetalHost) actionProvisioning(prov provisioner.Provisioner, info *reconcileInfo) actionResult {
 	hostConf := &hostConfigData{
@@ -586,6 +607,16 @@ func (r *ReconcileBareMetalHost) actionProvisioning(prov provisioner.Provisioner
 		return actionContinueNoWrite{}
 	}
 
+	// Ensure the root device hints we're going to use are stored.
+	dirty, err := updateRootDeviceHintsForHost(info.host)
+	if err != nil {
+		return actionError{errors.Wrap(err, "Could not update root device hints")}
+	}
+	if dirty {
+		info.log.Info("updating root device hints")
+		return actionContinue{}
+	}
+
 	provResult, err := prov.Provision(hostConf)
 	if err != nil {
 		return actionError{errors.Wrap(err, "failed to provision")}
@@ -642,6 +673,7 @@ func (r *ReconcileBareMetalHost) actionDeprovisioning(prov provisioner.Provision
 	// After the provisioner is done, clear the image settings so we
 	// transition to the next state.
 	info.host.Status.Provisioning.Image = metal3v1alpha1.Image{}
+	info.host.Status.Provisioning.RootDeviceHints = nil
 
 	return actionComplete{}
 }

pkg/controller/baremetalhost/baremetalhost_controller_test.go

@@ -7,6 +7,8 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+
 	corev1 "k8s.io/api/core/v1"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -1037,3 +1039,111 @@ 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",
+						SizeGigabytes:      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",
+				SizeGigabytes:      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 := updateRootDeviceHintsForHost(&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)
+		})
+	}
+}