Approval controller fixes (#252)

- Increase the default delay to two minutes
- Check the Ready status of the owning PackageVariant

Along with
nephio-project/nephio-example-packages#42 this
should:

Fixes #248
Fixes #249
Fixes #251

controllers/pkg/go.mod

@@ -5,8 +5,9 @@ go 1.20
 require (
 	code.gitea.io/sdk/gitea v0.15.1-0.20230509035020-970776d1c1e9
 	github.com/GoogleContainerTools/kpt v1.0.0-beta.29.0.20230327202912-01513604feaa
-	github.com/GoogleContainerTools/kpt-functions-sdk/go/fn v0.0.0-20230302070146-e8e9cb3c3ae2
+	github.com/GoogleContainerTools/kpt-functions-sdk/go/fn v0.0.0-20230427202446-3255accc518d
 	github.com/GoogleContainerTools/kpt/porch/api v0.0.0-20230504200302-14c7b353e6b6
+	github.com/GoogleContainerTools/kpt/porch/controllers v0.0.0-20230526213300-77a54e3b8e88
 	github.com/go-logr/logr v1.2.4
 	github.com/google/go-cmp v0.5.9
 	github.com/nephio-project/api v0.0.0-20230522173958-63a41669b495

controllers/pkg/go.sum

@@ -7,10 +7,12 @@ github.com/GoogleContainerTools/kpt v1.0.0-beta.29.0.20230327202912-01513604feaa
 github.com/GoogleContainerTools/kpt v1.0.0-beta.29.0.20230327202912-01513604feaa/go.mod h1:eAERMIKb67/4uZU56CULVA8Pc/OQ/YWsha0ja/sFHHM=
 github.com/GoogleContainerTools/kpt-functions-sdk/go/api v0.0.0-20220720212527-133180134b93 h1:c1GhPzYzU2a3E+/2WB9OxoljK2pNYfsBF5Fz9GkdYXs=
 github.com/GoogleContainerTools/kpt-functions-sdk/go/api v0.0.0-20220720212527-133180134b93/go.mod h1:gkK43tTaPXFNASpbIbQImzhmt1hdcdin++kvzTblykc=
-github.com/GoogleContainerTools/kpt-functions-sdk/go/fn v0.0.0-20230302070146-e8e9cb3c3ae2 h1:GDUCDAY2ijsUjg70QPMvWKezRxGKKzU07ckVc5uTgZA=
-github.com/GoogleContainerTools/kpt-functions-sdk/go/fn v0.0.0-20230302070146-e8e9cb3c3ae2/go.mod h1:Pnd3ImgaWS3OBVjztSiGMACMf+CDs20l5nT5Oljy/tA=
+github.com/GoogleContainerTools/kpt-functions-sdk/go/fn v0.0.0-20230427202446-3255accc518d h1:kgC/R6Kl+tBjsRvcPr4Beae1MiHumNMtbmUTy7qlPZI=
+github.com/GoogleContainerTools/kpt-functions-sdk/go/fn v0.0.0-20230427202446-3255accc518d/go.mod h1:Pnd3ImgaWS3OBVjztSiGMACMf+CDs20l5nT5Oljy/tA=
 github.com/GoogleContainerTools/kpt/porch/api v0.0.0-20230504200302-14c7b353e6b6 h1:U5QOwyapVo8PcFzMr4hSPOMIYlNRVn35tINRk9WntXQ=
 github.com/GoogleContainerTools/kpt/porch/api v0.0.0-20230504200302-14c7b353e6b6/go.mod h1:xQ4kNXVytubqrhwIqABBO/Ya4/v7m6VehpKEv43gCtE=
+github.com/GoogleContainerTools/kpt/porch/controllers v0.0.0-20230526213300-77a54e3b8e88 h1:gGboMcueEcYG7/XbV5vpjor8jD725vtOut8THRrtL3M=
+github.com/GoogleContainerTools/kpt/porch/controllers v0.0.0-20230526213300-77a54e3b8e88/go.mod h1:u73DWUyHPj896LCaDXwxjbA1g8atK5V5k5IT3Fj+5eQ=
 github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU=
 github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY=
 github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=

controllers/pkg/porch/client/client.go

@@ -17,6 +17,7 @@ package client
 import (
 	porchapi "github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1"
 	configapi "github.com/GoogleContainerTools/kpt/porch/api/porchconfig/v1alpha1"
+	pvapi "github.com/GoogleContainerTools/kpt/porch/controllers/packagevariants/api/v1alpha1"
 	coreapi "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -67,6 +68,7 @@ func AddToScheme(scheme *runtime.Scheme) error {
 	for _, api := range (runtime.SchemeBuilder{
 		configapi.AddToScheme,
 		porchapi.AddToScheme,
+		pvapi.AddToScheme,
 	}) {
 		if err := api(scheme); err != nil {
 			return err

controllers/pkg/reconcilers/approval/README.md

@@ -15,14 +15,14 @@ create a new Draft and have it not be automatically published.
 To enable this policy, annotate the package revision with
 `approval.nephio.org/policy: initial`.
 
-This controller will automatically delay taking any action for thirty seconds
+This controller will automatically delay taking any action for two minutes
 after the creation of the package revision. This is due to some current issues
 during the early lifecycle of a package generated by a PackageVariant. Hopefully
 we will fix this soon and can relax the delay.
 
-You can make this longer than thirty seconds by using the annotation
-`approval.nephio.org/delay` and setting it to a duration string. For example,
-setting `approval.nephio.org/delay: 2m` to require the revision to have
-existed for at least two minutes before it is proposed and approved.
+You can change the delay time by setting `approval.nephio.org/delay` to a
+duration string. For example, setting `approval.nephio.org/delay: 5m` to require
+the revision to have existed for at least five minutes before it is proposed and
+approved.
 
 If you set it to less than 30s, a delay of 30s will be used.

controllers/pkg/reconcilers/approval/reconciler.go

@@ -26,12 +26,16 @@ import (
 
 	ctrlconfig "github.com/nephio-project/nephio/controllers/pkg/reconcilers/config"
 	reconcilerinterface "github.com/nephio-project/nephio/controllers/pkg/reconcilers/reconciler-interface"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	types "k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 
 	"k8s.io/client-go/tools/record"
 
 	porchv1alpha1 "github.com/GoogleContainerTools/kpt/porch/api/porch/v1alpha1"
+	porchconfig "github.com/GoogleContainerTools/kpt/porch/api/porchconfig/v1alpha1"
+	pvapi "github.com/GoogleContainerTools/kpt/porch/controllers/packagevariants/api/v1alpha1"
 	"github.com/go-logr/logr"
 	porchclient "github.com/nephio-project/nephio/controllers/pkg/porch/client"
 	porchconds "github.com/nephio-project/nephio/controllers/pkg/porch/condition"
@@ -57,6 +61,8 @@ func init() {
 // +kubebuilder:rbac:groups=porch.kpt.dev,resources=packagerevisions,verbs=get;list;watch;update;patch
 // +kubebuilder:rbac:groups=porch.kpt.dev,resources=packagerevisions/status,verbs=get
 // +kubebuilder:rbac:groups=porch.kpt.dev,resources=packagerevisions/approval,verbs=get;update;patch
+// +kubebuilder:rbac:groups=config.porch.kpt.dev,resources=packagevariants,verbs=get;list;watch
+// +kubebuilder:rbac:groups=config.porch.kpt.dev,resources=packagevariants/status,verbs=get
 // +kubebuilder:rbac:groups="",resources=events,verbs=create;patch
 // SetupWithManager sets up the controller with the Manager.
 func (r *reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, c interface{}) (map[schema.GroupVersionKind]chan event.GenericEvent, error) {
@@ -70,6 +76,7 @@ func (r *reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, c i
 	r.recorder = mgr.GetEventRecorderFor("approval-controller")
 
 	return nil, ctrl.NewControllerManagedBy(mgr).
+		Named("ApprovalController").
 		For(&porchv1alpha1.PackageRevision{}).
 		Complete(r)
 }
@@ -85,6 +92,7 @@ type reconciler struct {
 
 func (r *reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	r.l = log.FromContext(ctx).WithValues("req", req)
+	r.l.Info("reconcile approval")
 
 	pr := &porchv1alpha1.PackageRevision{}
 	if err := r.Get(ctx, req.NamespacedName, pr); err != nil {
@@ -117,6 +125,8 @@ func (r *reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	// if requeue is > 0, then we should do nothing more with this PackageRevision
 	// for at least that long
 	if requeue > 0 {
+		r.recorder.Event(pr, corev1.EventTypeNormal,
+			"NotApproved", "delay time not met")
 		return ctrl.Result{RequeueAfter: requeue}, nil
 	}
 
@@ -127,6 +137,45 @@ func (r *reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		return ctrl.Result{}, nil
 	}
 
+	// If the package revision is owned by a PackageVariant, check the Ready condition
+	// of the package variant. If it is not Ready, then we should not approve yet. The
+	// lack of readiness could indicate an error which even impacts whether or not the
+	// readiness gates have been properly set.
+	//
+	for _, ownerRef := range pr.GetOwnerReferences() {
+		if ownerRef.Controller == nil || !*ownerRef.Controller {
+			continue
+		}
+		if porchconfig.GroupVersion.String() != ownerRef.APIVersion {
+			continue
+		}
+		if ownerRef.Kind != "PackageVariant" {
+			continue
+		}
+
+		var pv pvapi.PackageVariant
+		if err := r.Get(ctx, types.NamespacedName{Namespace: pr.Namespace, Name: ownerRef.Name}, &pv); err != nil {
+			r.recorder.Event(pr, corev1.EventTypeWarning,
+				"Error", fmt.Sprintf("could not get owning PackageVariant: %s", err.Error()))
+
+			return ctrl.Result{}, nil
+		}
+
+		for _, cond := range pv.Status.Conditions {
+			if cond.Type != "Ready" {
+				continue
+			}
+
+			if cond.Status != metav1.ConditionTrue {
+				r.recorder.Event(pr, corev1.EventTypeNormal,
+					"NotApproved", "owning PackageVariant not Ready")
+
+				return ctrl.Result{}, nil
+			}
+		}
+
+	}
+
 	// All policies require readiness gates to be met, so if they
 	// are not, we are done for now.
 	if !porchconds.PackageRevisionIsReady(pr.Spec.ReadinessGates, pr.Status.Conditions) {
@@ -184,7 +233,7 @@ func (r *reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 func (r *reconciler) manageDelay(ctx context.Context, pr *porchv1alpha1.PackageRevision) (time.Duration, error) {
 	delay, ok := pr.GetAnnotations()[DelayAnnotationName]
 	if !ok {
-		delay = "30s"
+		delay = "2m"
 	}
 	d, err := time.ParseDuration(delay)
 	if err != nil {

operators/nephio-controller-manager/go.mod

@@ -21,65 +21,38 @@ require (
 
 require (
 	code.gitea.io/sdk/gitea v0.15.1-0.20230509035020-970776d1c1e9 // indirect
-	github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 // indirect
 	github.com/GoogleContainerTools/kpt v1.0.0-beta.29.0.20230327202912-01513604feaa // indirect
 	github.com/GoogleContainerTools/kpt-functions-sdk/go/api v0.0.0-20220720212527-133180134b93 // indirect
-	github.com/GoogleContainerTools/kpt-functions-sdk/go/fn v0.0.0-20230302070146-e8e9cb3c3ae2 // indirect
+	github.com/GoogleContainerTools/kpt-functions-sdk/go/fn v0.0.0-20230427202446-3255accc518d // indirect
 	github.com/GoogleContainerTools/kpt/porch/api v0.0.0-20230504200302-14c7b353e6b6 // indirect
-	github.com/MakeNowJust/heredoc v1.0.0 // indirect
-	github.com/Masterminds/goutils v1.1.1 // indirect
-	github.com/Masterminds/semver/v3 v3.2.0 // indirect
-	github.com/Masterminds/sprig/v3 v3.2.3 // indirect
-	github.com/antlr/antlr4/runtime/Go/antlr v1.4.10 // indirect
-	github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a // indirect
+	github.com/GoogleContainerTools/kpt/porch/controllers v0.0.0-20230526213300-77a54e3b8e88 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/blang/semver v3.5.1+incompatible // indirect
-	github.com/blang/semver/v4 v4.0.0 // indirect
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect
-	github.com/chai2010/gettext-go v1.0.2 // indirect
-	github.com/coredns/caddy v1.1.0 // indirect
-	github.com/coredns/corefile-migration v1.0.20 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
-	github.com/docker/distribution v2.8.2+incompatible // indirect
 	github.com/emicklei/go-restful/v3 v3.10.2 // indirect
-	github.com/evanphx/json-patch v5.6.0+incompatible // indirect
 	github.com/evanphx/json-patch/v5 v5.6.0 // indirect
-	github.com/exponent-io/jsonpath v0.0.0-20151013193312-d6023ce2651d // indirect
 	github.com/fsnotify/fsnotify v1.6.0 // indirect
 	github.com/go-errors/errors v1.4.2 // indirect
 	github.com/go-logr/logr v1.2.4 // indirect
-	github.com/go-logr/zapr v1.2.4 // indirect
 	github.com/go-openapi/jsonpointer v0.19.6 // indirect
 	github.com/go-openapi/jsonreference v0.20.2 // indirect
 	github.com/go-openapi/swag v0.22.3 // indirect
-	github.com/gobuffalo/flect v1.0.2 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/golang/protobuf v1.5.3 // indirect
-	github.com/google/btree v1.1.2 // indirect
-	github.com/google/cel-go v0.12.6 // indirect
 	github.com/google/gnostic v0.6.9 // indirect
 	github.com/google/go-cmp v0.5.9 // indirect
 	github.com/google/gofuzz v1.2.0 // indirect
-	github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect
 	github.com/google/uuid v1.3.0 // indirect
-	github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79 // indirect
 	github.com/hansthienpondt/nipam v0.0.5 // indirect
 	github.com/hashicorp/go-version v1.5.0 // indirect
-	github.com/huandu/xstrings v1.3.3 // indirect
 	github.com/imdario/mergo v0.3.16 // indirect
-	github.com/inconshreveable/mousetrap v1.1.0 // indirect
 	github.com/josharian/intern v1.0.0 // indirect
 	github.com/json-iterator/go v1.1.12 // indirect
 	github.com/kentik/patricia v1.2.0 // indirect
-	github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de // indirect
 	github.com/mailru/easyjson v0.7.7 // indirect
 	github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect
-	github.com/mitchellh/copystructure v1.2.0 // indirect
-	github.com/mitchellh/go-wordwrap v1.0.0 // indirect
-	github.com/mitchellh/reflectwalk v1.0.2 // indirect
-	github.com/moby/spdystream v0.2.0 // indirect
-	github.com/moby/term v0.0.0-20221205130635-1aeaba878587 // indirect
 	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 // indirect
@@ -89,26 +62,13 @@ require (
 	github.com/nephio-project/nephio/krm-functions/lib v0.0.0-20230508215739-b13457eda5c9 // indirect
 	github.com/nephio-project/nephio/krm-functions/vlan-fn v0.0.0-20230519080401-f95bbb7f58a6 // indirect
 	github.com/nokia/k8s-ipam v0.0.4-0.20230530110122-75deb73d617a // indirect
-	github.com/onsi/gomega v1.27.7 // indirect
-	github.com/opencontainers/go-digest v1.0.0 // indirect
-	github.com/peterbourgon/diskv v2.0.1+incompatible // indirect
 	github.com/pkg/errors v0.9.1 // indirect
 	github.com/prometheus/client_golang v1.15.1 // indirect
 	github.com/prometheus/client_model v0.4.0 // indirect
 	github.com/prometheus/common v0.44.0 // indirect
 	github.com/prometheus/procfs v0.10.1 // indirect
-	github.com/russross/blackfriday/v2 v2.1.0 // indirect
-	github.com/shopspring/decimal v1.3.1 // indirect
-	github.com/spf13/cast v1.5.0 // indirect
-	github.com/spf13/cobra v1.7.0 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
-	github.com/stoewer/go-strcase v1.2.0 // indirect
-	github.com/valyala/fastjson v1.6.4 // indirect
 	github.com/xlab/treeprint v1.1.0 // indirect
-	go.starlark.net v0.0.0-20210901212718-87f333178d59 // indirect
-	go.uber.org/atomic v1.10.0 // indirect
-	go.uber.org/multierr v1.11.0 // indirect
-	go.uber.org/zap v1.24.0 // indirect
 	go4.org/netipx v0.0.0-20230303233057-f1b76eb4bb35 // indirect
 	golang.org/x/net v0.10.0 // indirect
 	golang.org/x/oauth2 v0.8.0 // indirect
@@ -126,12 +86,8 @@ require (
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 	k8s.io/api v0.27.2 // indirect
 	k8s.io/apiextensions-apiserver v0.27.2 // indirect
-	k8s.io/apiserver v0.27.2 // indirect
-	k8s.io/cli-runtime v0.27.2 // indirect
-	k8s.io/cluster-bootstrap v0.27.2 // indirect
 	k8s.io/component-base v0.27.2 // indirect
 	k8s.io/kube-openapi v0.0.0-20230525220651-2546d827e515 // indirect
-	k8s.io/kubectl v0.27.2 // indirect
 	k8s.io/utils v0.0.0-20230505201702-9f6742963106 // indirect
 	sigs.k8s.io/cluster-api v1.4.0-beta.2.0.20230527123250-e111168cdff3 // indirect
 	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect