Skip to content

Conversation

@camilamacedo86
Copy link

No description provided.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 13, 2025

Hello @camilamacedo86! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

@openshift-ci openshift-ci bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Oct 13, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 13, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign everettraven for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@camilamacedo86 camilamacedo86 changed the title [OLMv1]: Promote NewOLMOwnSingleNamespace to GA OPRUN-4200: [OLMv1]: Promote NewOLMOwnSingleNamespace to GA Oct 13, 2025
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Oct 13, 2025
@openshift-ci-robot
Copy link

openshift-ci-robot commented Oct 13, 2025

@camilamacedo86: This pull request references OPRUN-4200 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set.

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 openshift-eng/jira-lifecycle-plugin repository.

@everettraven
Copy link
Contributor

@camilamacedo86 I took a look at the feature promotion verification and it looks like we are missing testing on metal-ipv6 (disconnected) and only have 3 tests.

Is there a reason that this isn't being tested in disconnected environments and that we do not have the required minimum of 5 tests?

@camilamacedo86
Copy link
Author

Hi @everettraven,

This feature is quite small, so it was a bit tricky to come up with 5 tests.
However, we managed to add two new extra tests and merged them yesterday:
👉 PR #502

With that, we should soon meet the 5 test criteria for compliance.

Regarding disconnected environments — the current tests use content from the OCP catalog.
We’re checking whether we can update them to use sample content instead, since the OCP catalog isn’t available in disconnected setups.

@openshift-ci openshift-ci bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 15, 2025
@openshift-ci-robot
Copy link

openshift-ci-robot commented Oct 15, 2025

@camilamacedo86: This pull request references OPRUN-4200 which is a valid jira issue.

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 openshift-eng/jira-lifecycle-plugin repository.

@camilamacedo86
Copy link
Author

/test verify-feature-promotion

1 similar comment
@camilamacedo86
Copy link
Author

/test verify-feature-promotion

@JoelSpeed
Copy link
Contributor

@camilamacedo86 There's at least one out of date generated file on this PR, please run PROTO_OPTIONAL=1 make update and fix the verify job

@camilamacedo86
Copy link
Author

/test verify-feature-promotion

@openshift-ci openshift-ci bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 20, 2025
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 20, 2025
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 20, 2025
@camilamacedo86
Copy link
Author

/test minor-e2e-upgrade-minor

Copy link
Contributor

@everettraven everettraven left a comment

Choose a reason for hiding this comment

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

The generated manifests that are included here seem off. It looks like it regenerated manifests under a new openshift-api/ sub-directory that doesn't exist today.

Let's remove that please.

@everettraven
Copy link
Contributor

@camilamacedo86 Looking at verify-feature-promotion shows that this is good on all but the metal-ipv6 runs. You need at least 14 runs on the metal-ipv6 jobs. Looks like you need ~5 more runs for that platform to meet the promotion criteria.

Thanks!

@camilamacedo86
Copy link
Author

camilamacedo86 commented Oct 20, 2025

Hi @everettraven

Thank you for checking out.

The generated manifests that are included here seem off. It looks like it regenerated manifests under a new openshift-api/ sub-directory that doesn't exist today.

I cloned the project in a local dir called openshift-api
I re-cloned to api only and re-run the make command, now it is fixed,
Good catcher. 🎉

@camilamacedo86 Looking at verify-feature-promotion shows that this is good on all but the metal-ipv6 runs. You need at least 14 runs on the metal-ipv6 jobs. Looks like you need ~5 more runs for that platform to meet the promotion criteria.

Yes, I trigged more jobs but seems that today we have been facing many issues with ocp ci ecosystem.
I am trying to collect those ones.

@camilamacedo86
Copy link
Author

/retest-required

@everettraven
Copy link
Contributor

