Skip to content

🌱 Migrate from Requeue to RequeueAfter in kcp #13028

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+15 −20
Open

🌱 Migrate from Requeue to RequeueAfter in kcp #13028

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions controlplane/kubeadm/internal/controllers/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,7 @@ func (r *KubeadmControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.
// Initialize the patch helper.
patchHelper, err := patch.NewHelper(kcp, r.Client)
if err != nil {
log.Error(err, "Failed to configure the patch helper")
return ctrl.Result{Requeue: true}, nil
return ctrl.Result{}, err
}

if isPaused, requeue, err := paused.EnsurePausedCondition(ctx, r.Client, cluster, kcp); err != nil || isPaused || requeue {
Expand Down Expand Up @@ -558,8 +557,7 @@ func (r *KubeadmControlPlaneReconciler) reconcile(ctx context.Context, controlPl
// Get the workload cluster client.
workloadCluster, err := controlPlane.GetWorkloadCluster(ctx)
if err != nil {
log.V(2).Info("cannot get remote client to workload cluster, will requeue", "cause", err)
return ctrl.Result{Requeue: true}, nil
return ctrl.Result{}, err
}

// Update kube-proxy daemonset.
Expand Down
2 changes: 1 addition & 1 deletion controlplane/kubeadm/internal/controllers/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ func TestReconcileClusterNoEndpoints(t *testing.T) {
result, err = r.Reconcile(ctx, ctrl.Request{NamespacedName: util.ObjectKey(kcp)})
g.Expect(err).ToNot(HaveOccurred())
// TODO: this should stop to re-queue as soon as we have a proper remote cluster cache in place.
g.Expect(result).To(BeComparableTo(ctrl.Result{Requeue: false, RequeueAfter: 20 * time.Second}))
g.Expect(result.RequeueAfter).To(Equal(20 * time.Second))
g.Expect(r.Client.Get(ctx, util.ObjectKey(kcp), kcp)).To(Succeed())

// Always expect that the Finalizer is set on the passed in resource
Expand Down
2 changes: 1 addition & 1 deletion controlplane/kubeadm/internal/controllers/remediation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ func (r *KubeadmControlPlaneReconciler) reconcileUnhealthyMachines(ctx context.C
controlplanev1.RemediationInProgressAnnotation: remediationInProgressValue,
})

return ctrl.Result{Requeue: true}, nil
return ctrl.Result{}, nil // No need to requeue here. Machine deletion above triggers reconciliation.
Copy link
Member

@sbueringer sbueringer Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return ctrl.Result{}, nil // No need to requeue here. Machine deletion above triggers reconciliation.
return ctrl.Result{RequeueAfter: time.Millisecond}, nil // Technically there is no need to requeue here. Machine deletion above triggers reconciliation. But we have to return a non-zero Result so reconcile above returns.

My bad. We have to requeue here otherwise the reconcile func one level higher does not return which it absolutely has to

(using 1 ms so we don't get an additional requeue on top of the requeue we get from Machine deletion. With e.g. 10s we might get an additional requeue)

}

// Gets the machine to be remediated, which is the "most broken" among the unhealthy machines, determined as the machine
Expand Down
9 changes: 3 additions & 6 deletions controlplane/kubeadm/internal/controllers/scale.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@ func (r *KubeadmControlPlaneReconciler) initializeControlPlane(ctx context.Conte
newMachine.Spec.InfrastructureRef.Kind, klog.KRef(newMachine.Namespace, newMachine.Spec.InfrastructureRef.Name),
newMachine.Spec.Bootstrap.ConfigRef.Kind, klog.KRef(newMachine.Namespace, newMachine.Spec.Bootstrap.ConfigRef.Name))

// Requeue the control plane, in case there are additional operations to perform
return ctrl.Result{Requeue: true}, nil
return ctrl.Result{}, nil // No need to requeue here. Machine creation above triggers reconciliation.
}

func (r *KubeadmControlPlaneReconciler) scaleUpControlPlane(ctx context.Context, controlPlane *internal.ControlPlane) (ctrl.Result, error) {
Expand Down Expand Up @@ -93,8 +92,7 @@ func (r *KubeadmControlPlaneReconciler) scaleUpControlPlane(ctx context.Context,
newMachine.Spec.InfrastructureRef.Kind, klog.KRef(newMachine.Namespace, newMachine.Spec.InfrastructureRef.Name),
newMachine.Spec.Bootstrap.ConfigRef.Kind, klog.KRef(newMachine.Namespace, newMachine.Spec.Bootstrap.ConfigRef.Name))

// Requeue the control plane, in case there are other operations to perform
return ctrl.Result{Requeue: true}, nil
return ctrl.Result{}, nil // No need to requeue here. Machine creation above triggers reconciliation.
}

func (r *KubeadmControlPlaneReconciler) scaleDownControlPlane(
Expand Down Expand Up @@ -147,8 +145,7 @@ func (r *KubeadmControlPlaneReconciler) scaleDownControlPlane(
log.WithValues(controlPlane.StatusToLogKeyAndValues(nil, machineToDelete)...).
Info(fmt.Sprintf("Machine %s deleting (scale down)", machineToDelete.Name), "Machine", klog.KObj(machineToDelete))

// Requeue the control plane, in case there are additional operations to perform
return ctrl.Result{Requeue: true}, nil
return ctrl.Result{}, nil // No need to requeue here. Machine deletion above triggers reconciliation.
}

// preflightChecks checks if the control plane is stable before proceeding with a scale up/scale down operation,
Expand Down
8 changes: 4 additions & 4 deletions controlplane/kubeadm/internal/controllers/scale_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func TestKubeadmControlPlaneReconciler_initializeControlPlane(t *testing.T) {
}

result, err := r.initializeControlPlane(ctx, controlPlane)
g.Expect(result).To(BeComparableTo(ctrl.Result{Requeue: true}))
g.Expect(result.IsZero()).To(BeTrue())
g.Expect(err).ToNot(HaveOccurred())

machineList := &clusterv1.MachineList{}
Expand Down Expand Up @@ -164,7 +164,7 @@ func TestKubeadmControlPlaneReconciler_scaleUpControlPlane(t *testing.T) {
}

result, err := r.scaleUpControlPlane(ctx, controlPlane)
g.Expect(result).To(BeComparableTo(ctrl.Result{Requeue: true}))
g.Expect(result.IsZero()).To(BeTrue())
g.Expect(err).ToNot(HaveOccurred())

controlPlaneMachines := clusterv1.MachineList{}
Expand Down Expand Up @@ -297,7 +297,7 @@ func TestKubeadmControlPlaneReconciler_scaleDownControlPlane_NoError(t *testing.
g.Expect(err).ToNot(HaveOccurred())
result, err := r.scaleDownControlPlane(context.Background(), controlPlane, machineToDelete)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(result).To(BeComparableTo(ctrl.Result{Requeue: true}))
g.Expect(result.IsZero()).To(BeTrue())

controlPlaneMachines := clusterv1.MachineList{}
g.Expect(fakeClient.List(context.Background(), &controlPlaneMachines)).To(Succeed())
Expand Down Expand Up @@ -341,7 +341,7 @@ func TestKubeadmControlPlaneReconciler_scaleDownControlPlane_NoError(t *testing.
g.Expect(err).ToNot(HaveOccurred())
result, err := r.scaleDownControlPlane(context.Background(), controlPlane, machineToDelete)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(result).To(BeComparableTo(ctrl.Result{Requeue: true}))
g.Expect(result.IsZero()).To(BeTrue())

controlPlaneMachines := clusterv1.MachineList{}
g.Expect(fakeClient.List(context.Background(), &controlPlaneMachines)).To(Succeed())
Expand Down
8 changes: 4 additions & 4 deletions controlplane/kubeadm/internal/controllers/update_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func TestKubeadmControlPlaneReconciler_RolloutStrategy_ScaleUp(t *testing.T) {
controlPlane.InjectTestManagementCluster(r.managementCluster)

result, err := r.initializeControlPlane(ctx, controlPlane)
g.Expect(result).To(BeComparableTo(ctrl.Result{Requeue: true}))
g.Expect(result.RequeueAfter).To(Equal(10 * time.Second))
g.Expect(err).ToNot(HaveOccurred())

// initial setup
Expand All @@ -142,7 +142,7 @@ func TestKubeadmControlPlaneReconciler_RolloutStrategy_ScaleUp(t *testing.T) {
machinesUpToDateResults[m.Name] = internal.UpToDateResult{EligibleForInPlaceUpdate: false}
}
result, err = r.updateControlPlane(ctx, controlPlane, needingUpgrade, machinesUpToDateResults)
g.Expect(result).To(BeComparableTo(ctrl.Result{Requeue: true}))
g.Expect(result.IsZero()).To(BeTrue())
g.Expect(err).ToNot(HaveOccurred())
bothMachines := &clusterv1.MachineList{}
g.Eventually(func(g Gomega) {
Expand Down Expand Up @@ -193,7 +193,7 @@ func TestKubeadmControlPlaneReconciler_RolloutStrategy_ScaleUp(t *testing.T) {
// run upgrade the second time, expect we scale down
result, err = r.updateControlPlane(ctx, controlPlane, machinesRequireUpgrade, machinesUpToDateResults)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(result).To(BeComparableTo(ctrl.Result{Requeue: true}))
g.Expect(result.IsZero()).To(BeTrue())
finalMachine := &clusterv1.MachineList{}
g.Eventually(func(g Gomega) {
g.Expect(env.List(ctx, finalMachine, client.InNamespace(cluster.Namespace))).To(Succeed())
Expand Down Expand Up @@ -287,7 +287,7 @@ func TestKubeadmControlPlaneReconciler_RolloutStrategy_ScaleDown(t *testing.T) {
machinesUpToDateResults[m.Name] = internal.UpToDateResult{EligibleForInPlaceUpdate: false}
}
result, err = r.updateControlPlane(ctx, controlPlane, needingUpgrade, machinesUpToDateResults)
g.Expect(result).To(BeComparableTo(ctrl.Result{Requeue: true}))
g.Expect(result.IsZero()).To(BeTrue())
g.Expect(err).ToNot(HaveOccurred())
remainingMachines := &clusterv1.MachineList{}
g.Expect(fakeClient.List(ctx, remainingMachines, client.InNamespace(cluster.Namespace))).To(Succeed())
Expand Down
Loading