fix(health): a draining gateway stays live

crates/aisix-admin/src/lib.rs

@@ -380,11 +380,12 @@ fn normalized_prometheus_path(path: &str) -> String {
     }
 }
 
+/// Takes no [`AdminState`], for the same reason the proxy listener's
+/// does not: see [`aisix_proxy::health::livez_response`].
 async fn livez(
-    axum::extract::State(state): axum::extract::State<AdminState>,
     axum::extract::Query(params): axum::extract::Query<std::collections::HashMap<String, String>>,
 ) -> Response {
-    aisix_proxy::health::livez_response(&state.livez_state, params.contains_key("verbose"))
+    aisix_proxy::health::livez_response(params.contains_key("verbose"))
 }
 
 async fn readyz(
@@ -1087,7 +1088,7 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn livez_returns_503_when_shutting_down() {
+    async fn livez_stays_200_when_shutting_down() {
         let state = build_state();
         state.livez_state.mark_shutting_down();
         let app = build_router(state);
@@ -1096,10 +1097,12 @@ mod tests {
             .body(Body::empty())
             .unwrap();
         let resp = run(app, req).await;
-        assert_eq!(resp.status(), StatusCode::SERVICE_UNAVAILABLE);
-        let bytes = to_bytes(resp.into_body(), 1024 * 1024).await.unwrap();
-        let text = std::str::from_utf8(&bytes).unwrap();
-        assert!(text.contains("livez check failed"));
+        assert_eq!(
+            resp.status(),
+            StatusCode::OK,
+            "the admin listener answers the same liveness question as the \
+             proxy one, and a draining process is not one to restart",
+        );
     }
 
     #[tokio::test]

crates/aisix-admin/src/openapi.rs

@@ -67,22 +67,12 @@ const OPENAPI_JSON_BASE: &str = r##"{
                 }
               }
             }
-          },
-          "503": {
-            "description": "Process is shutting down (graceful drain)",
-            "content": {
-              "text/plain": {
-                "schema": {
-                  "type": "string"
-                }
-              }
-            }
           }
         },
         "tags": [
           "Health"
         ],
-        "description": "Process liveness: 200 while the process is alive, 503 once graceful shutdown has begun (an expected drain, not a crash). Use /readyz for traffic eligibility (readiness)."
+        "description": "Process liveness: should this instance be restarted? Answers 200 whenever it answers at all, including throughout a graceful drain \u2014 draining is deliberate work, and restarting an instance that is finishing the requests it accepted would kill exactly those. Use /readyz for traffic eligibility (readiness), which is what reports the drain."
       }
     },
     "/readyz": {
@@ -3472,8 +3462,12 @@ mod tests {
             "verbose"
         );
         // Graceful shutdown is documented as 503 (drain), not 500 (#591).
-        assert!(parsed["paths"]["/livez"]["get"]["responses"]["503"].is_object());
+        // Liveness has no failure response to document: a draining
+        // instance answers 200 like any other, and an instance that
+        // cannot answer does not reply at all.
+        assert!(parsed["paths"]["/livez"]["get"]["responses"]["503"].is_null());
         assert!(parsed["paths"]["/livez"]["get"]["responses"]["500"].is_null());
+        assert!(parsed["paths"]["/readyz"]["get"]["responses"]["503"].is_object());
         // /readyz is documented with 200 + 503 (readiness).
         assert!(parsed["paths"]["/readyz"]["get"]["responses"]["200"].is_object());
         assert!(parsed["paths"]["/readyz"]["get"]["responses"]["503"].is_object());

crates/aisix-proxy/src/health.rs

@@ -143,46 +143,40 @@ pub fn config_readiness_block(last_apply_age: Option<Duration>) -> Option<&'stat
     }
 }
 
