Fix controller not timeouting on start hook failure (#222)

Not sure why the check order was wrong in the first place. Timeout condition is now checked *before* checking start job execution.

Author: bastjan

controllers/upgradejob_controller.go

@@ -179,14 +179,6 @@ func (r *UpgradeJobReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 func (r *UpgradeJobReconciler) reconcileStartedJob(ctx context.Context, uj *managedupgradev1beta1.UpgradeJob) (ctrl.Result, error) {
 	l := log.FromContext(ctx).WithName("UpgradeJobReconciler.reconcileStartedJob")
 
-	cont, err := r.executeHooks(ctx, uj, managedupgradev1beta1.EventStart, noTrackingKey, eventInfoWithReason(managedupgradev1beta1.UpgradeJobReasonStarted), time.Time{})
-	if err != nil {
-		return ctrl.Result{}, err
-	}
-	if !cont {
-		return ctrl.Result{}, nil
-	}
-
 	if r.timeSinceStartAfter(uj) > uj.Spec.UpgradeTimeout.Duration {
 		r.setStatusCondition(&uj.Status.Conditions, metav1.Condition{
 			Type:    managedupgradev1beta1.UpgradeJobConditionFailed,
@@ -197,6 +189,14 @@ func (r *UpgradeJobReconciler) reconcileStartedJob(ctx context.Context, uj *mana
 		return ctrl.Result{}, r.Status().Update(ctx, uj)
 	}
 
+	cont, err := r.executeHooks(ctx, uj, managedupgradev1beta1.EventStart, noTrackingKey, eventInfoWithReason(managedupgradev1beta1.UpgradeJobReasonStarted), time.Time{})
+	if err != nil {
+		return ctrl.Result{}, err
+	}
+	if !cont {
+		return ctrl.Result{}, nil
+	}
+
 	var version configv1.ClusterVersion
 	if err := r.Get(ctx, types.NamespacedName{
 		Name: r.ManagedUpstreamClusterVersionName,