Fix csv e2e tests #2689

perdasilva · 2022-03-13T12:06:34Z

Signed-off-by: perdasilva [email protected]

Description of the change:
This PR refactors CSV e2e tests such that each test uses its own namespace. It also factors the disabling copied CSV e2e tests to their own file.

Motivation for the change:
Flaky tests driving everyone nuts

Closes: #2646
Closes: #2639
Closes: #2643
Closes: #2634
Closes: #2641

Reviewer Checklist

Implementation matches the proposed design, or proposal is updated to match implementation
Sufficient unit test coverage
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

perdasilva · 2022-03-13T12:09:14Z

Flakiness tests:

csv e2e

$ for i in (seq 0 10); make e2e-local TEST="ClusterServiceVersion2" >>  run.txt; end
$ cat run.txt | grep "SUCCESS" | wc -l
11

copied csv

$ for i in (seq 0 10); make e2e-local TEST="Disabling copied CSVs2" >>  run2.txt; end
$ cat run2.txt | grep "SUCCESS" | wc -l
11

NOTE: turns out seq is inclusive. The test name has 2 at the end because I was making the changes in new files and needed a unique name

perdasilva · 2022-03-13T12:37:57Z

/retest-required

perdasilva · 2022-03-13T14:44:38Z

/test flaky-e2e-tests

openshift-ci · 2022-03-13T14:44:40Z

@perdasilva: No presubmit jobs available for operator-framework/operator-lifecycle-manager@master

In response to this:

/test flaky-e2e-tests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

perdasilva · 2022-03-14T07:41:47Z

/test flaky-e2e-tests

openshift-ci · 2022-03-14T07:41:50Z

@perdasilva: No presubmit jobs available for operator-framework/operator-lifecycle-manager@master

In response to this:

/test flaky-e2e-tests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

perdasilva · 2022-03-14T07:42:27Z

/test flaky-e2e-tests

openshift-ci · 2022-03-14T07:42:29Z

@perdasilva: No presubmit jobs available for operator-framework/operator-lifecycle-manager@master

In response to this:

/test flaky-e2e-tests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

test/e2e/csv_e2e_test.go

perdasilva · 2022-03-14T16:08:56Z

@exdx regarding the Owned to Required change: I think that's due to the order of the tests having changed. If you look at master that test uses Required. There are others that use Owned. Sorry for the mess. It was easier for me to create a new file and bring over each test or group of tests one by one and test. Then I just copied everything back over the original file...

perdasilva · 2022-03-15T07:35:37Z

/test flaky-e2e-tests

openshift-ci · 2022-03-15T07:35:39Z

@perdasilva: No presubmit jobs available for operator-framework/operator-lifecycle-manager@master

In response to this:

/test flaky-e2e-tests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

timflannagan · 2022-03-15T20:15:36Z

Pulling this down locally and playing around with it now.

timflannagan · 2022-03-15T21:01:49Z

Ran this locally and it passed - It seems like we're still leaking a good chunk of resources:

$ k get ns
csv-e2e-lstsz                   Active   11m
csv-sa-non-csv-e2e-mljdzrwjw6   Active   10m
$ k get crds
apiczm5ks.example.com                         2022-03-15T20:42:45Z
apil9fwzs.example.com                         2022-03-15T20:42:47Z
apitwfb8s.example.com                         2022-03-15T20:42:55Z
apizmkmns.example.com                         2022-03-15T20:42:58Z
$ k get apiservices
v1.example.com                         Local                       True        14m
$ k get clusterroles
apil9fwzs.example.com-v1-view                                          2022-03-15T20:42:54Z
apil9fwzs.example.com-v1-admin                                         2022-03-15T20:42:54Z
apil9fwzs.example.com-v1-crdview                                       2022-03-15T20:42:54Z
apil9fwzs.example.com-v1-edit                                          2022-03-15T20:42:54Z
csv-e2e-lstsz-operatorgroup-view                                       2022-03-15T20:42:58Z
csv-e2e-lstsz-operatorgroup-edit                                       2022-03-15T20:42:58Z
csv-e2e-lstsz-operatorgroup-admin                                      2022-03-15T20:42:58Z
csv-sa-non-csv-e2e-mljdzrwjw6-operatorgroup-edit                       2022-03-15T20:43:42Z
csv-sa-non-csv-e2e-mljdzrwjw6-operatorgroup-view                       2022-03-15T20:43:42Z
csv-sa-non-csv-e2e-mljdzrwjw6-operatorgroup-admin                      2022-03-15T20:43:42Z
dep-799x5                                                              2022-03-15T20:47:03Z
dep-k58qp                                                              2022-03-15T20:47:03Z
dep-mnjvd                                                              2022-03-15T20:47:16Z
$ k get clusterrolebindings
dep-sszsz                                              ClusterRole/dep-k58qp                                                              11m
dep-9nfrk                                              ClusterRole/dep-799x5                                                              11m
dep-qn8x9                                              ClusterRole/dep-mnjvd                                                              11m
hat-server9rslt-service-system:auth-delegator          ClusterRole/system:auth-delegator                                                  10m

njhale

See comments below.

test/e2e/csv_e2e_test.go

test/e2e/util/resource_manager.go

test/e2e/util/resource_queue.go

test/e2e/util/resource_manager.go

test/e2e/copied_csv_e2e_test.go

timflannagan · 2022-03-29T16:03:23Z

It looks like we're still leaking a namespace + CRD:

$ k get ns
NAME                            STATUS   AGE
csv-sa-non-csv-e2e-vplnjkxkrr   Active   18m
...
$ k get crd
k get crd
NAME                                          CREATED AT
api8srgns.example.com                         2022-03-29T15:43:55Z
...

timflannagan · 2022-03-30T18:14:25Z

test/e2e/copied_csv_e2e_test.go

+			err := ctx.Ctx().Client().Delete(context.Background(), &csv)
+			if err != nil && k8serrors.IsNotFound(err) {
+				return err
+			}
+
+			return nil


nit: return client.IgnoreNotFound(...)

timflannagan · 2022-03-30T18:14:43Z

test/e2e/copied_csv_e2e_test.go

+	operatorsv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
+	"github.com/operator-framework/operator-lifecycle-manager/test/e2e/ctx"
+	corev1 "k8s.io/api/core/v1"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"


nit: apierrors

timflannagan · 2022-03-30T18:16:31Z

test/e2e/webhook_e2e_test.go

@@ -415,7 +415,7 @@ var _ = Describe("CSVs with a Webhook", func() {

 			// Make sure old resources are cleaned up.
 			Eventually(func() bool {
-				return csvExists(crc, csv.Spec.Replaces)
+				return csvExists(testNamespace, crc, csv.Spec.Replaces)


#2718 landed since this PR has opened - do we need to update this to use the test generated namespace?

(I fixed this in the latest force push)

timflannagan · 2022-03-30T18:16:54Z

test/e2e/util.go

-			fetched, err := newKubeClient().KubernetesInterface().CoreV1().Pods(catalogSource.GetNamespace()).List(context.Background(), metav1.ListOptions{LabelSelector: "olm.catalogSource=" + catalogSource.GetName()})
+			listOpts := metav1.ListOptions{
+				LabelSelector: "olm.catalogSource=" + catalogSource.GetName(),
+				FieldSelector: "status.phase=Running",


👍 looks reasonable to me

timflannagan · 2022-03-30T18:19:23Z

test/e2e/csv_e2e_test.go

-		// issue: https://github.com/operator-framework/operator-lifecycle-manager/issues/2646
-		It("[FLAKE] can satisfy an associated ClusterServiceVersion's ownership requirement", func() {


It looks like we've dropped this flake label when looking at the full file now. Should we graduate this test once these changes land and we have sufficient proof?

timflannagan · 2022-03-30T18:23:53Z

test/e2e/csv_e2e_test.go

 		})
-	})
+		When("a copied csv exists", func() {


Is this a duplicate test case compared to the dedicated copied CSV e2e suite?

timflannagan · 2022-03-30T18:25:07Z

test/e2e/csv_e2e_test.go

-		)
+			AfterEach(func() {
+				if target.GetName() != "" {
+					Expect(ctx.Ctx().Client().Delete(context.Background(), &target)).To(Succeed())


Do we need to use the TeardownNamespace helper function so we can also gather artifacts if this test case fails?

timflannagan · 2022-03-30T19:40:22Z

• Failure [68.716 seconds]
ClusterServiceVersion
/home/runner/work/operator-lifecycle-manager/operator-lifecycle-manager/test/e2e/csv_e2e_test.go:38
  AllNamespaces OperatorGroup
  /home/runner/work/operator-lifecycle-manager/operator-lifecycle-manager/test/e2e/csv_e2e_test.go:403
    when a copied csv exists
    /home/runner/work/operator-lifecycle-manager/operator-lifecycle-manager/test/e2e/csv_e2e_test.go:528
      is synchronized with the original csv [It]
      /home/runner/work/operator-lifecycle-manager/operator-lifecycle-manager/test/e2e/csv_e2e_test.go:577

      Timed out after 60.001s.
      Change to status of copy should have been reverted
      Expected
          <bool>: false
      to be true

      /home/runner/work/operator-lifecycle-manager/operator-lifecycle-manager/test/e2e/csv_e2e_test.go:603

      Full Stack Trace
      github.com/operator-framework/operator-lifecycle-manager/test/e2e.glob..func7.4.3.3()
      	/home/runner/work/operator-lifecycle-manager/operator-lifecycle-manager/test/e2e/csv_e2e_test.go:603 +0x18f
      github.com/operator-framework/operator-lifecycle-manager/test/e2e.TestEndToEnd(0x10768f9)
      	/home/runner/work/operator-lifecycle-manager/operator-lifecycle-manager/test/e2e/e2e_test.go:68 +0x268
      testing.tRunner(0xc0008af380, 0x33311e0)
      	/opt/hostedtoolcache/go/1.17.7/x64/src/testing/testing.go:[1259](https://github.com/operator-framework/operator-lifecycle-manager/runs/5760028839?check_suite_focus=true#step:4:1259) +0x102
      created by testing.(*T).Run
      	/opt/hostedtoolcache/go/1.17.7/x64/src/testing/testing.go:1306 +0x35a

Signed-off-by: perdasilva <[email protected]>

timflannagan

/lgtm

openshift-ci · 2022-03-31T13:20:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: perdasilva, timflannagan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [perdasilva,timflannagan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci bot requested review from anik120 and kevinrizza March 13, 2022 12:06

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 13, 2022

perdasilva force-pushed the fix_csv_e2e branch from d7efbd7 to 065bdd5 Compare March 13, 2022 12:10

perdasilva force-pushed the fix_csv_e2e branch from 065bdd5 to 92e817a Compare March 13, 2022 13:09

exdx reviewed Mar 14, 2022

View reviewed changes

test/e2e/csv_e2e_test.go Show resolved Hide resolved

test/e2e/csv_e2e_test.go Outdated Show resolved Hide resolved

test/e2e/csv_e2e_test.go Outdated Show resolved Hide resolved

perdasilva force-pushed the fix_csv_e2e branch from 92e817a to e485631 Compare March 14, 2022 15:53

perdasilva force-pushed the fix_csv_e2e branch from e485631 to 7ed746c Compare March 15, 2022 07:08

perdasilva force-pushed the fix_csv_e2e branch 2 times, most recently from 43f1d23 to a0bc8ef Compare March 18, 2022 17:13

njhale reviewed Mar 18, 2022

View reviewed changes

perdasilva force-pushed the fix_csv_e2e branch 5 times, most recently from 1bc3766 to d5b2e13 Compare March 23, 2022 10:48

perdasilva changed the title ~~Refactor csv e2e~~ Add e2e client and refactor csv and catsrc e2e suites Mar 23, 2022

timflannagan reviewed Mar 29, 2022

View reviewed changes

test/e2e/copied_csv_e2e_test.go Outdated Show resolved Hide resolved

perdasilva force-pushed the fix_csv_e2e branch from 84d712e to 9637dfb Compare March 29, 2022 15:22

perdasilva force-pushed the fix_csv_e2e branch from 9637dfb to 169ab85 Compare March 29, 2022 16:37

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 29, 2022

perdasilva force-pushed the fix_csv_e2e branch 2 times, most recently from e796376 to b85a279 Compare March 30, 2022 07:05

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 30, 2022

perdasilva force-pushed the fix_csv_e2e branch 3 times, most recently from cd0d8b1 to a1c404d Compare March 30, 2022 16:28

timflannagan reviewed Mar 30, 2022

View reviewed changes

timflannagan force-pushed the fix_csv_e2e branch from a1c404d to 2df6478 Compare March 30, 2022 19:12

timflannagan mentioned this pull request Mar 31, 2022

Update staging OLM 03-30 openshift/operator-framework-olm#274

Merged

perdasilva force-pushed the fix_csv_e2e branch 2 times, most recently from 4d6401f to d2a6a9d Compare March 31, 2022 07:51

Refactor csv e2e

7a694e0

Signed-off-by: perdasilva <[email protected]>

perdasilva force-pushed the fix_csv_e2e branch from d2a6a9d to 7a694e0 Compare March 31, 2022 11:18

timflannagan approved these changes Mar 31, 2022

View reviewed changes

openshift-ci bot assigned timflannagan Mar 31, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 31, 2022

openshift-merge-robot merged commit 19ea2cc into operator-framework:master Mar 31, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix csv e2e tests #2689

Fix csv e2e tests #2689

perdasilva commented Mar 13, 2022 •

edited

Loading

perdasilva commented Mar 13, 2022

perdasilva commented Mar 13, 2022

perdasilva commented Mar 13, 2022

openshift-ci bot commented Mar 13, 2022

perdasilva commented Mar 14, 2022

openshift-ci bot commented Mar 14, 2022

perdasilva commented Mar 14, 2022

openshift-ci bot commented Mar 14, 2022

perdasilva commented Mar 14, 2022

perdasilva commented Mar 15, 2022

openshift-ci bot commented Mar 15, 2022

timflannagan commented Mar 15, 2022

timflannagan commented Mar 15, 2022 •

edited

Loading

njhale left a comment

timflannagan commented Mar 29, 2022

timflannagan Mar 30, 2022

timflannagan Mar 30, 2022

timflannagan Mar 30, 2022

timflannagan Mar 30, 2022

timflannagan Mar 30, 2022

timflannagan Mar 30, 2022

timflannagan Mar 30, 2022

timflannagan Mar 30, 2022

timflannagan commented Mar 30, 2022

timflannagan left a comment

openshift-ci bot commented Mar 31, 2022

		// issue: https://github.com/operator-framework/operator-lifecycle-manager/issues/2646
		It("[FLAKE] can satisfy an associated ClusterServiceVersion's ownership requirement", func() {

Fix csv e2e tests #2689

Fix csv e2e tests #2689

Conversation

perdasilva commented Mar 13, 2022 • edited Loading

perdasilva commented Mar 13, 2022

perdasilva commented Mar 13, 2022

perdasilva commented Mar 13, 2022

openshift-ci bot commented Mar 13, 2022

perdasilva commented Mar 14, 2022

openshift-ci bot commented Mar 14, 2022

perdasilva commented Mar 14, 2022

openshift-ci bot commented Mar 14, 2022

perdasilva commented Mar 14, 2022

perdasilva commented Mar 15, 2022

openshift-ci bot commented Mar 15, 2022

timflannagan commented Mar 15, 2022

timflannagan commented Mar 15, 2022 • edited Loading

njhale left a comment

Choose a reason for hiding this comment

timflannagan commented Mar 29, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timflannagan commented Mar 30, 2022

timflannagan left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Mar 31, 2022

perdasilva commented Mar 13, 2022 •

edited

Loading

timflannagan commented Mar 15, 2022 •

edited

Loading