🌱 Revert Single/Own Namespace GA promotion #2292
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull Request Overview
This PR changes the SingleOwnNamespaceInstallSupport feature gate from General Availability (GA, enabled by default) to Alpha (disabled by default), reflecting that this feature requires additional validation before being production-ready.
- Demoted
SingleOwnNamespaceInstallSupportfeature gate from GA to Alpha status - Updated experimental manifests and Helm configurations to explicitly enable the feature gate
- Consolidated test files by moving content from
boxcutter_support_test.goandexperimental_test.gointosingle_namespace_support_test.go - Updated documentation to reflect the Alpha status and added instructions for enabling the feature gate
- Marked the
configfield in ClusterExtension API as experimental
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
internal/operator-controller/features/features.go |
Changed SingleOwnNamespaceInstallSupport from GA (default: true) to Alpha (default: false) |
manifests/experimental.yaml |
Added feature gate flag to enable SingleOwnNamespaceInstallSupport |
manifests/experimental-e2e.yaml |
Added feature gate flag for e2e testing environment |
helm/experimental.yaml |
Added feature to enabled features list in Helm config |
helm/tilt.yaml |
Added feature to enabled features list for Tilt development |
test/experimental-e2e/single_namespace_support_test.go |
Consolidated test setup and added version update test from deleted file |
test/experimental-e2e/experimental_test.go |
Deleted - moved content to single_namespace_support_test.go |
test/experimental-e2e/boxcutter_support_test.go |
Deleted - moved test to single_namespace_support_test.go |
hack/demo/single-namespace-demo-script.sh |
Updated to use experimental manifests and enable feature gate via kubectl patch |
hack/demo/own-namespace-demo-script.sh |
Updated to use experimental manifests and enable feature gate via kubectl patch |
docs/draft/howto/single-ownnamespace-install.md |
Updated documentation to reflect Alpha status and added feature gate enablement instructions |
docs/tutorials/explore-available-content.md |
Added note about feature gate requirement for single/own namespace support |
docs/draft/tutorials/explore-available-content-metas-endpoint.md |
Added note about feature gate requirement for single/own namespace support |
docs/project/olmv1_limitations.md |
Reverted documentation to state only AllNamespaces is supported by default |
api/v1/clusterextension_types.go |
Marked config field as experimental |
docs/api-reference/olmv1-api-reference.md |
Updated API documentation to reflect config field as experimental |
manifests/standard.yaml |
Removed config field from standard CRD |
manifests/standard-e2e.yaml |
Removed config field from standard e2e CRD |
helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml |
Removed config field from base CRD |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # remove feature gate from deployment | ||
| echo "Removing feature gate from operator-controller..." | ||
| kubectl patch deployment -n olmv1-system operator-controller-controller-manager --type='json' -p='[{"op": "remove", "path": "/spec/template/spec/containers/0/args", "value": "--feature-gates=SingleOwnNamespaceInstallSupport=true"}]' || true |
There was a problem hiding this comment.
The kubectl patch 'remove' operation should not include a 'value' field. The 'remove' operation in JSON Patch only requires the 'path' field, and including 'value' is invalid. This will cause the patch to fail (silently due to '|| true'). The correct operation should remove the specific argument from the array by index or use a different approach.
| kubectl patch deployment -n olmv1-system operator-controller-controller-manager --type='json' -p='[{"op": "remove", "path": "/spec/template/spec/containers/0/args", "value": "--feature-gates=SingleOwnNamespaceInstallSupport=true"}]' || true | |
| kubectl patch deployment -n olmv1-system operator-controller-controller-manager --type='json' -p='[{"op": "remove", "path": "/spec/template/spec/containers/0/args/-"}]' || true |
|
|
||
| # remove feature gate from deployment | ||
| echo "Removing feature gate from operator-controller..." | ||
| kubectl patch deployment -n olmv1-system operator-controller-controller-manager --type='json' -p='[{"op": "remove", "path": "/spec/template/spec/containers/0/args", "value": "--feature-gates=SingleOwnNamespaceInstallSupport=true"}]' || true |
There was a problem hiding this comment.
The kubectl patch 'remove' operation should not include a 'value' field. The 'remove' operation in JSON Patch only requires the 'path' field, and including 'value' is invalid. This will cause the patch to fail (silently due to '|| true'). The correct operation should remove the specific argument from the array by index or use a different approach.
| kubectl patch deployment -n olmv1-system operator-controller-controller-manager --type='json' -p='[{"op": "remove", "path": "/spec/template/spec/containers/0/args", "value": "--feature-gates=SingleOwnNamespaceInstallSupport=true"}]' || true | |
| # Find the index of the last argument in the args array | |
| last_index=$(kubectl get deployment -n olmv1-system operator-controller-controller-manager -o json | jq '.spec.template.spec.containers[0].args | length - 1') | |
| # Remove the last argument (feature gate) from the args array | |
| kubectl patch deployment -n olmv1-system operator-controller-controller-manager --type='json' -p="[{'op': 'remove', 'path': '/spec/template/spec/containers/0/args/${last_index}'}]" || true |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2292 +/- ##
==========================================
- Coverage 71.38% 71.25% -0.13%
==========================================
Files 90 90
Lines 7003 7003
==========================================
- Hits 4999 4990 -9
- Misses 1592 1600 +8
- Partials 412 413 +1
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:
|
camilamacedo86
left a comment
camilamacedo86
left a comment
There was a problem hiding this comment.
We can re-add after. 👍
/lgtm
|
/approve |
openshift-ci
bot
added
the
approved
|
/override crd-diff |
|
@tmshort: Overrode contexts on behalf of tmshort: crd-diff In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/override crd-diff/crd-diff |
|
@perdasilva: /override requires failed status contexts, check run or a prowjob name to operate on.
Only the following failed contexts/checkruns were expected:
If you are trying to override a checkrun that has a space in it, you must put a double quote on the context. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/override crd-diff |
|
@perdasilva: Overrode contexts on behalf of perdasilva: crd-diff In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
…o GA (OPRUN-4098) (operator-framework#2268)" This reverts commit 3c2fcb4. Signed-off-by: Per G. da Silva <pegoncal@redhat.com>
perdasilva
force-pushed
the
revert-singleownns-ga
branch
from
89c1329 to
0b65330
Compare
|
/override crd-diff |
|
@perdasilva: Overrode contexts on behalf of perdasilva: crd-diff In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, tmshort The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/override crd-diff |
|
@tmshort: Overrode contexts on behalf of tmshort: crd-diff In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/lgtm cancel |
|
/lgtm |
openshift-merge-bot
bot
merged commit 760855f
into
operator-framework:main
|
I ended up disabling crd-diff temporarily. |
3 participants
Description
There's still some contention around the API spec downstream. In case changes are required, let's revert this before we cut a new release and make sure there's upstream/downstream parity in the API spec.
Reviewer Checklist