Merge pull request #1899 from awgreene/fix-operatorcondition-generation

Bug 1906134: Don't create OperatorConditions for copied CSVs

Author: openshift-merge-robot

pkg/controller/operators/operatorconditiongenerator_controller.go

@@ -9,8 +9,11 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/source"
 
@@ -34,9 +37,35 @@ func (r *OperatorConditionGeneratorReconciler) SetupWithManager(mgr ctrl.Manager
 		IsController: true,
 		OwnerType:    &operatorsv1alpha1.ClusterServiceVersion{},
 	}
+	p := predicate.Funcs{
+		CreateFunc: func(e event.CreateEvent) bool {
+			if _, ok := e.Meta.GetLabels()[operatorsv1alpha1.CopiedLabelKey]; ok {
+				return false
+			}
+			return true
+		},
+		DeleteFunc: func(e event.DeleteEvent) bool {
+			if _, ok := e.Meta.GetLabels()[operatorsv1alpha1.CopiedLabelKey]; ok {
+				return false
+			}
+			return true
+		},
+		UpdateFunc: func(e event.UpdateEvent) bool {
+			if _, ok := e.MetaOld.GetLabels()[operatorsv1alpha1.CopiedLabelKey]; ok {
+				return false
+			}
+			return true
+		},
+		GenericFunc: func(e event.GenericEvent) bool {
+			if _, ok := e.Meta.GetLabels()[operatorsv1alpha1.CopiedLabelKey]; ok {
+				return false
+			}
+			return true
+		},
+	}
 
 	return ctrl.NewControllerManagedBy(mgr).
-		For(&operatorsv1alpha1.ClusterServiceVersion{}).
+		For(&operatorsv1alpha1.ClusterServiceVersion{}, builder.WithPredicates(p)).
 		Watches(&source.Kind{Type: &operatorsv1.OperatorCondition{}}, handler).
 		Complete(r)
 }
@@ -61,7 +90,6 @@ var _ reconcile.Reconciler = &OperatorConditionGeneratorReconciler{}
 func (r *OperatorConditionGeneratorReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	// Set up a convenient log object so we don't have to type request over and over again
 	log := r.log.WithValues("request", req)
-	log.V(2).Info("reconciling ClusterServiceVersion")
 
 	in := &operatorsv1alpha1.ClusterServiceVersion{}
 	err := r.Client.Get(context.TODO(), req.NamespacedName, in)

pkg/controller/operators/operatorconditiongenerator_controller_test.go

@@ -8,6 +8,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apiserver/pkg/storage/names"
@@ -99,6 +100,57 @@ var _ = Describe("The OperatorConditionsGenerator Controller", func() {
 		}, timeout, interval).Should(Succeed())
 	})
 
+	It("does not create an OperatorCondition for a Copied CSV", func() {
+		depName := genName("dep-")
+		csv := &operatorsv1alpha1.ClusterServiceVersion{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       operatorsv1alpha1.ClusterServiceVersionKind,
+				APIVersion: operatorsv1alpha1.ClusterServiceVersionAPIVersion,
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      genName("csv-"),
+				Namespace: namespace,
+				Labels: map[string]string{
+					operatorsv1alpha1.CopiedLabelKey: "",
+				},
+			},
+			Spec: operatorsv1alpha1.ClusterServiceVersionSpec{
+				InstallModes: []operatorsv1alpha1.InstallMode{
+					{
+						Type:      operatorsv1alpha1.InstallModeTypeOwnNamespace,
+						Supported: true,
+					},
+					{
+						Type:      operatorsv1alpha1.InstallModeTypeSingleNamespace,
+						Supported: true,
+					},
+					{
+						Type:      operatorsv1alpha1.InstallModeTypeMultiNamespace,
+						Supported: true,
+					},
+					{
+						Type:      operatorsv1alpha1.InstallModeTypeAllNamespaces,
+						Supported: true,
+					},
+				},
+				InstallStrategy: newNginxInstallStrategy(depName, nil, nil),
+			},
+		}
+
+		Expect(k8sClient.Create(ctx, csv)).To(Succeed())
+		namespacedName := types.NamespacedName{Name: csv.GetName(), Namespace: csv.GetNamespace()}
+		operatorCondition := &operatorsv1.OperatorCondition{}
+
+		// Wait 10 seconds
+		// Background: This test could pass simply because the controller hasn't reconciled the Copied CSV yet.
+		// However, this test does sound an alarm if the controller ever reconciles a copied CSV.
+		// TODO: Improve this test by identifying a way to identify that the controller has not reconciling a resource.
+		time.Sleep(time.Second * 10)
+		err := k8sClient.Get(ctx, namespacedName, operatorCondition)
+		Expect(err).ToNot(BeNil())
+		Expect(k8serrors.IsNotFound(err)).To(BeTrue())
+	})
+
 	It("creates an OperatorCondition for a CSV with multiple ServiceAccounts and Deployments", func() {
 		depName := genName("dep-")
 		csv := &operatorsv1alpha1.ClusterServiceVersion{