Improve error handling and update to ResourceVersionChangedPredicate for OperandRequest controller (#1179)

* Improve error handling for OperandRegistry notFound case

Signed-off-by: Daniel Fan <[email protected]>

* update to ResourceVersionChangedPredicate in OperandRequest controllers

Signed-off-by: Daniel Fan <[email protected]>

* Add operatorsv1 to scheme in namespace scope tests

Signed-off-by: Daniel Fan <[email protected]>

---------

Signed-off-by: Daniel Fan <[email protected]>

Author: Daniel-Fan

controllers/namespacescope/namespacescope_suite_test.go

@@ -28,6 +28,7 @@ import (
 	"github.com/onsi/gomega/gexec"
 	olmv1 "github.com/operator-framework/api/pkg/operators/v1"
 	olmv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
+	operatorsv1 "github.com/operator-framework/operator-lifecycle-manager/pkg/package-server/apis/operators/v1"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -95,6 +96,8 @@ var _ = BeforeSuite(func(ctx SpecContext) {
 	Expect(err).NotTo(HaveOccurred())
 	err = jaegerv1.AddToScheme(clientgoscheme.Scheme)
 	Expect(err).NotTo(HaveOccurred())
+	err = operatorsv1.AddToScheme(clientgoscheme.Scheme)
+	Expect(err).NotTo(HaveOccurred())
 
 	k8sClient, err = client.New(cfg, client.Options{Scheme: clientgoscheme.Scheme})
 	Expect(err).ToNot(HaveOccurred())

controllers/operandrequest/operandrequest_controller.go

@@ -181,7 +181,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re
 
 	// Check if all csv deploy succeed
 	if requestInstance.Status.Phase != operatorv1alpha1.ClusterPhaseRunning {
-		klog.V(2).Info("Waiting for all operators and operands to be deployed successfully ...")
+		klog.Info("Waiting for all operators and operands to be deployed successfully ...")
 		return ctrl.Result{RequeueAfter: constant.DefaultRequeueDuration}, nil
 	}
 
@@ -406,7 +406,7 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 	}
 	return ctrl.NewControllerManagedBy(mgr).
 		WithOptions(options).
-		For(&operatorv1alpha1.OperandRequest{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})).
+		For(&operatorv1alpha1.OperandRequest{}, builder.WithPredicates(predicate.ResourceVersionChangedPredicate{})).
 		Watches(&olmv1alpha1.Subscription{}, handler.EnqueueRequestsFromMapFunc(r.getSubToRequestMapper), builder.WithPredicates(predicate.Funcs{
 			UpdateFunc: func(e event.UpdateEvent) bool {
 				oldObject := e.ObjectOld.(*olmv1alpha1.Subscription)

controllers/operandrequest/reconcile_operator.go

@@ -85,6 +85,7 @@ func (r *Reconciler) reconcileOperator(ctx context.Context, requestInstance *ope
 				return utilerrors.NewAggregate([]error{err, patchErr})
 			}
 			klog.Errorf("Failed to get suitable OperandRegistry %s: %v", registryKey.String(), err)
+			return utilerrors.NewAggregate([]error{err, fmt.Errorf("failed to get suitable OperandRegistry %s", registryKey.String())})
 		}
 		merr := &util.MultiErr{}
 

controllers/operandrequestnoolm/operandrequestnoolm_controller.go

@@ -179,7 +179,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re
 	//TODO update this block to check deployments and their operands instead
 	// Check if all csv deploy succeed
 	if requestInstance.Status.Phase != operatorv1alpha1.ClusterPhaseRunning {
-		klog.V(2).Info("Waiting for all operators and operands to be deployed successfully ...")
+		klog.Info("Waiting for all operators and operands to be deployed successfully ...")
 		return ctrl.Result{RequeueAfter: constant.DefaultRequeueDuration}, nil
 	}
 
@@ -351,7 +351,7 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 	}
 	return ctrl.NewControllerManagedBy(mgr).
 		WithOptions(options).
-		For(&operatorv1alpha1.OperandRequest{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})).
+		For(&operatorv1alpha1.OperandRequest{}, builder.WithPredicates(predicate.ResourceVersionChangedPredicate{})).
 		Watches(&corev1.ConfigMap{}, handler.EnqueueRequestsFromMapFunc(r.getReferenceToRequestMapper), builder.WithPredicates(predicate.Funcs{
 			UpdateFunc: func(e event.UpdateEvent) bool {
 				oldObject := e.ObjectOld.(*corev1.ConfigMap)

go.mod

@@ -19,14 +19,15 @@ require (
 	github.com/stretchr/testify v1.9.0
 	golang.org/x/mod v0.17.0
 	k8s.io/api v0.28.2
-	k8s.io/apiextensions-apiserver v0.28.2
 	k8s.io/apimachinery v0.28.2
 	k8s.io/client-go v0.28.2
 	k8s.io/klog v1.0.0
 	k8s.io/utils v0.0.0-20230726121419-3b25d923346b
 	sigs.k8s.io/controller-runtime v0.16.1
 )
 
+require k8s.io/apiextensions-apiserver v0.28.2 // indirect
+
 require (
 	github.com/Shopify/logrus-bugsnag v0.0.0-20240507214313-004243a594f4 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect