Simplify timeouts (#30)

Author: tardieu

api/v1beta1/appwrapper_types.go

@@ -93,9 +93,6 @@ type AppWrapperDispatcherStatus struct {
 	// When last dispatched
 	LastDispatchingTime metav1.Time `json:"lastDispatchingTime,omitempty"`
 
-	// When last requeued
-	LastRequeuingTime metav1.Time `json:"lastRequeuingTime,omitempty"`
-
 	// How many times requeued
 	Requeued int32 `json:"requeued,omitempty"`
 

api/v1beta1/zz_generated.deepcopy.go

@@ -52,10 +52,6 @@ spec:
                     description: When last dispatched
                     format: date-time
                     type: string
-                  lastRequeuingTime:
-                    description: When last requeued
-                    format: date-time
-                    type: string
                   phase:
                     description: Phase
                     type: string

config/crd/bases/workload.codeflare.dev_appwrappers.yaml

@@ -78,35 +78,22 @@ func (r *Dispatcher) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 
 	case mcadv1beta1.Dispatching:
 		if len(appWrapper.Spec.DispatchingGates) > 0 {
-			appWrapper.Spec.DispatcherStatus.LastRequeuingTime = metav1.Now()
 			return r.update(ctx, appWrapper, mcadv1beta1.Requeuing, "requeued due to dispatching gate")
 		}
 		if appWrapper.Status.RunnerStatus.Phase == mcadv1beta1.Dispatching {
 			// runner is ready to dispatch
 			return r.update(ctx, appWrapper, mcadv1beta1.Running)
 		}
-		if isSlowDispatching(appWrapper) {
-			// runner has not acknowledged the job
-			// requeue or fail if max retries exhausted
-			return r.requeueOrFail(ctx, appWrapper)
-		} else {
-			// requeue reconciliation
-			return ctrl.Result{Requeue: true}, nil
-		}
+		// requeue reconciliation
+		return ctrl.Result{Requeue: true}, nil
 
 	case mcadv1beta1.Requeuing:
 		if appWrapper.Status.RunnerStatus.Phase == mcadv1beta1.Empty {
 			// runner has deleted/never created the wrapped resources
 			return r.update(ctx, appWrapper, mcadv1beta1.Queued)
 		}
-		if isSlowRequeuing(appWrapper) {
-			// runner has not completed deletion
-			// give up requeuing and fail instead
-			return r.update(ctx, appWrapper, mcadv1beta1.Failed)
-		} else {
-			// requeue reconciliation
-			return ctrl.Result{Requeue: true}, nil
-		}
+		// requeue reconciliation
+		return ctrl.Result{Requeue: true}, nil
 
 	case mcadv1beta1.Running:
 		if appWrapper.Status.RunnerStatus.Phase == mcadv1beta1.Succeeded {
@@ -122,7 +109,6 @@ func (r *Dispatcher) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 			return r.requeueOrFail(ctx, appWrapper)
 		}
 		if len(appWrapper.Spec.DispatchingGates) > 0 {
-			appWrapper.Spec.DispatcherStatus.LastRequeuingTime = metav1.Now()
 			return r.update(ctx, appWrapper, mcadv1beta1.Requeuing, "requeued due to dispatching gate")
 		}
 		// let the runner monitor the running job
@@ -190,7 +176,6 @@ func (r *Dispatcher) update(ctx context.Context, appWrapper *mcadv1beta1.AppWrap
 func (r *Dispatcher) requeueOrFail(ctx context.Context, appWrapper *mcadv1beta1.AppWrapper) (ctrl.Result, error) {
 	if appWrapper.Spec.DispatcherStatus.Requeued < appWrapper.Spec.Scheduling.Requeuing.MaxNumRequeuings {
 		appWrapper.Spec.DispatcherStatus.Requeued += 1
-		appWrapper.Spec.DispatcherStatus.LastRequeuingTime = metav1.Now()
 		return r.update(ctx, appWrapper, mcadv1beta1.Requeuing)
 	}
 	return r.update(ctx, appWrapper, mcadv1beta1.Failed, "maxNumRequeuings exceeded")
@@ -230,13 +215,3 @@ func (r *Dispatcher) dispatch(ctx context.Context) (ctrl.Result, error) {
 	// requeue to continue to dispatch queued appWrappers
 	return ctrl.Result{Requeue: true}, nil
 }
-
-// Is requeuing too slow?
-func isSlowRequeuing(appWrapper *mcadv1beta1.AppWrapper) bool {
-	return metav1.Now().After(appWrapper.Spec.DispatcherStatus.LastRequeuingTime.Add(requeuingTimeout))
-}
-
-// Is dispatching too slow?
-func isSlowDispatching(appWrapper *mcadv1beta1.AppWrapper) bool {
-	return metav1.Now().After(appWrapper.Spec.DispatcherStatus.LastDispatchingTime.Add(dispatchingTimeout))
-}

internal/controller/dispatcher.go

@@ -87,7 +87,7 @@ func (r *Runner) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result,
 	case mcadv1beta1.Requeuing:
 		// delete wrapped resources
 		if r.deleteResources(ctx, appWrapper) != 0 {
-			if time.Now().After(appWrapper.Status.RunnerStatus.LastRequeuingTime.Add(time.Minute)) {
+			if isSlowRequeuing(appWrapper) {
 				r.forceDelete(ctx, appWrapper)
 			}
 			// requeue reconciliation
@@ -117,15 +117,6 @@ func (r *Runner) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result,
 		if err != nil {
 			return ctrl.Result{}, err
 		}
-		// check for failure conditions
-		if counts.Failed > 0 {
-			// set errored status
-			return r.updateStatus(ctx, appWrapper, mcadv1beta1.Errored, "failed pod")
-		}
-		if isSlowRunning(appWrapper) && counts.Other > 0 {
-			// set errored status
-			return r.updateStatus(ctx, appWrapper, mcadv1beta1.Errored, "pod not ready")
-		}
 		if isSlowRunning(appWrapper) && counts.Running < int(appWrapper.Spec.Scheduling.MinAvailable) {
 			// set errored status
 			return r.updateStatus(ctx, appWrapper, mcadv1beta1.Errored, "too few running pods")
@@ -199,3 +190,8 @@ func (r *Runner) updateStatus(ctx context.Context, appWrapper *mcadv1beta1.AppWr
 func isSlowRunning(appWrapper *mcadv1beta1.AppWrapper) bool {
 	return metav1.Now().After(appWrapper.Status.RunnerStatus.LastRunningTime.Add(runningTimeout))
 }
+
+// Is requeuing too slow?
+func isSlowRequeuing(appWrapper *mcadv1beta1.AppWrapper) bool {
+	return metav1.Now().After(appWrapper.Status.RunnerStatus.LastRequeuingTime.Add(time.Minute))
+}

internal/controller/runner.go

@@ -25,16 +25,11 @@ import (
 const (
 	// Timeouts
 	requeuingTimeout     = 2 * time.Minute // minimum wait before aborting Requeuing
-	dispatchingTimeout   = 2 * time.Minute // minimum wait before aborting Dispatching
 	runningTimeout       = 5 * time.Minute // minimum wait before aborting Running
 	cacheConflictTimeout = 5 * time.Minute // minimum wait before invalidating the cache
-	clusterInfoTimeout   = time.Minute     // how long to cache cluster capacity
+	clusterInfoTimeout   = time.Minute     // how often to refresh cluster capacity
 
 	// RequeueAfter delays
-	runDelay      = time.Minute // maximum delay before next reconciliation of a Running AppWrapper
-	dispatchDelay = time.Minute // maximum delay before next "*/*" reconciliation (dispatch)
-
-	// The RequeueAfter delay is the maximum delay before the next reconciliation event.
-	// Reconciliation may be triggered earlier due for instance to pod phase changes.
-	// Reconciliation may be delayed due to the on-going reconciliation of other events.
+	runDelay      = time.Minute // how often to force check running AppWrapper health
+	dispatchDelay = time.Minute // how often to force dispatch
 )