(Partially) mitigate flaky tests

Signed-off-by: perdasilva <perdasilva@redhat.com>

Signed-off-by: perdasilva <perdasilva@redhat.com>

.github/ISSUE_TEMPLATE/flaky-test.md

@@ -0,0 +1,23 @@
+---
+name: Flaky Test Report
+about: If an unrelated test is failing for mysterious reasons
+title: ''
+labels: 'kind/flake'
+assignees: ''
+
+---
+
+## Flaky Test Report
+
+<!-- 
+Note: Make sure to first check the prerequisites that can be found in the main README file!
+Thanks for filing an issue! Before hitting the button, please provide the following information:
+-->
+
+**Failure Log Link**
+[Failure Log](put link here)
+You can get the link by clicking on the line number in the job logs and copying the url from your browser.
+
+**Relevant Failure Log**
+<!--- Paste any relevant failure logs from the job -->
+

.github/PULL_REQUEST_TEMPLATE.md

@@ -17,6 +17,8 @@ Note: Make sure your branch is rebased to the latest upstream master.
 - [ ] Sufficient end-to-end test coverage
 - [ ] Docs updated or added to `/doc`
 - [ ] Commit messages sensible and descriptive
+- [ ] Tests marked as `[FLAKE]` are truly flaky
+- [ ] Tests that remove the `[FLAKE]` tag are no longer flaky
 
 
 <!--

.github/workflows/e2e-tests.yml

@@ -19,7 +19,7 @@ jobs:
       - uses: actions/setup-go@v2
         with:
           go-version: '~1.17'
-      - run: make e2e-local E2E_TEST_CHUNK=${{ matrix.parallel-id }} E2E_TEST_NUM_CHUNKS=${{ strategy.job-total }} E2E_NODES=2 ARTIFACTS_DIR=./artifacts/
+      - run: make e2e-local E2E_TEST_CHUNK=${{ matrix.parallel-id }} E2E_TEST_NUM_CHUNKS=${{ strategy.job-total }} E2E_NODES=2 ARTIFACTS_DIR=./artifacts/ SKIP='\[FLAKY\]'
       - name: Archive Test Artifacts # test results, failed or not, are always uploaded.
         if: ${{ always() }}
         uses: actions/upload-artifact@v2

.github/workflows/flaky-e2e.yml

@@ -0,0 +1,24 @@
+name: flaky-e2e-tests
+on:
+  schedule:
+    - cron: '30 5,17 * * *' # run this every day at 5:30 and 17:30 UTC (00:30 and 12:30 ET)
+  push:
+    branches:
+      - master
+  pull_request:
+  workflow_dispatch:
+jobs:
+  flaky-e2e-tests:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v1
+      - uses: actions/setup-go@v2
+        with:
+          go-version: '~1.17'
+      - run: make e2e-local E2E_NODES=1 TEST='\[FLAKY\]' ARTIFACTS_DIR=./artifacts/
+      - name: Archive Test Artifacts # test results, failed or not, are always uploaded.
+        if: ${{ always() }}
+        uses: actions/upload-artifact@v2
+        with:
+          name: e2e-test-output-${{(github.event.pull_request.head.sha||github.sha)}}-${{ github.run_id }}
+          path: ${{ github.workspace }}/bin/artifacts/*

Makefile

@@ -131,7 +131,7 @@ E2E_TEST_NUM_CHUNKS ?= 4
 ifneq (all,$(E2E_TEST_CHUNK))
 TEST := $(shell go run ./test/e2e/split/... -chunks $(E2E_TEST_NUM_CHUNKS) -print-chunk $(E2E_TEST_CHUNK) ./test/e2e)
 endif
-E2E_OPTS ?= $(if $(E2E_SEED),-seed '$(E2E_SEED)') $(if $(TEST),-focus '$(TEST)') -flakeAttempts $(E2E_FLAKE_ATTEMPTS) -nodes $(E2E_NODES) -timeout $(E2E_TIMEOUT) -v -randomizeSuites -race -trace -progress
+E2E_OPTS ?= $(if $(E2E_SEED),-seed '$(E2E_SEED)') $(if $(SKIP), -skip '$(SKIP)') $(if $(TEST),-focus '$(TEST)') -flakeAttempts $(E2E_FLAKE_ATTEMPTS) -nodes $(E2E_NODES) -timeout $(E2E_TIMEOUT) -v -randomizeSuites -race -trace -progress
 E2E_INSTALL_NS ?= operator-lifecycle-manager
 E2E_TEST_NS ?= operators
 

README.md

@@ -170,6 +170,10 @@ Check out the [contributor documentation][contributor-documentation]. Also, see
 
 See [reporting bugs][bug_guide] for details about reporting any issues.
 
+## Reporting flaky tests
+
+See [reporting flaky tests][flaky_test_guide] for details about reporting flaky tests.
+
 ## License
 
 Operator Lifecycle Manager is under Apache 2.0 license. See the [LICENSE][license_file] file for details.
@@ -179,6 +183,7 @@ Operator Lifecycle Manager is under Apache 2.0 license. See the [LICENSE][licens
 [proposals_docs]: ./doc/contributors/design-proposals
 [license_file]:./LICENSE
 [bug_guide]:./doc/dev/reporting_bugs.md
+[flaky_test_guide]:./doc/dev/reporting_flakes.md
 [git_tool]:https://git-scm.com/downloads
 [go_tool]:https://golang.org/dl/
 [docker_tool]:https://docs.docker.com/install/

doc/dev/reporting_flakes.md

@@ -0,0 +1,21 @@
+# Reporting flakes
+
+If you are struggling to get your PR through because unrelated e2e or unit tests are randomly failing, it's likely
+you are being plagued by a flaky test 😱, a test that wasn't constructed as carefully as it should have been as is
+failing even when it should be succeeding. When this happens, check our [issues](https://github.com/operator-framework/operator-lifecycle-manager/issues) 
+to see if it has been filed before. Search also in the `closed issues`. If you find one, re-open it if necessary. 
+Otherwise, [file](https://github.com/operator-framework/operator-lifecycle-manager/issues/new) a flaky test issue.
+
+Once you have an issue link, you can disable the flaky test by adding the `[FLAKE]` tag to the test name and linking the issue in the code.
+
+Example:
+
+```
+    // issue: https://github.com/operator-framework/operator-lifecycle-manager/issues/2635 
+    It("[FLAKE] updates multiple intermediates", func() {
+...
+```
+
+You may be asked by the reviewer to supply evidence that the test is indeed flaky and not an unfortunate side effect of
+your contribution. We'll endeavor to make this an easy as process as possible to merge your contribution in as quickly
+and safely as possible.

test/e2e/catalog_e2e_test.go

@@ -1,4 +1,5 @@
-//  +build !bare
+//go:build !bare
+// +build !bare
 
 package e2e
 
@@ -870,7 +871,8 @@ var _ = Describe("Catalog represents a store of bundles which OLM can use to ins
 		Expect(v).Should(Equal(version.OperatorVersion{Version: busyboxVersion}), "latest version of operator not installed: catalog source update failed")
 	})
 
-	It("Dependency has correct replaces field", func() {
+	// issue: https://github.com/operator-framework/operator-lifecycle-manager/issues/2642
+	It("[FLAKE] Dependency has correct replaces field", func() {
 		// Create a CatalogSource that contains the busybox v1 and busybox-dependency v1 images
 		// Create a Subscription for busybox v1, which has a dependency on busybox-dependency v1.
 		// Wait for the busybox and busybox2 Subscriptions to succeed

test/e2e/crd_e2e_test.go

@@ -21,7 +21,8 @@ import (
 var _ = Describe("CRD Versions", func() {
 	AfterEach(func() { TearDown(testNamespace) }, float64(30))
 
-	It("creates v1 CRDs with a v1 schema successfully", func() {
+	// issue: https://github.com/operator-framework/operator-lifecycle-manager/issues/2640
+	It("[FLAKE] creates v1 CRDs with a v1 schema successfully", func() {
 		By("v1 crds with a valid openapiv3 schema should be created successfully by OLM")
 		c := newKubeClient()
 		crc := newCRClient()
@@ -94,7 +95,8 @@ var _ = Describe("CRD Versions", func() {
 		Expect(fetchedInstallPlan.Status.Phase).To(Equal(operatorsv1alpha1.InstallPlanPhaseComplete))
 	})
 
-	It("blocks a CRD upgrade that could cause data loss", func() {
+	// issue:https://github.com/operator-framework/operator-lifecycle-manager/issues/2638
+	It("[FLAKE] blocks a CRD upgrade that could cause data loss", func() {
 		By("checking the storage versions in the existing CRD status and the spec of the new CRD")
 
 		c := newKubeClient()

test/e2e/csv_e2e_test.go

@@ -19,8 +19,8 @@ import (
 	"k8s.io/apimachinery/pkg/api/equality"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
-	"k8s.io/apimachinery/pkg/fields"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/fields"
 	k8slabels "k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/selection"
@@ -64,10 +64,10 @@ var _ = Describe("ClusterServiceVersion", func() {
 
 	When("a CustomResourceDefinition was installed alongside a ClusterServiceVersion", func() {
 		var (
-			ns  corev1.Namespace
-			crd apiextensionsv1.CustomResourceDefinition
-			og operatorsv1.OperatorGroup
-			apiname string
+			ns          corev1.Namespace
+			crd         apiextensionsv1.CustomResourceDefinition
+			og          operatorsv1.OperatorGroup
+			apiname     string
 			apifullname string
 		)
 
@@ -144,7 +144,8 @@ var _ = Describe("ClusterServiceVersion", func() {
 			}).Should(WithTransform(k8serrors.IsNotFound, BeTrue()))
 		})
 
-		It("can satisfy an associated ClusterServiceVersion's ownership requirement", func() {
+		// issue: https://github.com/operator-framework/operator-lifecycle-manager/issues/2646
+		It("[FLAKE] can satisfy an associated ClusterServiceVersion's ownership requirement", func() {
 			associated := operatorsv1alpha1.ClusterServiceVersion{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "associated-csv",
@@ -261,7 +262,8 @@ var _ = Describe("ClusterServiceVersion", func() {
 			}).Should(Succeed())
 		})
 
-		It("can satisfy an unassociated ClusterServiceVersion's non-ownership requirement", func() {
+		// issue:https://github.com/operator-framework/operator-lifecycle-manager/issues/2639
+		It("[FLAKE] can satisfy an unassociated ClusterServiceVersion's non-ownership requirement", func() {
 			unassociated := operatorsv1alpha1.ClusterServiceVersion{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:      "unassociated-csv",
@@ -4206,7 +4208,7 @@ var _ = Describe("ClusterServiceVersion", func() {
 
 var _ = Describe("Disabling copied CSVs", func() {
 	var (
-		ns corev1.Namespace
+		ns  corev1.Namespace
 		csv operatorsv1alpha1.ClusterServiceVersion
 	)
 
@@ -4255,7 +4257,8 @@ var _ = Describe("Disabling copied CSVs", func() {
 	})
 
 	When("an operator is installed in AllNamespace mode", func() {
-		It("should have Copied CSVs in all other namespaces", func() {
+		// issue: https://github.com/operator-framework/operator-lifecycle-manager/issues/2643
+		It("[FLAKE] should have Copied CSVs in all other namespaces", func() {
 			Eventually(func() error {
 				requirement, err := k8slabels.NewRequirement(operatorsv1alpha1.CopiedLabelKey, selection.Equals, []string{csv.GetNamespace()})
 				if err != nil {
@@ -4335,7 +4338,8 @@ var _ = Describe("Disabling copied CSVs", func() {
 			}).Should(Succeed())
 		})
 
-		It("should be reflected in the olmConfig.Status.Condition array that the expected number of copied CSVs exist", func() {
+		// issue: https://github.com/operator-framework/operator-lifecycle-manager/issues/2634
+		It("[FLAKE] should be reflected in the olmConfig.Status.Condition array that the expected number of copied CSVs exist", func() {
 			Eventually(func() error {
 				var olmConfig operatorsv1.OLMConfig
 				if err := ctx.Ctx().Client().Get(context.TODO(), apitypes.NamespacedName{Name: "cluster"}, &olmConfig); err != nil {
@@ -4391,7 +4395,8 @@ var _ = Describe("Disabling copied CSVs", func() {
 			}).Should(Succeed())
 		})
 
-		It("should have copied CSVs in all other Namespaces", func() {
+		// issue: https://github.com/operator-framework/operator-lifecycle-manager/issues/2634
+		It("[FLAKE] should have copied CSVs in all other Namespaces", func() {
 			Eventually(func() error {
 				// find copied csvs...
 				requirement, err := k8slabels.NewRequirement(operatorsv1alpha1.CopiedLabelKey, selection.Equals, []string{csv.GetNamespace()})
@@ -4420,7 +4425,8 @@ var _ = Describe("Disabling copied CSVs", func() {
 			}).Should(Succeed())
 		})
 
-		It("should be reflected in the olmConfig.Status.Condition array that the expected number of copied CSVs exist", func() {
+		// issue: https://github.com/operator-framework/operator-lifecycle-manager/issues/2641
+		It("[FLAKE] should be reflected in the olmConfig.Status.Condition array that the expected number of copied CSVs exist", func() {
 			Eventually(func() error {
 				var olmConfig operatorsv1.OLMConfig
 				if err := ctx.Ctx().Client().Get(context.TODO(), apitypes.NamespacedName{Name: "cluster"}, &olmConfig); err != nil {

test/e2e/gc_e2e_test.go

@@ -646,7 +646,8 @@ var _ = Describe("Garbage collection for dependent resources", func() {
 				}).Should(BeNil())
 			})
 
-			It("should have removed the old configmap and put the new configmap in place", func() {
+			// flake issue: https://github.com/operator-framework/operator-lifecycle-manager/issues/2626
+			It("[FLAKE] should have removed the old configmap and put the new configmap in place", func() {
 				Eventually(func() bool {
 					_, err := kubeClient.GetConfigMap(testNamespace, configmapName)
 					return k8serrors.IsNotFound(err)

test/e2e/installplan_e2e_test.go

@@ -834,7 +834,7 @@ var _ = Describe("Install Plan", func() {
 			}()
 
 			// Create the catalog source
-			mainCatalogSourceName := genName("mock-ocs-main-" + strings.ToLower(CurrentGinkgoTestDescription().TestText) + "-")
+			mainCatalogSourceName := genName("mock-ocs-main-" + strings.ToLower(K8sSafeCurrentTestDescription()) + "-")
 			_, cleanupCatalogSource := createInternalCatalogSource(c, crc, mainCatalogSourceName, testNamespace, mainManifests, []apiextensions.CustomResourceDefinition{dependentCRD, mainCRD}, []operatorsv1alpha1.ClusterServiceVersion{dependentBetaCSV, dependentStableCSV, mainStableCSV, mainBetaCSV})
 			defer cleanupCatalogSource()
 
@@ -3006,7 +3006,8 @@ var _ = Describe("Install Plan", func() {
 
 	// This It spec verifies that, in cases where there are multiple options to fulfil a dependency
 	// across multiple catalogs, we only generate one installplan with one set of resolved resources.
-	It("consistent generation", func() {
+	//issue: https://github.com/operator-framework/operator-lifecycle-manager/issues/2633
+	It("[FLAKE] consistent generation", func() {
 
 		// Configure catalogs:
 		//  - one catalog with a package that has a dependency
@@ -3190,15 +3191,16 @@ var _ = Describe("Install Plan", func() {
 			Expect(err).NotTo(HaveOccurred())
 		})
 
-		It("should clear clear up the condition in the InstallPlan status that contains an error message when a valid OperatorGroup is created", func() {
+		// issue: https://github.com/operator-framework/operator-lifecycle-manager/issues/2636
+		It("[FLAKE] should clear up the condition in the InstallPlan status that contains an error message when a valid OperatorGroup is created", func() {
 
 			// first wait for a condition with a message exists
 			cond := operatorsv1alpha1.InstallPlanCondition{Type: operatorsv1alpha1.InstallPlanInstalled, Status: corev1.ConditionFalse, Reason: operatorsv1alpha1.InstallPlanReasonInstallCheckFailed,
 				Message: "no operator group found that is managing this namespace"}
 
 			Eventually(func() bool {
 				fetchedInstallPlan, err := fetchInstallPlanWithNamespace(GinkgoT(), crc, installPlanName, ns.GetName(), buildInstallPlanPhaseCheckFunc(operatorsv1alpha1.InstallPlanPhaseInstalling))
-				if err != nil || fetchedInstallPlan == nil{
+				if err != nil || fetchedInstallPlan == nil {
 					return false
 				}
 				if fetchedInstallPlan.Status.Phase != operatorsv1alpha1.InstallPlanPhaseInstalling {

test/e2e/operator_groups_e2e_test.go

@@ -1419,7 +1419,7 @@ var _ = Describe("Operator Group", func() {
 		aCSV := newCSV(csvName, opGroupNamespace, "", semver.MustParse("0.0.0"), []apiextensions.CustomResourceDefinition{mainCRD}, nil, &namedStrategy)
 
 		// Use the It spec name as label after stripping whitespaces
-		aCSV.Labels = map[string]string{"label": strings.Replace(CurrentGinkgoTestDescription().TestText, " ", "", -1)}
+		aCSV.Labels = map[string]string{"label": K8sSafeCurrentTestDescription()}
 		createdCSV, err := crc.OperatorsV1alpha1().ClusterServiceVersions(opGroupNamespace).Create(context.TODO(), &aCSV, metav1.CreateOptions{})
 		require.NoError(GinkgoT(), err)
 
@@ -1498,7 +1498,7 @@ var _ = Describe("Operator Group", func() {
 		})
 		require.NoError(GinkgoT(), err)
 
-		csvList, err := crc.OperatorsV1alpha1().ClusterServiceVersions(corev1.NamespaceAll).List(context.TODO(), metav1.ListOptions{LabelSelector: fmt.Sprintf("label=%s", strings.Replace(CurrentGinkgoTestDescription().TestText, " ", "", -1))})
+		csvList, err := crc.OperatorsV1alpha1().ClusterServiceVersions(corev1.NamespaceAll).List(context.TODO(), metav1.ListOptions{LabelSelector: fmt.Sprintf("label=%s", K8sSafeCurrentTestDescription())})
 		require.NoError(GinkgoT(), err)
 		GinkgoT().Logf("Found CSV count of %v", len(csvList.Items))
 		GinkgoT().Logf("Create other namespace %s", otherNamespaceName)
@@ -1795,7 +1795,8 @@ var _ = Describe("Operator Group", func() {
 
 	// Versions of OLM at 0.14.1 and older had a bug that would place the wrong namespace annotation on copied CSVs,
 	// preventing them from being GCd. This ensures that any leftover CSVs in that state are properly cleared up.
-	It("cleanup csvs with bad owner operator groups", func() {
+	// issue: https://github.com/operator-framework/operator-lifecycle-manager/issues/2644
+	It("[FLAKE] cleanup csvs with bad owner operator groups", func() {
 
 		c := newKubeClient()
 		crc := newCRClient()
@@ -1899,7 +1900,7 @@ var _ = Describe("Operator Group", func() {
 		aCSV := newCSV(csvName, opGroupNamespace, "", semver.MustParse("0.0.0"), []apiextensions.CustomResourceDefinition{mainCRD}, nil, &namedStrategy)
 
 		// Use the It spec name as label after stripping whitespaces
-		aCSV.Labels = map[string]string{"label": strings.Replace(CurrentGinkgoTestDescription().TestText, " ", "", -1)}
+		aCSV.Labels = map[string]string{"label": K8sSafeCurrentTestDescription()}
 		createdCSV, err := crc.OperatorsV1alpha1().ClusterServiceVersions(opGroupNamespace).Create(context.TODO(), &aCSV, metav1.CreateOptions{})
 		require.NoError(GinkgoT(), err)
 
@@ -1978,7 +1979,7 @@ var _ = Describe("Operator Group", func() {
 		})
 		require.NoError(GinkgoT(), err)
 
-		csvList, err := crc.OperatorsV1alpha1().ClusterServiceVersions(corev1.NamespaceAll).List(context.TODO(), metav1.ListOptions{LabelSelector: fmt.Sprintf("label=%s", strings.Replace(CurrentGinkgoTestDescription().TestText, " ", "", -1))})
+		csvList, err := crc.OperatorsV1alpha1().ClusterServiceVersions(corev1.NamespaceAll).List(context.TODO(), metav1.ListOptions{LabelSelector: fmt.Sprintf("label=%s", K8sSafeCurrentTestDescription())})
 		require.NoError(GinkgoT(), err)
 		GinkgoT().Logf("Found CSV count of %v", len(csvList.Items))
 		GinkgoT().Logf("Create other namespace %s", otherNamespaceName)
@@ -2288,7 +2289,8 @@ var _ = Describe("Operator Group", func() {
 					Expect(err).ToNot(HaveOccurred())
 				})
 
-				It("OLM applies labels to Namespaces that are associated with an OperatorGroup", func() {
+				// issue: https://github.com/operator-framework/operator-lifecycle-manager/issues/2637
+				It("[FLAKE] OLM applies labels to Namespaces that are associated with an OperatorGroup", func() {
 					ogLabel, err := getOGLabelKey(operatorGroup)
 					Expect(err).ToNot(HaveOccurred())
 

test/e2e/operator_test.go

@@ -70,7 +70,8 @@ var _ = Describe("Operator API", func() {
 	// 14. Ensure the reference to ns-a is eventually removed from o's status.components.refs field
 	// 15. Delete o
 	// 16. Ensure o is not re-created
-	It("should surface components in its status", func() {
+	// issue: https://github.com/operator-framework/operator-lifecycle-manager/issues/2628
+	It("[FLAKE] should surface components in its status", func() {
 		o := &operatorsv1.Operator{}
 		o.SetName(genName("o-"))
 

test/e2e/subscription_e2e_test.go

@@ -508,7 +508,8 @@ var _ = Describe("Subscription", func() {
 		require.Len(GinkgoT(), ips.Items, 2)
 	})
 
-	It("updates multiple intermediates", func() {
+	// issue: https://github.com/operator-framework/operator-lifecycle-manager/issues/2635
+	It("[FLAKE] updates multiple intermediates", func() {
 
 		crd := newCRD("ins")
 
@@ -1022,7 +1023,8 @@ var _ = Describe("Subscription", func() {
 	// - Delete the referenced InstallPlan
 	// - Wait for sub to have status condition SubscriptionInstallPlanMissing true
 	// - Ensure original non-InstallPlan status conditions remain after InstallPlan transitions
-	It("can reconcile InstallPlan status", func() {
+	// issue: https://github.com/operator-framework/operator-lifecycle-manager/issues/2645
+	It("[FLAKE] can reconcile InstallPlan status", func() {
 		c := newKubeClient()
 		crc := newCRClient()