🌱 Add health probes to test-operator bundles #2311

pedjak · 2025-11-06T16:13:02Z

Description

Enhance test-operator v1.0.0 and v1.2.0 bundles with HTTP-based health probes
to enable testing of startup, liveness, and readiness probe behavior. Replaces
the simple sleep container with a busybox httpd server that exposes probe
endpoints via a ConfigMap-mounted script.

The probes can be flipped from true to false by executing rm /var/www{stared,ready,live}
and back to true by performin touch /var/www{stared,ready,live}

Reviewer Checklist

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

netlify · 2025-11-06T16:13:08Z

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	`81c3cab`
🔍 Latest deploy log	https://app.netlify.com/projects/olmv1/deploys/6911b61adfab2f0008027b2c
😎 Deploy Preview	https://deploy-preview-2311--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.

Copilot

Pull Request Overview

This PR updates the test operator bundles (v1.0.0 and v1.2.0) by replacing a simple sleep-based busybox container with an httpd-based container that includes health probes. The changes add ConfigMaps with startup scripts and configure Kubernetes health checks for better container lifecycle management.

Replaces the sleep command with an httpd server implementation
Adds startup, liveness, and readiness probes with HTTP endpoints
Introduces ConfigMaps to mount executable scripts into containers

Reviewed Changes

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

File	Description
testdata/images/bundles/test-operator/v1.2.0/manifests/testoperator.clusterserviceversion.yaml	Updates container configuration with httpd server, health probes, and volume mounts for v1.2.0
testdata/images/bundles/test-operator/v1.2.0/manifests/script.configmap.yaml	Adds ConfigMap with httpd startup script for v1.2.0
testdata/images/bundles/test-operator/v1.0.0/manifests/testoperator.clusterserviceversion.yaml	Updates container configuration with httpd server, health probes, and volume mounts for v1.0.0
testdata/images/bundles/test-operator/v1.0.0/manifests/script.configmap.yaml	Adds ConfigMap with httpd startup script for v1.0.0

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

Copilot · 2025-11-06T16:14:31Z

testdata/images/bundles/test-operator/v1.2.0/manifests/testoperator.clusterserviceversion.yaml

+                        port: 80
+                      initialDelaySeconds: 1
+                      periodSeconds: 1
                    serviceAccountName: simple-bundle-manager


The serviceAccountName field is incorrectly placed inside the container spec. It should be at the pod spec level (after the containers array, not inside it). This will cause the deployment to fail.

Copilot · 2025-11-06T16:14:32Z

testdata/images/bundles/test-operator/v1.0.0/manifests/testoperator.clusterserviceversion.yaml

+                        port: 80
+                      initialDelaySeconds: 1
+                      periodSeconds: 1
                    serviceAccountName: simple-bundle-manager


The serviceAccountName field is incorrectly placed inside the container spec. It should be at the pod spec level (after the containers array, not inside it). This will cause the deployment to fail.

Incorrect, all e2e tests are passing.

The fact that the tests are passing doesn’t necessarily mean the implementation is correct.
It depends on what the end-to-end (E2E) tests are actually verifying with respect to this test data.

Also, note that the serviceAccountName should not be defined inside the container specification.
You can refer to this example for the correct structure:
https://github.com/operator-framework/operator-sdk/blob/master/testdata/go/v4/memcached-operator/bundle/manifests/memcached-operator.clusterserviceversion.yaml#L198

I'll check this out and poke at it. It seems weird that the e2es would pass. But, I guess they are only checking for the installed status and maybe the underlying helm operation succeeded while the operator is itself it bung?

yeah - the pod is getting pushed onto the default service account:

$ kubectl get pods -n test-operator -o yaml | grep -i serviceaccount - mountPath: /var/run/secrets/kubernetes.io/serviceaccount serviceAccount: default serviceAccountName: default - serviceAccountToken: - mountPath: /var/run/secrets/kubernetes.io/serviceaccount

I've updated the resources:

$ kubectl get pods -n test-operator -o yaml | grep -i serviceaccount serviceAccount: simple-bundle-manager serviceAccountName: simple-bundle-manager ...

codecov · 2025-11-06T16:44:46Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.24%. Comparing base (b3f85d5) to head (81c3cab).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2311      +/-   ##
==========================================
+ Coverage   71.14%   74.24%   +3.09%     
==========================================
  Files          91       91              
  Lines        7046     7046              
==========================================
+ Hits         5013     5231     +218     
+ Misses       1618     1402     -216     
+ Partials      415      413       -2

Flag	Coverage Δ
e2e	`45.94% <ø> (ø)`
experimental-e2e	`48.20% <ø> (?)`
unit	`58.55% <ø> (-0.03%)`	⬇️

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.

camilamacedo86

Hi @pedjak 👋

Thank you for the contribution!

I’m not sure what problem this change solves — could you please clarify?

How does it help us test OLM features?

Otherwise, I’d prefer to keep the samples simple and focused only on what’s needed to test OLM.