-pub fn livez_response(livez: &LivezState, verbose: bool) -> Response {
-    let mut body = String::new();
-    let mut failed = false;
-
-    body.push_str("[+]ping ok\n");
-    match livez.shutdown_check() {
-        Ok(()) => body.push_str("[+]shutdown ok\n"),
-        Err(_) => {
-            failed = true;
-            body.push_str("[-]shutdown failed: reason withheld\n");
-        }
-    }
-
+/// `GET /livez` — process liveness: should this instance be RESTARTED?
+///
+/// Answering at all is the check. The listener is bound and the runtime
+/// is servicing requests, which is the whole of what a liveness probe
+/// decides, so this never fails.
+///
+/// A draining instance answers `200` like any other. Draining is
+/// deliberate work, not a fault: an instance that has been told to shut
+/// down is finishing the requests it already accepted, and restarting it
+/// kills exactly those. That is what a failing liveness probe asks a
+/// platform to do, which is why "stop sending traffic here" belongs on
+/// [`readyz_response`] instead — the two questions are what separates
+/// the endpoints, and answering both with the drain state collapses them
+/// into one.
+///
+/// The platform is not the only caller that matters: the gateway also
+/// runs as a single container under docker or systemd, where a
+/// supervisor watching this endpoint would restart a healthy draining
+/// process mid-flight.
+///
+/// [`LivezState`] is deliberately not a parameter. The drain state is the
+/// one thing this answer must not depend on, and not taking it is a
+/// stronger guarantee than a comment saying so.
+pub fn livez_response(verbose: bool) -> Response {
     let headers = [
         (CONTENT_TYPE, TEXT_PLAIN_UTF8.clone()),
         (X_CONTENT_TYPE_OPTIONS.clone(), NOSNIFF.clone()),
     ];
 
-    if failed {
-        // Graceful shutdown is an expected drain, not an internal error —
-        // 503 so Kubernetes stops routing without treating it as a crash
-        // loop (#591).
-        return (
-            StatusCode::SERVICE_UNAVAILABLE,
-            headers,
-            format!("{body}livez check failed"),
-        )
-            .into_response();
-    }
-
     if !verbose {
         return (StatusCode::OK, headers, "ok").into_response();
     }
 
-    (
-        StatusCode::OK,
-        headers,
-        format!("{body}livez check passed\n"),
-    )
-        .into_response()
+    (StatusCode::OK, headers, "[+]ping ok\nlivez check passed\n").into_response()
 }
 
 /// `GET /readyz` — readiness (traffic eligibility), distinct from `/livez`
@@ -1188,8 +1182,7 @@ mod tests {
 
     #[tokio::test]
     async fn livez_default_success_is_plain_ok() {
-        let state = LivezState::new();
-        let resp = livez_response(&state, false);
+        let resp = livez_response(false);
 
         assert_eq!(resp.status(), StatusCode::OK);
         let body = to_bytes(resp.into_body(), 1024).await.unwrap();
@@ -1198,28 +1191,34 @@ mod tests {
 
     #[tokio::test]
     async fn livez_verbose_success_lists_checks() {
-        let state = LivezState::new();
-        let resp = livez_response(&state, true);
+        let resp = livez_response(true);
 
         assert_eq!(resp.status(), StatusCode::OK);
         let body = to_bytes(resp.into_body(), 1024).await.unwrap();
         let text = std::str::from_utf8(&body).unwrap();
         assert!(text.contains("[+]ping ok"));
-        assert!(text.contains("[+]shutdown ok"));
         assert!(text.contains("livez check passed"));
     }
 
+    /// A draining instance is healthy and must not be restarted:
+    /// restarting it kills the in-flight requests the drain exists to
+    /// finish. Liveness therefore stays `200` throughout, and it is
+    /// `/readyz` that withdraws the instance from traffic — the test
+    /// below pins the other half.
     #[tokio::test]
-    async fn livez_failure_returns_503_with_reason_withheld() {
+    async fn livez_stays_ok_while_draining() {
         let state = LivezState::new();
         state.mark_shutting_down();
-        let resp = livez_response(&state, false);
 
-        assert_eq!(resp.status(), StatusCode::SERVICE_UNAVAILABLE);
-        let body = to_bytes(resp.into_body(), 1024).await.unwrap();
-        let text = std::str::from_utf8(&body).unwrap();
-        assert!(text.contains("[-]shutdown failed: reason withheld"));
-        assert!(text.contains("livez check failed"));
+        let resp = livez_response(false);
+        assert_eq!(resp.status(), StatusCode::OK);
+
+        let readyz = readyz_response(&state, None, false);
+        assert_eq!(
+            readyz.status(),
+            StatusCode::SERVICE_UNAVAILABLE,
+            "the drain has to be visible somewhere, and readiness is where",
+        );
     }
 
     /// The count has to survive an unpaired decrement: it is reported on

crates/aisix-proxy/src/lib.rs

@@ -1010,11 +1010,13 @@ fn warn_allowed(outcome: DrainOutcome) -> bool {
     true
 }
 
+/// Takes no [`ProxyState`]: liveness answers whether the process should
+/// be restarted, and nothing about the gateway's state changes that
+/// answer. See [`crate::health::livez_response`].
 async fn livez(
-    State(state): State<ProxyState>,
     axum::extract::Query(params): axum::extract::Query<std::collections::HashMap<String, String>>,
 ) -> Response {
-    crate::health::livez_response(&state.livez, params.contains_key("verbose"))
+    crate::health::livez_response(params.contains_key("verbose"))
 }
 
 async fn readyz(
@@ -1800,7 +1802,7 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn livez_returns_503_when_shutting_down() {
+    async fn livez_stays_200_when_shutting_down() {
         let hub = Arc::new(Hub::new());
         let snap = seed_snapshot("my-gpt4", &["my-gpt4"], "http://unused");
         let state = build_state(snap, hub);
@@ -1814,10 +1816,12 @@ mod tests {
             .unwrap();
 
         let resp = run(app, req).await;
-        assert_eq!(resp.status(), StatusCode::SERVICE_UNAVAILABLE);
-        let bytes = to_bytes(resp.into_body(), 1024).await.unwrap();
-        let text = std::str::from_utf8(&bytes).unwrap();
-        assert!(text.contains("livez check failed"));
+        assert_eq!(
+            resp.status(),
+            StatusCode::OK,
+            "a draining process is healthy; failing liveness asks the platform \
+             to restart it, which kills the requests the drain is finishing",
+        );
     }
 
     #[tokio::test]

tests/e2e/src/cases/health-minimal-e2e.test.ts

@@ -11,7 +11,13 @@ describe("livez e2e: public liveness route is /livez and /health is gone", () =>
     if (!etcdReachable) return;
     // Held-back: this test drives the admin listener's health endpoint,
     // so it keeps admin bound (the suite default is now admin-off).
-    app = await spawnApp({ admin: true });
+    //
+    // A drain window is needed too. The harness default is
+    // `min_drain_secs: 0`, which lets the process exit within
+    // milliseconds of SIGTERM — leaving no interval in which to observe
+    // what the health endpoints report WHILE draining, which is the
+    // whole of what the last test asserts.
+    app = await spawnApp({ admin: true, extra: { shutdown: { min_drain_secs: 5 } } });
   });
 
   afterAll(async () => {
@@ -90,33 +96,53 @@ describe("livez e2e: public liveness route is /livez and /health is gone", () =>
     expect(await adminReadyz.body.text()).toBe("ok");
   });
 
-  test("proxy /livez turns unhealthy after SIGTERM before exit", async (ctx) => {
+  // A drain withdraws traffic; it does not make the process a candidate
+  // for restarting. Those are the two different questions the two
+  // endpoints answer, and a drain has to move exactly one of them:
+  // `/readyz` reports it so the balancer stops routing here, `/livez`
+  // stays `200` because a failing liveness probe asks the platform to
+  // restart the instance — which would kill the very requests the drain
+  // is staying alive to finish.
+  //
+  // Kubernetes stops probing liveness once a pod enters graceful
+  // termination, so a rolling update does not act on the answer. That
+  // makes this a contract about the endpoint rather than about kubelet,
+  // and it is not academic: the gateway also runs as a single container
+  // under docker or systemd, where a supervisor watching `/livez` does
+  // act on it.
+  test("a drain moves /readyz to 503 and leaves /livez at 200", async (ctx) => {
     if (!etcdReachable || !app) {
       ctx.skip();
       return;
     }
 
     app.signal("SIGTERM");
 
+    // Gate on readiness having withdrawn rather than on a sleep: it
+    // proves the drain has actually begun AND that the process is still
+    // serving, which is the window the liveness assertion below is
+    // about. `beforeAll` gives that window 5 seconds, and the poll below
+    // gives up before it closes.
     const deadline = Date.now() + 3000;
-    let observedUnhealthy = false;
+    let draining = false;
     while (Date.now() < deadline) {
-      try {
-        const res = await harnessRequest(`${app.proxyUrl}/livez`, { method: "GET" });
-        if (res.statusCode !== 200) {
-          observedUnhealthy = true;
-          await res.body.dump();
-          break;
-        }
-        await res.body.dump();
-      } catch {
-        observedUnhealthy = true;
+      const res = await harnessRequest(`${app.proxyUrl}/readyz`, { method: "GET" });
+      const status = res.statusCode;
+      await res.body.dump();
+      if (status === 503) {
+        draining = true;
         break;
       }
       await new Promise((r) => setTimeout(r, 50));
     }
-
-    expect(observedUnhealthy).toBe(true);
-    app = undefined;
+    expect(draining, "/readyz never reported 503 after SIGTERM").toBe(true);
+
+    for (const url of [`${app.proxyUrl}/livez`, `${app.adminUrl}/livez`]) {
+      const res = await harnessRequest(url, { method: "GET" });
+      const status = res.statusCode;
+      const body = await res.body.text();
+      expect(status, `${url} must stay live while draining`).toBe(200);
+      expect(body).toBe("ok");
+    }
   });
 });