Merge pull request #154 from abays/select_specific_bmh

Allow selection of specific BMH per compute node in OSBMS

api/bases/baremetal.openstack.org_openstackbaremetalsets.yaml

@@ -69,6 +69,12 @@ spec:
                 additionalProperties:
                   description: InstanceSpec Instance specific attributes
                   properties:
+                    bmhLabelSelector:
+                      additionalProperties:
+                        type: string
+                      description: BmhLabelSelector allows for the selection of a
+                        particular BaremetalHost based on arbitrary labels
+                      type: object
                     ctlPlaneIP:
                       description: CtlPlaneIP - Control Plane IP in CIDR notation
                       type: string

api/v1beta1/openstackbaremetalset.go

@@ -37,10 +37,10 @@ func GetBaremetalHosts(
 
 	err := c.List(ctx, bmhHostsList, listOpts...)
 	if err != nil {
-		return bmhHostsList, err
+		return nil, err
 	}
-
 	return bmhHostsList, nil
+
 }
 
 // VerifyBaremetalStatusBmhRefs - Verify that BMHs haven't been improperly deleted
@@ -90,33 +90,32 @@ func VerifyBaremetalSetScaleUp(
 	l logr.Logger,
 	instance *OpenStackBaremetalSet,
 	allBmhs *metal3v1.BareMetalHostList,
-	existingBmhs *metal3v1.BareMetalHostList) ([]string, error) {
+	existingBmhs *metal3v1.BareMetalHostList) (map[string]metal3v1.BareMetalHost, error) {
 	// How many new BaremetalHost allocations do we need (if any)?
 	newBmhsNeededCount := len(instance.Spec.BaremetalHosts) - len(existingBmhs.Items)
-	availableBaremetalHosts := []string{}
-
-	if newBmhsNeededCount > 0 {
-		// We have new BaremetalHosts requested, so search for BaremetalHosts that don't have consumerRef or online set
+	selectedBaremetalHosts := map[string]metal3v1.BareMetalHost{}
 
-		labelStr := ""
+	labelStr := ""
 
+	if newBmhsNeededCount > 0 {
 		if len(instance.Spec.BmhLabelSelector) > 0 {
 			labelStr = fmt.Sprintf("%v", instance.Spec.BmhLabelSelector)
 			labelStr = strings.Replace(labelStr, "map[", "[", 1)
 		}
 
-		l.Info("Attempting to find BaremetalHosts for scale-up of OpenStackBaremetalSet", "OpenStackBaremetalSet", instance.Name, "namespace", instance.Spec.BmhNamespace, "quantity", newBmhsNeededCount, "labels", labelStr)
+		l.Info("Attempting to find BaremetalHosts for scale-up of OpenStackBaremetalSet", "OpenStackBaremetalSet",
+			instance.Name, "namespace", instance.Spec.BmhNamespace, "quantity", newBmhsNeededCount, "labels", labelStr)
 
+		selectedCount := 0
 		for _, baremetalHost := range allBmhs.Items {
-			mismatch := false
 
-			if baremetalHost.Spec.Online {
-				l.Info("BaremetalHost cannot be used because it is already online", "BMH", baremetalHost.ObjectMeta.Name)
-				mismatch = true
+			if selectedCount == newBmhsNeededCount {
+				break
 			}
-
-			if baremetalHost.Spec.ConsumerRef != nil {
-				l.Info("BaremetalHost cannot be used because it already has a consumerRef", "BMH", baremetalHost.ObjectMeta.Name)
+			mismatch := false
+			hostName, matched := verifyBaremetalSetInstanceLabelMatch(l, instance, &baremetalHost)
+			if !matched {
+				l.Info("BaremetalHost cannot be used as it does not match node labels for", "BMH", baremetalHost.ObjectMeta.Name)
 				mismatch = true
 			}
 
@@ -130,36 +129,48 @@ func VerifyBaremetalSetScaleUp(
 				mismatch = true
 			}
 
+			if baremetalHost.Spec.Online {
+				l.Info("BaremetalHost cannot be used because it is already online", "BMH", baremetalHost.ObjectMeta.Name)
+				mismatch = true
+			}
+
+			if baremetalHost.Spec.ConsumerRef != nil {
+				l.Info("BaremetalHost cannot be used because it already has a consumerRef", "BMH", baremetalHost.ObjectMeta.Name)
+				mismatch = true
+			}
+
 			// If for any reason we can't use this BMH, do not add to the list of available BMHs
 			if mismatch {
 				continue
 			}
 
 			l.Info("Available BaremetalHost", "BMH", baremetalHost.ObjectMeta.Name)
 
-			availableBaremetalHosts = append(availableBaremetalHosts, baremetalHost.ObjectMeta.Name)
+			selectedBaremetalHosts[hostName] = baremetalHost
+			selectedCount++
 		}
+	}
+	// If we can't satisfy the new requested BaremetalHost count, explicitly state so
+	if newBmhsNeededCount > len(selectedBaremetalHosts) {
+		errLabelStr := ""
 
-		// If we can't satisfy the new requested BaremetalHost count, explicitly state so
-		if newBmhsNeededCount > len(availableBaremetalHosts) {
-			errLabelStr := ""
-
-			if labelStr != "" {
-				errLabelStr = fmt.Sprintf(" with labels %s", labelStr)
-			}
-
-			return nil, fmt.Errorf("unable to find %d requested BaremetalHosts%s in namespace %s for scale-up (%d in use, %d available)",
-				len(instance.Spec.BaremetalHosts),
-				errLabelStr,
-				instance.Spec.BmhNamespace,
-				len(existingBmhs.Items),
-				len(availableBaremetalHosts))
+		if labelStr != "" {
+			errLabelStr = fmt.Sprintf(" with labels %s", labelStr)
 		}
 
-		l.Info("Found sufficient quantity of BaremetalHosts for scale-up of OpenStackBaremetalSet", "OpenStackBaremetalSet", instance.Name, "namespace", instance.Spec.BmhNamespace, "BMHs", availableBaremetalHosts, "labels", labelStr)
+		return nil, fmt.Errorf("unable to find %d requested BaremetalHosts%s in namespace %s for scale-up (%d in use, %d available)",
+			len(instance.Spec.BaremetalHosts),
+			errLabelStr,
+			instance.Spec.BmhNamespace,
+			len(existingBmhs.Items),
+			len(selectedBaremetalHosts))
 	}
 
-	return availableBaremetalHosts, nil
+	l.Info("Found sufficient quantity of BaremetalHosts for scale-up of OpenStackBaremetalSet",
+		"OpenStackBaremetalSet", instance.Name, "namespace", instance.Spec.BmhNamespace, "BMHs",
+		selectedBaremetalHosts, "labels", labelStr)
+
+	return selectedBaremetalHosts, nil
 }
 
 // VerifyBaremetalSetScaleDown - TODO: not needed at the current moment
@@ -179,6 +190,36 @@ func VerifyBaremetalSetScaleDown(
 	return nil
 }
 
+func verifyBaremetalSetInstanceLabelMatch(
+	l logr.Logger,
+	instance *OpenStackBaremetalSet,
+	bmh *metal3v1.BareMetalHost) (string, bool) {
+
+	bmhLabels := bmh.GetLabels()
+	for hostName, instanceSpec := range instance.Spec.BaremetalHosts {
+		if IsMapSubset(bmhLabels, instanceSpec.BmhLabelSelector) {
+			return hostName, true
+		}
+	}
+	l.Info("BaremetalHost does not match any of the node labels as requested", "BMH", bmh.ObjectMeta.Name)
+	return "", false
+}
+
+func IsMapSubset[K, V comparable](m map[K]V, sub map[K]V) bool {
+	if sub == nil {
+		return true
+	}
+	if len(sub) > len(m) {
+		return false
+	}
+	for k, vsub := range sub {
+		if vm, found := m[k]; !found || vm != vsub {
+			return false
+		}
+	}
+	return true
+}
+
 func verifyBaremetalSetHardwareMatch(
 	l logr.Logger,
 	instance *OpenStackBaremetalSet,

api/v1beta1/openstackbaremetalset_types.go

@@ -28,6 +28,9 @@ type AutomatedCleaningMode string
 
 // InstanceSpec Instance specific attributes
 type InstanceSpec struct {
+	// +kubebuilder:validation:Optional
+	// BmhLabelSelector allows for the selection of a particular BaremetalHost based on arbitrary labels
+	BmhLabelSelector map[string]string `json:"bmhLabelSelector,omitempty"`
 	// +kubebuilder:validation:Optional
 	// CtlPlaneIP - Control Plane IP in CIDR notation
 	CtlPlaneIP string `json:"ctlPlaneIP,omitempty"`

config/crd/bases/baremetal.openstack.org_openstackbaremetalsets.yaml

@@ -69,6 +69,12 @@ spec:
                 additionalProperties:
                   description: InstanceSpec Instance specific attributes
                   properties:
+                    bmhLabelSelector:
+                      additionalProperties:
+                        type: string
+                      description: BmhLabelSelector allows for the selection of a
+                        particular BaremetalHost based on arbitrary labels
+                      type: object
                     ctlPlaneIP:
                       description: CtlPlaneIP - Control Plane IP in CIDR notation
                       type: string

controllers/openstackbaremetalset_controller.go

@@ -19,7 +19,6 @@ package controllers
 import (
 	"context"
 	"fmt"
-	"sort"
 	"time"
 
 	corev1 "k8s.io/api/core/v1"
@@ -42,6 +41,7 @@ import (
 	"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/labels"
 	oko_secret "github.com/openstack-k8s-operators/lib-common/modules/common/secret"
+	"github.com/openstack-k8s-operators/lib-common/modules/common/util"
 	baremetalv1 "github.com/openstack-k8s-operators/openstack-baremetal-operator/api/v1beta1"
 	"github.com/openstack-k8s-operators/openstack-baremetal-operator/pkg/openstackbaremetalset"
 	openstackprovisionserver "github.com/openstack-k8s-operators/openstack-baremetal-operator/pkg/openstackprovisionserver"
@@ -575,6 +575,19 @@ func (r *OpenStackBaremetalSetReconciler) deleteBmh(
 	return nil
 }
 
+func (r *OpenStackBaremetalSetReconciler) buildExistingHostBMHMap(instance *baremetalv1.OpenStackBaremetalSet,
+	existingBMHs *metal3v1.BareMetalHostList) map[string]metal3v1.BareMetalHost {
+	existingHostBMHMap := make(map[string]metal3v1.BareMetalHost)
+	instanceBmhOwnershipLabelKey := fmt.Sprintf("%s%s", instance.Name, openstackbaremetalset.HostnameLabelSelectorSuffix)
+	for _, bmh := range existingBMHs.Items {
+		hostName, ok := bmh.Labels[instanceBmhOwnershipLabelKey]
+		if ok {
+			existingHostBMHMap[hostName] = bmh
+		}
+	}
+	return existingHostBMHMap
+}
+
 // Provision BaremetalHost resources based on replica count
 func (r *OpenStackBaremetalSetReconciler) ensureBaremetalHosts(
 	ctx context.Context,
@@ -586,6 +599,7 @@ func (r *OpenStackBaremetalSetReconciler) ensureBaremetalHosts(
 	bmhLabels map[string]string,
 	envVars *map[string]env.Setter,
 ) error {
+
 	// Get all BaremetalHosts (and, optionally, only those that match instance.Spec.BmhLabelSelector if there is one)
 	baremetalHostsList, err := baremetalv1.GetBaremetalHosts(
 		ctx,
@@ -604,41 +618,20 @@ func (r *OpenStackBaremetalSetReconciler) ensureBaremetalHosts(
 	}
 
 	// Verify that we have enough hosts with the right hardware reqs available for scaling-up
-	availableBaremetalHosts, err := baremetalv1.VerifyBaremetalSetScaleUp(log.FromContext(ctx), instance, baremetalHostsList, existingBaremetalHosts)
+	selectedHostBMHMap, err := baremetalv1.VerifyBaremetalSetScaleUp(log.FromContext(ctx), instance, baremetalHostsList, existingBaremetalHosts)
 	if err != nil {
 		return err
 	}
 
-	// Sort the list of available BaremetalHosts
-	sort.Strings(availableBaremetalHosts)
-
-	instanceBmhOwnershipLabelKey := fmt.Sprintf("%s%s", instance.Name, openstackbaremetalset.HostnameLabelSelectorSuffix)
-	desiredHostnamesToBmhNames := map[string]string{}
-
-	// How many new BaremetalHost allocations do we need (if any) and which ones already exist (if any)?
-	for desiredHostName := range instance.Spec.BaremetalHosts {
-		// Do the existingBaremetalHosts already contain this desired host?
-		for _, existingBmh := range existingBaremetalHosts.Items {
-			if existingBmh.Labels[instanceBmhOwnershipLabelKey] == desiredHostName {
-				desiredHostnamesToBmhNames[desiredHostName] = existingBmh.Name
-				break
-			}
-		}
-
-		// If there isn't already a BMH for this desired host name, assign the host name
-		// to one of the available BMHs
-		if desiredHostnamesToBmhNames[desiredHostName] == "" {
-			desiredHostnamesToBmhNames[desiredHostName] = availableBaremetalHosts[0]
-			availableBaremetalHosts = availableBaremetalHosts[1:]
-		}
-	}
+	existingHostBMHMap := r.buildExistingHostBMHMap(instance, existingBaremetalHosts)
+	selectedHostBMHMap = util.MergeMaps(selectedHostBMHMap, existingHostBMHMap)
 
-	for desiredHostName, bmhName := range desiredHostnamesToBmhNames {
+	for desiredHostName, bmh := range selectedHostBMHMap {
 		err := openstackbaremetalset.BaremetalHostProvision(
 			ctx,
 			helper,
 			instance,
-			bmhName,
+			bmh.Name,
 			desiredHostName,
 			instance.Spec.BaremetalHosts[desiredHostName].CtlPlaneIP, // ctlPlaneIP
 			provisionServer,

pkg/openstackbaremetalset/baremetalhost.go

@@ -34,11 +34,9 @@ func BaremetalHostProvision(
 	envVars *map[string]env.Setter,
 ) error {
 	l := log.FromContext(ctx)
-
 	//
-	// Get the associated BaremetalHost status entry for this hostname
+	// Update status with BMH provisioning details
 	//
-	// TODO: To be reworked when IPAM integrated
 	var ok bool
 	var bmhStatus baremetalv1.HostStatus
 
@@ -53,7 +51,6 @@ func BaremetalHostProvision(
 		}
 		bmhStatus.IPAddresses["ctlplane"] = ctlPlaneIP
 	}
-
 	// Instance UserData/NetworkData overrides the default
 	userDataSecret := instance.Spec.BaremetalHosts[hostName].UserData
 	networkDataSecret := instance.Spec.BaremetalHosts[hostName].NetworkData
@@ -71,12 +68,12 @@ func BaremetalHostProvision(
 	if userDataSecret == nil {
 		templateParameters := make(map[string]interface{})
 		templateParameters["AuthorizedKeys"] = strings.TrimSuffix(string(sshSecret.Data["authorized_keys"]), "\n")
-		templateParameters["HostName"] = bmhStatus.Hostname
+		templateParameters["HostName"] = hostName
 		//If Hostname is fqdn, use it
-		if !hostNameIsFQDN(bmhStatus.Hostname) && instance.Spec.DomainName != "" {
-			templateParameters["FQDN"] = strings.Join([]string{bmhStatus.Hostname, instance.Spec.DomainName}, ".")
+		if !hostNameIsFQDN(hostName) && instance.Spec.DomainName != "" {
+			templateParameters["FQDN"] = strings.Join([]string{hostName, instance.Spec.DomainName}, ".")
 		} else {
-			templateParameters["FQDN"] = bmhStatus.Hostname
+			templateParameters["FQDN"] = hostName
 		}
 		templateParameters["CloudUserName"] = instance.Spec.CloudUserName
 
@@ -86,7 +83,7 @@ func BaremetalHostProvision(
 			templateParameters["NodeRootPassword"] = string(passwordSecret.Data["NodeRootPassword"])
 		}
 
-		userDataSecretName := fmt.Sprintf(CloudInitUserDataSecretName, instance.Name, bmh)
+		userDataSecretName := fmt.Sprintf(CloudInitUserDataSecretName, instance.Name, hostName)
 
 		userDataSt := util.Template{
 			Name:               userDataSecretName,
@@ -165,7 +162,7 @@ func BaremetalHostProvision(
 			templateParameters["CtlplaneDnsSearch"] = []string{}
 		}
 
-		networkDataSecretName := fmt.Sprintf(CloudInitNetworkDataSecretName, instance.Name, bmh)
+		networkDataSecretName := fmt.Sprintf(CloudInitNetworkDataSecretName, instance.Name, hostName)
 
 		// Flag the network data secret as safe to collect with must-gather
 		secretLabelsWithMustGather := labels.GetLabels(instance, labels.GetGroupLabel(baremetalv1.ServiceName), map[string]string{
@@ -200,7 +197,7 @@ func BaremetalHostProvision(
 		// belongs to the particular OSBMS.Spec.BaremetalHosts entry we have passed to this function.
 		// Set ownership labels that can be found by the respective controller kind
 		labelSelector := labels.GetLabels(instance, labels.GetGroupLabel(baremetalv1.ServiceName), map[string]string{
-			fmt.Sprintf("%s%s", instance.Name, HostnameLabelSelectorSuffix): bmhStatus.Hostname,
+			fmt.Sprintf("%s%s", instance.Name, HostnameLabelSelectorSuffix): hostName,
 		})
 		foundBaremetalHost.Labels = util.MergeStringMaps(
 			foundBaremetalHost.GetLabels(),