diff --git a/api/v1alpha1/hivelocitymachine_types.go b/api/v1alpha1/hivelocitymachine_types.go index bb55d6ede..e870df845 100644 --- a/api/v1alpha1/hivelocitymachine_types.go +++ b/api/v1alpha1/hivelocitymachine_types.go @@ -24,6 +24,7 @@ import ( "github.com/hivelocity/cluster-api-provider-hivelocity/pkg/services/hivelocity/hvtag" hv "github.com/hivelocity/hivelocity-client-go/client" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/selection" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" capierrors "sigs.k8s.io/cluster-api/errors" ) @@ -91,8 +92,9 @@ type HivelocityMachineSpec struct { // +optional ProviderID *string `json:"providerID,omitempty"` - // Type is the Hivelocity Machine Type for this machine. - Type HivelocityDeviceType `json:"type"` + // DeviceSelector can be used to limit the set of devices that this HivelocityMachine can claim. + // +optional + DeviceSelector DeviceSelector `json:"deviceSelector,omitempty"` // ImageName is the reference to the Machine Image from which to create the device. // +kubebuilder:validation:MinLength=1 @@ -103,6 +105,25 @@ type HivelocityMachineSpec struct { Status ControllerGeneratedStatus `json:"status,omitempty"` } +// DeviceSelector specifies matching criteria for tags on devices. +// This is used to target a specific set of devices that can be claimed by the HivelocityMachine. +type DeviceSelector struct { + // Key/value pairs of labels that must exist on a chosen Device + // +optional + MatchLabels map[string]string `json:"matchLabels,omitempty"` + + // MatchExpressions match expressions that must be true on a chosen Device + // +optional + MatchExpressions []DeviceSelectorRequirement `json:"matchExpressions,omitempty"` +} + +// DeviceSelectorRequirement defines a requirement used for MatchExpressions to select device. +type DeviceSelectorRequirement struct { + Key string `json:"key"` + Operator selection.Operator `json:"operator"` + Values []string `json:"values"` +} + // ControllerGeneratedStatus contains all status information which is important to persist. type ControllerGeneratedStatus struct { // Information tracked by the provisioner. diff --git a/api/v1alpha1/hivelocitymachine_webhook.go b/api/v1alpha1/hivelocitymachine_webhook.go index 759f2c20a..e9c2874b9 100644 --- a/api/v1alpha1/hivelocitymachine_webhook.go +++ b/api/v1alpha1/hivelocitymachine_webhook.go @@ -64,10 +64,10 @@ func (r *HivelocityMachine) ValidateUpdate(oldRaw runtime.Object) error { var allErrs field.ErrorList - // Type is immutable - if !reflect.DeepEqual(old.Spec.Type, r.Spec.Type) { + // DeviceSelector is immutable + if !reflect.DeepEqual(old.Spec.DeviceSelector, r.Spec.DeviceSelector) { allErrs = append(allErrs, - field.Invalid(field.NewPath("spec", "type"), r.Spec.Type, "field is immutable"), + field.Invalid(field.NewPath("spec", "DeviceSelector"), r.Spec.DeviceSelector, "field is immutable"), ) } diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 554423ec6..67d4c2c63 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -47,6 +47,55 @@ func (in *ControllerGeneratedStatus) DeepCopy() *ControllerGeneratedStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DeviceSelector) DeepCopyInto(out *DeviceSelector) { + *out = *in + if in.MatchLabels != nil { + in, out := &in.MatchLabels, &out.MatchLabels + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + if in.MatchExpressions != nil { + in, out := &in.MatchExpressions, &out.MatchExpressions + *out = make([]DeviceSelectorRequirement, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DeviceSelector. +func (in *DeviceSelector) DeepCopy() *DeviceSelector { + if in == nil { + return nil + } + out := new(DeviceSelector) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DeviceSelectorRequirement) DeepCopyInto(out *DeviceSelectorRequirement) { + *out = *in + if in.Values != nil { + in, out := &in.Values, &out.Values + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DeviceSelectorRequirement. +func (in *DeviceSelectorRequirement) DeepCopy() *DeviceSelectorRequirement { + if in == nil { + return nil + } + out := new(DeviceSelectorRequirement) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *HivelocityCluster) DeepCopyInto(out *HivelocityCluster) { *out = *in @@ -319,6 +368,7 @@ func (in *HivelocityMachineSpec) DeepCopyInto(out *HivelocityMachineSpec) { *out = new(string) **out = **in } + in.DeviceSelector.DeepCopyInto(&out.DeviceSelector) in.Status.DeepCopyInto(&out.Status) } diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_hivelocitymachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_hivelocitymachines.yaml index d639ba587..10322ec84 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_hivelocitymachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_hivelocitymachines.yaml @@ -76,6 +76,41 @@ spec: spec: description: HivelocityMachineSpec defines the desired state of HivelocityMachine. properties: + deviceSelector: + description: DeviceSelector can be used to limit the set of devices + that this HivelocityMachine can claim. + properties: + matchExpressions: + description: MatchExpressions match expressions that must be true + on a chosen Device + items: + description: DeviceSelectorRequirement defines a requirement + used for MatchExpressions to select device. + properties: + key: + type: string + operator: + description: Operator represents a key/field's relationship + to value(s). See labels.Requirement and fields.Requirement + for more details. + type: string + values: + items: + type: string + type: array + required: + - key + - operator + - values + type: object + type: array + matchLabels: + additionalProperties: + type: string + description: Key/value pairs of labels that must exist on a chosen + Device + type: object + type: object imageName: description: ImageName is the reference to the Machine Image from which to create the device. @@ -97,19 +132,8 @@ spec: description: Information tracked by the provisioner. type: string type: object - type: - description: Type is the Hivelocity Machine Type for this machine. - enum: - - pool - - hvCustom - - hvControlPlane - - hvWorker - - e2eControlPlane - - e2eWorker - type: string required: - imageName - - type type: object status: description: HivelocityMachineStatus defines the observed state of HivelocityMachine. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_hivelocitymachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_hivelocitymachinetemplates.yaml index 98a900731..b7306ffe0 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_hivelocitymachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_hivelocitymachinetemplates.yaml @@ -78,6 +78,41 @@ spec: description: Spec is the specification of the desired behavior of the machine. properties: + deviceSelector: + description: DeviceSelector can be used to limit the set of + devices that this HivelocityMachine can claim. + properties: + matchExpressions: + description: MatchExpressions match expressions that must + be true on a chosen Device + items: + description: DeviceSelectorRequirement defines a requirement + used for MatchExpressions to select device. + properties: + key: + type: string + operator: + description: Operator represents a key/field's relationship + to value(s). See labels.Requirement and fields.Requirement + for more details. + type: string + values: + items: + type: string + type: array + required: + - key + - operator + - values + type: object + type: array + matchLabels: + additionalProperties: + type: string + description: Key/value pairs of labels that must exist + on a chosen Device + type: object + type: object imageName: description: ImageName is the reference to the Machine Image from which to create the device. @@ -99,20 +134,8 @@ spec: description: Information tracked by the provisioner. type: string type: object - type: - description: Type is the Hivelocity Machine Type for this - machine. - enum: - - pool - - hvCustom - - hvControlPlane - - hvWorker - - e2eControlPlane - - e2eWorker - type: string required: - imageName - - type type: object required: - spec diff --git a/controllers/hivelocitycluster_controller.go b/controllers/hivelocitycluster_controller.go index d263564c9..5bb5f1490 100644 --- a/controllers/hivelocitycluster_controller.go +++ b/controllers/hivelocitycluster_controller.go @@ -185,6 +185,7 @@ func (r *HivelocityClusterReconciler) reconcileNormal(ctx context.Context, clust if hvCluster.Spec.ControlPlaneEndpoint.Host == "" { var hmt = infrav1.HivelocityMachineTemplate{} name := hvCluster.Name + "-control-plane" + err := r.Client.Get(ctx, client.ObjectKey{ Namespace: hvCluster.ObjectMeta.Namespace, Name: name, @@ -192,16 +193,12 @@ func (r *HivelocityClusterReconciler) reconcileNormal(ctx context.Context, clust if err != nil { return ctrl.Result{}, fmt.Errorf("failed to get HivelocityMachineTemplate %q: %w", name, err) } - machineType := hmt.Spec.Template.Spec.Type - if machineType == "" { - return ctrl.Result{}, fmt.Errorf("Spec.Template.Spec.Type of HivelocityMachineTemplate %q is empty", name) - } - hvDevice, err := device.GetFirstFreeDevice(ctx, clusterScope.HVClient, machineType, hvCluster, "") + + hvDevice, err := device.GetFirstFreeDevice(ctx, clusterScope.HVClient, hmt.Spec.Template.Spec, hvCluster) if err != nil { return ctrl.Result{}, fmt.Errorf("device.GetFirstFreeDevice() failed: %w", err) } - log.Info(fmt.Sprintf("Setting hvCluster.Spec.ControlPlaneEndpoint.Host to %q (machineType=%s).", - hvDevice.PrimaryIp, machineType)) + log.Info(fmt.Sprintf("Setting hvCluster.Spec.ControlPlaneEndpoint.Host to %q", hvDevice.PrimaryIp)) hvCluster.Spec.ControlPlaneEndpoint.Host = hvDevice.PrimaryIp hvCluster.Spec.ControlPlaneEndpoint.Port = 6443 } diff --git a/controllers/hivelocitycluster_controller_test.go b/controllers/hivelocitycluster_controller_test.go index 23c45754c..346183fbd 100644 --- a/controllers/hivelocitycluster_controller_test.go +++ b/controllers/hivelocitycluster_controller_test.go @@ -216,7 +216,11 @@ var _ = Describe("Hivelocity ClusterReconciler", func() { }, Spec: infrav1.HivelocityMachineSpec{ ImageName: "Ubuntu 20.x", - Type: "pool", + DeviceSelector: infrav1.DeviceSelector{ + MatchLabels: map[string]string{ + "deviceType": "pool", + }, + }, }, } Expect(testEnv.Create(ctx, hvMachine)).To(Succeed()) diff --git a/controllers/hivelocitymachine_controller_test.go b/controllers/hivelocitymachine_controller_test.go index c4eff26a0..b7998bf7c 100644 --- a/controllers/hivelocitymachine_controller_test.go +++ b/controllers/hivelocitymachine_controller_test.go @@ -148,7 +148,11 @@ var _ = Describe("HivelocityMachineReconciler", func() { }, Spec: infrav1.HivelocityMachineSpec{ ImageName: "Ubuntu 20.x", - Type: "hvCustom", + DeviceSelector: infrav1.DeviceSelector{ + MatchLabels: map[string]string{ + "deviceType": "hvCustom", + }, + }, }, } @@ -231,7 +235,7 @@ var _ = Describe("HivelocityMachineReconciler", func() { device, err := hvClient.GetDevice(ctx, mock.FreeDeviceID) Expect(err).ShouldNot(HaveOccurred()) Expect(device.Tags).Should(BeEquivalentTo([]string{ - "caphv-device-type=hvCustom", + "caphvlabel:deviceType=hvCustom", "caphv-use=allow", "caphv-cluster-hv-test1=owned", "caphv-cluster-name=hv-test1", @@ -337,7 +341,11 @@ var _ = Describe("Hivelocity secret", func() { }, Spec: infrav1.HivelocityMachineSpec{ ImageName: "Ubuntu 20.x", - Type: "hvCustom", + DeviceSelector: infrav1.DeviceSelector{ + MatchLabels: map[string]string{ + "deviceType": "hvCustom", + }, + }, }, } Expect(testEnv.Create(ctx, hivelocityMachine)).To(Succeed()) @@ -427,7 +435,6 @@ var _ = Describe("HivelocityMachine validation", func() { }, Spec: infrav1.HivelocityMachineSpec{ ImageName: "Ubuntu 20.x", - Type: "hvCustom", }, } }) @@ -436,11 +443,6 @@ var _ = Describe("HivelocityMachine validation", func() { Expect(testEnv.Cleanup(ctx, testNs, hvMachine)).To(Succeed()) }) - It("should fail with wrong type", func() { - hvMachine.Spec.Type = "wrong-type" - Expect(testEnv.Create(ctx, hvMachine)).ToNot(Succeed()) - }) - It("should fail without imageName", func() { hvMachine.Spec.ImageName = "" Expect(testEnv.Create(ctx, hvMachine)).ToNot(Succeed()) diff --git a/pkg/labels/labels.go b/pkg/labels/labels.go new file mode 100644 index 000000000..912862d99 --- /dev/null +++ b/pkg/labels/labels.go @@ -0,0 +1,62 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package labels + +import ( + "strings" +) + +// Tags is a slice of string. It implements https://pkg.go.dev/k8s.io/apimachinery/pkg/labels#Labels interface. +type Tags []string + +// Has returns whether the provided label exists in the slice. +// It implements the Has function of the Labels interface. +func (ls Tags) Has(label string) bool { + for _, tag := range ls { + if !strings.HasPrefix(tag, "caphvlabel:") { + continue + } + + trimmedTag := strings.TrimPrefix(tag, "caphvlabel:") + splittedTrimTag := strings.Split(trimmedTag, "=") + + if len(splittedTrimTag) == 2 && splittedTrimTag[0] == label { + return true + } + } + + return false +} + +// Get returns the value in the slice for the provided label. +// It implements the Has function of the Labels interface. +func (ls Tags) Get(label string) string { + for _, tag := range ls { + if !strings.HasPrefix(tag, "caphvlabel:") { + continue + } + + trimmedTag := strings.TrimPrefix(tag, "caphvlabel:") + splittedTrimTag := strings.Split(trimmedTag, "=") + + if len(splittedTrimTag) == 2 && splittedTrimTag[0] == label { + return splittedTrimTag[1] + } + } + + return "" +} diff --git a/pkg/services/hivelocity/client/mock/mock_client.go b/pkg/services/hivelocity/client/mock/mock_client.go index d640ea1b1..df4e8613d 100644 --- a/pkg/services/hivelocity/client/mock/mock_client.go +++ b/pkg/services/hivelocity/client/mock/mock_client.go @@ -40,11 +40,8 @@ const ( // FreeDevice is a device which is not associated with a node. var FreeDevice = hv.BareMetalDevice{ - Hostname: "host-FreeDevice", - Tags: []string{ - hvtag.DeviceTag{Key: hvtag.DeviceTagKeyDeviceType, Value: "hvCustom"}.ToString(), - hvtag.DeviceTag{Key: hvtag.DeviceTagKeyCAPHVUseAllowed, Value: "allow"}.ToString(), - }, + Hostname: "host-FreeDevice", + Tags: []string{"caphvlabel:deviceType=hvCustom", "caphv-use=allow"}, DeviceId: FreeDeviceID, PowerStatus: "ON", OsName: defaultImage, @@ -52,11 +49,8 @@ var FreeDevice = hv.BareMetalDevice{ // FreeDevicePool1 is a device which is not associated with a node. var FreeDevicePool1 = hv.BareMetalDevice{ - Hostname: "host-FreeDevice", - Tags: []string{ - hvtag.DeviceTag{Key: hvtag.DeviceTagKeyDeviceType, Value: "pool"}.ToString(), - hvtag.DeviceTag{Key: hvtag.DeviceTagKeyCAPHVUseAllowed, Value: "allow"}.ToString(), - }, + Hostname: "host-FreeDevice", + Tags: []string{"caphvlabel:deviceType=pool", "caphv-use=allow"}, DeviceId: 51, PowerStatus: "ON", OsName: defaultImage, @@ -64,11 +58,8 @@ var FreeDevicePool1 = hv.BareMetalDevice{ // FreeDevicePool2 is a device which is not associated with a node. var FreeDevicePool2 = hv.BareMetalDevice{ - Hostname: "host-FreeDevice", - Tags: []string{ - hvtag.DeviceTag{Key: hvtag.DeviceTagKeyDeviceType, Value: "pool"}.ToString(), - hvtag.DeviceTag{Key: hvtag.DeviceTagKeyCAPHVUseAllowed, Value: "allow"}.ToString(), - }, + Hostname: "host-FreeDevice", + Tags: []string{"caphvlabel:deviceType=pool", "caphv-use=allow"}, DeviceId: 52, PowerStatus: "ON", OsName: defaultImage, @@ -76,11 +67,8 @@ var FreeDevicePool2 = hv.BareMetalDevice{ // FreeDevicePool3 is a device which is not associated with a node. var FreeDevicePool3 = hv.BareMetalDevice{ - Hostname: "host-FreeDevice", - Tags: []string{ - hvtag.DeviceTag{Key: hvtag.DeviceTagKeyDeviceType, Value: "pool"}.ToString(), - hvtag.DeviceTag{Key: hvtag.DeviceTagKeyCAPHVUseAllowed, Value: "allow"}.ToString(), - }, + Hostname: "host-FreeDevice", + Tags: []string{"caphvlabel:deviceType=pool", "caphv-use=allow"}, DeviceId: 53, PowerStatus: "ON", OsName: defaultImage, @@ -114,11 +102,22 @@ var NoTagsDevice = hv.BareMetalDevice{ // CaphNotAllowDevice is a device which is has no "caphv-use=allow" tag. var CaphNotAllowDevice = hv.BareMetalDevice{ - Hostname: "host-FreeDevice", + Hostname: "host-FreeDevice", + Tags: []string{"caphvlabel:deviceType=hvCustom"}, + DeviceId: FreeDeviceID, + PowerStatus: "ON", + OsName: defaultImage, +} + +// MultiLabelsDevice is a device which has multiple tags. +var MultiLabelsDevice = hv.BareMetalDevice{ + Hostname: "host3-unused", Tags: []string{ - hvtag.DeviceTag{Key: hvtag.DeviceTagKeyDeviceType, Value: "hvCustom"}.ToString(), + "caphvlabel:foo1=bar1", + "caphvlabel:foo2=bar2", + "caphv-use=allow", }, - DeviceId: FreeDeviceID, + DeviceId: NoTagsDeviceID, PowerStatus: "ON", OsName: defaultImage, } diff --git a/pkg/services/hivelocity/client/mock/mock_client_test.go b/pkg/services/hivelocity/client/mock/mock_client_test.go index a559707b7..03bb7561d 100644 --- a/pkg/services/hivelocity/client/mock/mock_client_test.go +++ b/pkg/services/hivelocity/client/mock/mock_client_test.go @@ -53,7 +53,7 @@ func Test_NewMockedHVClientFactory(t *testing.T) { ctx := context.Background() device, err := client.GetDevice(ctx, FreeDeviceID) require.NoError(t, err) - require.ElementsMatch(t, device.Tags, []string{"caphv-device-type=hvCustom", "caphv-use=allow"}) + require.ElementsMatch(t, device.Tags, []string{"caphvlabel:deviceType=hvCustom", "caphv-use=allow"}) err = client.SetDeviceTags(ctx, FreeDeviceID, []string{"new-tag"}) require.NoError(t, err) @@ -70,7 +70,7 @@ func Test_NewMockedHVClientFactory(t *testing.T) { clientNewF := factoryNewF.NewClient("dummy-key") device, err = clientNewF.GetDevice(ctx, FreeDeviceID) require.NoError(t, err) - require.ElementsMatch(t, device.Tags, []string{"caphv-device-type=hvCustom", "caphv-use=allow"}) + require.ElementsMatch(t, device.Tags, []string{"caphvlabel:deviceType=hvCustom", "caphv-use=allow"}) } func Test_ProvisionDevice(t *testing.T) { diff --git a/pkg/services/hivelocity/device/device.go b/pkg/services/hivelocity/device/device.go index c262bfdfc..a3229635f 100644 --- a/pkg/services/hivelocity/device/device.go +++ b/pkg/services/hivelocity/device/device.go @@ -27,12 +27,15 @@ import ( "time" infrav1 "github.com/hivelocity/cluster-api-provider-hivelocity/api/v1alpha1" + hvlabels "github.com/hivelocity/cluster-api-provider-hivelocity/pkg/labels" "github.com/hivelocity/cluster-api-provider-hivelocity/pkg/scope" hvclient "github.com/hivelocity/cluster-api-provider-hivelocity/pkg/services/hivelocity/client" "github.com/hivelocity/cluster-api-provider-hivelocity/pkg/services/hivelocity/hvtag" "github.com/hivelocity/cluster-api-provider-hivelocity/pkg/utils" hv "github.com/hivelocity/hivelocity-client-go/client" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" capierrors "sigs.k8s.io/cluster-api/errors" "sigs.k8s.io/cluster-api/util/conditions" @@ -107,8 +110,7 @@ func (s *Service) actionAssociateDevice(ctx context.Context) actionResult { log := s.scope.Logger.WithValues("function", "actionAssociateDevice") log.V(1).Info("Started function") - device, err := GetFirstFreeDevice(ctx, s.scope.HVClient, s.scope.HivelocityMachine.Spec.Type, - s.scope.HivelocityCluster, s.scope.Name()) + device, err := GetFirstFreeDevice(ctx, s.scope.HVClient, s.scope.HivelocityMachine.Spec, s.scope.HivelocityCluster) if err != nil { s.handleRateLimitExceeded(err, "ListDevices") return actionError{err: fmt.Errorf("failed to find available device: %w", err)} @@ -123,9 +125,8 @@ func (s *Service) actionAssociateDevice(ctx context.Context) actionResult { "no available device found", ) record.Warnf(s.scope.HivelocityMachine, "NoFreeDeviceFound", "Failed to find free device") - return actionContinue{delay: time.Minute} + return actionContinue{delay: 30 * time.Second} } - conditions.Delete(s.scope.HivelocityMachine, infrav1.DeviceAssociateSucceededCondition) // associate this device with the machine object by setting tags @@ -149,9 +150,7 @@ func (s *Service) actionAssociateDevice(ctx context.Context) actionResult { } // GetFirstFreeDevice finds the first free matching device. The parameter machineName is optional. -func GetFirstFreeDevice(ctx context.Context, hvclient hvclient.Client, machineType infrav1.HivelocityDeviceType, - hvCluster *infrav1.HivelocityCluster, machineName string, -) (*hv.BareMetalDevice, error) { +func GetFirstFreeDevice(ctx context.Context, hvclient hvclient.Client, hvMachineSpec infrav1.HivelocityMachineSpec, hvCluster *infrav1.HivelocityCluster) (*hv.BareMetalDevice, error) { // list all devices devices, err := hvclient.ListDevices(ctx) if err != nil { @@ -168,13 +167,15 @@ func GetFirstFreeDevice(ctx context.Context, hvclient hvclient.Client, machineTy return devices[i].DeviceId < devices[j].DeviceId }) - device := findAvailableDeviceFromList(devices, machineType, hvCluster.Name) + device := findAvailableDeviceFromList(devices, hvMachineSpec.DeviceSelector, hvCluster.Name) + return device, nil } -func findAvailableDeviceFromList(devices []hv.BareMetalDevice, deviceType infrav1.HivelocityDeviceType, clusterName string) *hv.BareMetalDevice { - for _, device := range devices { +func findAvailableDeviceFromList(devices []hv.BareMetalDevice, deviceSelector infrav1.DeviceSelector, clusterName string) *hv.BareMetalDevice { + labelSelector := getLabelSelector(deviceSelector) + for _, device := range devices { // Skip if caphv-permanent-error exists _, err := hvtag.PermanentErrorTagFromList(device.Tags) if err == nil { @@ -193,14 +194,7 @@ func findAvailableDeviceFromList(devices []hv.BareMetalDevice, deviceType infrav continue } - deviceTypeTag, err := hvtag.DeviceTypeTagFromList(device.Tags) - if err != nil { - // No device type has been set, don't use it. - continue - } - - // Ignore if has wrong device type - if deviceTypeTag.Value != string(deviceType) { + if !labelSelector.Matches(hvlabels.Tags(device.Tags)) { continue } @@ -225,6 +219,27 @@ func findAvailableDeviceFromList(devices []hv.BareMetalDevice, deviceType infrav return nil } +func getLabelSelector(deviceSelector infrav1.DeviceSelector) labels.Selector { + labelSelector := labels.NewSelector() + var reqs labels.Requirements + + for labelKey, labelVal := range deviceSelector.MatchLabels { + r, err := labels.NewRequirement(labelKey, selection.Equals, []string{labelVal}) + if err == nil { // ignore invalid host selector + reqs = append(reqs, *r) + } + } + for _, req := range deviceSelector.MatchExpressions { + lowercaseOperator := selection.Operator(strings.ToLower(string(req.Operator))) + r, err := labels.NewRequirement(req.Key, lowercaseOperator, req.Values) + if err == nil { // ignore invalid host selector + reqs = append(reqs, *r) + } + } + + return labelSelector.Add(reqs...) +} + // actionVerifyAssociate verifies that the HV device has actually been associated to this machine and only this. // Checking whether there are other machines also associated avoids situations where machines are selected at the same time. func (s *Service) actionVerifyAssociate(ctx context.Context) actionResult { diff --git a/pkg/services/hivelocity/device/device_test.go b/pkg/services/hivelocity/device/device_test.go index ee7a38866..deeae422a 100644 --- a/pkg/services/hivelocity/device/device_test.go +++ b/pkg/services/hivelocity/device/device_test.go @@ -34,25 +34,49 @@ func Test_findAvailableDeviceFromList(t *testing.T) { tests := []struct { description string devices []hv.BareMetalDevice - deviceType infrav1.HivelocityDeviceType + deviceType infrav1.DeviceSelector + wantMachine *hv.BareMetalDevice shouldNil bool }{ { - description: "checks no device selected if no deviceType matches", + description: "checks that no device is selected if no DeviceSelector matches", devices: []hv.BareMetalDevice{ mockclient.NoTagsDevice, mockclient.FreeDevice, }, - deviceType: "fooDeviceType", - shouldNil: true, + deviceType: infrav1.DeviceSelector{ + MatchLabels: map[string]string{ + "foo1": "bar1", + }, + }, + shouldNil: true, }, { description: "check no device selected if device has no caphv-use=allow tag", devices: []hv.BareMetalDevice{ mockclient.CaphNotAllowDevice, }, - deviceType: "hvCustom", - shouldNil: true, + deviceType: infrav1.DeviceSelector{ + MatchLabels: map[string]string{ + "deviceType": "hvCustom", + }, + }, + shouldNil: true, + }, + { + description: "selects device which has all the tags", + devices: []hv.BareMetalDevice{ + mockclient.MultiLabelsDevice, + mockclient.FreeDevice, + }, + deviceType: infrav1.DeviceSelector{ + MatchLabels: map[string]string{ + "foo1": "bar1", + "foo2": "bar2", + }, + }, + wantMachine: &mockclient.MultiLabelsDevice, + shouldNil: false, }, } @@ -64,6 +88,7 @@ func Test_findAvailableDeviceFromList(t *testing.T) { require.Nil(t, device) } else { require.NotNil(t, device) + require.Equal(t, test.wantMachine, device) } }) } diff --git a/pkg/services/hivelocity/hvtag/hvtag.go b/pkg/services/hivelocity/hvtag/hvtag.go index 7192c766e..ce13d909a 100644 --- a/pkg/services/hivelocity/hvtag/hvtag.go +++ b/pkg/services/hivelocity/hvtag/hvtag.go @@ -34,9 +34,6 @@ const ( // DeviceTagKeyCluster is the key for the name of the associated HivelocityCluster object. DeviceTagKeyCluster DeviceTagKey = "caphv-cluster-name" - // DeviceTagKeyDeviceType is the key for the device type that users can reference in HivelocityMachine.Spec. - DeviceTagKeyDeviceType DeviceTagKey = "caphv-device-type" - // DeviceTagKeyMachineType is the key for the machine type, i.e. worker, control_plane. DeviceTagKeyMachineType DeviceTagKey = "caphv-machine-type" @@ -73,7 +70,6 @@ var ( func (key DeviceTagKey) IsValid() bool { return key == DeviceTagKeyMachine || key == DeviceTagKeyCluster || - key == DeviceTagKeyDeviceType || key == DeviceTagKeyMachineType || key == DeviceTagKeyPermanentError || key == DeviceTagKeyCAPHVUseAllowed @@ -134,11 +130,6 @@ func ClusterTagFromList(tagList []string) (DeviceTag, error) { return DeviceTagFromList(DeviceTagKeyCluster, tagList) } -// DeviceTypeTagFromList returns the device type tag from a list of tag strings. -func DeviceTypeTagFromList(tagList []string) (DeviceTag, error) { - return DeviceTagFromList(DeviceTagKeyDeviceType, tagList) -} - // PermanentErrorTagFromList returns the permanent error tag from a list of tag strings. func PermanentErrorTagFromList(tagList []string) (DeviceTag, error) { return DeviceTagFromList(DeviceTagKeyPermanentError, tagList) @@ -214,7 +205,6 @@ func isEphemeralTag(tag string) bool { // ignore tags that are only allowed to be changed or removed by the user for _, keepPrefix := range []string{ string(DeviceTagKeyPermanentError), - string(DeviceTagKeyDeviceType), string(DeviceTagKeyCAPHVUseAllowed), } { if strings.HasPrefix(tag, keepPrefix+"=") { diff --git a/pkg/services/hivelocity/hvtag/hvtag_test.go b/pkg/services/hivelocity/hvtag/hvtag_test.go index 7b63dacc0..70e653028 100644 --- a/pkg/services/hivelocity/hvtag/hvtag_test.go +++ b/pkg/services/hivelocity/hvtag/hvtag_test.go @@ -82,10 +82,6 @@ var _ = Describe("Test DeviceTagKey.IsValid", func() { key: DeviceTagKeyMachine, expectIsValid: true, }), - Entry("device type key", testCaseDeviceTagKeyIsValid{ - key: DeviceTagKeyDeviceType, - expectIsValid: true, - }), Entry("machine type key", testCaseDeviceTagKeyIsValid{ key: DeviceTagKeyMachineType, expectIsValid: true, @@ -175,14 +171,6 @@ var _ = Describe("TestClusterTagFromList", func() { }) }) -var _ = Describe("TestDeviceTypeTagFromList", func() { - It("returns a DeviceTag with DeviceTagKeyDeviceType", func() { - deviceTag, err := DeviceTypeTagFromList([]string{DeviceTagKeyDeviceType.Prefix() + "value", string(DeviceTagKeyDeviceType) + "value"}) - Expect(err).To(BeNil()) - Expect(deviceTag).To(Equal(DeviceTag{Key: DeviceTagKeyDeviceType, Value: "value"})) - }) -}) - var _ = Describe("deviceTagFromString", func() { type testCaseDeviceTagFromString struct { tagString string @@ -345,7 +333,6 @@ var _ = Describe("RemoveEphemeralTags", func() { It("removes ephemeral tags, but keeps permanent tags", func() { newTags := RemoveEphemeralTags([]string{ // non-ephemeral (keep) - DeviceTagKeyDeviceType.Prefix() + "my-device-type", DeviceTagKeyPermanentError.Prefix() + "my-permantent-error", DeviceTagKeyCAPHVUseAllowed.Prefix() + "allow", @@ -355,7 +342,6 @@ var _ = Describe("RemoveEphemeralTags", func() { "some-other-tag", }) Expect(newTags).To(Equal([]string{ - "caphv-device-type=my-device-type", "caphv-permanent-error=my-permantent-error", "caphv-use=allow", "some-other-tag"})) @@ -365,7 +351,6 @@ var _ = Describe("RemoveEphemeralTags", func() { var _ = Describe("PermanentErrorTagFromList", func() { It("return permantent error from list", func() { tag, err := PermanentErrorTagFromList([]string{ - DeviceTagKeyDeviceType.Prefix() + "my-device-type", DeviceTagKeyPermanentError.Prefix() + "my-permantent-error", DeviceTagKeyCluster.Prefix() + "my-cluster", DeviceTagKeyMachine.Prefix() + "my-machine", diff --git a/templates/cluster-templates/bases/hivelocity-mt-control-plane-ubuntu.yaml b/templates/cluster-templates/bases/hivelocity-mt-control-plane-ubuntu.yaml index f7eb922f7..3ed14e3e4 100644 --- a/templates/cluster-templates/bases/hivelocity-mt-control-plane-ubuntu.yaml +++ b/templates/cluster-templates/bases/hivelocity-mt-control-plane-ubuntu.yaml @@ -5,5 +5,7 @@ metadata: spec: template: spec: - type: "${HIVELOCITY_CONTROL_PLANE_MACHINE_TYPE}" + deviceSelector: + matchLabels: + deviceType: e2eControlPlane imageName: "ubuntu-22.04" diff --git a/templates/cluster-templates/bases/hivelocity-mt-md-0-ubuntu.yaml b/templates/cluster-templates/bases/hivelocity-mt-md-0-ubuntu.yaml index 642d82c66..c30d89daf 100644 --- a/templates/cluster-templates/bases/hivelocity-mt-md-0-ubuntu.yaml +++ b/templates/cluster-templates/bases/hivelocity-mt-md-0-ubuntu.yaml @@ -5,5 +5,7 @@ metadata: spec: template: spec: - type: "${HIVELOCITY_WORKER_MACHINE_TYPE}" + deviceSelector: + matchLabels: + deviceType: e2eWorker imageName: "ubuntu-22.04" diff --git a/test/claim-devices-or-fail/claim-devices-or-fail.go b/test/claim-devices-or-fail/claim-devices-or-fail.go index e875b9218..231551dc5 100644 --- a/test/claim-devices-or-fail/claim-devices-or-fail.go +++ b/test/claim-devices-or-fail/claim-devices-or-fail.go @@ -47,18 +47,17 @@ func main() { } for i := 1; i < len(os.Args); i++ { - deviceType := os.Args[i] - err := releaseOldMachines(ctx, apiClient, deviceType, allDevices) + tag := os.Args[i] + err := releaseOldMachines(ctx, apiClient, tag, allDevices) if err != nil { log.Fatalln(err) } } } -func releaseOldMachines(ctx context.Context, apiClient *hv.APIClient, deviceType string, +func releaseOldMachines(ctx context.Context, apiClient *hv.APIClient, tag string, allDevices []hv.BareMetalDevice) error { devicesWithTag := make([]hv.BareMetalDevice, 0) - tag := fmt.Sprintf("%s=%s", hvtag.DeviceTagKeyDeviceType, deviceType) for _, device := range allDevices { if !slices.Contains(device.Tags, tag) { @@ -78,10 +77,9 @@ func releaseOldMachines(ctx context.Context, apiClient *hv.APIClient, deviceType devicesWithTag = append(devicesWithTag, device) } if len(devicesWithTag) == 0 { - return fmt.Errorf("no device found with %s=%s", hvtag.DeviceTagKeyDeviceType, deviceType) + return fmt.Errorf("no device found with %q, Please make a corresponding device available. For example, by giving a machine the appropriate label via the HV web UI", tag) } - fmt.Printf("resetting labels of all devices which have %s=%s. Found %d devices\n", - hvtag.DeviceTagKeyDeviceType, deviceType, len(devicesWithTag)) + fmt.Printf("resetting labels of all devices which have %s. Found %d devices\n", tag, len(devicesWithTag)) for _, device := range devicesWithTag { fmt.Printf(" resetting labels of device %d\n", device.DeviceId) newTags := hvtag.RemoveEphemeralTags(device.Tags) diff --git a/test/e2e/config/hivelocity-ci.yaml b/test/e2e/config/hivelocity-ci.yaml index f732551e4..d8569f08f 100644 --- a/test/e2e/config/hivelocity-ci.yaml +++ b/test/e2e/config/hivelocity-ci.yaml @@ -119,8 +119,8 @@ variables: HIVELOCITY_SSH_PUB: "secret" HIVELOCITY_IMAGE_NAME: ${HIVELOCITY_IMAGE_NAME:-} HIVELOCITY_REGION: LAX2 - HIVELOCITY_CONTROL_PLANE_MACHINE_TYPE: e2eControlPlane - HIVELOCITY_WORKER_MACHINE_TYPE: e2eWorker + HIVELOCITY_CONTROL_PLANE_MACHINE_TYPE: caphvlabel:deviceType=e2eControlPlane + HIVELOCITY_WORKER_MACHINE_TYPE: caphvlabel:deviceType=e2eWorker REDACT_LOG_SCRIPT: "../../hack/log/redact.sh" intervals: diff --git a/test/e2e/config/hivelocity.yaml b/test/e2e/config/hivelocity.yaml index d393d3fd2..8ca72b995 100644 --- a/test/e2e/config/hivelocity.yaml +++ b/test/e2e/config/hivelocity.yaml @@ -125,8 +125,8 @@ variables: HIVELOCITY_SSH_PUB: "secret" HIVELOCITY_IMAGE_NAME: ${HIVELOCITY_IMAGE_NAME:-} HIVELOCITY_REGION: LAX2 - HIVELOCITY_CONTROL_PLANE_MACHINE_TYPE: e2eControlPlane - HIVELOCITY_WORKER_MACHINE_TYPE: e2eWorker + HIVELOCITY_CONTROL_PLANE_MACHINE_TYPE: caphvlabel:deviceType=e2eControlPlane + HIVELOCITY_WORKER_MACHINE_TYPE: caphvlabel:deviceType=e2eWorker REDACT_LOG_SCRIPT: "../../hack/log/redact.sh" intervals: