🌱 Requeue for Machine Available condition (#12953)

* Requeue for machine Available condition

* Address feedback

Author: fabriziopandini

internal/controllers/machine/machine_controller.go

@@ -195,7 +195,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt
 	return nil
 }
 
-func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) {
+func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (retres ctrl.Result, reterr error) {
 	// Fetch the Machine instance
 	m := &clusterv1.Machine{}
 	if err := r.Client.Get(ctx, req.NamespacedName, m); err != nil {
@@ -255,7 +255,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re
 	}
 
 	defer func() {
-		r.updateStatus(ctx, s)
+		updateRes := r.updateStatus(ctx, s)
+		retres = util.LowestNonZeroResult(retres, updateRes)
 
 		// Always attempt to patch the object and status after each reconciliation.
 		// Patch ObservedGeneration only if the reconciliation completed successfully

internal/controllers/machine/machine_controller_status.go

@@ -42,7 +42,7 @@ import (
 // machine being partially deleted but also for running machines being disrupted e.g. by deleting the node.
 // Additionally, this func should ensure that the conditions managed by this controller are always set in order to
 // comply with the recommendation in the Kubernetes API guidelines.
-func (r *Reconciler) updateStatus(ctx context.Context, s *scope) {
+func (r *Reconciler) updateStatus(ctx context.Context, s *scope) ctrl.Result {
 	// Update status from the Bootstrap Config external resource.
 	// Note: some of the status fields derived from the Bootstrap Config are managed in reconcileBootstrap, e.g. status.BootstrapReady, etc.
 	// here we are taking care only of the delta (condition).
@@ -67,9 +67,9 @@ func (r *Reconciler) updateStatus(ctx context.Context, s *scope) {
 	setDeletingCondition(ctx, s.machine, s.reconcileDeleteExecuted, s.deletingReason, s.deletingMessage)
 	setUpdatingCondition(ctx, s.machine, s.updatingReason, s.updatingMessage)
 	setReadyCondition(ctx, s.machine)
-	setAvailableCondition(ctx, s.machine)
-
 	setMachinePhaseAndLastUpdated(ctx, s.machine)
+
+	return setAvailableCondition(ctx, s.machine)
 }
 
 func setBootstrapReadyCondition(_ context.Context, machine *clusterv1.Machine, bootstrapConfig *unstructured.Unstructured, bootstrapConfigIsNotFound bool) {
@@ -774,7 +774,7 @@ func calculateDeletingConditionForSummary(machine *clusterv1.Machine) conditions
 	}
 }
 
-func setAvailableCondition(ctx context.Context, machine *clusterv1.Machine) {
+func setAvailableCondition(ctx context.Context, machine *clusterv1.Machine) ctrl.Result {
 	log := ctrl.LoggerFrom(ctx)
 	readyCondition := conditions.Get(machine, clusterv1.MachineReadyCondition)
 
@@ -788,7 +788,7 @@ func setAvailableCondition(ctx context.Context, machine *clusterv1.Machine) {
 			Reason:  clusterv1.MachineAvailableInternalErrorReason,
 			Message: "Please check controller logs for errors",
 		})
-		return
+		return ctrl.Result{}
 	}
 
 	if readyCondition.Status != metav1.ConditionTrue {
@@ -797,23 +797,25 @@ func setAvailableCondition(ctx context.Context, machine *clusterv1.Machine) {
 			Status: metav1.ConditionFalse,
 			Reason: clusterv1.MachineNotReadyReason,
 		})
-		return
+		return ctrl.Result{}
 	}
 
-	if time.Since(readyCondition.LastTransitionTime.Time) >= time.Duration(ptr.Deref(machine.Spec.MinReadySeconds, 0))*time.Second {
+	t := time.Since(readyCondition.LastTransitionTime.Time) - time.Duration(ptr.Deref(machine.Spec.MinReadySeconds, 0))*time.Second
+	if t >= 0 {
 		conditions.Set(machine, metav1.Condition{
 			Type:   clusterv1.MachineAvailableCondition,
 			Status: metav1.ConditionTrue,
 			Reason: clusterv1.MachineAvailableReason,
 		})
-		return
+		return ctrl.Result{}
 	}
 
 	conditions.Set(machine, metav1.Condition{
 		Type:   clusterv1.MachineAvailableCondition,
 		Status: metav1.ConditionFalse,
 		Reason: clusterv1.MachineWaitingForMinReadySecondsReason,
 	})
+	return ctrl.Result{RequeueAfter: -t}
 }
 
 func setMachinePhaseAndLastUpdated(_ context.Context, m *clusterv1.Machine) {

internal/controllers/machine/machine_controller_status_test.go

@@ -28,6 +28,7 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	utilfeature "k8s.io/component-base/featuregate/testing"
 	"k8s.io/utils/ptr"
+	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
@@ -1967,6 +1968,7 @@ func TestAvailableCondition(t *testing.T) {
 		name            string
 		machine         *clusterv1.Machine
 		expectCondition metav1.Condition
+		expectRes       ctrl.Result
 	}{
 		{
 			name: "Not Ready",
@@ -1990,6 +1992,7 @@ func TestAvailableCondition(t *testing.T) {
 				Status: metav1.ConditionFalse,
 				Reason: clusterv1.MachineNotReadyReason,
 			},
+			expectRes: ctrl.Result{},
 		},
 		{
 			name: "Ready but still waiting for MinReadySeconds",
@@ -2014,6 +2017,7 @@ func TestAvailableCondition(t *testing.T) {
 				Status: metav1.ConditionFalse,
 				Reason: clusterv1.MachineWaitingForMinReadySecondsReason,
 			},
+			expectRes: ctrl.Result{RequeueAfter: 10 * time.Second},
 		},
 		{
 			name: "Ready and available",
@@ -2038,18 +2042,24 @@ func TestAvailableCondition(t *testing.T) {
 				Status: metav1.ConditionTrue,
 				Reason: clusterv1.MachineAvailableReason,
 			},
+			expectRes: ctrl.Result{},
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			g := NewWithT(t)
 
-			setAvailableCondition(ctx, tc.machine)
+			res := setAvailableCondition(ctx, tc.machine)
 
 			availableCondition := conditions.Get(tc.machine, clusterv1.MachineAvailableCondition)
 			g.Expect(availableCondition).ToNot(BeNil())
 			g.Expect(*availableCondition).To(conditions.MatchCondition(tc.expectCondition, conditions.IgnoreLastTransitionTime(true)))
+
+			g.Expect(res.IsZero()).To(Equal(tc.expectRes.IsZero()))
+			if !tc.expectRes.IsZero() {
+				g.Expect(res.RequeueAfter).To(BeNumerically("~", tc.expectRes.RequeueAfter, 100*time.Millisecond))
+			}
 		})
 	}
 }