Merge pull request kubernetes-sigs#9810 from chrischdi/pr-errclusterlocked-requeue-minute

k8s-ci-robot · web-flow · commit 1b56bf9c427a · 2023-12-05T12:51:59.000+01:00
🐛 ClusterCacheTracker: Use RequeueAfter instead of immediate requeue on ErrClusterLocked to not have exponentially increasing requeue time
diff --git a/bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go b/bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go
@@ -239,7 +239,7 @@ func (r *KubeadmConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		// Requeue if the reconcile failed because the ClusterCacheTracker was locked for
 		// the current cluster because of concurrent access.
 		log.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")
-		return ctrl.Result{Requeue: true}, nil
+		return ctrl.Result{RequeueAfter: time.Minute}, nil
 	}
 	return res, err
 }
diff --git a/controlplane/kubeadm/internal/controllers/controller.go b/controlplane/kubeadm/internal/controllers/controller.go
@@ -243,7 +243,7 @@ func (r *KubeadmControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.
 		// the current cluster because of concurrent access.
 		if errors.Is(err, remote.ErrClusterLocked) {
 			log.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")
-			return ctrl.Result{Requeue: true}, nil
+			return ctrl.Result{RequeueAfter: time.Minute}, nil
 		}
 		return res, err
 	}
@@ -254,7 +254,7 @@ func (r *KubeadmControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.
 	// the current cluster because of concurrent access.
 	if errors.Is(err, remote.ErrClusterLocked) {
 		log.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")
-		return ctrl.Result{Requeue: true}, nil
+		return ctrl.Result{RequeueAfter: time.Minute}, nil
 	}
 	return res, err
 }
diff --git a/exp/internal/controllers/machinepool_controller.go b/exp/internal/controllers/machinepool_controller.go
@@ -19,6 +19,7 @@ package controllers
 import (
 	"context"
 	"fmt"
+	"time"
 
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
@@ -197,7 +198,7 @@ func (r *MachinePoolReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		// the current cluster because of concurrent access.
 		if errors.Is(err, remote.ErrClusterLocked) {
 			log.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")
-			return ctrl.Result{Requeue: true}, nil
+			return ctrl.Result{RequeueAfter: time.Minute}, nil
 		}
 		return ctrl.Result{}, err
 	}
@@ -215,7 +216,7 @@ func (r *MachinePoolReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	// the current cluster because of concurrent access.
 	if errors.Is(err, remote.ErrClusterLocked) {
 		log.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")
-		return ctrl.Result{Requeue: true}, nil
+		return ctrl.Result{RequeueAfter: time.Minute}, nil
 	}
 	return res, err
 }
diff --git a/internal/controllers/machine/machine_controller.go b/internal/controllers/machine/machine_controller.go
@@ -206,7 +206,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re
 		// the current cluster because of concurrent access.
 		if errors.Is(err, remote.ErrClusterLocked) {
 			log.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")
-			return ctrl.Result{Requeue: true}, nil
+			return ctrl.Result{RequeueAfter: time.Minute}, nil
 		}
 		return res, err
 	}
@@ -224,7 +224,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re
 	// the current cluster because of concurrent access.
 	if errors.Is(err, remote.ErrClusterLocked) {
 		log.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")
-		return ctrl.Result{Requeue: true}, nil
+		return ctrl.Result{RequeueAfter: time.Minute}, nil
 	}
 	return res, err
 }
@@ -606,7 +606,7 @@ func (r *Reconciler) drainNode(ctx context.Context, cluster *clusterv1.Cluster,
 	if err != nil {
 		if errors.Is(err, remote.ErrClusterLocked) {
 			log.V(5).Info("Requeuing drain Node because another worker has the lock on the ClusterCacheTracker")
-			return ctrl.Result{Requeue: true}, nil
+			return ctrl.Result{RequeueAfter: time.Minute}, nil
 		}
 		log.Error(err, "Error creating a remote client for cluster while draining Node, won't retry")
 		return ctrl.Result{}, nil
diff --git a/internal/controllers/machinehealthcheck/machinehealthcheck_controller.go b/internal/controllers/machinehealthcheck/machinehealthcheck_controller.go
@@ -176,7 +176,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re
 		// the current cluster because of concurrent access.
 		if errors.Is(err, remote.ErrClusterLocked) {
 			log.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")
-			return ctrl.Result{Requeue: true}, nil
+			return ctrl.Result{RequeueAfter: time.Minute}, nil
 		}
 		log.Error(err, "Failed to reconcile MachineHealthCheck")
 		r.recorder.Eventf(m, corev1.EventTypeWarning, "ReconcileError", "%v", err)
diff --git a/internal/controllers/machineset/machineset_controller.go b/internal/controllers/machineset/machineset_controller.go
@@ -184,7 +184,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re
 		// the current cluster because of concurrent access.
 		if errors.Is(err, remote.ErrClusterLocked) {
 			log.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")
-			return ctrl.Result{Requeue: true}, nil
+			return ctrl.Result{RequeueAfter: time.Minute}, nil
 		}
 		r.recorder.Eventf(machineSet, corev1.EventTypeWarning, "ReconcileError", "%v", err)
 	}
diff --git a/internal/controllers/topology/cluster/cluster_controller.go b/internal/controllers/topology/cluster/cluster_controller.go
@@ -209,7 +209,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re
 		// the current cluster because of concurrent access.
 		if errors.Is(err, remote.ErrClusterLocked) {
 			log.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")
-			return ctrl.Result{Requeue: true}, nil
+			return ctrl.Result{RequeueAfter: time.Minute}, nil
 		}
 	}
 	return result, err
