Skip to content

add liveness, readiness, and startup probes to noobaa-operator #1684

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+71 −4
Open

add liveness, readiness, and startup probes to noobaa-operator #1684

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f87a9a7
add liveness, readiness, and startup probes to noobaa-operator
OdedViner Aug 21, 2025
3fa2e24
fix code
OdedViner Sep 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions deploy/operator.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ spec:
containers:
- name: noobaa-operator
image: NOOBAA_OPERATOR_IMAGE
args:
- operator
- run
- --health-probe-bind-address=:8081
Comment on lines +44 to +47
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Trim trailing spaces and normalize args indentation.

Lines 45–46 have trailing spaces; fix to keep CI/yamllint green and avoid noisy diffs.

-          args:
-            - operator  
-            - run       
-            - --health-probe-bind-address=:8081
+          args:
+            - operator
+            - run
+            - --health-probe-bind-address=:8081

To verify no trailing spaces remain:

🏁 Script executed:

#!/usr/bin/env bash
grep -nE ' +$' deploy/operator.yaml || echo "No trailing spaces"

Length of output: 93

Trim trailing spaces in deploy/operator.yaml args
Lines 45–46 contain trailing spaces; remove them to keep CI/yamllint green.

🧰 Tools
🪛 YAMLlint (1.37.1)

[warning] 45-45: wrong indentation: expected 10 but found 12

(indentation)

[error] 45-45: trailing spaces

(trailing-spaces)

[error] 46-46: trailing spaces

(trailing-spaces)

🤖 Prompt for AI Agents 
In deploy/operator.yaml around lines 44 to 47, the args entries ("operator" and
"run") include trailing spaces on lines 45–46; remove the trailing whitespace
characters from those two args so each arg is trimmed to exact tokens (e.g.,
"operator" and "run") to satisfy yamllint/CI and keep YAML clean.

ports:
- name: healthz
containerPort: 8081
volumeMounts:
- name: bound-sa-token
mountPath: /var/run/secrets/openshift/serviceaccount
Expand All @@ -50,6 +57,29 @@ spec:
# SHOULD BE RETURNED ONCE COSI IS BACK
# - name: socket
# mountPath: /var/lib/cosi
readinessProbe:
httpGet:
path: /readyz
port: healthz
initialDelaySeconds: 5
periodSeconds: 10
timeoutSeconds: 5
failureThreshold: 3
livenessProbe:
httpGet:
path: /readyz
Copy link
Member

Choose a reason for hiding this comment

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

Should all probes be the same?

Copy link
Member

Choose a reason for hiding this comment

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

Since we only perform ping, I suppose it's okay for them to be set to the same path. In the future, we might want to use more advanced probes, but for now, it's enough.

port: healthz
initialDelaySeconds: 15
periodSeconds: 10
timeoutSeconds: 5
failureThreshold: 3
startupProbe:
Comment on lines +60 to +76
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Use /healthz for liveness; /readyz for readiness (current YAML uses /readyz for liveness).
This avoids killing a live process just because it isn’t ready yet and matches the PR description.

           readinessProbe:
             httpGet:
               path: /readyz
               port: healthz
             initialDelaySeconds: 5
             periodSeconds: 10
             timeoutSeconds: 5
             failureThreshold: 3
           livenessProbe:
             httpGet:
-              path: /readyz
+              path: /healthz
               port: healthz
             initialDelaySeconds: 15
             periodSeconds: 10
             timeoutSeconds: 5
             failureThreshold: 3

🏁 Script executed:

#!/usr/bin/env bash
set -e
# Locate the operator.yaml file
OP_FILE=$(fd --type f operator.yaml || true)
echo "Found file: $OP_FILE"
[ -n "$OP_FILE" ] || { echo "operator.yaml not found"; exit 1; }
# Print lines 60–76 for verification
sed -n '60,76p' "$OP_FILE"

Length of output: 698

Use /healthz for livenessProbe
Swap the livenessProbe HTTP path to /healthz (keep readinessProbe on /readyz) to avoid killing a live process before it’s ready.

🤖 Prompt for AI Agents 
In deploy/operator.yaml around lines 60 to 76, the livenessProbe is using the
/readyz path which can cause a live process to be killed before it is fully
ready; change the livenessProbe httpGet.path value from /readyz to /healthz
while leaving the readinessProbe on /readyz (no other probe changes required).

httpGet:
path: /readyz
port: healthz
periodSeconds: 10
timeoutSeconds: 5
failureThreshold: 30
resources:
limits:
cpu: "250m"
Expand Down
32 changes: 31 additions & 1 deletion pkg/bundle/deploy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6189,7 +6189,7 @@ spec:
sourceNamespace: default
`

const Sha256_deploy_operator_yaml = "aa8da1c289a05b3c94b9393b04307d38814a67625ac6a8006dace4d09366f35b"
const Sha256_deploy_operator_yaml = "5b33c842b603ff507c385c1f65fc2b2e86a3f3e131fc2e2e216d1bb966c032e2"

const File_deploy_operator_yaml = `apiVersion: apps/v1
kind: Deployment
Expand Down Expand Up @@ -6234,6 +6234,13 @@ spec:
containers:
- name: noobaa-operator
image: NOOBAA_OPERATOR_IMAGE
args:
- operator
- run
- --health-probe-bind-address=:8081
ports:
- name: healthz
containerPort: 8081
volumeMounts:
- name: bound-sa-token
mountPath: /var/run/secrets/openshift/serviceaccount
Expand All @@ -6243,6 +6250,29 @@ spec:
# SHOULD BE RETURNED ONCE COSI IS BACK
# - name: socket
# mountPath: /var/lib/cosi
readinessProbe:
httpGet:
path: /readyz
port: healthz
initialDelaySeconds: 5
periodSeconds: 10
timeoutSeconds: 5
failureThreshold: 3
livenessProbe:
httpGet:
path: /readyz
port: healthz
initialDelaySeconds: 15
periodSeconds: 10
timeoutSeconds: 5
failureThreshold: 3
startupProbe:
Comment on lines +6262 to +6269
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Liveness probe should hit /healthz, not /readyz.

controller-runtime exposes both /healthz and /readyz; liveness is expected to target /healthz. Pointing liveness to /readyz can mask unhealthy-but-ready states. Switch the path to /healthz.

           livenessProbe:
             httpGet:
-              path: /readyz
+              path: /healthz
               port: healthz
             initialDelaySeconds: 15
             periodSeconds: 10
             timeoutSeconds: 5
             failureThreshold: 3
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
httpGet:
path: /readyz
port: healthz
initialDelaySeconds: 15
periodSeconds: 10
timeoutSeconds: 5
failureThreshold: 3
startupProbe:
httpGet:
path: /healthz
port: healthz
initialDelaySeconds: 15
periodSeconds: 10
timeoutSeconds: 5
failureThreshold: 3
startupProbe:
🤖 Prompt for AI Agents 
In pkg/bundle/deploy.go around lines 6262 to 6269 the livenessProbe httpGet path
is set to /readyz but should target /healthz; change the livenessProbe httpGet
path value from "/readyz" to "/healthz" so the liveness check uses the
controller-runtime health endpoint and does not mask unhealthy-but-ready states.

httpGet:
path: /readyz
port: healthz
periodSeconds: 10
timeoutSeconds: 5
failureThreshold: 30
resources:
limits:
cpu: "250m"
Expand Down
13 changes: 10 additions & 3 deletions pkg/operator/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,18 @@ import (

"github.com/operator-framework/operator-lib/leader"
"sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/healthz"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/manager/signals"
metricsServer "sigs.k8s.io/controller-runtime/pkg/metrics/server"
)

// Change below variables to serve metrics on different host or port.
var (
metricsHost = "0.0.0.0"
metricsPort int32 = 8383
log = util.Logger()
metricsHost = "0.0.0.0"
metricsPort int32 = 8383
healthProbeAddr = ":8081"
log = util.Logger()
)

// RunOperator is the main function of the operator but it is called from a cobra.Command
Expand Down Expand Up @@ -66,6 +68,7 @@ func RunOperator(cmd *cobra.Command, args []string) {
Metrics: metricsServer.Options{
BindAddress: fmt.Sprintf("%s:%d", metricsHost, metricsPort),
},
HealthProbeBindAddress: healthProbeAddr, // Serve /healthz and /readyz here
})
if err != nil {
log.Fatalf("Failed to create manager: %s", err)
Expand Down Expand Up @@ -97,6 +100,10 @@ func RunOperator(cmd *cobra.Command, args []string) {
log.Fatalf("Failed AddToClusterScopedManager: %s", err)
}

if err := mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil {
log.Fatalf("Failed to add readiness check: %s", err)
}

util.Panic(mgr.Add(manager.RunnableFunc(func(ctx context.Context) error {
system.RunOperatorCreate(cmd, args)
return nil
Expand Down
Loading