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

Remove OgSourceProvider #3360

Merged

Conversation

anik120
Copy link
Contributor

@anik120 anik120 commented Aug 6, 2024

Description of the change:

2788 introduced the OgSourceProvider
that allows for the exclusion of Catalogs in the global namespace. This OgSourceProvider
was used by the resolver as the SourceProvider implementation.

3349 modified the resolver, to list the CatalogSources
in the cluster by itself on the basis of the annotation in the OperatorGroup
in the resolving namespace, so that that information can be passed to the RegistrySourceProvider,
which now list the CatalogSources using a client instead of reaching out to the
registry-server cache.

This PR removes the OgSourceProvider, since the feature is now being directly implemented
in the resolver, and the OgSourceProvider implementation has become redundant.

Note that the feature parity is still maintained, as attested by the successful execution
of these tests introduced in 2788.

Motivation for the change:

Architectural changes:

Testing remarks:

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 Aug 6, 2024
[2788](operator-framework#2788) introduced the `OgSourceProvider`
that allows for the exclusion of Catalogs in the global namespace. This `OgSourceProvider`
was used by the resolver as the `SourceProvider` implementation.

[3349](operator-framework#3349) modified the resolver, to list the CatalogSources
in the cluster by itself on the basis of the annotation in the `OperatorGroup`
in the resolving namespace, so that that information can be passed to the `RegistrySourceProvider`,
which now list the CatalogSources using a client  instead of reaching out to the
registry-server cache.

This PR removes the `OgSourceProvider`, since the feature is now being directly implemented
in the resolver, and the `OgSourceProvider` implementation has become redundant.

Note that the feature parity is still maintained, as attested by the successful execution
of [these tests](https://github.com/operator-framework/operator-lifecycle-manager/blob/master/test/e2e/catalog_exclusion_test.go) introduced in 2788.
@anik120 anik120 changed the title WIP: Remove og source provider Remove OgSourceProvider Aug 8, 2024
@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 Aug 8, 2024
Copy link
Collaborator

@perdasilva perdasilva left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 2, 2024
@perdasilva perdasilva added this pull request to the merge queue Sep 12, 2024
Merged via the queue into operator-framework:master with commit 183a7f2 Sep 12, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants