operator-framework / operator-controller

A new and improved management framework for extending Kubernetes with Operators
https://operator-framework.github.io/operator-controller/
Apache License 2.0
55 stars 53 forks source link

⚠ REMOVE extension api from main branch #820

Closed acmenezes closed 4 months ago

acmenezes commented 5 months ago

Description

The intent is to remove this API and its controllers for the v1.0.0 release. Once we've released 1.0.0, we plan to re-introduce the Extension API and controllers.

@joelanford, please take a look and tell me if that's what you had in mind. I did a flat removal of all the references to the api as well. Didn't try to circumvent or redesign around any errors. Any suggestions or changes are welcomed. Saving this as a draft PR to be safe.

My question, and I think it's an important one, would we pull the main branch's code to some secondary branch as a reference for after v1.0.0 before merging this if we plan to reintroduce it?

Solves #735

Reviewer Checklist

netlify[bot] commented 5 months ago

Deploy Preview for olmv1 ready!

Name Link
Latest commit 5f838135a940bcc0d650582dc22e264976e920b1
Latest deploy log https://app.netlify.com/sites/olmv1/deploys/66437b470d70e70008b3e7a8
Deploy Preview https://deploy-preview-820--olmv1.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

codecov[bot] commented 5 months ago

Codecov Report

Attention: Patch coverage is 66.66667% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 81.80%. Comparing base (6d73b73) to head (ca00f4a).

Files Patch % Lines
...nternal/controllers/clusterextension_controller.go 57.14% 4 Missing and 2 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #820 +/- ## =========================================== + Coverage 71.02% 81.80% +10.78% =========================================== Files 17 15 -2 Lines 1301 907 -394 =========================================== - Hits 924 742 -182 + Misses 304 120 -184 + Partials 73 45 -28 ``` | [Flag](https://app.codecov.io/gh/operator-framework/operator-controller/pull/820/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) | Coverage Δ | | |---|---|---| | [e2e](https://app.codecov.io/gh/operator-framework/operator-controller/pull/820/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) | `59.75% <61.11%> (+17.02%)` | :arrow_up: | | [unit](https://app.codecov.io/gh/operator-framework/operator-controller/pull/820/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) | `73.76% <66.66%> (+10.16%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

joelanford commented 5 months ago

I don't think we need to save the code off. The git history is there, so we can just come back to the commit prior to this PR if necessary.

But I suspect we don't even do that. I think it is more likely that we'll essentially copy ClusterExtension back over to Extension and make the few tweaks necessary for it to work as a namespace-scoped API. And we'll do a similar thing for the controller.

acmenezes commented 5 months ago

IMO the lint failures can be addressed via a //nolint:unused comment for now and be updated in #747 when they are used again.

This should help keep the scope of this PR smaller

@everettraven good idea. I just added the comments there. I think no more lint issues are found. Thanks.