[CELEBORN-2447] Add a /health endpoint to master and worker

[strelok89]

What changes were proposed in this pull request?

Add a GET /health endpoint served by both master and worker, whose HTTP status code reflects whether the service is able to serve: 200 when healthy and 503 when not.

• HttpService#healthCheck defaults to a shallow check that reports healthy once the HTTP service is available. This is what the master uses.
• Worker overrides it to report healthy only when the worker is registered with the master and its state is Normal.
• /health is added to the default HTTP authentication bypass paths.

The endpoint is mounted at the root rather than under /api/v1 so that a probe is not coupled to the API version, consistent with the other operational paths (/ping, /metrics/prometheus). The checked-in OpenAPI spec covers only /api/v1, so the generated client is unaffected.

Example:

$ curl -s -o /dev/null -w '%{http_code}\n' http://worker:9096/health
200

$ curl -s http://worker:9096/health
{"service":"worker","healthy":false,"reason":"worker is not registered with master"}

Why are the changes needed?

Celeborn currently exposes no endpoint whose status code reflects whether the process can serve, so a meaningful Kubernetes probe cannot be written. /ping always returns 200 regardless of state, and /api/v1/workers returns 200 even when isRegistered is false, so operators have to fall back to an exec probe that shells out and greps the JSON body.

The practical cost is rolling updates. Without a readiness signal, a StatefulSet marks a worker pod Ready as soon as its container process starts and immediately proceeds to the next ordinal, before the restarted worker has re-registered with the master. On a large cluster this can remove a meaningful fraction of workers from service before any of them rejoin.

Two design points worth calling out for review:

The worker check mirrors the master's own definition. AbstractMetaManager#isWorkerAvailable accepts only Normal, so a worker that is idle, decommissioning or exiting is already excluded from availableWorkers and never selected in offerSlots. Reporting such a worker as healthy would put /health at odds with the master. Whether Idle specifically should fail readiness is discussed on CELEBORN-2447 — it is reachable only through an explicit DecommissionThenIdle event, so the alternative (200 for Idle, 503 only for the decommission/exit states) is a reasonable variation if reviewers prefer it.

The master check is deliberately shallow. Masters are fronted by a headless Service that does not set publishNotReadyAddresses. A quorum-aware or leader-aware check would report every master as unhealthy during a cold start, withholding their DNS records and preventing them from discovering each other to form quorum. A follower is also a healthy replica. Quorum and leadership remain observable through /api/v1/ratis and /api/v1/masters.

/health bypasses authentication by default because a kubelet cannot present credentials, and celeborn.http.auth.bypass.api.paths defaults to empty (CELEBORN-2278).

Wiring probes into the Helm chart, which currently defines no livenessProbe, readinessProbe or startupProbe for either role, is intended as a follow-up so the two changes can be reviewed independently.

Does this PR resolve a correctness bug?

• Yes

Does this PR introduce any user-facing change?

• Yes

A new /health endpoint on master and worker, documented in docs/restapi.md.

How was this patch tested?

New tests:

• ApiBaseResourceSuite: /health returns 200 and reports the correct service name, for both master and worker.
• ApiBaseResourceAuthenticationSuite: /health is reachable without credentials.
• ApiWorkerResourceSuite: /health returns 503 when the worker is not registered.

Verified locally on JDK 11: celeborn-service/Test/compile and celeborn-worker/Test/compile pass, ApiMasterResourceSuite (19/19) and ApiMasterResourceAuthenticationSuite (9/9) pass, and spotless:check passes for the service and worker modules.

The worker-side suites were not run locally: they were developed on Windows, where any MiniClusterFeature test fails during setup because CelebornConf#workerBaseDirs splits each storage dir on : and rejects C:\... paths. That is pre-existing and unrelated to this change, and those suites are covered by CI.

[Copilot]

Pull request overview

This pull request adds a root-level GET /health endpoint on both Celeborn master and worker HTTP services, returning 200 when the process is considered able to serve and 503 otherwise, primarily to support Kubernetes readiness probing.

Changes:

• Introduces a new /health JAX-RS resource returning a structured JSON response and appropriate HTTP status codes.
• Adds a default HttpService#healthCheck implementation and a worker override that gates health on registration and Normal worker state.
• Extends HTTP auth bypass defaults to allow unauthenticated access to /health, and adds/extends tests plus REST API documentation.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
worker/src/test/scala/org/apache/celeborn/service/deploy/worker/http/api/ApiWorkerResourceSuite.scala	Adds a worker-specific test asserting /health returns 503 when the worker is not registered.
worker/src/main/scala/org/apache/celeborn/service/deploy/worker/Worker.scala	Implements worker-specific healthCheck() logic based on registration and worker state.
service/src/test/scala/org/apache/celeborn/server/common/http/ApiBaseResourceSuite.scala	Adds a base test validating /health returns 200 and includes the service name when healthy.
service/src/test/scala/org/apache/celeborn/server/common/http/ApiBaseResourceAuthenticationSuite.scala	Adds a test verifying /health is reachable without credentials.
service/src/main/scala/org/apache/celeborn/server/common/HttpService.scala	Adds a default healthCheck() hook for services to implement health semantics.
service/src/main/scala/org/apache/celeborn/server/common/http/authentication/AuthenticationFilter.scala	Adds /health to the default authentication bypass path set.
service/src/main/scala/org/apache/celeborn/server/common/http/api/HealthResource.scala	Adds the new /health endpoint and its response schema.
docs/restapi.md	Documents the new /health endpoint behavior and intent for readiness probing.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[zaynt4606]

Thanks @strelok89 for working on this. I found two cases where the worker readiness endpoint can report a false positive.

[strelok89]

@zaynt4606 fixed the things waiting for you check

[SteNicholas]

@strelok89, thanks for updates. I left one comment for updates. PTAL.

[SteNicholas]

@strelok89, initialize() starts the HTTP server and calls registerWithMaster() before initializing the push/fetch handlers and registering the controller endpoint. Successful registration sets both flags checked here to true, and the state is already Normal, so a probe in that interval receives 200 even though the handlers still reject traffic as "Worker Not Registered!". This can mark the pod Ready and let a rolling update advance before the worker can serve.
I reproduced this by observing the real /healthz response at the cleaner.submit(...) call in initialize(), without changing either registration flag: HTTP 200 while both pushDataHandler.checkRegistered() and fetchHandler.checkRegistered were false.
Could we also gate readiness on a safely published initialization-complete flag, set after handler initialization and rpcEnv.setupEndpoint(...), and add a startup test covering this interval?

[strelok89]

Good catch, thanks — you're right, and fixed in f753d09.

Worker now has an initialized AtomicBoolean, set immediately after the handler initialization and rpcEnv.setupEndpoint(RpcNameConstants.WORKER_EP, controller), and healthCheck() checks it first, before the registration checks, returning worker is still initializing. The ordering matters: in the interval you reproduced, registered and registeredInMasterView are already true and the state is Normal, so the init check has to come first for the reason to be accurate.

Test-wise, ApiWorkerResourceSuite gains a case that asserts the worker is registered, master-visible and Normal, then clears initialized and asserts /healthz returns 503 with that reason. It reproduces the window by clearing the flag rather than racing a live initialize(), which would be flaky in CI. I also made MiniClusterFeature's startup wait block on worker.initialized in addition to worker.registered, otherwise the existing suites could intermittently hit the new 503 window.

Master is unchanged — its healthCheck() is still the default (true, "") from HttpService.