Skip to content

refactor: check /health/cluster response body and use ProbeBuilder - #982

Merged
razvan merged 8 commits into
mainfrom
fix/readiness-probe-cluster-connected-check
Aug 18, 2026
Merged

refactor: check /health/cluster response body and use ProbeBuilder#982
razvan merged 8 commits into
mainfrom
fix/readiness-probe-cluster-connected-check

Conversation

@razvan

@razvan razvan commented Aug 17, 2026

Copy link
Copy Markdown
Member

Description

--- PASS: kuttl (318.65s)
    --- PASS: kuttl/harness (0.00s)
        --- PASS: kuttl/harness/smoke_nifi-2.9.0_use-zookeeper-manager-true_zookeeper-3.9.5_openshift-false_listener-class-external-unstable (318.64s)
PASS

Definition of Done Checklist

  • Not all of these items are applicable to all PRs, the author should update this template to only leave the boxes in that are relevant
  • Please make sure all these things are done and tick the boxes

Author

  • Changes are OpenShift compatible
  • CRD changes approved
  • CRD documentation for all fields, following the style guide.
  • Helm chart can be installed and deployed operator works
  • Integration tests passed (for non trivial changes)
  • Changes need to be "offline" compatible
  • Links to generated (nightly) docs added
  • Release note snippet added

Reviewer

  • Code contains useful comments
  • Code contains useful logging statements
  • (Integration-)Test cases added
  • Documentation added or updated. Follows the style guide.
  • Changelog updated
  • Cargo.toml only contains references to git tags (not specific commits or branches)

Acceptance

  • Feature Tracker has been updated
  • Proper release label has been added
  • Links to generated (nightly) docs added
  • Release note snippet added
  • Add type/deprecation label & add to the deprecation schedule
  • Add type/experimental label & add to the experimental features tracker

razvan and others added 4 commits August 17, 2026 16:11
NiFi's /health/cluster management-server endpoint returns HTTP 200 for
both CONNECTING and CONNECTED nodes (only DISCONNECTED etc. get a
non-2xx status), so a bare `curl --fail` could not tell a node still
joining the cluster from one that has actually joined it. Verified
against a live NiFi 2.9.0 node in minikube: /health/cluster returned
200 with body "Cluster Status: CONNECTING" while joining, and 200
with "Cluster Status: CONNECTED" once fully joined.

The readiness probe now greps the response body for
"Cluster Status: CONNECTED" instead of only checking the return code.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Replaces manual Probe struct literals with ProbeBuilder, which validates
that duration fields fit into K8s's i32 seconds fields instead of
silently truncating.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@razvan razvan self-assigned this Aug 17, 2026
@razvan razvan moved this to Development: Waiting for Review in Stackable Engineering Aug 17, 2026
@sbernauer
sbernauer self-requested a review August 18, 2026 06:59
@sbernauer sbernauer moved this from Development: Waiting for Review to Development: In Review in Stackable Engineering Aug 18, 2026
Comment thread rust/operator-binary/src/controller/build/resource/probes.rs Outdated
Comment thread rust/operator-binary/src/controller/build/resource/probes.rs Outdated
Comment thread rust/operator-binary/src/controller/build/resource/probes.rs Outdated
Comment thread rust/operator-binary/src/controller/build/resource/probes.rs Outdated
Comment thread rust/operator-binary/src/controller/build/resource/probes.rs Outdated
Comment thread rust/operator-binary/src/controller/build/resource/probes.rs Outdated
Comment thread rust/operator-binary/src/controller/build/resource/probes.rs
razvan and others added 3 commits August 18, 2026 09:31
sbernauer
sbernauer previously approved these changes Aug 18, 2026

@sbernauer sbernauer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM thanks! Only minor comment suggestion

Comment thread rust/operator-binary/src/controller/build/resource/probes.rs Outdated
Comment thread rust/operator-binary/src/controller/build/resource/probes.rs Outdated
Co-authored-by: Sebastian Bernauer <sebastian.bernauer@stackable.tech>

@sbernauer sbernauer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@razvan
razvan added this pull request to the merge queue Aug 18, 2026
Merged via the queue into main with commit 674adbb Aug 18, 2026
12 checks passed
@razvan
razvan deleted the fix/readiness-probe-cluster-connected-check branch August 18, 2026 09:36
@soenkeliebau

soenkeliebau commented Aug 18, 2026

Copy link
Copy Markdown
Member

TL/DR: I had Claude run a field test with the improved readiness probe from this patch and it seems to significantly improve cluster stability in restart-heavy situations.

I didn't test this exact code, because it made running the test in parallel much harder, but I did take the changed readiness probe and apply it to a NiFi cluster via PodOverride

    podOverrides:
      spec:
        containers:
          - name: nifi
            readinessProbe:
              exec:
                command:
                  - /bin/bash
                  - -euo
                  - pipefail
                  - -c
                  - 'curl -fsS http://127.0.0.1:52020/health/cluster | grep -q "Cluster Status: CONNECTED"'
              periodSeconds: 10
              timeoutSeconds: 5
              failureThreshold: 3

Both clusters ran a light workload with a generateflowfile processor and a few loadbalancing queues ..

I then had Claude stand up a vanilla NiFi three node cluster and a cluster with this override applied and configure both with 5 minute cert lifetimes to ensure near constant restart pressure.

I let this run for a bit and had Claude observe the state especially with an eye on overlapping restarts, and this is the report that came out:

metric vanilla patched
% time fully connected (3/3) 64.3% 89.3%
% time degraded (<3 connected) 32.1% 7.1%
min connected during window 0 2
overlap episodes (entered ≤1 connected) 1 (persistent ~1.5 min) 0
samples with 2+ nodes down (conn≤1) 5 0
samples whole cluster down (conn=0) 5 0
false-green % (k8s Ready > actually connected) 21.4% 0.0%

What happened: on vanilla, pods report Ready at port-open → the PDB considers the cluster healthy and lets the restarter evict the next node before the previous rejoined → overlapping restarts → the cluster collapsed to 0/3 connected while k8s still showed all pods Ready (logs: "There is currently no Cluster Coordinator").

Patched's readiness=CONNECTED made the PDB pace evictions one-at-a-time, it never dropped below 2/3 and never overlapped. This is a direct, mechanistic reproduction of the customer's "restarted a node, cluster fell over, health looked fine."

@soenkeliebau

Copy link
Copy Markdown
Member

Additionally: this readiness check will actually significantly improve behavior after an unclean shutdown. The old check I believe reported ready before wal replay was done - with this, ready is only reported after an unclean state following a non-graceful shutdown has been cleaned up.

@lfrancke lfrancke moved this from Development: In Review to Development: Done in Stackable Engineering Sep 1, 2026
@lfrancke lfrancke moved this from Acceptance: In Progress to Done in Stackable Engineering Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

4 participants