✨Namespace and PVC probes by dtfranz · Pull Request #2535 · operator-framework/operator-controller

dtfranz · 2026-03-03T15:07:15Z

Adds readiness probing via CEL for namespaces and PVCs, to prevent subsequent phases from installing until their readiness checks have passed. Also adds e2e coverage via direct CER creation.

Description

Reviewer Checklist

API Go Documentation
Tests: Unit Tests (and E2E Tests, if appropriate)
Comprehensive Commit Messages
Links to related GitHub Issue(s)

netlify · 2026-03-03T15:07:21Z

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	`1f0a99f`
🔍 Latest deploy log	https://app.netlify.com/projects/olmv1/deploys/69a8079a1b07a4000807034e
😎 Deploy Preview	https://deploy-preview-2535--olmv1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

openshift-ci · 2026-03-03T15:08:10Z

[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 pedjak for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

OWNERS

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

Copilot

Pull request overview

Adds readiness probing for additional Kubernetes resource types (Namespaces and PVCs) using CEL-based probes in the ClusterExtensionRevision reconciliation flow, and extends the e2e framework + scenarios to validate direct CER application and phase-blocking behavior when probes fail.

Changes:

Add CEL readiness probes for Namespace (Active) and PersistentVolumeClaim (Bound) and wire them into the CER progress probe chain.
Extend e2e step framework to support direct ClusterExtensionRevision application and scenario templating via ${CER_NAME}.
Add new e2e feature coverage for CER install success and probe-driven phase progression/halting (PVC scenarios), plus required RBAC.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
test/e2e/steps/testdata/rbac-template.yaml	Grants e2e SA permissions for PV/PVC resources needed by new scenarios.
test/e2e/steps/steps.go	Adds `${CER_NAME}` templating and helper to fetch CER objects; expands step matching for “not installed”.
test/e2e/steps/hooks.go	Extends scenario context with CER name/state and updates cleanup behavior.
test/e2e/features/revision.feature	New e2e feature validating direct CER apply + probe gating for PVC phases.
test/e2e/README.md	Documents the new `${CER_NAME}` scenario variable.
internal/operator-controller/controllers/clusterextensionrevision_controller.go	Initializes and registers CEL probes for Namespace/PVC readiness in CER reconcile options.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

internal/operator-controller/controllers/clusterextensionrevision_controller.go

test/e2e/steps/hooks.go

test/e2e/steps/steps.go

internal/operator-controller/controllers/clusterextensionrevision_controller.go

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

test/e2e/steps/steps.go

codecov · 2026-03-03T16:21:01Z

Codecov Report

❌ Patch coverage is 64.70588% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.61%. Comparing base (55473d8) to head (1f0a99f).

Files with missing lines	Patch %	Lines
...controllers/clusterextensionrevision_controller.go	64.70%	3 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2535      +/-   ##
==========================================
- Coverage   68.62%   68.61%   -0.02%     
==========================================
  Files         131      131              
  Lines        9288     9304      +16     
==========================================
+ Hits         6374     6384      +10     
- Misses       2427     2430       +3     
- Partials      487      490       +3

Flag	Coverage Δ
e2e	`42.26% <0.00%> (-0.09%)`	⬇️
experimental-e2e	`52.09% <64.70%> (+0.07%)`	⬆️
unit	`53.62% <5.88%> (-0.10%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

test/e2e/features/revision.feature

Copilot · 2026-03-04T09:37:44Z

internal/operator-controller/controllers/clusterextensionrevision_controller.go

 func (c *ClusterExtensionRevisionReconciler) SetupWithManager(mgr ctrl.Manager) error {
+	// Initialize probes once at setup time
+	if err := initializeProbes(); err != nil {
+		return err
+	}


initializeProbes() is only invoked from SetupWithManager, but unit tests (and any direct calls to Reconcile without calling SetupWithManager) construct the reconciler and call Reconcile directly. In that case namespaceActiveProbe/pvcBoundProbe remain zero values, so the new readiness probes are effectively disabled (and could diverge from production behavior). Consider initializing these probes via a sync.Once guard that’s called from toBoxcutterRevision/Reconcile (and optionally from SetupWithManager), so probes are always available when reconciliation runs.

test/e2e/steps/steps.go

Adds readiness probing via CEL for namespaces and PVCs, to prevent subsequent phases from installing until their readiness checks have passed. Also adds e2e coverage via direct CER creation. Increased e2e timeout to 15m for experimental target. Signed-off-by: Daniel Franz <dfranz@redhat.com>

pedjak

not sure if for this PR is necessary to expose direct creation of CERs, instead of going through CE and appropriate bundles.

pedjak · 2026-03-04T20:44:01Z

internal/operator-controller/controllers/clusterextensionrevision_controller.go

 }

+// initializeProbes is used to initialize CEL probes at startup time, so we don't recreate them on every reconcile
+func initializeProbes() error {


I would actually wrap this function into sync.OnceValue and then call it from reconcile loop - it looks cleaner from workflow point of view.

pedjak · 2026-03-04T20:47:52Z

test/e2e/features/revision.feature

+    And ServiceAccount "olm-sa" with needed permissions is available in ${TEST_NAMESPACE}
+
+  @BoxcutterRuntime
+  Scenario:  Install simple revision


how is this scenario related to the logic introduced in PR? We have plenty of scenarios under install.feature that install exactly these resources.

pedjak · 2026-03-04T20:48:43Z

internal/operator-controller/controllers/clusterextensionrevision_controller.go

 }

+// initializeProbes is used to initialize CEL probes at startup time, so we don't recreate them on every reconcile
+func initializeProbes() error {


I would wrap this into sync.OnceValue and then call it from reconcile loop, it feels more natural and matches the flow.

pedjak · 2026-03-04T20:49:34Z

test/e2e/features/revision.feature

+      """
+
+    And resource "persistentvolumeclaim/test-pvc" is installed
+    And ClusterExtensionRevision "${CER_NAME}" reports Available as False with Reason ProbeFailure


is there any message we could assert?

pedjak · 2026-03-04T20:51:20Z

test/e2e/steps/hooks.go

-		for _, r := range forDeletion {
-			if _, err := k8sClient("delete", r.kind, r.name, "--ignore-not-found=true"); err != nil {
-				logger.Info("Error deleting resource", "name", r.name, "namespace", sc.namespace, "stderr", stderrOutput(err))
+	for _, r := range forDeletion {


how is this change related to the PR?

Copilot AI review requested due to automatic review settings March 3, 2026 15:07

openshift-ci bot requested review from ankitathomas and bentito March 3, 2026 15:08

Copilot started reviewing on behalf of dtfranz March 3, 2026 15:10 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

dtfranz force-pushed the pvc-namespace-probes branch 2 times, most recently from d87dfaf to 6a9f311 Compare March 3, 2026 15:45

Copilot AI review requested due to automatic review settings March 3, 2026 15:45

dtfranz force-pushed the pvc-namespace-probes branch from 6a9f311 to ec5019e Compare March 3, 2026 15:47

Copilot AI reviewed Mar 3, 2026

View reviewed changes

test/e2e/steps/steps.go Outdated Show resolved Hide resolved

test/e2e/steps/steps.go Outdated Show resolved Hide resolved

Copilot started reviewing on behalf of dtfranz March 3, 2026 16:09 View session

dtfranz force-pushed the pvc-namespace-probes branch 2 times, most recently from 493a1a7 to 1c7882b Compare March 4, 2026 09:31

Copilot AI review requested due to automatic review settings March 4, 2026 09:31

Copilot started reviewing on behalf of dtfranz March 4, 2026 09:31 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

dtfranz force-pushed the pvc-namespace-probes branch from 1c7882b to 1f0a99f Compare March 4, 2026 10:21

pedjak reviewed Mar 4, 2026

View reviewed changes

Conversation

dtfranz commented Mar 3, 2026

Description

Reviewer Checklist

Uh oh!

netlify bot commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for olmv1 ready!

Uh oh!

openshift-ci bot commented Mar 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pedjak left a comment

Choose a reason for hiding this comment

Uh oh!

pedjak Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

pedjak Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

pedjak Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

pedjak Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

pedjak Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

netlify bot commented Mar 3, 2026 •

edited

Loading

codecov bot commented Mar 3, 2026 •

edited

Loading