test/e2e: Reduce subscription e2e test pollution #2438
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
All test cases in subscription suite shares the same test namespace. In some cases, the resource cleanup which doesn't work properly from previous test causes failure on subsequent test. This commit will create new namespace for each test so it can be run in insolation. Signed-off-by: Vu Dinh <vudinh@outlook.com>
timflannagan
force-pushed
the
test/reduce-subscription-e2e-test-pollution
branch
from
ca56d21
to
aff1154
Compare
- Update the helper function SetUpGeneratedTestNamespace -> SetupGeneratedTestNamespace. - Update SetupGeneratedTestNamespace to only return a corev1.Namespace object. Call sites can access the generated namespace by indexing into the object's GetName() method or the metdata.Name field. - Update SetupGeneratedTestNamespace and remove the logic that generates the namespace's metadata.name in the function. Call sites are responsible for generating a unique name now. - Update any subscription e2e tests to use the returned corev1.Namespace resource's GetName() method. - Remove the usage of the TearDown helper function in the subscription e2e suite. Generated namespaces (and their underlying testing resources) are cleaned up by deleting the namespace itself. Signed-off-by: timflannagan <timflannagan@gmail.com>
Remove duplicate package import for the operator v1alpha1 API library. Signed-off-by: timflannagan <timflannagan@gmail.com>
timflannagan
force-pushed
the
test/reduce-subscription-e2e-test-pollution
branch
from
aff1154
to
de4e4b3
Compare
Update the subscription and util e2e packages and replace any instances of context.TODO() with context.Background(). Signed-off-by: timflannagan <timflannagan@gmail.com>
|
Hmm interesting: Likely also creating a custom OG for that test case. |
…concile InstallPlan status' test
The 'can reconcile InstallPlan status' test case creates a custom OG
that targets the namespace that was created. This leads to the following
error message being logged in CI:
``log
2021-11-09T03:05:18.130980249Z stderr F E1109 03:05:18.130887 1 queueinformer_operator.go:290] sync {"update" "subscription-e2e-9bxh2/install-mqx5p"} failed: attenuated service account query failed - more than one operator group(s) are managing this namespace count=2
2021-11-09T03:05:18.130992349Z stderr F time="2021-11-09T03:05:18Z" level=info msg=syncing id=XXo2z ip=install-mqx5p namespace=subscription-e2e-9bxh2 phase=Installing
```
Remove this custom OG and use the OG that was created in the BeforeEach
clause instead.
Signed-off-by: timflannagan <timflannagan@gmail.com>
This was referenced Nov 9, 2021
|
/approve |
openshift-ci
bot
added
the
approved
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dinhxuanvu, kevinrizza, 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 |
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
|
Poked around that podconfig test failure further and found the following in the olm operator logs: It looks like the InstallPlan generated is reported a completed state, but the underlying nginx deploymentspec wasn't able to be created. We use that helper function for generating a CSV based on the nginx deployment quite a bit so I'm not too sure how to diagnose further. |
timflannagan
force-pushed
the
test/reduce-subscription-e2e-test-pollution
branch
2 times, most recently
from
cd0e512
to
2563e69
Compare
|
/lgtm |
timflannagan
removed
the
do-not-merge/hold
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of the change:
Reduce the overall testing pollution in the subscription e2e suite by generating a testing namespace for individual tests.
Motivation for the change:
Various flakes within this e2e package are popping up more frequently. Recent changes made to reduce the flake tolerance budget from three attempts to a single attempt were hiding inadequate isolation and cleanup between individual tests. A simple example is a test that created test fixtures in a shared namespace but neglected to cleanup those resources. Another, unrelated test will build up it's own set of text fixtures, and fail that test's individual assertions as the previously created and improperly cleaned up resources have unattended consequences, ex: a subscription fails resolution as another test had created a subscription in the same namespace, waited for an installplan to be created and report a completed state, that installplan is removed after that test had finished but the subscription still exists and the new test fails assertions that expect the newer subscription to report a ready state.
Reviewer Checklist
/doc