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.

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

refactor operator-framework-e2e #503

Merged

Conversation

everettraven
Copy link
Contributor

@everettraven everettraven commented Oct 31, 2023

Description

  • Adds a script called setup.sh to perform all the setup logic for the coast-to-coast e2e test
    • This makes it easier to read and modify the setup logic in the future
  • Removes obsolete Go code and updates the Ginkgo tests as necessary
  • Removes obsolete Makefile targets and updates the Makefile as necessary

The combination of the changes here makes the test still generally do the same things of:

  • Create a temp directory for building operators
  • Use the operator-sdk to:
    • create two operators and build their images, one for the registry+v1 and one for the plain+v0 bundle formats
    • generate and build the bundle image for the registry+v1 bundle format
  • For the plain+v0 operator, create and build the plain+v0 bundle image
  • Creates a File-Based Catalog (FBC) image that contains both the operators
  • Load the operator, bundle, and catalog images onto a KinD cluster
  • Clean up the temp directories
  • Run the Ginkgo test suite that:
    • Creates a Catalog that references a catalog image, either the one with the plain+v0 bundle or the one with the registry+v1 bundle
    • Creates an Operator that references a package, either the one with the plain+v0 bundle or the one with the registry+v1 bundle
    • Verifies that the Operator eventually has the condition Installed with a status of True and a reason of Success

Motivation

Open Questions

  • The previous implementation included "upgrading" an operator by changing its .spec.version to be the "new" version. This implementation does not have that and instead focuses on the coast-to-coast of build, package, deploy. Is it necessary to include the "upgrading" of an operator as part of this e2e test?

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

@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 Oct 31, 2023
@everettraven everettraven changed the title WIP: refactor coast-to-coast e2e WIP: refactor operator-framework-e2e Oct 31, 2023
Copy link

codecov bot commented Oct 31, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7156464) 83.75% compared to head (cc277fc) 83.75%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #503   +/-   ##
=======================================
  Coverage   83.75%   83.75%           
=======================================
  Files          23       23           
  Lines         868      868           
=======================================
  Hits          727      727           
  Misses         96       96           
  Partials       45       45           
Flag Coverage Δ
e2e 65.43% <ø> (+0.34%) ⬆️
unit 78.44% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 31, 2023
@everettraven everettraven changed the title WIP: refactor operator-framework-e2e refactor operator-framework-e2e Oct 31, 2023
@everettraven everettraven marked this pull request as ready for review October 31, 2023 20:12
@everettraven everettraven requested a review from a team as a code owner October 31, 2023 20:12
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 31, 2023
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 1, 2023
to perform setup in a bash script, making
it easier to read+modify the setup and
easier to read the e2e logic written in Go

Signed-off-by: Bryce Palmer <everettraven@gmail.com>
test/operator-framework-e2e/operator_framework_test.go Outdated Show resolved Hide resolved
test/operator-framework-e2e/setup.sh Outdated Show resolved Hide resolved
test/operator-framework-e2e/setup.sh Outdated Show resolved Hide resolved
test/operator-framework-e2e/setup.sh Outdated Show resolved Hide resolved
test/operator-framework-e2e/setup.sh Outdated Show resolved Hide resolved
Signed-off-by: Bryce Palmer <everettraven@gmail.com>
@m1kola
Copy link
Member

m1kola commented Nov 2, 2023

Correct me if I'm wrong, but it seems that both #349 and #350 are resolved by this since we no longer use local-registry.

@everettraven
Copy link
Contributor Author

everettraven commented Nov 2, 2023

Correct me if I'm wrong, but it seems that both #349 and #350 are resolved by this since we no longer use local-registry.

Looks like it. I looked through the issues to try and find the ones this resolved and missed these. Will add them as resolved to the PR description, thanks!

Copy link
Member

@m1kola m1kola left a comment

Choose a reason for hiding this comment

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

Looks good overall. Just one question

@m1kola
Copy link
Member

m1kola commented Nov 2, 2023

Re: go-apidiff - we removed a bunch of e2e helper functions which were exported. I think we can ignore this.

@everettraven everettraven added this pull request to the merge queue Nov 2, 2023
Merged via the queue into operator-framework:main with commit e78fd95 Nov 2, 2023
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants