diff --git a/workspaces/controller/internal/controller/workspace_activity_test.go b/workspaces/controller/internal/controller/workspace_activity_test.go index 192bd7797..f6befc738 100644 --- a/workspaces/controller/internal/controller/workspace_activity_test.go +++ b/workspaces/controller/internal/controller/workspace_activity_test.go @@ -60,10 +60,10 @@ var ( ) var _ = Describe("mergeReconcileResult", func() { - It("should prefer an immediate requeue", func() { - a := ctrl.Result{Requeue: true} - b := ctrl.Result{RequeueAfter: time.Second} - Expect(mergeReconcileResult(a, b)).To(Equal(a)) + It("should return an empty result when neither requests a requeue", func() { + a := ctrl.Result{} + b := ctrl.Result{} + Expect(mergeReconcileResult(a, b)).To(Equal(ctrl.Result{})) }) It("should prefer the sooner RequeueAfter", func() { diff --git a/workspaces/controller/internal/controller/workspace_controller.go b/workspaces/controller/internal/controller/workspace_controller.go index d46564021..7487aea66 100644 --- a/workspaces/controller/internal/controller/workspace_controller.go +++ b/workspaces/controller/internal/controller/workspace_controller.go @@ -63,6 +63,10 @@ const ( // pod template constants workspacePodTemplateContainerName = "main" + // requeue delay when the local cache is known to be stale (fixed delay, since a stale cache + // is not write contention, so no exponential backoff is needed) + requeueAfterStaleCache = 2 * time.Second + // lengths for resource names generateNameSuffixLength = 6 nameHashLength = 8 @@ -195,7 +199,7 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( if err := r.Update(ctx, workspaceKind); err != nil { if apierrors.IsConflict(err) { log.V(2).Info("update conflict while adding finalizer to WorkspaceKind, will requeue") - return ctrl.Result{Requeue: true}, nil + return ctrl.Result{}, err } log.Error(err, "unable to add finalizer to WorkspaceKind") return ctrl.Result{}, err @@ -292,9 +296,10 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( existingServiceAccount := &corev1.ServiceAccount{} if getErr := r.Get(ctx, client.ObjectKeyFromObject(serviceAccount), existingServiceAccount); getErr != nil { if apierrors.IsNotFound(getErr) { - // the cache is stale, the watch on owned ServiceAccounts will requeue us + // the cache is stale; requeue after a short delay instead of relying on the + // owned-object watch, which will not fire if the ServiceAccount is owned by another controller log.V(2).Info("ServiceAccount already exists but is not in the cache yet, will requeue") - return ctrl.Result{Requeue: true}, nil + return ctrl.Result{RequeueAfter: requeueAfterStaleCache}, nil } log.Error(getErr, "unable to get existing ServiceAccount") return ctrl.Result{}, getErr @@ -306,7 +311,9 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( fmt.Sprintf(stateMsgErrorServiceAccountNotOwned, existingServiceAccount.Name), ) } - return ctrl.Result{Requeue: true}, nil + // the cache is stale (the owner index did not return the ServiceAccount), requeue after + // a short delay so we pick it up once the cache catches up + return ctrl.Result{RequeueAfter: requeueAfterStaleCache}, nil } log.Error(err, "unable to create ServiceAccount") return ctrl.Result{}, err @@ -320,7 +327,7 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( if err := r.Update(ctx, foundServiceAccount); err != nil { if apierrors.IsConflict(err) { log.V(2).Info("update conflict while updating ServiceAccount, will requeue") - return ctrl.Result{Requeue: true}, nil + return ctrl.Result{}, err } log.Error(err, "unable to update ServiceAccount") return ctrl.Result{}, err @@ -384,7 +391,7 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( if err := r.Update(ctx, foundStatefulSet); err != nil { if apierrors.IsConflict(err) { log.V(2).Info("update conflict while updating StatefulSet, will requeue") - return ctrl.Result{Requeue: true}, nil + return ctrl.Result{}, err } log.Error(err, "unable to update StatefulSet") return ctrl.Result{}, err @@ -449,7 +456,7 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( if err := r.Update(ctx, foundService); err != nil { if apierrors.IsConflict(err) { log.V(2).Info("update conflict while updating Service, will requeue") - return ctrl.Result{Requeue: true}, nil + return ctrl.Result{}, err } log.Error(err, "unable to update Service") return ctrl.Result{}, err @@ -516,7 +523,7 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( if err := r.Update(ctx, foundVirtualService); err != nil { if apierrors.IsConflict(err) { log.V(2).Info("update conflict while updating VirtualService, will requeue") - return ctrl.Result{Requeue: true}, nil + return ctrl.Result{}, err } log.Error(err, "unable to update VirtualService") return ctrl.Result{}, err @@ -529,9 +536,6 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( // reconcile RoleBindings if err := r.reconcileRoleBindings(ctx, log, workspace, workspaceKind, serviceAccountName); err != nil { // NOTE: `reconcileRoleBindings()` has already logged the cause, including the conflict case - if apierrors.IsConflict(err) { - return ctrl.Result{Requeue: true}, nil - } return ctrl.Result{}, err } @@ -570,7 +574,7 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( if err := r.Status().Update(ctx, workspace); err != nil { if apierrors.IsConflict(err) { log.V(2).Info("update conflict while updating Workspace status, will requeue") - return ctrl.Result{Requeue: true}, nil + return ctrl.Result{}, err } log.Error(err, "unable to update Workspace status") return ctrl.Result{}, err @@ -589,7 +593,7 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( if err := r.Patch(ctx, workspace, client.MergeFrom(originalWorkspace)); err != nil { if apierrors.IsConflict(err) { log.V(2).Info("update conflict while pausing Workspace, will requeue") - return ctrl.Result{Requeue: true}, nil + return ctrl.Result{}, err } log.Error(err, "unable to pause Workspace") return ctrl.Result{}, err @@ -604,14 +608,7 @@ func (r *WorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( // mergeReconcileResult combines two reconcile results, preferring the sooner requeue. func mergeReconcileResult(a, b ctrl.Result) ctrl.Result { - // TODO: fix the requeue deprecation below switch { - //nolint:staticcheck - case a.Requeue: - return a - //nolint:staticcheck - case b.Requeue: - return b case a.RequeueAfter <= 0: return b case b.RequeueAfter <= 0: @@ -686,7 +683,7 @@ func (r *WorkspaceReconciler) updateWorkspaceState(ctx context.Context, log logr if err := r.Status().Update(ctx, workspace); err != nil { if apierrors.IsConflict(err) { log.V(2).Info("update conflict while updating Workspace status, will requeue") - return ctrl.Result{Requeue: true}, nil + return ctrl.Result{}, err } log.Error(err, "unable to update Workspace status") return ctrl.Result{}, err diff --git a/workspaces/controller/internal/controller/workspacekind_controller.go b/workspaces/controller/internal/controller/workspacekind_controller.go index c260a9796..af6744382 100644 --- a/workspaces/controller/internal/controller/workspacekind_controller.go +++ b/workspaces/controller/internal/controller/workspacekind_controller.go @@ -105,7 +105,7 @@ func (r *WorkspaceKindReconciler) Reconcile(ctx context.Context, req ctrl.Reques if err := r.Update(ctx, workspaceKind); err != nil { if apierrors.IsConflict(err) { log.V(2).Info("update conflict while removing finalizer from WorkspaceKind, will requeue") - return ctrl.Result{Requeue: true}, nil + return ctrl.Result{}, err } log.Error(err, "unable to remove finalizer from WorkspaceKind") return ctrl.Result{}, err @@ -157,7 +157,7 @@ func (r *WorkspaceKindReconciler) Reconcile(ctx context.Context, req ctrl.Reques if err := r.Status().Update(ctx, workspaceKind); err != nil { if apierrors.IsConflict(err) { log.V(2).Info("update conflict while updating WorkspaceKind status, will requeue") - return ctrl.Result{Requeue: true}, nil + return ctrl.Result{}, err } log.Error(err, "unable to update WorkspaceKind status") return ctrl.Result{}, err