fix(olm): refactor e2e and check annotations

Author: Jeff Peeler

pkg/controller/operators/olm/operatorgroup.go

@@ -57,7 +57,7 @@ func (a *Operator) syncOperatorGroups(obj interface{}) error {
 			return err
 		}
 
-		if err := a.copyCsvToTargetNamespace(csv, op, targetedNamespaces); err!=nil {
+		if err := a.copyCsvToTargetNamespace(csv, op, targetedNamespaces); err != nil {
 			return err
 		}
 	}
@@ -109,35 +109,41 @@ func (a *Operator) syncOperatorGroups(obj interface{}) error {
 }
 
 func (a *Operator) copyCsvToTargetNamespace(csv *v1alpha1.ClusterServiceVersion, operatorGroup *v1alpha2.OperatorGroup, targetNamespaces []*corev1.Namespace) error {
-	var nsList []string
-	for _, ns := range targetNamespaces {
-		nsList = append(nsList, ns.Name)
-	}
-	nsListJoined := strings.Join(nsList, ",")
-
-
 	for _, ns := range targetNamespaces {
 		if ns.Name == operatorGroup.GetNamespace() {
 			continue
 		}
+		// create new CSV instead of DeepCopy as namespace and resource version (and status) will be different
 		newCSV := v1alpha1.ClusterServiceVersion{
 			ObjectMeta: metav1.ObjectMeta{
-				Name: csv.Name,
+				Name:        csv.Name,
+				Annotations: csv.Annotations,
 			},
 			Spec: *csv.Spec.DeepCopy(),
-			Status: v1alpha1.ClusterServiceVersionStatus{
+		}
+		newCSV.SetNamespace(ns.Name)
+
+		log.Debugf("Copying/updating CSV %v to/in namespace %v", csv.GetName(), ns.Name)
+		createdCSV, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(ns.Name).Create(&newCSV)
+		if err == nil {
+			createdCSV.Status = v1alpha1.ClusterServiceVersionStatus{
 				Message:        "CSV copied to target namespace",
 				Reason:         v1alpha1.CSVReasonCopied,
 				LastUpdateTime: timeNow(),
-			},
+			}
+			if _, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(ns.Name).UpdateStatus(createdCSV); err != nil {
+				log.Errorf("Status update for CSV failed: %v", err)
+				return err
+			}
 		}
-		a.addAnnotationsToCSV(&newCSV, operatorGroup, nsListJoined)
-		newCSV.SetNamespace(ns.Name)
-
-		log.Debugf("Copying CSV %v to namespace %v", csv.GetName(), ns.Name)
-		_, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(ns.Name).Create(&newCSV)
 		if k8serrors.IsAlreadyExists(err) {
-			if _, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(ns.Name).Update(&newCSV); err != nil {
+			fetchedCSV, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(ns.Name).Get(csv.GetName(), metav1.GetOptions{})
+			if err != nil {
+				log.Errorf("Create failed, yet get failed: %v", err)
+			}
+			// update the potentially different annotations
+			fetchedCSV.Annotations = csv.Annotations
+			if _, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(ns.Name).Update(fetchedCSV); err != nil {
 				log.Errorf("Update CSV in target namespace failed: %v", err)
 				return err
 			}

test/e2e/operator_groups_e2e_test.go

@@ -63,9 +63,23 @@ func patchOlmDeployment(t *testing.T, c operatorclient.ClientInterface, newNames
 	return command
 }
 
+func checkOperatorGroupAnnotations(obj metav1.Object, op *v1alpha2.OperatorGroup, targetNamespaces string) error {
+	if annotation, ok := obj.GetAnnotations()["olm.targetNamespaces"]; !ok || annotation != targetNamespaces {
+		return fmt.Errorf("missing targetNamespaces annotation on %v", obj.GetName())
+	}
+	if annotation, ok := obj.GetAnnotations()["olm.operatorNamespace"]; !ok || annotation != op.GetNamespace() {
+		return fmt.Errorf("missing operatorNamespace on %v", obj.GetName())
+	}
+	if annotation, ok := obj.GetAnnotations()["olm.operatorGroup"]; !ok || annotation != op.GetName() {
+		return fmt.Errorf("missing operatorGroup annotation on %v", obj.GetName())
+	}
+
+	return nil
+}
+
 func TestCreateOperatorGroupWithMatchingNamespace(t *testing.T) {
 	// Create namespace with specific label
-	// Create CSV in opoerator namespace
+	// Create CSV in operator namespace
 	// Create operator group that watches namespace and uses specific label
 	// Verify operator group status contains correct status
 	// Verify csv in target namespace exists, has copied status, has annotations
@@ -171,8 +185,7 @@ func TestCreateOperatorGroupWithMatchingNamespace(t *testing.T) {
 			fmt.Println(fetchErr)
 			return false, fetchErr
 		}
-		//TODO: check other annotations
-		if fetchedCSV.Annotations["olm.targetNamespaces"] == otherNamespaceName {
+		if checkOperatorGroupAnnotations(fetchedCSV, &operatorGroup, otherNamespaceName) == nil {
 			return true, nil
 		}
 		return false, nil
@@ -185,16 +198,17 @@ func TestCreateOperatorGroupWithMatchingNamespace(t *testing.T) {
 			fmt.Println(fetchErr)
 			return false, fetchErr
 		}
-		//TODO: check other annotations
-		if fetchedCSV.Annotations["olm.targetNamespaces"] == otherNamespaceName {
+		if checkOperatorGroupAnnotations(fetchedCSV, &operatorGroup, otherNamespaceName) == nil {
 			return true, nil
 		}
+
 		return false, nil
 	})
-
-
-	// TODO: verify CSV in target namespace has correct annotations and status
-	// TODO: verify CSV in operator namespace has correct annotations
+	// since annotations are set along with status, no reason to poll for this check as done above
+	log.Debug("Checking status on CSV in target namespace")
+	fetchedCSV, err := crc.OperatorsV1alpha1().ClusterServiceVersions(otherNamespaceName).Get(csvName, metav1.GetOptions{})
+	require.NoError(t, err)
+	require.EqualValues(t, v1alpha1.CSVReasonCopied, fetchedCSV.Status.Reason)
 
 	//log.Debug("Waiting on deployment to have correct annotation")
 	//err = wait.Poll(pollInterval, pollDuration, func() (bool, error) {