Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

e2e: Refactor the CatalogSource tests to reduce flakes #2693

Merged
merged 2 commits into from
Mar 15, 2022
Merged

e2e: Refactor the CatalogSource tests to reduce flakes #2693

merged 2 commits into from
Mar 15, 2022

Conversation

timflannagan
Copy link
Contributor

Description of the change:
Follow up to #2690 as it doesn't look like I have the right permissions to push to Per's fork.

Motivation for the change:
See #2690 for more information.

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 and others added 2 commits March 14, 2022 17:09
@perdasilva
Refactor catalog source e2e tests to unique namespaces
6eed805 
Signed-off-by: perdasilva <perdasilva@redhat.com>
@timflannagan
e2e: Ensure the CatalogSource resource created in the global catalog …
000ee89 
…test is deleted

Signed-off-by: timflannagan <timflannagan@gmail.com>
@openshift-ci
Copy link

openshift-ci bot commented Mar 14, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 14, 2022
@timflannagan timflannagan changed the title Fix/catsrc e2e e2e: Refactor the CatalogSource tests to reduce flakes Mar 14, 2022
@timflannagan timflannagan mentioned this pull request Mar 14, 2022
Closed
7 tasks
timflannagan
timflannagan commented Mar 14, 2022
View reviewed changes
test/e2e/catalog_e2e_test.go
Comment on lines +150 to +151
cs, cleanup := createInternalCatalogSource(c, crc, mainCatalogName, globalNS, mainManifests, []apiextensions.CustomResourceDefinition{mainCRD}, []v1alpha1.ClusterServiceVersion{mainCSV})
defer cleanup()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: only substantial difference between Per's original PR and this one. I noticed we weren't cleaning up this CatalogSource that's deployed in the global namespace correctly.

timflannagan
timflannagan commented Mar 14, 2022
View reviewed changes
test/e2e/catalog_e2e_test.go

catalogSourceName := genName("mock-ocs-")
_, cleanupSource := createInternalCatalogSource(c, crc, catalogSourceName, operatorNamespace, manifests, []apiextensions.CustomResourceDefinition{crd}, []v1alpha1.ClusterServiceVersion{csv})
_, cleanupSource := createInternalCatalogSource(c, crc, catalogSourceName, ns.GetName(), manifests, []apiextensions.CustomResourceDefinition{crd}, []v1alpha1.ClusterServiceVersion{csv})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I noticed this test case was missed when ensuring that CatalogSources get deployed in their own namespace (outside of any tests that are testing the global namespace behavior), so any change you see for the remainder of this test spec's diff is substituting operatorNamespace -> the generated namespace's metadata.Name.

@timflannagan
Copy link
Contributor Author

Hmm interesting:

• Failure [323.808 seconds]
Catalog represents a store of bundles which OLM can use to install Operators
/home/runner/work/operator-lifecycle-manager/operator-lifecycle-manager/test/e2e/catalog_e2e_test.go:38
  delete internal registry pod triggers recreation [It]
  /home/runner/work/operator-lifecycle-manager/operator-lifecycle-manager/test/e2e/catalog_e2e_test.go:539

  
  	Error Trace:	util.go:110
  	            				catalog_e2e_test.go:596
  	            				runner.go:113
  	            				runner.go:64
  	            				it_node.go:26
  	            				spec.go:215
  	            				spec.go:138
  	            				spec_runner.go:200
  	            				spec_runner.go:170
  	            				spec_runner.go:66
  	            				suite.go:79
  	            				ginkgo_dsl.go:245
  	            				ginkgo_dsl.go:228
  	            				e2e_test.go:59
  	Error:      	Received unexpected error:
  	            	timed out waiting for the condition
  	Test:       	Catalog represents a store of bundles which OLM can use to install Operators delete internal registry pod triggers recreation

https://github.com/operator-framework/operator-lifecycle-manager/runs/5545060704?check_suite_focus=true

@akihikokuroda
Copy link
Member

^^^ may be related to 2 registry pods and probably fixed by #2614 (comment)

@timflannagan
Copy link
Contributor Author

timflannagan commented Mar 14, 2022 •
edited
Loading

#2693 (comment) x2 maybe a cached view? it looks like it only failed a single time in the e2e workflow status.

re-spinning again.

@akihikokuroda
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 15, 2022
@openshift-merge-robot openshift-merge-robot merged commit 14c0b96 into operator-framework:master Mar 15, 2022
@timflannagan timflannagan deleted the fix/catsrc-e2e branch March 15, 2022 02:35
@exdx exdx mentioned this pull request Mar 23, 2022
Merged
7 tasks
nsapse pushed a commit to nsapse/operator-lifecycle-manager that referenced this pull request Mar 28, 2022
@timflannagan @perdasilva @nsapse
e2e: Refactor the CatalogSource tests to reduce flakes (operator-fram…
e742e7a 
…ework#2693)

* Refactor catalog source e2e tests to unique namespaces

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

* e2e: Ensure the CatalogSource resource created in the global catalog test is deleted

Signed-off-by: timflannagan <timflannagan@gmail.com>

Co-authored-by: perdasilva <perdasilva@redhat.com>
Signed-off-by: Noah Sapse <nsapse@redhat.com>
@perdasilva perdasilva mentioned this pull request May 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akihikokuroda akihikokuroda Awaiting requested review from akihikokuroda

@awgreene awgreene Awaiting requested review from awgreene

Assignees

@akihikokuroda akihikokuroda

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@timflannagan @akihikokuroda @openshift-merge-robot @perdasilva