fix(lease): let a pool provision for a lease that matches nothing yet

controller/cmd/main.go

@@ -44,6 +44,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 
 	jumpstarterdevv1alpha1 "github.com/jumpstarter-dev/jumpstarter/controller/api/v1alpha1"
+	virtualtargetv1alpha1 "github.com/jumpstarter-dev/jumpstarter/controller/api/virtualtarget/v1alpha1"
 	"github.com/jumpstarter-dev/jumpstarter/controller/internal/authentication"
 	"github.com/jumpstarter-dev/jumpstarter/controller/internal/authorization"
 	"github.com/jumpstarter-dev/jumpstarter/controller/internal/config"
@@ -102,6 +103,10 @@ func init() {
 
 	utilruntime.Must(jumpstarterdevv1alpha1.AddToScheme(scheme))
 
+	// Read-only, for the lease controller: whether a selector that matches no
+	// exporter today could be satisfied by a pool provisioning one.
+	utilruntime.Must(virtualtargetv1alpha1.AddToScheme(scheme))
+
 	// +kubebuilder:scaffold:scheme
 	apiserverinstall.Install(scheme)
 }

controller/deploy/operator/internal/controller/jumpstarter/rbac.go

@@ -207,6 +207,15 @@ func (r *JumpstarterReconciler) createRole(jumpstarter *operatorv1alpha1.Jumpsta
 				Resources: []string{"clients/finalizers", "exporters/finalizers", "leases/finalizers", "exporteraccesspolicies/finalizers"},
 				Verbs:     []string{"update"},
 			},
+			{
+				// Read-only: the lease controller asks whether a selector that
+				// matches no exporter today could be satisfied by a pool
+				// provisioning one, which decides between pending and
+				// unsatisfiable. It never writes exporter sets.
+				APIGroups: []string{"virtualtarget.jumpstarter.dev"},
+				Resources: []string{"exportersets"},
+				Verbs:     []string{"get", "list", "watch"},
+			},
 			{
 				APIGroups: []string{""},
 				Resources: []string{"events"},

controller/internal/controller/lease_controller.go

@@ -24,6 +24,7 @@ import (
 	"time"
 
 	jumpstarterdevv1alpha1 "github.com/jumpstarter-dev/jumpstarter/controller/api/v1alpha1"
+	virtualtargetv1alpha1 "github.com/jumpstarter-dev/jumpstarter/controller/api/virtualtarget/v1alpha1"
 	jmpmetrics "github.com/jumpstarter-dev/jumpstarter/controller/internal/metrics"
 	corev1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
@@ -63,6 +64,7 @@ type ApprovedExporter struct {
 // +kubebuilder:rbac:groups=jumpstarter.dev,resources=leases,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=jumpstarter.dev,resources=leases/status,verbs=get;update;patch
 // +kubebuilder:rbac:groups=jumpstarter.dev,resources=leases/finalizers,verbs=update
+// +kubebuilder:rbac:groups=virtualtarget.jumpstarter.dev,resources=exportersets,verbs=get;list;watch
 // Reconcile is part of the main kubernetes reconciliation loop which aims to
 // move the current state of the cluster closer to the desired state.
 // TODO(user): Modify the Reconcile function to compare the state specified by
@@ -207,6 +209,98 @@ func (r *LeaseReconciler) reconcileStatusBeginEndTimes(
 	return nil
 }
 
+// candidateExporters resolves the exporters a lease could be given: the one it
+// names, or every exporter its selector matches.
+//
+// It reports decided=true when it has already settled the lease's status and
+// the caller should stop — the named exporter is missing, does not match, or is
+// disabled; every match is disabled; or nothing matches yet and a pool can
+// provide one.
+func (r *LeaseReconciler) candidateExporters(
+	ctx context.Context,
+	result *ctrl.Result,
+	lease *jumpstarterdevv1alpha1.Lease,
+	selector labels.Selector,
+) ([]jumpstarterdevv1alpha1.Exporter, bool, error) {
+	if lease.Spec.ExporterRef != nil {
+		var exporter jumpstarterdevv1alpha1.Exporter
+		if err := r.Get(ctx, types.NamespacedName{
+			Namespace: lease.Namespace,
+			Name:      lease.Spec.ExporterRef.Name,
+		}, &exporter); err != nil {
+			if k8serrors.IsNotFound(err) {
+				lease.SetStatusUnsatisfiable(
+					"ExporterNotFound",
+					"Requested exporter %s was not found",
+					lease.Spec.ExporterRef.Name,
+				)
+				return nil, true, nil
+			}
+			return nil, false, fmt.Errorf("candidateExporters: failed to get requested exporter: %w", err)
+		}
+		if !selector.Empty() && !selector.Matches(labels.Set(exporter.Labels)) {
+			lease.SetStatusUnsatisfiable(
+				"SelectorMismatch",
+				"Requested exporter %s does not match selector %s",
+				exporter.Name,
+				metav1.FormatLabelSelector(&lease.Spec.Selector),
+			)
+			return nil, true, nil
+		}
+		// Check if the explicitly requested exporter is disabled
+		if !exporter.IsEnabled() && !lease.Spec.AllowDisabled {
+			lease.SetStatusUnsatisfiable(
+				"ExporterDisabled",
+				"Requested exporter %s is disabled. "+
+					"To lease a disabled exporter, set spec.allowDisabled: true on the Lease, "+
+					"or use --allow-disabled with jmp create lease or jmp shell",
+				exporter.Name,
+			)
+			return nil, true, nil
+		}
+		return []jumpstarterdevv1alpha1.Exporter{exporter}, false, nil
+	}
+
+	// List all exporters matching selector
+	listed, err := r.ListMatchingExporters(ctx, lease, selector)
+	if err != nil {
+		return nil, false, fmt.Errorf("candidateExporters: failed to list matching exporters: %w", err)
+	}
+	if len(listed.Items) == 0 {
+		// Nothing matches yet, which is the normal state of a pool that
+		// keeps nothing warm. An exporter set that could provision a
+		// match makes this lease pending rather than unsatisfiable —
+		// and pending is what the set counts as demand to scale up on.
+		// Calling it unsatisfiable here ends the lease in this same
+		// reconcile, so the set would never see it.
+		set, err := r.exporterSetCanProvision(ctx, lease, selector)
+		if err != nil {
+			return nil, false, fmt.Errorf("candidateExporters: %w", err)
+		}
+		if set != "" {
+			lease.SetStatusPending(
+				"Provisioning",
+				"No exporter matches the selector yet; exporter set %s can provision one",
+				set,
+			)
+			result.RequeueAfter = pendingRequeueAfter(lease)
+			return nil, true, nil
+		}
+	}
+
+	// Filter out disabled exporters from selector-based listing
+	matchingExporters := filterOutDisabledExporters(listed.Items)
+	if len(matchingExporters) == 0 && len(listed.Items) > 0 {
+		lease.SetStatusUnsatisfiable(
+			"AllDisabled",
+			"All %d exporters matching the selector are disabled",
+			len(listed.Items),
+		)
+		return nil, true, nil
+	}
+	return matchingExporters, false, nil
+}
+
 // Also manages LeaseConditionTypeUnsatisfiable and LeaseConditionTypePending
 func (r *LeaseReconciler) reconcileStatusExporterRef(
 	ctx context.Context,
@@ -245,60 +339,12 @@ func (r *LeaseReconciler) reconcileStatusExporterRef(
 			return nil
 		}
 
-		var matchingExporters []jumpstarterdevv1alpha1.Exporter
-		if lease.Spec.ExporterRef != nil {
-			var exporter jumpstarterdevv1alpha1.Exporter
-			if err := r.Get(ctx, types.NamespacedName{
-				Namespace: lease.Namespace,
-				Name:      lease.Spec.ExporterRef.Name,
-			}, &exporter); err != nil {
-				if k8serrors.IsNotFound(err) {
-					lease.SetStatusUnsatisfiable(
-						"ExporterNotFound",
-						"Requested exporter %s was not found",
-						lease.Spec.ExporterRef.Name,
-					)
-					return nil
-				}
-				return fmt.Errorf("reconcileStatusExporterRef: failed to get requested exporter: %w", err)
-			}
-			if !selector.Empty() && !selector.Matches(labels.Set(exporter.Labels)) {
-				lease.SetStatusUnsatisfiable(
-					"SelectorMismatch",
-					"Requested exporter %s does not match selector %s",
-					exporter.Name,
-					metav1.FormatLabelSelector(&lease.Spec.Selector),
-				)
-				return nil
-			}
-			// Check if the explicitly requested exporter is disabled
-			if !exporter.IsEnabled() && !lease.Spec.AllowDisabled {
-				lease.SetStatusUnsatisfiable(
-					"ExporterDisabled",
-					"Requested exporter %s is disabled. "+
-						"To lease a disabled exporter, set spec.allowDisabled: true on the Lease, "+
-						"or use --allow-disabled with jmp create lease or jmp shell",
-					exporter.Name,
-				)
-				return nil
-			}
-			matchingExporters = []jumpstarterdevv1alpha1.Exporter{exporter}
-		} else {
-			// List all exporters matching selector
-			listed, err := r.ListMatchingExporters(ctx, lease, selector)
-			if err != nil {
-				return fmt.Errorf("reconcileStatusExporterRef: failed to list matching exporters: %w", err)
-			}
-			// Filter out disabled exporters from selector-based listing
-			matchingExporters = filterOutDisabledExporters(listed.Items)
-			if len(matchingExporters) == 0 && len(listed.Items) > 0 {
-				lease.SetStatusUnsatisfiable(
-					"AllDisabled",
-					"All %d exporters matching the selector are disabled",
-					len(listed.Items),
-				)
-				return nil
-			}
+		matchingExporters, decided, err := r.candidateExporters(ctx, result, lease, selector)
+		if err != nil {
+			return err
+		}
+		if decided {
+			return nil
 		}
 
 		approvedExporters, unmatchedDescriptions, err := r.attachMatchingPolicies(ctx, lease, matchingExporters)
@@ -557,6 +603,51 @@ func (r *LeaseReconciler) ListMatchingExporters(ctx context.Context, lease *jump
 	return &matchingExporters, nil
 }
 
+// exporterSetCanProvision reports the name of an exporter set that could
+// provision an exporter matching the lease's selector, or "" if none can.
+//
+// It answers the question the lease controller cannot otherwise ask: is there
+// nothing for this lease because nothing will ever match, or because the pool
+// that would match keeps nothing warm? The two look identical from the exporter
+// listing and mean opposite things to the caller.
+//
+// A set qualifies when the labels it stamps on its members satisfy the selector
+// and it is not already at its ceiling. Whether it is actually willing to scale
+// right now is the set's own decision, made in its reconciler; this only has to
+// be right about whether waiting is worthwhile.
+func (r *LeaseReconciler) exporterSetCanProvision(
+	ctx context.Context,
+	lease *jumpstarterdevv1alpha1.Lease,
+	selector labels.Selector,
+) (string, error) {
+	var sets virtualtargetv1alpha1.ExporterSetList
+	if err := r.List(ctx, &sets, client.InNamespace(lease.Namespace)); err != nil {
+		if k8serrors.IsForbidden(err) || meta.IsNoMatchError(err) {
+			// A controller running against an operator that has not granted
+			// this read, or a cluster without the exporter set CRDs at all.
+			// Neither is transient, and neither is a reason to fail a lease:
+			// fall back to the behaviour from before pools existed.
+			log.FromContext(ctx).V(1).Info(
+				"cannot read exporter sets, treating the selector as unsatisfiable",
+				"lease", lease.Name, "reason", err.Error())
+			return "", nil
+		}
+		return "", fmt.Errorf("exporterSetCanProvision: failed to list exporter sets: %w", err)
+	}
+
+	for i := range sets.Items {
+		set := &sets.Items[i]
+		if !selector.Matches(labels.Set(set.Spec.Template.Metadata.Labels)) {
+			continue
+		}
+		if set.Spec.MaxReplicas > 0 && set.Status.Replicas >= set.Spec.MaxReplicas {
+			continue
+		}
+		return set.Name, nil
+	}
+	return "", nil
+}
+
 // ListActiveLeases returns a list of active leases in the namespace
 func (r *LeaseReconciler) ListActiveLeases(ctx context.Context, namespace string) (*jumpstarterdevv1alpha1.LeaseList, error) {
 	var activeLeases jumpstarterdevv1alpha1.LeaseList

controller/internal/controller/lease_controller_test.go

@@ -21,6 +21,7 @@ import (
 	"time"
 
 	jumpstarterdevv1alpha1 "github.com/jumpstarter-dev/jumpstarter/controller/api/v1alpha1"
+	virtualtargetv1alpha1 "github.com/jumpstarter-dev/jumpstarter/controller/api/virtualtarget/v1alpha1"
 	"github.com/jumpstarter-dev/jumpstarter/controller/internal/oidc"
 	cpb "github.com/jumpstarter-dev/jumpstarter/controller/internal/protocol/jumpstarter/client/v1"
 	. "github.com/onsi/ginkgo/v2"
@@ -2654,3 +2655,115 @@ var _ = Describe("pendingRequeueAfter", func() {
 		Entry("5m (capped)", 5*time.Minute, 30*time.Second),
 	)
 })
+
+var _ = Describe("Lease against an exporter set with nothing warm", func() {
+	const setName = "cold-pool"
+
+	newSet := func(maxReplicas, replicas int32) *virtualtargetv1alpha1.ExporterSet {
+		return &virtualtargetv1alpha1.ExporterSet{
+			ObjectMeta: metav1.ObjectMeta{Name: setName, Namespace: "default"},
+			Spec: virtualtargetv1alpha1.ExporterSetSpec{
+				VirtualTargetClassName: "qemu",
+				MinReplicas:            0,
+				MaxReplicas:            maxReplicas,
+				Selector:               metav1.LabelSelector{MatchLabels: map[string]string{"pool": "cold"}},
+				Template: virtualtargetv1alpha1.ExporterSetTemplate{
+					Metadata: virtualtargetv1alpha1.EmbeddedObjectMeta{
+						Labels: map[string]string{"pool": "cold"},
+					},
+				},
+			},
+			Status: virtualtargetv1alpha1.ExporterSetStatus{Replicas: replicas},
+		}
+	}
+
+	newLease := func(name string, matchLabels map[string]string) *jumpstarterdevv1alpha1.Lease {
+		return &jumpstarterdevv1alpha1.Lease{
+			ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "default"},
+			Spec: jumpstarterdevv1alpha1.LeaseSpec{
+				ClientRef: corev1.LocalObjectReference{Name: testClient.Name},
+				Selector:  metav1.LabelSelector{MatchLabels: matchLabels},
+				Duration:  &metav1.Duration{Duration: 2 * time.Second},
+			},
+		}
+	}
+
+	createSet := func(ctx context.Context, set *virtualtargetv1alpha1.ExporterSet) {
+		status := set.Status
+		Expect(k8sClient.Create(ctx, set)).To(Succeed())
+		set.Status = status
+		Expect(k8sClient.Status().Update(ctx, set)).To(Succeed())
+	}
+
+	AfterEach(func() {
+		ctx := context.Background()
+		set := &virtualtargetv1alpha1.ExporterSet{
+			ObjectMeta: metav1.ObjectMeta{Name: setName, Namespace: "default"},
+		}
+		_ = k8sClient.Delete(ctx, set)
+		deleteLeases(ctx, "cold-lease", "cold-lease-max", "no-pool-lease")
+	})
+
+	When("a pool could provision a matching exporter", func() {
+		It("should stay pending rather than ending the lease", func() {
+			ctx := context.Background()
+			createSet(ctx, newSet(4, 0))
+
+			lease := newLease("cold-lease", map[string]string{"pool": "cold"})
+			Expect(k8sClient.Create(ctx, lease)).To(Succeed())
+			_ = reconcileLease(ctx, lease)
+
+			updated := getLease(ctx, lease.Name)
+			// Ending it here is what stopped the set from ever seeing demand:
+			// countPendingLeases skips ended leases and requires Pending.
+			Expect(updated.Status.Ended).To(BeFalse())
+			Expect(meta.IsStatusConditionTrue(
+				updated.Status.Conditions,
+				string(jumpstarterdevv1alpha1.LeaseConditionTypeUnsatisfiable),
+			)).To(BeFalse())
+
+			condition := meta.FindStatusCondition(
+				updated.Status.Conditions,
+				string(jumpstarterdevv1alpha1.LeaseConditionTypePending),
+			)
+			Expect(condition).NotTo(BeNil())
+			Expect(condition.Reason).To(Equal("Provisioning"))
+			Expect(condition.Message).To(ContainSubstring(setName))
+		})
+	})
+
+	When("the pool is already at its ceiling", func() {
+		It("should be unsatisfiable, since waiting would not help", func() {
+			ctx := context.Background()
+			createSet(ctx, newSet(2, 2))
+
+			lease := newLease("cold-lease-max", map[string]string{"pool": "cold"})
+			Expect(k8sClient.Create(ctx, lease)).To(Succeed())
+			_ = reconcileLease(ctx, lease)
+
+			updated := getLease(ctx, lease.Name)
+			Expect(meta.IsStatusConditionTrue(
+				updated.Status.Conditions,
+				string(jumpstarterdevv1alpha1.LeaseConditionTypeUnsatisfiable),
+			)).To(BeTrue())
+		})
+	})
+
+	When("no pool matches the selector", func() {
+		It("should still fail fast rather than waiting forever", func() {
+			ctx := context.Background()
+			createSet(ctx, newSet(4, 0))
+
+			lease := newLease("no-pool-lease", map[string]string{"pool": "does-not-exist"})
+			Expect(k8sClient.Create(ctx, lease)).To(Succeed())
+			_ = reconcileLease(ctx, lease)
+
+			updated := getLease(ctx, lease.Name)
+			Expect(meta.IsStatusConditionTrue(
+				updated.Status.Conditions,
+				string(jumpstarterdevv1alpha1.LeaseConditionTypeUnsatisfiable),
+			)).To(BeTrue())
+			Expect(updated.Status.Ended).To(BeTrue())
+		})
+	})
+})