Replace imperative Requeue calls with a declarative Requeue event

Replace explicit Requeue reconciler-event (enqueue) calls with an
a return error (event) of type `requeueKeyError`

This approach lets the knative generated reconciler code to do the
requeuing when we pass our intent by returning an `event` of type
`requeueKeyError`.

Signed-off-by: Nikhil Thomas <[email protected]>

pkg/apis/operator/v1alpha1/const.go

@@ -16,7 +16,11 @@ limitations under the License.
 
 package v1alpha1
 
-import "fmt"
+import (
+	"fmt"
+	"knative.dev/pkg/controller"
+	"time"
+)
 
 const (
 	// operatorVersion
@@ -47,6 +51,8 @@ const (
 	InstallerSetType       = "operator.tekton.dev/type"
 
 	UpgradePending = "upgrade pending"
+
+	RequeueDelay = 10 * time.Second
 )
 
 var (
@@ -55,6 +61,8 @@ var (
 	// that we proceed ahead with updated object
 	RECONCILE_AGAIN_ERR = fmt.Errorf("reconcile again and proceed")
 
+	REQUEUE_EVENT_AFTER = controller.NewRequeueAfter(RequeueDelay)
+
 	// DEPENDENCY_UPGRADE_PENDING_ERR
 	// When a reconciler cannot proceed due to an upgrade in progress of a dependency
 	DEPENDENCY_UPGRADE_PENDING_ERR = fmt.Errorf("dependency upgrade pending")

pkg/reconciler/kubernetes/tektonchain/controller.go

@@ -72,9 +72,6 @@ func NewExtendedController(generator common.ExtensionGenerator) injection.Contro
 		}
 		impl := tektonChainreconciler.NewImpl(ctx, c)
 
-		// Add enqueue func in reconciler
-		c.enqueueAfter = impl.EnqueueAfter
-
 		logger.Info("Setting up event handlers for Tekton Chain")
 
 		tektonChaininformer.Get(ctx).Informer().AddEventHandler(controller.HandleAll(impl.Enqueue))

pkg/reconciler/kubernetes/tektonchain/tektonchain.go

@@ -19,8 +19,6 @@ package tektonchain
 import (
 	"context"
 	"fmt"
-	"time"
-
 	mf "github.com/manifestival/manifestival"
 	"github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"
 	clientset "github.com/tektoncd/operator/pkg/client/clientset/versioned"
@@ -45,9 +43,7 @@ type Reconciler struct {
 	// particular version
 	manifest mf.Manifest
 	// Platform-specific behavior to affect the transform
-	// enqueueAfter enqueues a obj after a duration
-	enqueueAfter func(obj interface{}, after time.Duration)
-	extension    common.Extension
+	extension common.Extension
 	// chainVersion describes the current chain version
 	chainVersion    string
 	operatorVersion string
@@ -176,8 +172,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tc *v1alpha1.TektonChain
 			Get(ctx, existingInstallerSet, metav1.GetOptions{})
 		if err == nil {
 			tc.Status.MarkNotReady("Waiting for previous installer set to get deleted")
-			r.enqueueAfter(tc, 10*time.Second)
-			return nil
+			return v1alpha1.REQUEUE_EVENT_AFTER
 		}
 		if !apierrors.IsNotFound(err) {
 			logger.Error("failed to get InstallerSet: %s", err)
@@ -233,8 +228,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tc *v1alpha1.TektonChain
 
 			// after updating installer set enqueue after a duration
 			// to allow changes to get deployed
-			r.enqueueAfter(tc, 20*time.Second)
-			return nil
+			return v1alpha1.REQUEUE_EVENT_AFTER
 		}
 	}
 
@@ -244,18 +238,15 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tc *v1alpha1.TektonChain
 	ready := installedTIS.Status.GetCondition(apis.ConditionReady)
 	if ready == nil {
 		tc.Status.MarkInstallerSetNotReady("Waiting for installation")
-		r.enqueueAfter(tc, 10*time.Second)
-		return nil
+		return v1alpha1.REQUEUE_EVENT_AFTER
 	}
 
 	if ready.Status == corev1.ConditionUnknown {
 		tc.Status.MarkInstallerSetNotReady("Waiting for installation")
-		r.enqueueAfter(tc, 10*time.Second)
-		return nil
+		return v1alpha1.REQUEUE_EVENT_AFTER
 	} else if ready.Status == corev1.ConditionFalse {
 		tc.Status.MarkInstallerSetNotReady(ready.Message)
-		r.enqueueAfter(tc, 10*time.Second)
-		return nil
+		return v1alpha1.REQUEUE_EVENT_AFTER
 	}
 
 	// Mark InstallerSet Ready

pkg/reconciler/kubernetes/tektondashboard/controller.go

@@ -76,9 +76,6 @@ func NewExtendedController(generator common.ExtensionGenerator) injection.Contro
 		}
 		impl := tektonDashboardreconciler.NewImpl(ctx, c)
 
-		// Add enqueue func in reconciler
-		c.enqueueAfter = impl.EnqueueAfter
-
 		logger.Info("Setting up event handlers for tekton-dashboard")
 
 		tektonDashboardInformer.Informer().AddEventHandler(controller.HandleAll(impl.Enqueue))

pkg/reconciler/kubernetes/tektondashboard/tektondashboard.go

@@ -19,8 +19,6 @@ package tektondashboard
 import (
 	"context"
 	"fmt"
-	"time"
-
 	mf "github.com/manifestival/manifestival"
 	"github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"
 	clientset "github.com/tektoncd/operator/pkg/client/clientset/versioned"
@@ -51,8 +49,6 @@ type Reconciler struct {
 	// a particular version with readonly value as false
 	fullaccessManifest mf.Manifest
 	// Platform-specific behavior to affect the transform
-	// enqueueAfter enqueues a obj after a duration
-	enqueueAfter     func(obj interface{}, after time.Duration)
 	extension        common.Extension
 	pipelineInformer pipelineinformer.TektonPipelineInformer
 	operatorVersion  string
@@ -137,8 +133,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, td *v1alpha1.TektonDashb
 		if err.Error() == common.PipelineNotReady {
 			td.Status.MarkDependencyInstalling("tekton-pipelines is still installing")
 			// wait for pipeline status to change
-			r.enqueueAfter(td, 10*time.Second)
-			return nil
+			return v1alpha1.REQUEUE_EVENT_AFTER
 
 		}
 		// (tektonpipeline.opeator.tekton.dev instance not available yet)
@@ -210,8 +205,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, td *v1alpha1.TektonDashb
 			Get(ctx, existingInstallerSet, metav1.GetOptions{})
 		if err == nil {
 			td.Status.MarkNotReady("Waiting for previous installer set to get deleted")
-			r.enqueueAfter(td, 10*time.Second)
-			return nil
+			return v1alpha1.REQUEUE_EVENT_AFTER
 		}
 		if !apierrors.IsNotFound(err) {
 			logger.Error("failed to get InstallerSet: %s", err)
@@ -263,8 +257,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, td *v1alpha1.TektonDashb
 
 			// after updating installer set enqueue after a duration
 			// to allow changes to get deployed
-			r.enqueueAfter(td, 20*time.Second)
-			return nil
+			return v1alpha1.REQUEUE_EVENT_AFTER
 		}
 	}
 
@@ -274,18 +267,15 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, td *v1alpha1.TektonDashb
 	ready := installedTIS.Status.GetCondition(apis.ConditionReady)
 	if ready == nil {
 		td.Status.MarkInstallerSetNotReady("Waiting for installation")
-		r.enqueueAfter(td, 10*time.Second)
-		return nil
+		return v1alpha1.REQUEUE_EVENT_AFTER
 	}
 
 	if ready.Status == corev1.ConditionUnknown {
 		td.Status.MarkInstallerSetNotReady("Waiting for installation")
-		r.enqueueAfter(td, 10*time.Second)
-		return nil
+		return v1alpha1.REQUEUE_EVENT_AFTER
 	} else if ready.Status == corev1.ConditionFalse {
 		td.Status.MarkInstallerSetNotReady(ready.Message)
-		r.enqueueAfter(td, 10*time.Second)
-		return nil
+		return v1alpha1.REQUEUE_EVENT_AFTER
 	}
 
 	// Mark InstallerSet Ready

pkg/reconciler/kubernetes/tektonhub/controller.go

@@ -68,9 +68,6 @@ func NewExtendedController(generator common.ExtensionGenerator) injection.Contro
 		}
 		impl := tektonHubReconciler.NewImpl(ctx, c)
 
-		// Add enqueue func in reconciler
-		c.enqueueAfter = impl.EnqueueAfter
-
 		logger.Info("Setting up event handlers")
 
 		tektonHubInformer.Informer().AddEventHandler(controller.HandleAll(impl.Enqueue))

pkg/reconciler/kubernetes/tektonhub/tektonhub.go

@@ -20,12 +20,10 @@ package tektonhub
 import (
 	"context"
 	"fmt"
-	"path/filepath"
-	"time"
-
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"path/filepath"
 
 	mf "github.com/manifestival/manifestival"
 	"github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"
@@ -51,8 +49,6 @@ type Reconciler struct {
 	manifest mf.Manifest
 	// Platform-specific behavior to affect the transform
 	extension common.Extension
-	// enqueueAfter enqueues a obj after a duration
-	enqueueAfter func(obj interface{}, after time.Duration)
 }
 
 var (
@@ -175,17 +171,15 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, th *v1alpha1.TektonHub)
 
 func (r *Reconciler) handleError(err error, th *v1alpha1.TektonHub) error {
 	if err == v1alpha1.RECONCILE_AGAIN_ERR {
-		r.enqueueAfter(th, 10*time.Second)
-		return nil
+		return v1alpha1.REQUEUE_EVENT_AFTER
 	}
 	return err
 }
 
 func (r *Reconciler) manageUiComponent(ctx context.Context, th *v1alpha1.TektonHub, hubDir, version string) error {
 	if err := r.validateUiConfigMap(ctx, th); err != nil {
 		th.Status.MarkUiDependencyMissing(fmt.Sprintf("UI config map not present: %v", err.Error()))
-		r.enqueueAfter(th, 10*time.Second)
-		return nil
+		return v1alpha1.REQUEUE_EVENT_AFTER
 	}
 
 	th.Status.MarkUiDependenciesInstalled()
@@ -217,8 +211,7 @@ func (r *Reconciler) manageApiComponent(ctx context.Context, th *v1alpha1.Tekton
 	// Validate whether the secrets and configmap are created for API
 	if err := r.validateApiDependencies(ctx, th); err != nil {
 		th.Status.MarkApiDependencyMissing("api secrets not present")
-		r.enqueueAfter(th, 10*time.Second)
-		return err
+		return v1alpha1.REQUEUE_EVENT_AFTER
 	}
 
 	th.Status.MarkApiDependenciesInstalled()

pkg/reconciler/kubernetes/tektoninstallerset/controller.go

@@ -59,9 +59,6 @@ func NewExtendedController() injection.ControllerConstructor {
 		}
 		impl := tektonInstallerReconciler.NewImpl(ctx, c)
 
-		// Add enqueue func in reconciler
-		c.enqueueAfter = impl.EnqueueAfter
-
 		logger.Info("Setting up event handlers for TektonInstallerSet")
 
 		tektonInstallerinformer.Get(ctx).Informer().AddEventHandler(controller.HandleAll(impl.Enqueue))

pkg/reconciler/kubernetes/tektoninstallerset/tektoninstallerset.go

@@ -19,8 +19,6 @@ package tektoninstallerset
 import (
 	"context"
 	"fmt"
-	"time"
-
 	"github.com/go-logr/logr"
 	mf "github.com/manifestival/manifestival"
 	"github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"
@@ -36,7 +34,6 @@ type Reconciler struct {
 	operatorClientSet clientset.Interface
 	mfClient          mf.Client
 	mfLogger          logr.Logger
-	enqueueAfter      func(obj interface{}, after time.Duration)
 }
 
 // Reconciler implements controller.Reconciler
@@ -148,8 +145,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, installerSet *v1alpha1.T
 	err = installer.IsWebhookReady()
 	if err != nil {
 		installerSet.Status.MarkWebhookNotReady(err.Error())
-		r.enqueueAfter(installerSet, time.Second*10)
-		return nil
+		return v1alpha1.REQUEUE_EVENT_AFTER
 	}
 
 	// Update Status for Webhook
@@ -159,8 +155,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, installerSet *v1alpha1.T
 	err = installer.IsControllerReady()
 	if err != nil {
 		installerSet.Status.MarkControllerNotReady(err.Error())
-		r.enqueueAfter(installerSet, time.Second*10)
-		return nil
+		return v1alpha1.REQUEUE_EVENT_AFTER
 	}
 
 	// Update Ready status of Controller
@@ -179,8 +174,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, installerSet *v1alpha1.T
 	err = installer.AllDeploymentsReady()
 	if err != nil {
 		installerSet.Status.MarkAllDeploymentsNotReady(err.Error())
-		r.enqueueAfter(installerSet, time.Second*10)
-		return nil
+		return v1alpha1.REQUEUE_EVENT_AFTER
 	}
 
 	// Mark all deployments ready
@@ -191,8 +185,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, installerSet *v1alpha1.T
 
 func (r *Reconciler) handleError(err error, installerSet *v1alpha1.TektonInstallerSet) error {
 	if err == v1alpha1.RECONCILE_AGAIN_ERR {
-		r.enqueueAfter(installerSet, 10*time.Second)
-		return nil
+		return v1alpha1.REQUEUE_EVENT_AFTER
 	}
 	return err
 }

pkg/reconciler/kubernetes/tektonpipeline/controller.go

@@ -74,9 +74,6 @@ func NewExtendedController(generator common.ExtensionGenerator) injection.Contro
 		}
 		impl := tektonPipelineReconciler.NewImpl(ctx, c)
 
-		// Add enqueue func in reconciler
-		c.enqueueAfter = impl.EnqueueAfter
-
 		logger.Info("Setting up event handlers for TektonPipeline")
 
 		tektonPipelineInformer.Get(ctx).Informer().AddEventHandler(controller.HandleAll(impl.Enqueue))

pkg/reconciler/kubernetes/tektonpipeline/tektonpipeline.go

@@ -19,8 +19,6 @@ package tektonpipeline
 import (
 	"context"
 	"fmt"
-	"time"
-
 	mf "github.com/manifestival/manifestival"
 	"github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"
 	clientset "github.com/tektoncd/operator/pkg/client/clientset/versioned"
@@ -58,8 +56,6 @@ type Reconciler struct {
 	manifest mf.Manifest
 	// Platform-specific behavior to affect the transform
 	extension common.Extension
-	// enqueueAfter enqueues a obj after a duration
-	enqueueAfter func(obj interface{}, after time.Duration)
 	// metrics handles metrics for pipeline install
 	metrics         *Recorder
 	kubeClientSet   kubernetes.Interface
@@ -212,8 +208,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tp *v1alpha1.TektonPipel
 			Get(ctx, existingInstallerSet, metav1.GetOptions{})
 		if err == nil {
 			tp.Status.MarkNotReady("Waiting for previous installer set to get deleted")
-			r.enqueueAfter(tp, 10*time.Second)
-			return nil
+			return v1alpha1.REQUEUE_EVENT_AFTER
 		}
 		if !apierrors.IsNotFound(err) {
 			logger.Error("failed to get InstallerSet: %s", err)
@@ -257,8 +252,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tp *v1alpha1.TektonPipel
 
 			// after updating installer set enqueue after a duration
 			// to allow changes to get deployed
-			r.enqueueAfter(tp, 20*time.Second)
-			return nil
+			return v1alpha1.REQUEUE_EVENT_AFTER
 		}
 	}
 
@@ -268,14 +262,12 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tp *v1alpha1.TektonPipel
 	ready := installedTIS.Status.GetCondition(apis.ConditionReady)
 	if ready == nil {
 		tp.Status.MarkInstallerSetNotReady("Waiting for installation")
-		r.enqueueAfter(tp, 10*time.Second)
-		return nil
+		return v1alpha1.REQUEUE_EVENT_AFTER
 	}
 
 	if ready.Status == corev1.ConditionUnknown {
 		tp.Status.MarkInstallerSetNotReady("Waiting for installation")
-		r.enqueueAfter(tp, 10*time.Second)
-		return nil
+		return v1alpha1.REQUEUE_EVENT_AFTER
 	} else if ready.Status == corev1.ConditionFalse {
 		tp.Status.MarkInstallerSetNotReady(ready.Message)
 		manifest := r.manifest
@@ -284,8 +276,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, tp *v1alpha1.TektonPipel
 			return err
 		}
 		err = common.PreemptDeadlock(ctx, &manifest, r.kubeClientSet, v1alpha1.PipelineResourceName)
-		r.enqueueAfter(tp, 10*time.Second)
-		return err
+		return v1alpha1.REQUEUE_EVENT_AFTER
 	}
 
 	// Mark InstallerSet Ready

pkg/reconciler/kubernetes/tektontrigger/controller.go

@@ -78,9 +78,6 @@ func NewExtendedController(generator common.ExtensionGenerator) injection.Contro
 		}
 		impl := tektonTriggerreconciler.NewImpl(ctx, c)
 
-		// Add enqueue func in reconciler
-		c.enqueueAfter = impl.EnqueueAfter
-
 		logger.Info("Setting up event handlers for TektonTrigger")
 
 		tektonTriggerinformer.Get(ctx).Informer().AddEventHandler(controller.HandleAll(impl.Enqueue))