From dfa6d8f2cde4edde4d34e323cf42c5297237fa99 Mon Sep 17 00:00:00 2001 From: Andrew Bays Date: Wed, 27 Mar 2024 09:27:50 +0000 Subject: [PATCH 1/2] Allow selection of specific BMH per compute node in OSBMS jira: OSPRH-10282 Co-Authored-By: @rabi ramishra@redhat.com Signed-off-by: rabi --- ....openstack.org_openstackbaremetalsets.yaml | 6 + api/v1beta1/openstackbaremetalset.go | 107 ++++++++++++------ api/v1beta1/openstackbaremetalset_types.go | 3 + api/v1beta1/zz_generated.deepcopy.go | 7 ++ ....openstack.org_openstackbaremetalsets.yaml | 6 + .../openstackbaremetalset_controller.go | 47 ++++---- pkg/openstackbaremetalset/baremetalhost.go | 19 ++-- 7 files changed, 124 insertions(+), 71 deletions(-) diff --git a/api/bases/baremetal.openstack.org_openstackbaremetalsets.yaml b/api/bases/baremetal.openstack.org_openstackbaremetalsets.yaml index 3ab8bd0..ebd3f81 100644 --- a/api/bases/baremetal.openstack.org_openstackbaremetalsets.yaml +++ b/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 diff --git a/api/v1beta1/openstackbaremetalset.go b/api/v1beta1/openstackbaremetalset.go index 3c79fc3..6b18998 100644 --- a/api/v1beta1/openstackbaremetalset.go +++ b/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,6 +129,16 @@ 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 @@ -137,29 +146,31 @@ func VerifyBaremetalSetScaleUp( 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, diff --git a/api/v1beta1/openstackbaremetalset_types.go b/api/v1beta1/openstackbaremetalset_types.go index 0f97a14..f24def7 100644 --- a/api/v1beta1/openstackbaremetalset_types.go +++ b/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"` diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 01666a9..c842f81 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -180,6 +180,13 @@ func (in *IPStatus) DeepCopy() *IPStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *InstanceSpec) DeepCopyInto(out *InstanceSpec) { *out = *in + if in.BmhLabelSelector != nil { + in, out := &in.BmhLabelSelector, &out.BmhLabelSelector + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.UserData != nil { in, out := &in.UserData, &out.UserData *out = new(v1.SecretReference) diff --git a/config/crd/bases/baremetal.openstack.org_openstackbaremetalsets.yaml b/config/crd/bases/baremetal.openstack.org_openstackbaremetalsets.yaml index 3ab8bd0..ebd3f81 100644 --- a/config/crd/bases/baremetal.openstack.org_openstackbaremetalsets.yaml +++ b/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 diff --git a/controllers/openstackbaremetalset_controller.go b/controllers/openstackbaremetalset_controller.go index d5f4233..36e2d53 100644 --- a/controllers/openstackbaremetalset_controller.go +++ b/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, diff --git a/pkg/openstackbaremetalset/baremetalhost.go b/pkg/openstackbaremetalset/baremetalhost.go index 93b2dd4..6b1fc9a 100644 --- a/pkg/openstackbaremetalset/baremetalhost.go +++ b/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(), From 1c0bf8bc1f10c12e31a147eaaddfaa396353edf2 Mon Sep 17 00:00:00 2001 From: rabi Date: Sat, 28 Sep 2024 15:41:36 +0530 Subject: [PATCH 2/2] Add functional tests for bmh selection Signed-off-by: rabi --- tests/functional/base_test.go | 97 ++++++++++++ .../openstackbaremetalset_controller_test.go | 3 +- .../openstackbaremetalset_webhook_test.go | 147 ++++++++++++++++++ 3 files changed, 246 insertions(+), 1 deletion(-) create mode 100644 tests/functional/openstackbaremetalset_webhook_test.go diff --git a/tests/functional/base_test.go b/tests/functional/base_test.go index 577540f..a79e4c6 100644 --- a/tests/functional/base_test.go +++ b/tests/functional/base_test.go @@ -21,6 +21,7 @@ import ( metal3v1 "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1" . "github.com/onsi/gomega" //revive:disable:dot-imports "github.com/openstack-k8s-operators/lib-common/modules/common/condition" + "github.com/openstack-k8s-operators/lib-common/modules/common/util" baremetalv1 "github.com/openstack-k8s-operators/openstack-baremetal-operator/api/v1beta1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -55,6 +56,7 @@ func DefaultBaremetalSetSpec(name types.NamespacedName, withProvInterface bool) "ctlPlaneIP": "10.0.0.1", }, }, + "bmhLabelSelector": map[string]string{"app": "openstack"}, "deploymentSSHSecret": "mysecret", "ctlplaneInterface": "eth0", "bmhNamespace": name.Namespace, @@ -71,6 +73,65 @@ func DefaultBaremetalSetSpec(name types.NamespacedName, withProvInterface bool) } +// Build BaremetalSetSpec struct for two nodes +func TwoNodeBaremetalSetSpec(namespace string) map[string]interface{} { + spec := map[string]interface{}{ + "baremetalHosts": map[string]interface{}{ + "compute-0": map[string]interface{}{ + "ctlPlaneIP": "10.0.0.1", + }, + "compute-1": map[string]interface{}{ + "ctlPlaneIP": "10.0.0.1", + }, + }, + "bmhLabelSelector": map[string]string{"app": "openstack"}, + "deploymentSSHSecret": "mysecret", + "ctlplaneInterface": "eth0", + "bmhNamespace": namespace, + } + return spec +} + +func TwoNodeBaremetalSetSpecWithNodeLabel(namespace string) map[string]interface{} { + spec := map[string]interface{}{ + "baremetalHosts": map[string]interface{}{ + "compute-0": map[string]interface{}{ + "ctlPlaneIP": "10.0.0.1", + "bmhLabelSelector": map[string]string{"nodeName": "compute-0"}, + }, + "compute-1": map[string]interface{}{ + "ctlPlaneIP": "10.0.0.1", + "bmhLabelSelector": map[string]string{"nodeName": "compute-1"}, + }, + }, + "bmhLabelSelector": map[string]string{"app": "openstack"}, + "deploymentSSHSecret": "mysecret", + "ctlplaneInterface": "eth0", + "bmhNamespace": namespace, + } + return spec +} + +func TwoNodeBaremetalSetSpecWithWrongNodeLabel(namespace string) map[string]interface{} { + spec := map[string]interface{}{ + "baremetalHosts": map[string]interface{}{ + "compute-0": map[string]interface{}{ + "ctlPlaneIP": "10.0.0.1", + "bmhLabelSelector": map[string]string{"nodeName": "compute-0"}, + }, + "compute-1": map[string]interface{}{ + "ctlPlaneIP": "10.0.0.1", + "bmhLabelSelector": map[string]string{"nodeName": "compute-2"}, + }, + }, + "bmhLabelSelector": map[string]string{"app": "openstack"}, + "deploymentSSHSecret": "mysecret", + "ctlplaneInterface": "eth0", + "bmhNamespace": namespace, + } + return spec +} + // Default BMH Template with preset values func DefaultBMHTemplate(name types.NamespacedName) map[string]interface{} { return map[string]interface{}{ @@ -79,6 +140,35 @@ func DefaultBMHTemplate(name types.NamespacedName) map[string]interface{} { "metadata": map[string]interface{}{ "name": name.Name, "namespace": name.Namespace, + "labels": map[string]string{ + "app": "openstack", + }, + "annotations": map[string]interface{}{ + "inspect.metal3.io": "disabled", + }, + }, + "spec": map[string]interface{}{ + "bmc": map[string]interface{}{ + "address": "fake_address", + "credentialsName": "fake_credential", + }, + "bootMACAddress": "52:54:00:39:a7:44", + "bootMode": "UEFI", + "online": false, + }, + } +} + +// Default BMH Template with preset values +func BMHTemplateWithNodeLabel(name types.NamespacedName, nodeLabel map[string]string) map[string]interface{} { + labels := util.MergeMaps(map[string]string{"app": "openstack"}, nodeLabel) + return map[string]interface{}{ + "apiVersion": "metal3.io/v1alpha1", + "kind": "BareMetalHost", + "metadata": map[string]interface{}{ + "name": name.Name, + "namespace": name.Namespace, + "labels": labels, "annotations": map[string]interface{}{ "inspect.metal3.io": "disabled", }, @@ -111,6 +201,13 @@ func CreateBaremetalHost(name types.NamespacedName) *unstructured.Unstructured { return th.CreateUnstructured(instance) } +// Create BaremetalHost with NodeLabel +func CreateBaremetalHostWithNodeLabel(name types.NamespacedName, + nodeLabel map[string]string) *unstructured.Unstructured { + instance := BMHTemplateWithNodeLabel(name, nodeLabel) + return th.CreateUnstructured(instance) +} + // Get BaremetalHost func GetBaremetalHost(name types.NamespacedName) *metal3v1.BareMetalHost { instance := &metal3v1.BareMetalHost{} diff --git a/tests/functional/openstackbaremetalset_controller_test.go b/tests/functional/openstackbaremetalset_controller_test.go index bbb702b..411359c 100644 --- a/tests/functional/openstackbaremetalset_controller_test.go +++ b/tests/functional/openstackbaremetalset_controller_test.go @@ -69,6 +69,7 @@ var _ = Describe("BaremetalSet Test", func() { UserData: nil, NetworkData: nil, PreprovisioningNetworkDataName: "", + BmhLabelSelector: nil, }, }, OSImage: "", @@ -85,7 +86,7 @@ var _ = Describe("BaremetalSet Test", func() { CtlplaneGateway: "", CtlplaneNetmask: "255.255.255.0", BmhNamespace: baremetalSetName.Namespace, - BmhLabelSelector: nil, + BmhLabelSelector: map[string]string{"app": "openstack"}, HardwareReqs: baremetalv1.HardwareReqs{ CPUReqs: baremetalv1.CPUReqs{ Arch: "", diff --git a/tests/functional/openstackbaremetalset_webhook_test.go b/tests/functional/openstackbaremetalset_webhook_test.go new file mode 100644 index 0000000..00038eb --- /dev/null +++ b/tests/functional/openstackbaremetalset_webhook_test.go @@ -0,0 +1,147 @@ +package functional + +import ( + "errors" + + metal3v1 "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1" + . "github.com/onsi/ginkgo/v2" //revive:disable:dot-imports + . "github.com/onsi/gomega" //revive:disable:dot-imports + v1 "k8s.io/api/core/v1" + k8s_errors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/types" + + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" +) + +var _ = Describe("OpenStackBaremetalSet Webhook", func() { + + var baremetalSetName types.NamespacedName + var bmhName types.NamespacedName + var bmhName1 types.NamespacedName + + BeforeEach(func() { + baremetalSetName = types.NamespacedName{ + Name: "edpm-compute-baremetalset", + Namespace: namespace, + } + bmhName = types.NamespacedName{ + Name: "compute-0", + Namespace: namespace, + } + bmhName1 = types.NamespacedName{ + Name: "compute-1", + Namespace: namespace, + } + }) + When("When creating BaremetalSet", func() { + BeforeEach(func() { + DeferCleanup(th.DeleteInstance, CreateBaremetalHost(bmhName)) + bmh := GetBaremetalHost(bmhName) + Eventually(func(g Gomega) { + // OpenStackBaremetalSet has the same name as OpenStackDataPlaneNodeSet + bmh.Status.Provisioning.State = metal3v1.StateAvailable + g.Expect(th.K8sClient.Status().Update(th.Ctx, bmh)).To(Succeed()) + + }, th.Timeout, th.Interval).Should(Succeed()) + + }) + It("It should not fail if enough bmhs are available", func() { + spec := DefaultBaremetalSetSpec(baremetalSetName, false) + object := DefaultBaremetalSetTemplate(baremetalSetName, spec) + unstructuredObj := &unstructured.Unstructured{Object: object} + _, err := controllerutil.CreateOrPatch( + th.Ctx, th.K8sClient, unstructuredObj, func() error { return nil }) + Expect(err).ShouldNot(HaveOccurred()) + }) + + It("It should fail if not enough bmhs are available", func() { + spec := TwoNodeBaremetalSetSpec(baremetalSetName.Namespace) + object := DefaultBaremetalSetTemplate(baremetalSetName, spec) + unstructuredObj := &unstructured.Unstructured{Object: object} + _, err := controllerutil.CreateOrPatch( + th.Ctx, th.K8sClient, unstructuredObj, func() error { return nil }) + Expect(err).Should(HaveOccurred()) + var statusError *k8s_errors.StatusError + Expect(errors.As(err, &statusError)).To(BeTrue()) + Expect(statusError.ErrStatus.Message).To( + ContainSubstring( + "unable to find 2 requested BaremetalHosts"), + ) + }) + + It("It should fail when suffiecient BMHs are not offine", func() { + bmh := GetBaremetalHost(bmhName) + Eventually(func(g Gomega) { + // OpenStackBaremetalSet has the same name as OpenStackDataPlaneNodeSet + bmh.Spec.Online = true + g.Expect(th.K8sClient.Status().Update(th.Ctx, bmh)).To(Succeed()) + + }, th.Timeout, th.Interval).Should(Succeed()) + spec := DefaultBaremetalSetSpec(baremetalSetName, false) + object := DefaultBaremetalSetTemplate(baremetalSetName, spec) + unstructuredObj := &unstructured.Unstructured{Object: object} + _, err := controllerutil.CreateOrPatch( + th.Ctx, th.K8sClient, unstructuredObj, func() error { return nil }) + Expect(err).ShouldNot(HaveOccurred()) + }) + It("It should fail when all BMHs have consumerRef", func() { + bmh := GetBaremetalHost(bmhName) + Eventually(func(g Gomega) { + // OpenStackBaremetalSet has the same name as OpenStackDataPlaneNodeSet + bmh.Spec.ConsumerRef = &v1.ObjectReference{} + g.Expect(th.K8sClient.Status().Update(th.Ctx, bmh)).To(Succeed()) + + }, th.Timeout, th.Interval).Should(Succeed()) + spec := DefaultBaremetalSetSpec(baremetalSetName, false) + object := DefaultBaremetalSetTemplate(baremetalSetName, spec) + unstructuredObj := &unstructured.Unstructured{Object: object} + _, err := controllerutil.CreateOrPatch( + th.Ctx, th.K8sClient, unstructuredObj, func() error { return nil }) + Expect(err).ShouldNot(HaveOccurred()) + }) + }) + + When("When creating BaremetalSet with nodeSelector", func() { + BeforeEach(func() { + DeferCleanup(th.DeleteInstance, CreateBaremetalHostWithNodeLabel( + bmhName, map[string]string{"nodeName": "compute-0"})) + bmh := GetBaremetalHost(bmhName) + DeferCleanup(th.DeleteInstance, CreateBaremetalHostWithNodeLabel( + bmhName1, map[string]string{"nodeName": "compute-1"})) + bmh1 := GetBaremetalHost(bmhName1) + Eventually(func(g Gomega) { + // OpenStackBaremetalSet has the same name as OpenStackDataPlaneNodeSet + bmh.Status.Provisioning.State = metal3v1.StateAvailable + bmh1.Status.Provisioning.State = metal3v1.StateAvailable + g.Expect(th.K8sClient.Status().Update(th.Ctx, bmh)).To(Succeed()) + g.Expect(th.K8sClient.Status().Update(th.Ctx, bmh1)).To(Succeed()) + }, th.Timeout, th.Interval).Should(Succeed()) + + }) + + It("It should pass if node labels match", func() { + spec := TwoNodeBaremetalSetSpecWithNodeLabel(baremetalSetName.Namespace) + object := DefaultBaremetalSetTemplate(baremetalSetName, spec) + unstructuredObj := &unstructured.Unstructured{Object: object} + _, err := controllerutil.CreateOrPatch( + th.Ctx, th.K8sClient, unstructuredObj, func() error { return nil }) + Expect(err).ShouldNot(HaveOccurred()) + }) + + It("It should fail if node labels don't match", func() { + spec := TwoNodeBaremetalSetSpecWithWrongNodeLabel(baremetalSetName.Namespace) + object := DefaultBaremetalSetTemplate(baremetalSetName, spec) + unstructuredObj := &unstructured.Unstructured{Object: object} + _, err := controllerutil.CreateOrPatch( + th.Ctx, th.K8sClient, unstructuredObj, func() error { return nil }) + Expect(err).Should(HaveOccurred()) + var statusError *k8s_errors.StatusError + Expect(errors.As(err, &statusError)).To(BeTrue()) + Expect(statusError.ErrStatus.Message).To( + ContainSubstring( + "unable to find 2 requested BaremetalHosts"), + ) + }) + }) +})