diff --git a/test/infrastructure/docker/exp/internal/controllers/dockermachinepool_controller.go b/test/infrastructure/docker/exp/internal/controllers/dockermachinepool_controller.go
@@ -20,6 +20,7 @@ package controllers
 import (
 	"context"
 	"fmt"
+	"time"
 
 	"github.com/pkg/errors"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -158,7 +159,7 @@ func (r *DockerMachinePoolReconciler) Reconcile(ctx context.Context, req ctrl.Re
 	// the current cluster because of concurrent access.
 	if errors.Is(err, remote.ErrClusterLocked) {
 		log.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")
-		return ctrl.Result{Requeue: true}, nil
+		return ctrl.Result{RequeueAfter: time.Minute}, nil
 	}
 	return res, err
 }
diff --git a/test/infrastructure/docker/internal/controllers/dockermachine_controller.go b/test/infrastructure/docker/internal/controllers/dockermachine_controller.go
@@ -195,7 +195,7 @@ func (r *DockerMachineReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	// the current cluster because of concurrent access.
 	if errors.Is(err, remote.ErrClusterLocked) {
 		log.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")
-		return ctrl.Result{Requeue: true}, nil
+		return ctrl.Result{RequeueAfter: time.Minute}, nil
 	}
 	return res, err
 }

Original file line number	Diff line number	Diff line change
`@@ -239,7 +239,7 @@ func (r *KubeadmConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques`
`239`	`239`	`// Requeue if the reconcile failed because the ClusterCacheTracker was locked for`
`240`	`240`	`// the current cluster because of concurrent access.`
`241`	`241`	`log.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")`
`242`		`- return ctrl.Result{Requeue: true}, nil`
	`242`	`+ return ctrl.Result{RequeueAfter: time.Minute}, nil`
`243`	`243`	`}`
`244`	`244`	`return res, err`
`245`	`245`	`}`
Original file line number	Diff line number	Diff line change
`@@ -243,7 +243,7 @@ func (r *KubeadmControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.`
`243`	`243`	`// the current cluster because of concurrent access.`
`244`	`244`	`if errors.Is(err, remote.ErrClusterLocked) {`
`245`	`245`	`log.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")`
`246`		`- return ctrl.Result{Requeue: true}, nil`
	`246`	`+ return ctrl.Result{RequeueAfter: time.Minute}, nil`
`247`	`247`	`}`
`248`	`248`	`return res, err`
`249`	`249`	`}`
`@@ -254,7 +254,7 @@ func (r *KubeadmControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.`
`254`	`254`	`// the current cluster because of concurrent access.`
`255`	`255`	`if errors.Is(err, remote.ErrClusterLocked) {`
`256`	`256`	`log.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")`
`257`		`- return ctrl.Result{Requeue: true}, nil`
	`257`	`+ return ctrl.Result{RequeueAfter: time.Minute}, nil`
`258`	`258`	`}`
`259`	`259`	`return res, err`
`260`	`260`	`}`
Original file line number	Diff line number	Diff line change
`@@ -19,6 +19,7 @@ package controllers`
`19`	`19`	`import (`
`20`	`20`	`"context"`
`21`	`21`	`"fmt"`
	`22`	`+ "time"`
`22`	`23`
`23`	`24`	`"github.com/pkg/errors"`
`24`	`25`	`corev1 "k8s.io/api/core/v1"`
`@@ -197,7 +198,7 @@ func (r *MachinePoolReconciler) Reconcile(ctx context.Context, req ctrl.Request)`
`197`	`198`	`// the current cluster because of concurrent access.`
`198`	`199`	`if errors.Is(err, remote.ErrClusterLocked) {`
`199`	`200`	`log.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")`
`200`		`- return ctrl.Result{Requeue: true}, nil`
	`201`	`+ return ctrl.Result{RequeueAfter: time.Minute}, nil`
`201`	`202`	`}`
`202`	`203`	`return ctrl.Result{}, err`
`203`	`204`	`}`
`@@ -215,7 +216,7 @@ func (r *MachinePoolReconciler) Reconcile(ctx context.Context, req ctrl.Request)`
`215`	`216`	`// the current cluster because of concurrent access.`
`216`	`217`	`if errors.Is(err, remote.ErrClusterLocked) {`
`217`	`218`	`log.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")`
`218`		`- return ctrl.Result{Requeue: true}, nil`
	`219`	`+ return ctrl.Result{RequeueAfter: time.Minute}, nil`
`219`	`220`	`}`
`220`	`221`	`return res, err`
`221`	`222`	`}`
Original file line number	Diff line number	Diff line change
`@@ -206,7 +206,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re`
`206`	`206`	`// the current cluster because of concurrent access.`
`207`	`207`	`if errors.Is(err, remote.ErrClusterLocked) {`
`208`	`208`	`log.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")`
`209`		`- return ctrl.Result{Requeue: true}, nil`
	`209`	`+ return ctrl.Result{RequeueAfter: time.Minute}, nil`
`210`	`210`	`}`
`211`	`211`	`return res, err`
`212`	`212`	`}`
`@@ -224,7 +224,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re`
`224`	`224`	`// the current cluster because of concurrent access.`
`225`	`225`	`if errors.Is(err, remote.ErrClusterLocked) {`
`226`	`226`	`log.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")`
`227`		`- return ctrl.Result{Requeue: true}, nil`
	`227`	`+ return ctrl.Result{RequeueAfter: time.Minute}, nil`
`228`	`228`	`}`
`229`	`229`	`return res, err`
`230`	`230`	`}`
`@@ -606,7 +606,7 @@ func (r Reconciler) drainNode(ctx context.Context, cluster clusterv1.Cluster,`
`606`	`606`	`if err != nil {`
`607`	`607`	`if errors.Is(err, remote.ErrClusterLocked) {`
`608`	`608`	`log.V(5).Info("Requeuing drain Node because another worker has the lock on the ClusterCacheTracker")`
`609`		`- return ctrl.Result{Requeue: true}, nil`
	`609`	`+ return ctrl.Result{RequeueAfter: time.Minute}, nil`
`610`	`610`	`}`
`611`	`611`	`log.Error(err, "Error creating a remote client for cluster while draining Node, won't retry")`
`612`	`612`	`return ctrl.Result{}, nil`
Original file line number	Diff line number	Diff line change
`@@ -176,7 +176,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re`
`176`	`176`	`// the current cluster because of concurrent access.`
`177`	`177`	`if errors.Is(err, remote.ErrClusterLocked) {`
`178`	`178`	`log.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")`
`179`		`- return ctrl.Result{Requeue: true}, nil`
	`179`	`+ return ctrl.Result{RequeueAfter: time.Minute}, nil`
`180`	`180`	`}`
`181`	`181`	`log.Error(err, "Failed to reconcile MachineHealthCheck")`
`182`	`182`	`r.recorder.Eventf(m, corev1.EventTypeWarning, "ReconcileError", "%v", err)`
Original file line number	Diff line number	Diff line change
`@@ -184,7 +184,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re`
`184`	`184`	`// the current cluster because of concurrent access.`
`185`	`185`	`if errors.Is(err, remote.ErrClusterLocked) {`
`186`	`186`	`log.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")`
`187`		`- return ctrl.Result{Requeue: true}, nil`
	`187`	`+ return ctrl.Result{RequeueAfter: time.Minute}, nil`
`188`	`188`	`}`
`189`	`189`	`r.recorder.Eventf(machineSet, corev1.EventTypeWarning, "ReconcileError", "%v", err)`
`190`	`190`	`}`
Original file line number	Diff line number	Diff line change
`@@ -209,7 +209,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re`
`209`	`209`	`// the current cluster because of concurrent access.`
`210`	`210`	`if errors.Is(err, remote.ErrClusterLocked) {`
`211`	`211`	`log.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")`
`212`		`- return ctrl.Result{Requeue: true}, nil`
	`212`	`+ return ctrl.Result{RequeueAfter: time.Minute}, nil`
`213`	`213`	`}`
`214`	`214`	`}`
`215`	`215`	`return result, err`
Original file line number	Diff line number	Diff line change
`@@ -195,7 +195,7 @@ func (r *DockerMachineReconciler) Reconcile(ctx context.Context, req ctrl.Reques`
`195`	`195`	`// the current cluster because of concurrent access.`
`196`	`196`	`if errors.Is(err, remote.ErrClusterLocked) {`
`197`	`197`	`log.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker")`
`198`		`- return ctrl.Result{Requeue: true}, nil`
	`198`	`+ return ctrl.Result{RequeueAfter: time.Minute}, nil`
`199`	`199`	`}`
`200`	`200`	`return res, err`
`201`	`201`	`}`