From 8215c7f3fe16b1135ddfc9de1d498fff2f75e659 Mon Sep 17 00:00:00 2001 From: Todd Short Date: Wed, 3 Jan 2024 17:53:34 -0500 Subject: [PATCH] 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 --- test/e2e/csv_e2e_test.go | 6 +++- test/e2e/installplan_e2e_test.go | 48 +++++++++++++++++++++++-------- test/e2e/subscription_e2e_test.go | 29 +++++++++++++++++++ test/e2e/util.go | 2 ++ 4 files changed, 72 insertions(+), 13 deletions(-) diff --git a/test/e2e/csv_e2e_test.go b/test/e2e/csv_e2e_test.go index 0d8e3e2abc..132571910e 100644 --- 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 index ad52f2a542..f3c8732f9f 100644 --- 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 index 9a8a487873..b96359e926 100644 --- 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 index fa7e3480e5..5032e67579 100644 --- 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 }