Requeue in approval and generic specializer (#452)

In the most recent Porch builds, I have cleaned up a bunch of noisy
watch notifications. This is a good thing, except that our approval
controller and specializers were relying on the verbose watch events
being thrown out. This means that our e2e test times will go up a lot if
we bump Porch version. This PR resolves that, by requeueing when it
makes sense to. There is more we could do (for example watching
PackageVariants so we find out immediately when they flip to Ready
state), but this is a good start. Here are the timings I have:

| Test | R1 | Current Nephio<br/>New Porch| This PR Nephio<br/>New Porch
|
| ---- | ---- | ----------------------- | -------------------------|
| 001.sh | 235 secs | 540 secs | 235 secs |
| 002.sh | 730 secs | 1307 secs | 584 secs |
| 003.sh | 112 secs | 53 secs | 69 secs |
| 004.sh | 152 secs | 432 secs | 91 secs |
| 005.sh | 358 secs | 388 secs | 222 secs |
| 006.sh | 307 secs | 413 secs | 176 secs |
| 007.sh | 371 secs | 484 secs | 186 secs |
| 008.sh | 291 secs | 289 secs |67 secs |
| 009.sh | 26 secs | 30 secs | 31 secs |

I also added events in the approval controller for when it proposes and
approves the PackageRevision.

controllers/pkg/reconcilers/approval/reconciler.go

@@ -47,6 +47,7 @@ const (
 	DelayAnnotationName          = "approval.nephio.org/delay"
 	PolicyAnnotationName         = "approval.nephio.org/policy"
 	InitialPolicyAnnotationValue = "initial"
+	RequeueDuration              = 10 * time.Second
 )
 
 func init() {
@@ -122,7 +123,7 @@ func (r *reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		r.recorder.Event(pr, corev1.EventTypeNormal,
 			"NotApproved", "owning PackageVariant not Ready")
 
-		return ctrl.Result{}, nil
+		return ctrl.Result{RequeueAfter: RequeueDuration}, nil
 	}
 
 	// All policies require readiness gates to be met, so if they
@@ -131,7 +132,7 @@ func (r *reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		r.recorder.Event(pr, corev1.EventTypeNormal,
 			"NotApproved", "readiness gates not met")
 
-		return ctrl.Result{}, nil
+		return ctrl.Result{RequeueAfter: RequeueDuration}, nil
 	}
 
 	// Readiness is met, so check our other policies
@@ -157,7 +158,7 @@ func (r *reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		r.recorder.Eventf(pr, corev1.EventTypeNormal,
 			"NotApproved", "approval policy %q not met", policy)
 
-		return ctrl.Result{}, nil
+		return ctrl.Result{RequeueAfter: RequeueDuration}, nil
 	}
 
 	// Delay if needed, and let the user know via an event
@@ -185,8 +186,13 @@ func (r *reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		return ctrl.Result{RequeueAfter: requeue}, nil
 	}
 
+	action := "approving"
+	reason := "Approved"
+
 	// All policies met
 	if pr.Spec.Lifecycle == porchv1alpha1.PackageRevisionLifecycleDraft {
+		action = "proposing"
+		reason = "Proposed"
 		pr.Spec.Lifecycle = porchv1alpha1.PackageRevisionLifecycleProposed
 		err = r.Update(ctx, pr)
 	} else {
@@ -198,7 +204,10 @@ func (r *reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 
 	if err != nil {
 		r.recorder.Eventf(pr, corev1.EventTypeWarning,
-			"Error", "error approving: %s", err.Error())
+			"Error", "error %s: %s", action, err.Error())
+	} else {
+		r.recorder.Eventf(pr, corev1.EventTypeNormal,
+			reason, "all approval policies met")
 	}
 
 	return ctrl.Result{}, err

controllers/pkg/reconcilers/generic-specializer/reconciler.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"reflect"
 	"strings"
+	"time"
 
 	"github.com/GoogleContainerTools/kpt-functions-sdk/go/fn"
 	kptv1 "github.com/GoogleContainerTools/kpt/pkg/api/kptfile/v1"
@@ -51,6 +52,10 @@ import (
 	"sigs.k8s.io/kustomize/kyaml/kio/kioutil"
 )
 
+const (
+	RequeueDuration = 10 * time.Second
+)
+
 func init() {
 	reconcilerinterface.Register("genericspecializer", &reconciler{})
 }
@@ -111,14 +116,14 @@ func (r *reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		r.recorder.Event(pr, corev1.EventTypeWarning,
 			"Error", fmt.Sprintf("could not get owning PackageVariant: %s", err.Error()))
 
-		return ctrl.Result{}, nil
+		return ctrl.Result{}, err
 	}
 
 	if !pvReady {
 		r.recorder.Event(pr, corev1.EventTypeNormal,
 			"Waiting", "owning PackageVariant not Ready")
 
-		return ctrl.Result{}, nil
+		return ctrl.Result{RequeueAfter: RequeueDuration}, nil
 	}
 
 	ipamf := ipamfn.New(r.ipamClientProxy)