Fix Create InstallPlan with Permissions test (#3129)

Fix #3108 The Subscription needs to be deleted before deleting the CSV, otherwise the Subscription will recreate the CSV, and subsequently, the CR/CRBs are not deleted. Update some test logging as well. Signed-off-by: Todd Short <[email protected]>
operator-framework · Jan 3, 2024 · 8215c7f · 8215c7f
1 parent a081d35
commit 8215c7f
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 13 deletions.
diff --git a/test/e2e/csv_e2e_test.go b/test/e2e/csv_e2e_test.go
@@ -4441,6 +4441,7 @@ func fetchCSV(c versioned.Interface, namespace, name string, checker csvConditio
 	var lastPhase operatorsv1alpha1.ClusterServiceVersionPhase
 	var lastReason operatorsv1alpha1.ConditionReason
 	var lastMessage string
+	var lastError string
 	lastTime := time.Now()
 	var csv *operatorsv1alpha1.ClusterServiceVersion
 
@@ -4449,7 +4450,10 @@ func fetchCSV(c versioned.Interface, namespace, name string, checker csvConditio
 		var err error
 		csv, err = c.OperatorsV1alpha1().ClusterServiceVersions(namespace).Get(context.TODO(), name, metav1.GetOptions{})
 		if err != nil || csv == nil {
-			ctx.Ctx().Logf("error getting csv %s/%s: %v", namespace, name, err)
+			if lastError != err.Error() {
+				ctx.Ctx().Logf("error getting csv %s/%s: %v", namespace, name, err)
+				lastError = err.Error()
+			}
 			return false, nil
 		}
 		phase, reason, message := csv.Status.Phase, csv.Status.Reason, csv.Status.Message

diff --git a/test/e2e/installplan_e2e_test.go b/test/e2e/installplan_e2e_test.go
@@ -2792,10 +2792,13 @@ var _ = Describe("Install Plan", func() {
 		_, err := fetchCatalogSourceOnStatus(crc, mainCatalogSourceName, generatedNamespace.GetName(), catalogSourceRegistryPodSynced())
 		require.NoError(GinkgoT(), err)
 
+		By("Creating a Subscription")
 		subscriptionName := genName("sub-nginx-")
-		subscriptionCleanup := createSubscriptionForCatalog(crc, generatedNamespace.GetName(), subscriptionName, mainCatalogSourceName, packageName, stableChannel, "", operatorsv1alpha1.ApprovalAutomatic)
-		defer subscriptionCleanup()
+		// Subscription is explitly deleted as part of the test to avoid CSV being recreated,
+		// so ignore cleanup function
+		_ = createSubscriptionForCatalog(crc, generatedNamespace.GetName(), subscriptionName, mainCatalogSourceName, packageName, stableChannel, "", operatorsv1alpha1.ApprovalAutomatic)
 
+		By("Attempt to get Subscription")
 		subscription, err := fetchSubscription(crc, generatedNamespace.GetName(), subscriptionName, subscriptionHasInstallPlanChecker())
 		require.NoError(GinkgoT(), err)
 		require.NotNil(GinkgoT(), subscription)
@@ -2867,22 +2870,38 @@ var _ = Describe("Install Plan", func() {
 		By("Should have removed every matching step")
 		require.Equal(GinkgoT(), 0, len(expectedSteps), "Actual resource steps do not match expected: %#v", expectedSteps)
 
-		GinkgoT().Logf("deleting csv %s/%s", generatedNamespace.GetName(), stableCSVName)
-		By("Explicitly delete the CSV")
-		err = crc.OperatorsV1alpha1().ClusterServiceVersions(generatedNamespace.GetName()).Delete(context.Background(), stableCSVName, metav1.DeleteOptions{})
+		By(fmt.Sprintf("Explicitly deleting subscription %s/%s", generatedNamespace.GetName(), subscriptionName))
+		err = crc.OperatorsV1alpha1().Subscriptions(generatedNamespace.GetName()).Delete(context.Background(), subscriptionName, metav1.DeleteOptions{})
 		By("Looking for no error OR IsNotFound error")
-		if err != nil && apierrors.IsNotFound(err) {
-			err = nil
-		}
-		require.NoError(GinkgoT(), err)
+		require.NoError(GinkgoT(), client.IgnoreNotFound(err))
+
+		By("Waiting for the Subscription to delete")
+		err = waitForSubscriptionToDelete(generatedNamespace.GetName(), subscriptionName, crc)
+		require.NoError(GinkgoT(), client.IgnoreNotFound(err))
 
+		By(fmt.Sprintf("Explicitly deleting csv %s/%s", generatedNamespace.GetName(), stableCSVName))
+		err = crc.OperatorsV1alpha1().ClusterServiceVersions(generatedNamespace.GetName()).Delete(context.Background(), stableCSVName, metav1.DeleteOptions{})
+		By("Looking for no error OR IsNotFound error")
+		require.NoError(GinkgoT(), client.IgnoreNotFound(err))
+		By("Waiting for the CSV to delete")
+		err = waitForCsvToDelete(generatedNamespace.GetName(), stableCSVName, crc)
+		require.NoError(GinkgoT(), client.IgnoreNotFound(err))
+
+		nCrs := 0
+		nCrbs := 0
+		By("Waiting for CRBs and CRs and SAs to delete")
 		Eventually(func() bool {
+
 			crbs, err := c.KubernetesInterface().RbacV1().ClusterRoleBindings().List(context.Background(), metav1.ListOptions{LabelSelector: fmt.Sprintf("%v=%v", ownerutil.OwnerKey, stableCSVName)})
 			if err != nil {
 				GinkgoT().Logf("error getting crbs: %v", err)
 				return false
 			}
-			if len(crbs.Items) != 0 {
+			if n := len(crbs.Items); n != 0 {
+				if n != nCrbs {
+					GinkgoT().Logf("CRBs remaining:  %v", n)
+					nCrbs = n
+				}
 				return false
 			}
 
@@ -2891,18 +2910,23 @@ var _ = Describe("Install Plan", func() {
 				GinkgoT().Logf("error getting crs: %v", err)
 				return false
 			}
-			if len(crs.Items) != 0 {
+			if n := len(crs.Items); n != 0 {
+				if n != nCrs {
+					GinkgoT().Logf("CRs remaining: %v", n)
+					nCrs = n
+				}
 				return false
 			}
 
 			_, err = c.KubernetesInterface().CoreV1().ServiceAccounts(generatedNamespace.GetName()).Get(context.Background(), serviceAccountName, metav1.GetOptions{})
-			if err != nil && !apierrors.IsNotFound(err) {
+			if client.IgnoreNotFound(err) != nil {
 				GinkgoT().Logf("error getting sa %s/%s: %v", generatedNamespace.GetName(), serviceAccountName, err)
 				return false
 			}
 
 			return true
 		}, pollDuration*2, pollInterval).Should(BeTrue())
+		By("Cleaning up the test")
 	})
 
 	It("CRD validation", func() {

diff --git a/test/e2e/subscription_e2e_test.go b/test/e2e/subscription_e2e_test.go
@@ -3365,6 +3365,35 @@ func createSubscriptionForCatalogWithSpec(t GinkgoTInterface, crc versioned.Inte
 	return buildSubscriptionCleanupFunc(crc, subscription)
 }
 
+func waitForSubscriptionToDelete(namespace, name string, c versioned.Interface) error {
+	var lastState operatorsv1alpha1.SubscriptionState
+	var lastReason operatorsv1alpha1.ConditionReason
+	lastTime := time.Now()
+
+	ctx.Ctx().Logf("waiting for subscription %s/%s to delete", namespace, name)
+	err := wait.Poll(pollInterval, pollDuration, func() (bool, error) {
+		sub, err := c.OperatorsV1alpha1().Subscriptions(namespace).Get(context.TODO(), name, metav1.GetOptions{})
+		if apierrors.IsNotFound(err) {
+			ctx.Ctx().Logf("subscription %s/%s deleted", namespace, name)
+			return true, nil
+		}
+		if err != nil {
+			ctx.Ctx().Logf("error getting subscription %s/%s: %v", namespace, name, err)
+		}
+		if sub != nil {
+			state, reason := sub.Status.State, sub.Status.Reason
+			if state != lastState || reason != lastReason {
+				ctx.Ctx().Logf("waited %s for subscription %s/%s status: %s (%s)", time.Since(lastTime), namespace, name, state, reason)
+				lastState, lastReason = state, reason
+				lastTime = time.Now()
+			}
+		}
+		return false, nil
+	})
+
+	return err
+}
+
 func checkDeploymentHasPodConfigNodeSelector(t GinkgoTInterface, client operatorclient.ClientInterface, csv *operatorsv1alpha1.ClusterServiceVersion, nodeSelector map[string]string) error {
 	resolver := install.StrategyResolver{}
 

diff --git a/test/e2e/util.go b/test/e2e/util.go
@@ -677,8 +677,10 @@ func createInternalCatalogSource(
 	ctx.Ctx().Logf("Catalog source %s created", name)
 
 	cleanupInternalCatalogSource := func() {
+		ctx.Ctx().Logf("Cleaning catalog source %s", name)
 		configMapCleanup()
 		buildCatalogSourceCleanupFunc(c, crc, namespace, catalogSource)()
+		ctx.Ctx().Logf("Done cleaning catalog source %s", name)
 	}
 	return catalogSource, cleanupInternalCatalogSource
 }