@camilamacedo86 I'm still not sure I'm following why we need to add the github/openshift/api/* directory and files?

The only files I'm expecting to be updated here are:

  • features/features.go
  • features.md
  • payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml

@camilamacedo86
Copy link
Author

camilamacedo86 commented Oct 21, 2025

Hi @everettraven

@camilamacedo86 I'm still not sure I'm following why we need to add the github/openshift/api/* directory and files?

They were generated with the command PROTO_OPTIONAL=1 make update
Which was requested to be executed here: #2527 (comment)

Should we run or not this one?

@openshift-ci openshift-ci bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 21, 2025
@camilamacedo86
Copy link
Author

Hi @everettraven and @JoelSpeed

For this case we either need to rename the test to change it to allow run in a disconnected so the [ci/prow/verify-feature-promotion](https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_api/2527/pull-ci-openshift-api-master-verify-feature-promotion/1980704120935288832) will not pass.

We have 5 tests running in a disconnect env:

Could you please overwrite to allow us to move forward?
OR what would still missing?

@everettraven
Copy link
Contributor

@camilamacedo86 Thanks for bringing it to my attention that this is in a state ready for another review.

This PR is in my queue to review today.

@camilamacedo86
Copy link
Author

camilamacedo86 commented Oct 22, 2025

/test verify-feature-promotion

Just for you be able to check the latest state.

@everettraven
Copy link
Contributor

Doing a manual analysis aggregating the results across rename, this is the latest state of verification (focusing on the renames that don't outright pass):

Test Name aws/ha azure/ha gcp/ha metal/ha/ipv4 metal/ha/ipv6 metal/ha/dual vsphere/ha aws/single
OLMv1 operator installation support for ownNamespace and single namespace watch mode with operator should install cluster extensions successfully in both watch modes 98% (66/67) 95% (45/47) 100% (40/40) 90% (20/22) FAIL 100% (41/41) 96% (28/29) 100% (51/51) 98% (67/68)
OLMv1 operator installation support for ownNamespace watch mode with an operator that does not support ownNamespace installation mode should fail to install a cluster extension successfully 100% (29/29) 100% (40/40) 100% (50/50) 96% (29/30) 100% (17/17) 100% (29/29) 100% (57/57) 100% (64/64)

With this in mind, it looks like even if the rename is taken into account the verify-feature-promotion check would have failed due to the test OLMv1 operator installation support for ownNamespace and single namespace watch mode with operator should install cluster extensions successfully in both watch modes having a pass rate of 90% on metal-ipv4 when the minimum bar is 95%.

@everettraven
Copy link
Contributor

Would it be possible for us to collect more runs for metal-ipv4 to get to a state where this would have passed the check without the rename?

@everettraven
Copy link
Contributor

@camilamacedo86 I just noticed that this feature gate is pointing to a closed OpenShift Enhancement Proposal that never received any review from OpenShift API reviewers despite there being an API change involved.

I'm assuming this is now related to openshift/enhancements#1849 which is currently under review.

This will not be able to be promoted until the review has been completed and the enhancement has been merged as accepted.

@camilamacedo86
Copy link
Author

Thank you, @everettraven! 🙌

The fix for the flake (openshift/api#2527) has been merged and is now included in the image:

Fix included in accepted release: 4.21.0-0.nightly-2025-10-25-183621

To move forward with promotion, we just need to gather more data to confirm that no test shows a success rate below 95% and get merged the PEP openshift/enhancements#1849.

Once that’s both are in place, then we can move forward.

c/c @perdasilva

@camilamacedo86
Copy link
Author

/retest-required

1 similar comment
@camilamacedo86
Copy link
Author

/retest-required

@camilamacedo86
Copy link
Author

/test verify-feature-promotion

1 similar comment
@camilamacedo86
Copy link
Author

/test verify-feature-promotion

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 10, 2025

@camilamacedo86: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/okd-scos-e2e-aws-ovn 3a93502 link false /test okd-scos-e2e-aws-ovn
ci/prow/verify-feature-promotion 3a93502 link true /test verify-feature-promotion

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

@oceanc80
Copy link

oceanc80 commented Dec 1, 2025

/retitle OPRUN-4226: [OLMv1]: Promote NewOLMOwnSingleNamespace to GA #2527

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 1, 2025
@openshift-merge-robot
Copy link
Contributor

PR needs rebase.

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.

@openshift-ci openshift-ci bot changed the title OPRUN-4200: [OLMv1]: Promote NewOLMOwnSingleNamespace to GA OPRUN-4226: [OLMv1]: Promote NewOLMOwnSingleNamespace to GA #2527 Dec 1, 2025
@openshift-ci-robot
Copy link

openshift-ci-robot commented Dec 1, 2025

@camilamacedo86: This pull request references OPRUN-4226 which is a valid jira issue.

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 openshift-eng/jira-lifecycle-plugin repository.

@coderabbitai
Copy link

coderabbitai bot commented Dec 1, 2025

Walkthrough

The NewOLMOwnSingleNamespace feature gate is being enabled for the Default cluster profile. Changes include updating documentation to reflect the new release-phase column placement, extending the feature gate initialization to accept the Default configuration level, and moving the feature from the disabled to enabled list in the SelfManagedHA Default manifest.

Changes

Cohort / File(s) Summary
Documentation
features.md
NewOLMOwnSingleNamespace row repositioned with shifted column placement across release-phase columns
Feature Gate Configuration
features/features.go
enableForClusterProfile for NewOLMOwnSingleNamespace extended to include Default alongside SelfManaged, DevPreviewNoUpgrade, and TechPreviewNoUpgrade profiles
Feature Gate Manifest
payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml
NewOLMOwnSingleNamespace moved from disabled list to enabled list

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Review the feature gate logic change in features/features.go to confirm Default profile addition is intentional and consistent with intended gating behavior
  • Verify the YAML manifest update properly reflects the enabled state and is correctly formatted
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between a2cb0c5 and 3a93502.

📒 Files selected for processing (3)
  • features.md (1 hunks)
  • features/features.go (1 hunks)
  • payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

  • payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml
  • features/features.go
  • features.md
🧬 Code graph analysis (1)
features/features.go (2)
features/util.go (1)
  • SelfManaged (36-36)
config/v1/types_feature.go (3)
  • Default (41-41)
  • DevPreviewNoUpgrade (49-49)
  • TechPreviewNoUpgrade (45-45)
🔇 Additional comments (2)
payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml (1)

284-286: Verify GA promotion prerequisites are met before merging.

Based on PR comments, there are critical blockers for promoting this feature to GA:

  1. The enhancement PR (OPRUN-4133: OLMv1 single/own namespace install mode support enhancements#1849) is closed and needs to be reviewed and accepted before promotion can proceed
  2. Test coverage shows 90% success rate (20/22) on metal/ha/ipv4, which is below the required 95% minimum for GA promotion
  3. Additional metal-ipv4 test runs are needed to meet promotion criteria

Per the comments: "promotion cannot proceed until that enhancement is reviewed and accepted."

Confirm these prerequisites are satisfied before merging.

features/features.go (1)

500-506: The enhancement PR reference is correct. PR #1774 ("OPRUN-3767: Add OLMv1 Single/OwnNamespace EP") is the authoritative enhancement PR for the NewOLMOwnSingleNamespace feature gate. No update needed.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.5.0)

Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented
The command is terminated due to an error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented


Comment @coderabbitai help to get the list of available commands and usage tips.

@jianzhangbjz
Copy link
Member

cc QE @Xia-Zhao-rh
/assign @Xia-Zhao-rh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants