Skip to content

Extra E2E coverage for #2942 - #2961

Closed
m1kola wants to merge 6 commits into
operator-framework:masterfrom
m1kola:unpack_before_creating_InstallPlan--exctra-e2e
Closed

Extra E2E coverage for #2942#2961
m1kola wants to merge 6 commits into
operator-framework:masterfrom
m1kola:unpack_before_creating_InstallPlan--exctra-e2e

Conversation

@m1kola

@m1kola m1kola commented May 5, 2023

Copy link
Copy Markdown
Member

Includes #2942 which needs to be merged first.

Description of the change:

This is a follow up for #2942

Keeping it separate from the main PR to make it easier to review (main PR is already large)

Motivation for the change:

Extra E2E coverage

Architectural changes:

None compared to #2942. Just extra E2E tests.

Testing remarks:

Extra E2E tests

Reviewer Checklist

  • Implementation matches the proposed design, or proposal is updated to match implementation
  • Sufficient unit test coverage
  • Sufficient end-to-end test coverage
  • Bug fixes are accompanied by regression test(s)
  • e2e tests and flake fixes are accompanied evidence of flake testing, e.g. executing the test 100(0) times
  • tech debt/todo is accompanied by issue link(s) in comments in the surrounding code
  • Tests are comprehensible, e.g. Ginkgo DSL is being used appropriately
  • Docs updated or added to /doc
  • Commit messages sensible and descriptive
  • Tests marked as [FLAKE] are truly flaky and have an issue
  • Code is properly formatted

@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 5, 2023
@openshift-ci

openshift-ci Bot commented May 5, 2023

Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci

openshift-ci Bot commented May 5, 2023

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: m1kola
Once this PR has been reviewed and has the lgtm label, please assign njhale for approval. For more information see the Kubernetes Code Review Process.

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

Details 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

@m1kola
m1kola force-pushed the unpack_before_creating_InstallPlan--exctra-e2e branch from c298534 to c8d19fc Compare May 5, 2023 12:34
@m1kola m1kola mentioned this pull request May 5, 2023
Merged
11 tasks
dtfranz
dtfranz reviewed May 9, 2023
View reviewed changes
Comment thread pkg/controller/operators/catalog/operator.go
})

// Remove BundleUnpackFailed condition from subscriptions
o.removeSubsCond(subs, v1alpha1.SubscriptionBundleUnpackFailed)

@dtfranz dtfranz May 9, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Out of scope for this, but it'd be nice if o.removeSubsCond accepted multiple conditions like the RemoveConditions func it calls does. That would remove some inefficiency from looping through the conditions multiple times.

Mikalai Radchuk added 3 commits May 22, 2023 17:52
Mikalai Radchuk @perdasilva
Changes how InstallPlans are being created
698e4d6 
Prevent OLM from creating `InstallPlan`s when bundle unpack fails

Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
Mikalai Radchuk @perdasilva
Updates unit tests for syncResolvingNamespace
c96f4d8 
Tests now include handling of unpacking errors

Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
Mikalai Radchuk @perdasilva
Updates E2E tests
67b8fc6 
Changes required to account for a new flow where we
prevent `InstallPlan` from being created when unpack
job fails

Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
@m1kola
m1kola force-pushed the unpack_before_creating_InstallPlan--exctra-e2e branch from c8d19fc to 937bda9 Compare May 24, 2023 13:17
m1kola
m1kola commented May 25, 2023
View reviewed changes
Comment thread test/e2e/subscription_e2e_test.go

Context("with bundle which OLM will fail to unpack", func() {
BeforeEach(func() {
By("patching the OperatorGroup to reduce the bundle unpacking timeout")

@m1kola m1kola May 25, 2023

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Suggested change
By("patching the OperatorGroup to reduce the bundle unpacking timeout")
By("patching the OperatorGroup to increase the bundle unpacking timeout")

@m1kola
m1kola force-pushed the unpack_before_creating_InstallPlan--exctra-e2e branch from 937bda9 to 255985c Compare May 25, 2023 16:20
Mikalai Radchuk added 2 commits May 25, 2023 17:21
Mikalai Radchuk
Renames test data files
01bc878 
Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
Mikalai Radchuk
Adds E2E coverage for unpack conditions
6846c99 
* Ensure no progress is reported after successfull unpacking
* Ensure OLM can auto recover when a broken catalog gets fixed

Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
@m1kola
m1kola force-pushed the unpack_before_creating_InstallPlan--exctra-e2e branch from 255985c to 8fce9f0 Compare May 25, 2023 16:22
Mikalai Radchuk
Changes fail forward E2E to use failed IP again
2a10f26 
Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
@m1kola
m1kola force-pushed the unpack_before_creating_InstallPlan--exctra-e2e branch from 8fce9f0 to 2a10f26 Compare May 26, 2023 09:11
@m1kola

m1kola commented May 26, 2023

Copy link
Copy Markdown
Member Author

I decided to merge this into the original PR #2942 since I needed extra test files which I already had in this PR. Closing

@m1kola m1kola closed this May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dtfranz dtfranz dtfranz left review comments

Assignees

No one assigned

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@m1kola @dtfranz