search

operator-framework / operator-controller

A new and improved management framework for extending Kubernetes with Operators
Apache License 2.0
35 stars 49 forks source link

✨ Modify ClusterExtension to use Helm under the hood to apply contents into the cluster #846

Closed bentito closed 1 month ago

bentito commented 2 months ago

Description

ClusterExtension reconciler needs to perform the following actions:

[See this doc for more detail if possible.]

  1. Based on the cluster Extension spec, get the list of bundles from the catalog and perform resolution.

    • The resolution here need not use dep.py, it can be simple filtering and sorting as done in extension controller currently.
      1. Ref: Extension controller code
      2. Resolution: Extension controller resolution
      3. The resolution step involves fetching the installed version. Since there are no APIs (like BundleDeployment or App) present in clusters that reference the installed bundle, we need to investigate on listing the installed helm charts or use helm secrets. This also means that when the helm chart is created, we need to store metadata that uniquely identifies an extension.

  2. Unpack the contents of the resolved bundle. For this iteration we can cover only registryV1 bundles from an image source.

  3. Conversion of registryV1 to plain bundle.

  4. Creating a Helm chart from the resources present in plain bundle.

  5. Installing helm chart onto the cluster.

    • Helm chart deployer
    • We need to be able to inject metadata into the chart that will be required for resolution as mentioned in 1.(iii).
      • Currently, we run a post render hook, that adds labels on the objects created by the chart: Post render hook details. Will need to figure out on how to store this info in the helm secret or on the helm chart directory.

Reviewer Checklist

netlify[bot] commented 2 months ago

Deploy Preview for olmv1 ready!

Name Link
Latest commit eafc251031d156d2a565ce592197048a2472f674
Latest deploy log https://app.netlify.com/sites/olmv1/deploys/6658a2435c1e0c00082a1028
Deploy Preview https://deploy-preview-846--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 2 months ago

Codecov Report

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

Project coverage is 75.38%. Comparing base (a458282) to head (60f4c80). Report is 9 commits behind head on main.

Files Patch % Lines
...nternal/controllers/clusterextension_controller.go 62.02% 94 Missing and 26 partials :warning:
internal/handler/registry.go 62.26% 10 Missing and 10 partials :warning:
internal/controllers/clusterextension_status.go 45.71% 19 Missing :warning:
cmd/manager/main.go 75.86% 9 Missing and 5 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #846 +/- ## ========================================== - Coverage 83.18% 75.38% -7.80% ========================================== Files 15 18 +3 Lines 874 1170 +296 ========================================== + Hits 727 882 +155 - Misses 102 208 +106 - Partials 45 80 +35 ``` | [Flag](https://app.codecov.io/gh/operator-framework/operator-controller/pull/846/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/846/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) | `56.06% <59.91%> (-6.18%)` | :arrow_down: | | [unit](https://app.codecov.io/gh/operator-framework/operator-controller/pull/846/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) | `50.60% <21.03%> (-24.33%)` | :arrow_down: | 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.

bentito commented 2 months ago

@tmshort thanks for deconflicting main.go 👍

dtfranz commented 2 months ago

FYI for reviewers: I've merged a new commit to the branch which re-enables all e2e tests; they should be passing now. If you like, you can view the standalone diff here.

tmshort commented 1 month ago

So, after a discussion, we decided to hopefully make it easier on all, to reduce churn on this PR.

We'd like to not make many changes to this PR unless absolutely necessary.

We will be creating follow up PRs for specific issues (e.g. errors fixes #878) Any PRs referenced (except #874) will be re-targeted against the main branch after this PR merges.

Those comments with "OWNER" (usually @varshaprasad96, sorry!) reference an upcoming change to this PR.

varshaprasad96 commented 1 month ago

The only part remaining to be fixed in this PR are the unit tests - #874 is in progress to handle the same.

codecov-commenter commented 1 month ago

Codecov Report

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

Project coverage is 77.21%. Comparing base (7727f8f) to head (eafc251). Report is 2 commits behind head on main.

Files Patch % Lines
...nternal/controllers/clusterextension_controller.go 67.43% 75 Missing and 24 partials :warning:
internal/handler/registry.go 62.26% 10 Missing and 10 partials :warning:
internal/controllers/common_controller.go 58.53% 17 Missing :warning:
cmd/manager/main.go 75.43% 9 Missing and 5 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #846 +/- ## ========================================== - Coverage 83.18% 77.21% -5.97% ========================================== Files 15 17 +2 Lines 874 1176 +302 ========================================== + Hits 727 908 +181 - Misses 102 189 +87 - Partials 45 79 +34 ``` | [Flag](https://app.codecov.io/gh/operator-framework/operator-controller/pull/846/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/846/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) | `59.86% <64.55%> (-2.38%)` | :arrow_down: | | [unit](https://app.codecov.io/gh/operator-framework/operator-controller/pull/846/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=operator-framework) | `58.52% <33.42%> (-16.42%)` | :arrow_down: | 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.

tmshort commented 1 month ago

@everettraven @m1kola @acornett21 PTAL We've rebased the PR and updated the unit tests, and will be going forward with this. Other significant changes to be done have been noted in various issues.