From ac27c34bcad0b8b4efa07c586f4a5363a76a49cf Mon Sep 17 00:00:00 2001 From: Bella Khizgiyaev Date: Mon, 19 Jan 2026 14:32:08 +0200 Subject: [PATCH] Support direct exporter leasing by name in addition to the existing label selector based flow. Signed-off-by: Bella Khizgiyaev --- api/v1alpha1/lease_helpers.go | 71 +++++++-- api/v1alpha1/lease_types.go | 7 +- internal/controller/lease_controller.go | 188 +++++++++++++++--------- 3 files changed, 187 insertions(+), 79 deletions(-) diff --git a/api/v1alpha1/lease_helpers.go b/api/v1alpha1/lease_helpers.go index 54b42de6..bfb531ca 100644 --- a/api/v1alpha1/lease_helpers.go +++ b/api/v1alpha1/lease_helpers.go @@ -174,9 +174,32 @@ func LeaseFromProtobuf( key types.NamespacedName, clientRef corev1.LocalObjectReference, ) (*Lease, error) { - selector, err := ParseLabelSelector(req.Selector) - if err != nil { - return nil, err + var selector *metav1.LabelSelector // nil initially, only set if selector-based selection is used + var deviceName *string + + switch selection := req.ExporterSelection.(type) { + case *cpb.Lease_Selector: + if selection.Selector == "" { + return nil, fmt.Errorf("selector cannot be empty") + } + var err error + selector, err = ParseLabelSelector(selection.Selector) + if err != nil { + return nil, err + } + case *cpb.Lease_ExporterName: + if selection.ExporterName == "" { + return nil, fmt.Errorf("exporter_name cannot be empty") + } + exporterKey, err := utils.ParseExporterIdentifier(selection.ExporterName) + if err != nil { + return nil, fmt.Errorf("invalid exporter_name: %w", err) + } + deviceName = &exporterKey.Name + case nil: + return nil, fmt.Errorf("either selector or exporter_name must be specified in exporter_selection") + default: + return nil, fmt.Errorf("unknown exporter_selection type: %T", selection) } var beginTime, endTime *metav1.Time @@ -195,18 +218,29 @@ func LeaseFromProtobuf( return nil, err } + // Build the LeaseSpec - only set Selector if deviceName is nil (selector-based selection) + // For name-based selection, Selector remains at zero value (unused) + spec := LeaseSpec{ + ClientRef: clientRef, + Duration: duration, + DeviceName: deviceName, + BeginTime: beginTime, + EndTime: endTime, + } + + if deviceName == nil { + if selector == nil { + return nil, fmt.Errorf("selector must be provided when deviceName is not set") + } + spec.Selector = *selector + } + return &Lease{ ObjectMeta: metav1.ObjectMeta{ Namespace: key.Namespace, Name: key.Name, }, - Spec: LeaseSpec{ - ClientRef: clientRef, - Duration: duration, - Selector: *selector, - BeginTime: beginTime, - EndTime: endTime, - }, + Spec: spec, }, nil } @@ -228,10 +262,25 @@ func (l *Lease) ToProtobuf() *cpb.Lease { lease := cpb.Lease{ Name: fmt.Sprintf("namespaces/%s/leases/%s", l.Namespace, l.Name), - Selector: metav1.FormatLabelSelector(&l.Spec.Selector), Client: ptr.To(fmt.Sprintf("namespaces/%s/clients/%s", l.Namespace, l.Spec.ClientRef.Name)), Conditions: conditions, } + + // Set the oneof exporter_selection field based on which is specified + if l.Spec.DeviceName != nil && *l.Spec.DeviceName != "" { + // Direct device selection by resource name + lease.ExporterSelection = &cpb.Lease_ExporterName{ + ExporterName: utils.UnparseExporterIdentifier(kclient.ObjectKey{ + Namespace: l.Namespace, + Name: *l.Spec.DeviceName, + }), + } + } else { + // Selector-based exporter matching + lease.ExporterSelection = &cpb.Lease_Selector{ + Selector: metav1.FormatLabelSelector(&l.Spec.Selector), + } + } if l.Spec.Duration != nil { lease.Duration = durationpb.New(l.Spec.Duration.Duration) } diff --git a/api/v1alpha1/lease_types.go b/api/v1alpha1/lease_types.go index 97d4f5e6..3d6a3bcf 100644 --- a/api/v1alpha1/lease_types.go +++ b/api/v1alpha1/lease_types.go @@ -29,8 +29,11 @@ type LeaseSpec struct { // Can be omitted (nil) when both BeginTime and EndTime are provided, // in which case it's calculated as EndTime - BeginTime. Duration *metav1.Duration `json:"duration,omitempty"` - // The selector for the exporter to be used - Selector metav1.LabelSelector `json:"selector"` + // The selector for the exporter to be used (mutually exclusive with DeviceName) + Selector metav1.LabelSelector `json:"selector,omitempty"` + // Direct device selection by name (mutually exclusive with Selector) + // When specified, the lease will only match this specific exporter + DeviceName *string `json:"deviceName,omitempty"` // The release flag requests the controller to end the lease now Release bool `json:"release,omitempty"` // Requested start time. If omitted, lease starts when exporter is acquired. diff --git a/internal/controller/lease_controller.go b/internal/controller/lease_controller.go index eb5c7596..acb3417c 100644 --- a/internal/controller/lease_controller.go +++ b/internal/controller/lease_controller.go @@ -25,6 +25,7 @@ import ( jumpstarterdevv1alpha1 "github.com/jumpstarter-dev/jumpstarter-controller/api/v1alpha1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -209,6 +210,12 @@ func (r *LeaseReconciler) reconcileStatusExporterRef( return nil } } + + // Check if direct device selection by name is specified + if lease.Spec.DeviceName != nil && *lease.Spec.DeviceName != "" { + return r.reconcileDirectExporterSelection(ctx, result, lease) + } + logger.Info("Looking for a matching exporter for lease", "lease", lease.Name, "client", lease.GetClientName(), "selector", lease.Spec.Selector) selector, err := lease.GetExporterSelector() @@ -225,90 +232,139 @@ func (r *LeaseReconciler) reconcileStatusExporterRef( return fmt.Errorf("reconcileStatusExporterRef: failed to list matching exporters: %w", err) } - approvedExporters, err := r.attachMatchingPolicies(ctx, lease, matchingExporters.Items) - if err != nil { - return fmt.Errorf("reconcileStatusExporterRef: failed to handle policy approval: %w", err) - } + return r.selectAndAssignExporter(ctx, result, lease, matchingExporters.Items) + } - if len(approvedExporters) == 0 { - lease.SetStatusUnsatisfiable( - "NoAccess", - "While there are %d exporters matching the selector, none of them are approved by any policy for your client", - len(matchingExporters.Items), - ) - return nil - } + return nil +} - onlineApprovedExporters := filterOutOfflineExporters(approvedExporters) - if len(onlineApprovedExporters) == 0 { - lease.SetStatusPending( - "Offline", - "While there are %d available exporters (i.e. %s), none of them are online", - len(approvedExporters), - approvedExporters[0].Exporter.Name, - ) - result.RequeueAfter = time.Second - return nil - } +// Unified function for selecting and assigning an exporter from a list of candidate exporters. +// Used by both selector-based and name-based selection. +func (r *LeaseReconciler) selectAndAssignExporter( + ctx context.Context, + result *ctrl.Result, + lease *jumpstarterdevv1alpha1.Lease, + candidateExporters []jumpstarterdevv1alpha1.Exporter, +) error { + approvedExporters, err := r.attachMatchingPolicies(ctx, lease, candidateExporters) + if err != nil { + return fmt.Errorf("selectAndAssignExporter: failed to handle policy approval: %w", err) + } - // Filter out exporters that are already leased - activeLeases, err := r.ListActiveLeases(ctx, lease.Namespace) - if err != nil { - return fmt.Errorf("reconcileStatusExporterRef: failed to list active leases: %w", err) + if len(approvedExporters) == 0 { + if len(candidateExporters) == 1 { + // Name-based selection: single exporter + lease.SetStatusUnsatisfiable("NoAccess", + "Device '%s' exists but no policy allows your client to access it", + candidateExporters[0].Name) + } else { + // Selector-based selection: multiple exporters + lease.SetStatusUnsatisfiable("NoAccess", + "While there are %d exporters matching the selector, none of them are approved by any policy for your client", + len(candidateExporters)) } + return nil + } - onlineApprovedExporters = attachExistingLeases(onlineApprovedExporters, activeLeases.Items) - orderedExporters := orderApprovedExporters(onlineApprovedExporters) + onlineApprovedExporters := filterOutOfflineExporters(approvedExporters) + if len(onlineApprovedExporters) == 0 { + lease.SetStatusPending( + "Offline", + "While there are %d available exporters (i.e. %s), none of them are online", + len(approvedExporters), + approvedExporters[0].Exporter.Name, + ) + result.RequeueAfter = time.Second + return nil + } - if len(orderedExporters) > 0 && orderedExporters[0].Policy.SpotAccess { - lease.SetStatusUnsatisfiable("SpotAccess", - "The only possible exporters are under spot access (i.e. %s), but spot access is still not implemented", - orderedExporters[0].Exporter.Name) - return nil - } + activeLeases, err := r.ListActiveLeases(ctx, lease.Namespace) + if err != nil { + return fmt.Errorf("selectAndAssignExporter: failed to list active leases: %w", err) + } - availableExporters := filterOutLeasedExporters(onlineApprovedExporters) - if len(availableExporters) == 0 { - lease.SetStatusPending("NotAvailable", - "There are %d approved exporters, (i.e. %s) but all of them are already leased", - len(onlineApprovedExporters), - onlineApprovedExporters[0].Exporter.Name, - ) - result.RequeueAfter = time.Second - return nil - } + onlineApprovedExporters = attachExistingLeases(onlineApprovedExporters, activeLeases.Items) + orderedExporters := orderApprovedExporters(onlineApprovedExporters) + + if len(orderedExporters) > 0 && orderedExporters[0].Policy.SpotAccess { + lease.SetStatusUnsatisfiable("SpotAccess", + "The only possible exporters are under spot access (i.e. %s), but spot access is still not implemented", + orderedExporters[0].Exporter.Name) + return nil + } - // TODO: here there's room for improvement, i.e. we could have multiple - // clients trying to lease the same exporters, we should look at priorities - // and spot access to decide which client gets the exporter, this probably means - // that we will need to construct a lease scheduler with the view of all leases - // and exporters in the system, and (maybe) a priority queue for the leases. + availableExporters := filterOutLeasedExporters(onlineApprovedExporters) + if len(availableExporters) == 0 { + lease.SetStatusPending("NotAvailable", + "There are %d approved exporters, (i.e. %s) but all of them are already leased", + len(onlineApprovedExporters), + onlineApprovedExporters[0].Exporter.Name, + ) + result.RequeueAfter = time.Second + return nil + } - // For now, we just select the best available exporter without considering other - // ongoing lease requests + // TODO: here there's room for improvement, i.e. we could have multiple + // clients trying to lease the same exporters, we should look at priorities + // and spot access to decide which client gets the exporter, this probably means + // that we will need to construct a lease scheduler with the view of all leases + // and exporters in the system, and (maybe) a priority queue for the leases. - selected := availableExporters[0] + // For now, we just select the best available exporter without considering other + // ongoing lease requests - if selected.ExistingLease != nil { - // TODO: Implement eviction of spot access leases - lease.SetStatusPending("NotAvailable", - "Exporter %s is already leased by another client under spot access, but spot access eviction still not implemented", - selected.Exporter.Name) - result.RequeueAfter = time.Second - return nil - } + selected := availableExporters[0] - lease.Status.Priority = selected.Policy.Priority - lease.Status.SpotAccess = selected.Policy.SpotAccess - lease.Status.ExporterRef = &corev1.LocalObjectReference{ - Name: selected.Exporter.Name, - } + if selected.ExistingLease != nil { + // TODO: Implement eviction of spot access leases + lease.SetStatusPending("NotAvailable", + "Exporter %s is already leased by another client under spot access, but spot access eviction still not implemented", + selected.Exporter.Name) + result.RequeueAfter = time.Second return nil } + lease.Status.Priority = selected.Policy.Priority + lease.Status.SpotAccess = selected.Policy.SpotAccess + lease.Status.ExporterRef = &corev1.LocalObjectReference{ + Name: selected.Exporter.Name, + } return nil } +// Handles direct exporter selection by name +func (r *LeaseReconciler) reconcileDirectExporterSelection( + ctx context.Context, + result *ctrl.Result, + lease *jumpstarterdevv1alpha1.Lease, +) error { + logger := log.FromContext(ctx) + deviceName := *lease.Spec.DeviceName + + logger.Info("Looking for specific device for lease", + "lease", lease.Name, + "client", lease.GetClientName(), + "deviceName", deviceName) + + // Get the specific exporter by device name + var exporter jumpstarterdevv1alpha1.Exporter + err := r.Get(ctx, types.NamespacedName{ + Namespace: lease.Namespace, + Name: deviceName, + }, &exporter) + + if err != nil { + if errors.IsNotFound(err) { + lease.SetStatusInvalid("ExporterNotFound", + "Device '%s' not found in namespace '%s'", + deviceName, lease.Namespace) + return nil + } + return fmt.Errorf("reconcileDirectExporterSelection: failed to get exporter: %w", err) + } + return r.selectAndAssignExporter(ctx, result, lease, []jumpstarterdevv1alpha1.Exporter{exporter}) +} + // attachMatchingPolicies attaches the matching policies to the list of online exporters // if the exporter matches the policy and the client matches the policy's client selector // the exporter is approved for leasing