camilamacedo86 · 2025-11-07T02:26:01Z

testdata/images/bundles/test-operator/v1.0.0/manifests/script.configmap.yaml

+    echo true > /var/www/started
+    echo true > /var/www/ready
+    echo true > /var/www/live
+    exec httpd -f -h /var/www -p 80


TL;DR (just to share):

Common case of content managed by OLM is scaffolded with Operator SDK/Kubebuilder, which use controller-runtime.

We already have a project used in downstream tests that we maintain — test-operator — which includes health and ready probes. See the default implementation: https://github.com/operator-framework/test-operator/blob/main/cmd/main.go#L238-L245 ( controller runtime )

If there’s a strong reason to add probe implementations here (which doesn’t seem to be the case right now), I’d prefer to rely on the existing setup — meaning we can create a bundle using test-operator.

Otherwise, adding probes here feels unnecessary. I’d suggest using the existing test-operator instead.

I don't think we ever reached a steady state on the test-operator or use it within our e2es. I'm actually wondering if we should nuke it for now.

The nice thing about having the health and readiness probes defined in this way is that we can very easily change their state at runtime by creating/deleting the files in /var/www, which is great for e2e testing. This is the real value added here by this PR.

That might be a true point. However, unless we add tests that actually take advantage of it, I don’t see a strong reason to include it.

Could we add some unit tests or use case scenarios that demonstrate where this feature would be helpful? If so, I’d be completely on board with it — that would show not only that this change is useful, but also that it works as expected.

These will come when we start to lay down more boxcutter tests. This was added to help with manual testing of the conditions changes we've been introducing. I think it's ok to add this now, to reduce the size of PRs later.

If you want to move forward, I’m okay with that.
But I think we should properly validate that:

a) the configuration here is correct,

b) it provides the flexibility we want, and

c) it’s actually useful in practice (and not just theoretical).

I’d prefer to have at least one test using it, but I won’t block this one! 👍

🤔 lemme see what I can do

Enhance test-operator v1.0.0 and v1.2.0 bundles with HTTP-based health probes to enable testing of startup, liveness, and readiness probe behavior. Replaces the simple sleep container with a busybox httpd server that exposes probe endpoints via a ConfigMap-mounted script. The probes can be flipped from true to false by executing `rm /var/www{stared,ready,live}` and back to true by performin `touch /var/www{stared,ready,live}`

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

@pedjak

@pedjak please fix the changes in the mock/testdata
Then, I think @per is ok with that.

camilamacedo86

/lgtm

We can add the test afterwords and check it
All fine. If it does not work well as we expected we can always change things up 👍

perdasilva · 2025-11-11T09:34:19Z

/approve

openshift-ci · 2025-11-11T09:34:24Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, perdasilva

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:

~~OWNERS~~ [perdasilva]

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

Copilot AI review requested due to automatic review settings November 6, 2025 16:13

pedjak requested a review from a team as a code owner November 6, 2025 16:13

openshift-ci bot requested review from OchiengEd and bentito November 6, 2025 16:13

Copilot AI reviewed Nov 6, 2025

View reviewed changes

pedjak requested a review from perdasilva November 6, 2025 17:03

camilamacedo86 previously requested changes Nov 7, 2025

View reviewed changes

openshift-ci bot assigned camilamacedo86 Nov 7, 2025

camilamacedo86 reviewed Nov 7, 2025

View reviewed changes

pedjak and others added 2 commits November 10, 2025 10:53

Adjust serviceAccount indent in ClusterServiceVersion

81c3cab

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

perdasilva force-pushed the adjustable-health-propes-test-operator branch from dd5384a to 81c3cab Compare November 10, 2025 09:53

perdasilva requested a review from camilamacedo86 November 10, 2025 09:53

camilamacedo86 approved these changes Nov 11, 2025

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 11, 2025

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 11, 2025

openshift-merge-bot bot merged commit 936797b into operator-framework:main Nov 11, 2025
28 of 29 checks passed

🌱 Add health probes to test-operator bundles #2311

🌱 Add health probes to test-operator bundles #2311

Conversation

pedjak commented Nov 6, 2025

Description

Reviewer Checklist

Uh oh!

netlify bot commented Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for olmv1 ready!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

pedjak Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

perdasilva Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

perdasilva Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

perdasilva Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

camilamacedo86 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

perdasilva Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

perdasilva Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

perdasilva Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 left a comment

Choose a reason for hiding this comment

Uh oh!

perdasilva commented Nov 11, 2025

Uh oh!

openshift-ci bot commented Nov 11, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

netlify bot commented Nov 6, 2025 •

edited

Loading

camilamacedo86 Nov 7, 2025 •

edited

Loading

codecov bot commented Nov 6, 2025 •

edited

Loading

camilamacedo86 left a comment •

edited

Loading

camilamacedo86 Nov 7, 2025 •

edited

Loading

perdasilva Nov 7, 2025 •

edited

Loading

camilamacedo86 Nov 7, 2025 •

edited

Loading