Merge pull request #168 from michalskrivanek/endtime

support future leases, calculate effective begin/end/duration lease times

Author: mangelajo

api/v1alpha1/lease_helpers.go

@@ -20,6 +20,59 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/log"
 )
 
+// ReconcileLeaseTimeFields calculates missing time fields and validates consistency
+// between BeginTime, EndTime, and Duration. Modifies pointers in place.
+//
+// Supported lease specification patterns:
+// 1. Duration only (no BeginTime/EndTime): immediate start, runs for Duration
+//   - BeginTime set by controller when exporter acquired
+//   - EndTime = Status.BeginTime + Duration (calculated at runtime)
+//
+// 2. EndTime only: INVALID - cannot infer Duration without BeginTime or explicit Duration
+//   - Returns error: "duration is required (must specify Duration, or both BeginTime and EndTime)"
+//
+// 3. BeginTime + Duration: scheduled start at BeginTime, runs for Duration
+//   - Lease waits until BeginTime, then acquires exporter
+//   - EndTime = BeginTime + Duration (calculated at runtime)
+//
+// 4. BeginTime + EndTime: scheduled window, Duration computed from times
+//   - Duration = EndTime - BeginTime (auto-calculated here)
+//   - Validates EndTime > BeginTime (positive duration)
+//
+// 5. EndTime + Duration: scheduled end, BeginTime computed as EndTime - Duration
+//   - BeginTime = EndTime - Duration (auto-calculated here)
+//   - Useful for "finish by" scheduling
+//
+// 6. BeginTime + EndTime + Duration: all three specified, validates consistency
+//   - Validates Duration == EndTime - BeginTime
+//   - Returns error if inconsistent: "duration conflicts with begin_time and end_time"
+//
+// Note: The controller never auto-populates Spec.EndTime. It calculates expiration time
+// on-demand from available fields, keeping Spec.EndTime meaningful only when explicitly
+// set by the user. See lease_controller.go reconcileStatusEnded for expiration logic.
+func ReconcileLeaseTimeFields(beginTime, endTime **metav1.Time, duration **metav1.Duration) error {
+	if *beginTime != nil && *endTime != nil {
+		// Calculate duration from explicit begin/end times
+		calculated := (*endTime).Sub((*beginTime).Time)
+		if *duration != nil && (*duration).Duration > 0 && (*duration).Duration != calculated {
+			return fmt.Errorf("duration conflicts with begin_time and end_time")
+		}
+		*duration = &metav1.Duration{Duration: calculated}
+	} else if *endTime != nil && *duration != nil && (*duration).Duration > 0 {
+		// Calculate BeginTime from EndTime - Duration (scheduled lease ending at specific time)
+		*beginTime = &metav1.Time{Time: (*endTime).Add(-(*duration).Duration)}
+	}
+
+	// Validate final duration is positive (rejects nil, negative, zero)
+	if *duration == nil {
+		return fmt.Errorf("duration is required (must specify Duration, or both BeginTime and EndTime)")
+	}
+	if (*duration).Duration <= 0 {
+		return fmt.Errorf("duration must be positive, got %v", (*duration).Duration)
+	}
+	return nil
+}
+
 func LeaseFromProtobuf(
 	req *cpb.Lease,
 	key types.NamespacedName,
@@ -30,15 +83,33 @@ func LeaseFromProtobuf(
 		return nil, err
 	}
 
+	var beginTime, endTime *metav1.Time
+	var duration *metav1.Duration
+
+	if req.BeginTime != nil {
+		beginTime = &metav1.Time{Time: req.BeginTime.AsTime()}
+	}
+	if req.EndTime != nil {
+		endTime = &metav1.Time{Time: req.EndTime.AsTime()}
+	}
+	if req.Duration != nil {
+		duration = &metav1.Duration{Duration: req.Duration.AsDuration()}
+	}
+	if err := ReconcileLeaseTimeFields(&beginTime, &endTime, &duration); err != nil {
+		return nil, err
+	}
+
 	return &Lease{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace: key.Namespace,
 			Name:      key.Name,
 		},
 		Spec: LeaseSpec{
 			ClientRef: clientRef,
-			Duration:  metav1.Duration{Duration: req.Duration.AsDuration()},
+			Duration:  duration,
 			Selector:  *selector,
+			BeginTime: beginTime,
+			EndTime:   endTime,
 		},
 	}, nil
 }
@@ -60,22 +131,34 @@ 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),
-		Duration:          durationpb.New(l.Spec.Duration.Duration),
-		EffectiveDuration: durationpb.New(l.Spec.Duration.Duration), // TODO: implement lease renewal
-		Client:            ptr.To(fmt.Sprintf("namespaces/%s/clients/%s", l.Namespace, l.Spec.ClientRef.Name)),
-		Conditions:        conditions,
-		// TODO: implement scheduled leases
-		BeginTime: nil,
-		EndTime:   nil,
+		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,
+	}
+	if l.Spec.Duration != nil {
+		lease.Duration = durationpb.New(l.Spec.Duration.Duration)
 	}
 
+	// Requested/planned times from Spec
+	if l.Spec.BeginTime != nil {
+		lease.BeginTime = timestamppb.New(l.Spec.BeginTime.Time)
+	}
+	if l.Spec.EndTime != nil {
+		lease.EndTime = timestamppb.New(l.Spec.EndTime.Time)
+	}
+
+	// Actual times from Status
 	if l.Status.BeginTime != nil {
 		lease.EffectiveBeginTime = timestamppb.New(l.Status.BeginTime.Time)
-	}
-	if l.Status.EndTime != nil {
-		lease.EffectiveEndTime = timestamppb.New(l.Status.EndTime.Time)
+		endTime := time.Now()
+		if l.Status.EndTime != nil {
+			endTime = l.Status.EndTime.Time
+			lease.EffectiveEndTime = timestamppb.New(endTime)
+		}
+		// Final effective duration or current one so far while active. Non-negative to handle clock skew.
+		effectiveDuration := max(endTime.Sub(l.Status.BeginTime.Time), 0)
+		lease.EffectiveDuration = durationpb.New(effectiveDuration)
 	}
 	if l.Status.ExporterRef != nil {
 		lease.Exporter = ptr.To(utils.UnparseExporterIdentifier(kclient.ObjectKey{

api/v1alpha1/lease_types.go

@@ -25,18 +25,26 @@ import (
 type LeaseSpec struct {
 	// The client that is requesting the lease
 	ClientRef corev1.LocalObjectReference `json:"clientRef"`
-	// The desired duration of the lease
-	Duration metav1.Duration `json:"duration"`
+	// Duration of the lease. Must be positive when provided.
+	// 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 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.
+	// Immutable after lease starts (cannot change the past).
+	BeginTime *metav1.Time `json:"beginTime,omitempty"`
+	// Requested end time. If specified with BeginTime, Duration is calculated.
+	// Can be updated to extend or shorten active leases.
+	EndTime *metav1.Time `json:"endTime,omitempty"`
 }
 
 // LeaseStatus defines the observed state of Lease
 type LeaseStatus struct {
 	// If the lease has been acquired an exporter name is assigned
-	// and then and then it can be used, it will be empty while still pending
+	// and then it can be used, it will be empty while still pending
 	BeginTime   *metav1.Time                 `json:"beginTime,omitempty"`
 	EndTime     *metav1.Time                 `json:"endTime,omitempty"`
 	ExporterRef *corev1.LocalObjectReference `json:"exporterRef,omitempty"`

api/v1alpha1/zz_generated.deepcopy.go

@@ -82,7 +82,7 @@ func (r *LeaseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
 		return result, err
 	}
 
-	if err := r.reconcileStatusBeginTime(ctx, &lease); err != nil {
+	if err := r.reconcileStatusBeginEndTimes(ctx, &lease); err != nil {
 		return result, err
 	}
 
@@ -141,34 +141,41 @@ func (r *LeaseReconciler) reconcileStatusEnded(
 			lease.Release(ctx)
 			return nil
 		} else if lease.Status.BeginTime != nil {
-			expiration := lease.Status.BeginTime.Add(lease.Spec.Duration.Duration)
+			var expiration time.Time
+			if lease.Spec.EndTime != nil {
+				// expires at Spec.EndTime when specified
+				expiration = lease.Spec.EndTime.Time
+			} else if lease.Spec.BeginTime != nil && lease.Spec.Duration != nil {
+				// expires at Spec.BeginTime + Spec.Duration - scheduled lease
+				expiration = lease.Spec.BeginTime.Add(lease.Spec.Duration.Duration)
+			} else if lease.Spec.Duration != nil {
+				// expires at actual BeginTime + Spec.Duration - immediate lease
+				expiration = lease.Status.BeginTime.Add(lease.Spec.Duration.Duration)
+			}
+
 			if expiration.Before(now) {
 				lease.Expire(ctx)
 				return nil
-			} else {
-				result.RequeueAfter = expiration.Sub(now)
-				return nil
 			}
+			result.RequeueAfter = expiration.Sub(now)
+			return nil
 		}
 
 	}
 	return nil
 }
 
 // nolint:unparam
-func (r *LeaseReconciler) reconcileStatusBeginTime(
+func (r *LeaseReconciler) reconcileStatusBeginEndTimes(
 	ctx context.Context,
 	lease *jumpstarterdevv1alpha1.Lease,
 ) error {
-	logger := log.FromContext(ctx)
-
-	now := time.Now()
 	if lease.Status.BeginTime == nil && lease.Status.ExporterRef != nil {
+		logger := log.FromContext(ctx)
 		logger.Info("Updating begin time for lease", "lease", lease.Name, "exporter", lease.GetExporterName(), "client", lease.GetClientName())
+		now := time.Now()
+		lease.Status.BeginTime = &metav1.Time{Time: now}
 		lease.SetStatusReady(true, "Ready", "An exporter has been acquired for the client")
-		lease.Status.BeginTime = &metav1.Time{
-			Time: now,
-		}
 	}
 
 	return nil
@@ -188,6 +195,20 @@ func (r *LeaseReconciler) reconcileStatusExporterRef(
 	}
 
 	if lease.Status.ExporterRef == nil {
+		// For scheduled leases: only assign exporter if requested BeginTime has arrived
+		if lease.Spec.BeginTime != nil {
+			now := time.Now()
+			if lease.Spec.BeginTime.After(now) {
+				// Requested BeginTime is in the future, wait until then
+				waitDuration := lease.Spec.BeginTime.Sub(now)
+				logger.Info("Lease is scheduled for the future, waiting",
+					"lease", lease.Name,
+					"requestedBeginTime", lease.Spec.BeginTime,
+					"waitDuration", waitDuration)
+				result.RequeueAfter = waitDuration
+				return nil
+			}
+		}
 		logger.Info("Looking for a matching exporter for lease", "lease", lease.Name, "client", lease.GetClientName(), "selector", lease.Spec.Selector)
 
 		selector, err := lease.GetExporterSelector()
@@ -338,7 +359,14 @@ func (r *LeaseReconciler) attachMatchingPolicies(ctx context.Context, lease *jum
 						}
 						if clientSelector.Matches(labels.Set(jclient.Labels)) {
 							if p.MaximumDuration != nil {
-								if lease.Spec.Duration.Duration > p.MaximumDuration.Duration {
+								// Calculate requested duration (may be from explicit Duration or computed from times)
+								requestedDuration := time.Duration(0)
+								if lease.Spec.Duration != nil {
+									requestedDuration = lease.Spec.Duration.Duration
+								} else if lease.Spec.BeginTime != nil && lease.Spec.EndTime != nil {
+									requestedDuration = lease.Spec.EndTime.Sub(lease.Spec.BeginTime.Time)
+								}
+								if requestedDuration > p.MaximumDuration.Duration {
 									// TODO: we probably should keep this on the list of approved exporters
 									// but mark as excessive duration so we can report it on the status
 									// of lease if no other options exist