Skip to content

fix(zookeeper): fail closed on uncertain role probe#3230

Draft
weicao wants to merge 1 commit into
mainfrom
logan/zk-roleprobe-fail-closed
Draft

fix(zookeeper): fail closed on uncertain role probe#3230
weicao wants to merge 1 commit into
mainfrom
logan/zk-roleprobe-fail-closed

Conversation

@weicao

@weicao weicao commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Problem

ZooKeeper roleProbe currently exits successfully when it cannot obtain a trustworthy role. On the base commit, an empty nc response and a refused connection both returned rc=0 with empty output, while Mode: looking returned rc=0 and published looking as a role.

The probe output drives Pod role labels and routing. An unknown state must be rejected instead of being published as a role.

Change

  • propagate failures from the nc/zkServer.sh probe pipelines
  • accept only the exact modes standalone, leader, follower, and observer
  • keep the existing standalone to leader mapping
  • return non-zero with empty stdout for empty, uncertain, refused, or ambiguous multi-line results
  • add direct behavior tests for the actual script, including the valid and fail-closed matrices

Evidence

The new tests against the old implementation produced 18 examples with 4 expected failures: empty output, Mode: looking, connection refusal, and ambiguous multi-line output. The same tests pass after this change.

Validation:

  • focused roleprobe ShellSpec: 18 examples, 0 failures
  • all ZooKeeper script ShellSpec: 34 examples, 0 failures
  • Helm render: updated script is present in the ConfigMap and the lifecycle action still invokes /bin/bash -c /kubeblocks/scripts/roleprobe.sh
  • ShellCheck: only pre-existing SC2086 and ShellSpec indirect-call SC2329 informational findings; no new blocker
  • independent implementation conformity review: GREEN on exact commit c0bfd8f285e69be4bf7025e5411c85aacbc1536e

Review focus

Please focus on the exact role whitelist and whether every uncertain probe path returns non-zero without publishing stdout.

Boundary

This PR validates the role-probe script contract and chart rendering. It does not rerun or change the existing ZooKeeper runtime repetition ledger, and it does not claim coverage of all controller routing timings.

@weicao
weicao requested review from a team and leon-ape as code owners July 17, 2026 06:18
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 68 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.00%. Comparing base (80dfb2b) to head (c0bfd8f).

Files with missing lines Patch % Lines
addons/zookeeper/scripts-ut-spec/roleprobe_spec.sh 0.00% 68 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main   #3230   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files        150     150           
  Lines      23606   23673   +67     
=====================================
- Misses     23606   23673   +67     

☔ View full report in Codecov by Harness.
📢 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.

@weicao
weicao marked this pull request as draft July 18, 2026 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants