-
Notifications
You must be signed in to change notification settings - Fork 71
✨Add DeploymentConfig support to registry+v1 bundle renderer #2469
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
✨Add DeploymentConfig support to registry+v1 bundle renderer #2469
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
|
||
| // DeploymentConfig is a type alias for v1alpha1.SubscriptionConfig | ||
| // to maintain clear naming in the OLMv1 context while reusing the v0 type. | ||
| type DeploymentConfig = v1alpha1.SubscriptionConfig |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we weren't going to use SubscriptionConfig because of the extra selector field?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the RFC:
NOTE: The v0 SubscriptionConfig struct also has a [Selector](https://github.com/operator-framework/api/blob/master/pkg/operators/v1alpha1/subscription_types.go#L47) field, however it is completely ignored by the v0 controller. According to the comment in the code, it was meant to:
Select which pods/ReplicaSets would receive the subscription’s configuration
Match pod template labels(similar to Deployment selectors)
However, the code that extracts subscription config overrides(pkg/controller/operators/olm/overrides/config.go) extracts all other fields like NodeSelector, Tolerations, Resources and are subsequently applied, but the Selector field is never extracted.
OLMv1 will maintain the same behavior—accepting but ignoring this field.
Notice that there's no applySelectorConfig() function, so we're just ignoring that field.
With our wish to reuse v0 SubscriptionConfig for maintainability, we carry over nuance/s into v1. Silently ignoring the selector field being one of them.
We have to remember to document this explicitly for our users everywhere for added clarity.
07bd38a to
f8d0a5a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Implements Phase 2 of the Deployment Configuration RFC for registry+v1 bundles by threading a DeploymentConfig through the renderer and applying those customizations to CSV-generated Deployment resources (matching OLMv0 behavior), with extensive unit test coverage.
Changes:
- Add
DeploymentConfigsupport torender.Optionsand a newrender.WithDeploymentConfig(...)option. - Apply deployment customizations (env/envFrom, volumes, mounts, tolerations, resources, node selector, affinity, annotations) during registry+v1 CSV
Deploymentgeneration. - Add/extend unit tests covering option propagation and deployment customization behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/operator-controller/rukpak/render/render.go | Adds DeploymentConfig to render options and introduces WithDeploymentConfig. |
| internal/operator-controller/rukpak/render/render_test.go | Adds tests ensuring DeploymentConfig is passed through render options correctly. |
| internal/operator-controller/rukpak/render/registryv1/generators/generators.go | Applies DeploymentConfig to generated CSV Deployment resources via new helper functions. |
| internal/operator-controller/rukpak/render/registryv1/generators/generators_test.go | Adds integration-style tests verifying BundleCSVDeploymentGenerator applies config fields as expected. |
| internal/operator-controller/rukpak/render/registryv1/generators/deployment_config_test.go | Adds unit tests targeting the new config-application helper functions. |
| internal/operator-controller/config/config.go | Introduces a DeploymentConfig alias to v1alpha1.SubscriptionConfig for OLMv1 naming clarity. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
internal/operator-controller/rukpak/render/registryv1/generators/generators.go
Outdated
Show resolved
Hide resolved
internal/operator-controller/rukpak/render/registryv1/generators/generators.go
Outdated
Show resolved
Hide resolved
internal/operator-controller/rukpak/render/registryv1/generators/deployment_config_test.go
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2469 +/- ##
==========================================
+ Coverage 69.52% 69.75% +0.22%
==========================================
Files 102 102
Lines 8231 8451 +220
==========================================
+ Hits 5723 5895 +172
- Misses 2056 2087 +31
- Partials 452 469 +17
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Lint needs to be fixed. |
| "github.com/operator-framework/operator-controller/internal/operator-controller/config" | ||
| ) | ||
|
|
||
| func Test_applyCustomConfigToDeployment(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we move these tests against the public API?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i.e. WithDeploymentConfig
2c38f50 to
8666d5f
Compare
4d755ab to
4170eae
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
internal/operator-controller/rukpak/render/registryv1/generators/generators.go
Show resolved
Hide resolved
internal/operator-controller/rukpak/render/registryv1/generators/generators.go
Show resolved
Hide resolved
This PR implements **Phase 2** of the [Deployment Configuration RFC](https://docs.google.com/document/d/18O4qBvu5I4WIJgo5KU1opyUKcrfgk64xsI3tyXxmVEU/edit?tab=t.0): extending the OLMv1 bundle renderer to apply `DeploymentConfig` customizations to operator deployments. Building on the foundation from operator-framework#2454, this PR enables the renderer to accept and apply deployment configuration when rendering registry+v1 bundles. The implementation follows OLMv0's behavior patterns to ensure compatibility and correctness. The next PR will wire up the config in the `ClusterExtension` controller by parsing `spec.install.config` to convert to `DeploymentConfig` and thread `DeploymentConfig` through the controller's render call chain
4170eae to
04b566e
Compare
|
@perdasilva @tmshort addressed all the feedback, PTAL |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tmshort The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
8daa35d
into
operator-framework:main
**Summary** Completes Phase 3 of the [Deployment Configuration RFC](https://docs.google.com/document/d/1bDo3W1asZqjJTgZy7BcVOGtKOEukp0vUi5CO1ac3vwc/edit?usp=sharing). Extracts `deploymentConfig` from validated ClusterExtension configuration and passes it to the bundle renderer. Builds on: - (PR2454)[operator-framework#2454]: Config API and JSON schema validation - (PR2469)[operator-framework#2469]: Renderer support for applying deployment customizations This completes the RFC - users can now customize operator deployments via ClusterExtension.spec.config.inline: ``` config: configType: Inline inline: watchNamespace: "some-namespace" deploymentConfig: env: - name: TEST_ENV value: test-value nodeSelector: kubernetes.io/os: linux ```
**Summary** Completes Phase 3 of the [Deployment Configuration RFC](https://docs.google.com/document/d/1bDo3W1asZqjJTgZy7BcVOGtKOEukp0vUi5CO1ac3vwc/edit?usp=sharing). Extracts `deploymentConfig` from validated ClusterExtension configuration and passes it to the bundle renderer. Builds on: - (PR2454)[operator-framework#2454]: Config API and JSON schema validation - (PR2469)[operator-framework#2469]: Renderer support for applying deployment customizations This completes the RFC - users can now customize operator deployments via ClusterExtension.spec.config.inline: ``` config: configType: Inline inline: watchNamespace: "some-namespace" deploymentConfig: env: - name: TEST_ENV value: test-value nodeSelector: kubernetes.io/os: linux ```
**Summary** Completes Phase 3 of the [Deployment Configuration RFC](https://docs.google.com/document/d/1bDo3W1asZqjJTgZy7BcVOGtKOEukp0vUi5CO1ac3vwc/edit?usp=sharing). Extracts `deploymentConfig` from validated ClusterExtension configuration and passes it to the bundle renderer. Builds on: - (PR2454)[operator-framework#2454]: Config API and JSON schema validation - (PR2469)[operator-framework#2469]: Renderer support for applying deployment customizations This completes the RFC - users can now customize operator deployments via ClusterExtension.spec.config.inline: ``` config: configType: Inline inline: watchNamespace: "some-namespace" deploymentConfig: env: - name: TEST_ENV value: test-value nodeSelector: kubernetes.io/os: linux ```
**Summary** Completes Phase 3 of the [Deployment Configuration RFC](https://docs.google.com/document/d/1bDo3W1asZqjJTgZy7BcVOGtKOEukp0vUi5CO1ac3vwc/edit?usp=sharing). Extracts `deploymentConfig` from validated ClusterExtension configuration and passes it to the bundle renderer. Builds on: - (PR2454)[#2454]: Config API and JSON schema validation - (PR2469)[#2469]: Renderer support for applying deployment customizations This completes the RFC - users can now customize operator deployments via ClusterExtension.spec.config.inline: ``` config: configType: Inline inline: watchNamespace: "some-namespace" deploymentConfig: env: - name: TEST_ENV value: test-value nodeSelector: kubernetes.io/os: linux ```
Description
This PR implements Phase 2 of the Deployment Configuration RFC: extending the OLMv1 bundle renderer to apply
DeploymentConfigcustomizations to operator deployments.Building on the foundation from #2454, this PR enables the renderer to accept and apply deployment configuration when rendering registry+v1 bundles. The implementation follows OLMv0's behavior patterns to ensure compatibility and correctness.
The next PR will wire up the config in the
ClusterExtensioncontroller by parsingspec.install.configto convert toDeploymentConfigand threadDeploymentConfigthrough the controller's render call chainReviewer Checklist