fix(aisix-etcd): deterministic wait for supervisor cache-write tests

.github/workflows/ci.yml

@@ -51,7 +51,14 @@ jobs:
     env:
       # Picked up by crates/aisix-cache/tests/redis_integration.rs.
       # The tests no-op when this is unset (local dev), so absence is safe.
-      AISIX_REDIS_URL: redis://127.0.0.1:6379
+      #
+      # MUST NOT start with the `AISIX_` prefix — Config::load_from_path
+      # merges every AISIX_* env var into the root Config (via
+      # config-rs Environment::with_prefix("AISIX")), and Config has
+      # `#[serde(deny_unknown_fields)]`. An AISIX_REDIS_URL leaks into
+      # every Config::load_from_path test as `redis_url` and breaks
+      # the entire `aisix-core::config::tests` module.
+      CACHE_TEST_REDIS_URL: redis://127.0.0.1:6379
     steps:
       - uses: actions/checkout@v4
       - uses: dtolnay/rust-toolchain@stable
@@ -66,6 +73,12 @@ jobs:
       - name: unit tests with coverage
         run: cargo llvm-cov --workspace --all-features --lcov --output-path lcov-unit.info
       - uses: actions/upload-artifact@v4
+        # Soft until the org-level Actions storage quota refreshes (every
+        # 6-12h per GH). Tests passing is the gating signal here; the
+        # coverage artifact only feeds the advisory coverage-gate job.
+        # Remove this `continue-on-error` when storage usage has been
+        # raised or has fallen below quota.
+        continue-on-error: true
         with:
           name: coverage-unit
           path: lcov-unit.info
@@ -92,6 +105,12 @@ jobs:
         working-directory: ui
         run: pnpm build
       - uses: actions/upload-artifact@v4
+        # Soft until the org-level Actions storage quota refreshes. The
+        # downstream build-bin / e2e jobs depend on this artifact; if
+        # the upload fails their download-artifact will fail and they
+        # will be skipped (e2e is already continue-on-error at the job
+        # level; coverage-gate is advisory). Remove when quota is OK.
+        continue-on-error: true
         with:
           name: ui-dist
           path: crates/aisix-admin/ui-dist
@@ -101,6 +120,11 @@ jobs:
     name: build aisix (instrumented)
     needs: build-ui
     runs-on: ubuntu-latest
+    # Soft while the artifact storage quota issue persists. This job
+    # only feeds the advisory e2e job; if its download-artifact for
+    # ui-dist fails because build-ui couldn't upload, don't fail the
+    # whole PR. Revert once quota is OK.
+    continue-on-error: true
     env:
       RUSTFLAGS: "-C instrument-coverage"
       LLVM_PROFILE_FILE: "coverage/aisix-%p-%m.profraw"

crates/aisix-cache/src/redis.rs

@@ -148,7 +148,7 @@ mod tests {
     }
 
     // The full integration path (real Redis round-trip) lives in
-    // `tests/redis_integration.rs` and is opt-in via the `AISIX_REDIS_URL`
+    // `tests/redis_integration.rs` and is opt-in via the `CACHE_TEST_REDIS_URL`
     // env var so the unit-test job stays hermetic. CI runs Redis as a
     // service so the integration test exercises the happy path.
 }

crates/aisix-cache/tests/redis_integration.rs

@@ -1,6 +1,6 @@
 //! End-to-end Redis tests against a live Redis instance.
 //!
-//! Runs only when `AISIX_REDIS_URL` is set (e.g. on CI which spins
+//! Runs only when `CACHE_TEST_REDIS_URL` is set (e.g. on CI which spins
 //! `redis:7-alpine` as a service). The unit test module in
 //! `src/redis.rs` handles hermetic checks; this file proves the
 //! request → upstream → cache round-trip actually round-trips.
@@ -13,7 +13,7 @@ use aisix_cache::{Cache, RedisCache};
 use aisix_gateway::{ChatMessage, ChatResponse, FinishReason, UsageStats};
 
 fn redis_url() -> Option<String> {
-    std::env::var("AISIX_REDIS_URL").ok()
+    std::env::var("CACHE_TEST_REDIS_URL").ok()
 }
 
 fn sample(content: &str) -> ChatResponse {
@@ -29,7 +29,7 @@ fn sample(content: &str) -> ChatResponse {
 #[tokio::test]
 async fn put_then_get_round_trips_against_real_redis() {
     let Some(url) = redis_url() else {
-        eprintln!("skipping: AISIX_REDIS_URL not set");
+        eprintln!("skipping: CACHE_TEST_REDIS_URL not set");
         return;
     };
 
@@ -48,7 +48,7 @@ async fn put_then_get_round_trips_against_real_redis() {
 #[tokio::test]
 async fn ttl_eviction_drops_entry_after_window() {
     let Some(url) = redis_url() else {
-        eprintln!("skipping: AISIX_REDIS_URL not set");
+        eprintln!("skipping: CACHE_TEST_REDIS_URL not set");
         return;
     };
 
@@ -70,7 +70,7 @@ async fn ttl_eviction_drops_entry_after_window() {
 #[tokio::test]
 async fn missing_key_returns_none() {
     let Some(url) = redis_url() else {
-        eprintln!("skipping: AISIX_REDIS_URL not set");
+        eprintln!("skipping: CACHE_TEST_REDIS_URL not set");
         return;
     };
 

crates/aisix-etcd/src/supervisor.rs

@@ -22,6 +22,7 @@ use std::collections::HashMap;
 use std::sync::Arc;
 use std::sync::Mutex;
 use std::time::Duration;
+use tokio::task::JoinHandle;
 
 use crate::backoff::ExpBackoff;
 use crate::key;
@@ -42,6 +43,16 @@ pub struct Supervisor<P: ConfigProvider> {
     state: Mutex<HashMap<String, RawEntry>>,
     revision: Mutex<i64>,
     cache: SnapshotCache,
+
+    // JoinHandles for in-flight `flush_cache` writes. Tests use
+    // [`Self::await_pending_cache_writes`] to deterministically wait
+    // for these without relying on a wall-clock sleep, which proved
+    // flaky on slow CI runners. Production code does not read this
+    // field; if a handle is dropped (e.g. during shutdown), the
+    // underlying write either completed or was cancelled — either
+    // way the on-disk cache is best-effort and the next live cycle
+    // re-publishes from etcd.
+    pending_writes: Mutex<Vec<JoinHandle<()>>>,
 }
 
 impl<P: ConfigProvider> Supervisor<P> {
@@ -63,6 +74,25 @@ impl<P: ConfigProvider> Supervisor<P> {
             state: Mutex::new(HashMap::new()),
             revision: Mutex::new(0),
             cache,
+            pending_writes: Mutex::new(Vec::new()),
+        }
+    }
+
+    /// Drain the JoinHandles for any in-flight cache writes spawned
+    /// by [`Self::flush_cache`] and await them. Test-only synchroniser:
+    /// production code never needs to block on disk persistence.
+    #[cfg(test)]
+    pub async fn await_pending_cache_writes(&self) {
+        let handles: Vec<JoinHandle<()>> = {
+            let mut pending = self.pending_writes.lock().unwrap();
+            std::mem::take(&mut *pending)
+        };
+        for handle in handles {
+            // Failures here are not test failures — a write that
+            // panicked is its own bug surfaced separately. We only
+            // need the await to deterministically order against the
+            // disk read that follows.
+            let _ = handle.await;
         }
     }
 
@@ -230,8 +260,14 @@ impl<P: ConfigProvider> Supervisor<P> {
         let cache = self.cache.clone();
         // Spawn the actual write so the apply path stays sync. If we
         // aren't inside a runtime (cache::disabled() tests), just skip.
-        if let Ok(handle) = tokio::runtime::Handle::try_current() {
-            handle.spawn(async move { cache.store(&entries, revision).await });
+        // Track the JoinHandle so tests can deterministically await
+        // the write via [`Self::await_pending_cache_writes`] instead
+        // of leaning on `tokio::time::sleep`, which under CI load
+        // raced the spawn (~50ms wasn't enough on heavily loaded
+        // GitHub Actions runners).
+        if let Ok(rt_handle) = tokio::runtime::Handle::try_current() {
+            let join = rt_handle.spawn(async move { cache.store(&entries, revision).await });
+            self.pending_writes.lock().unwrap().push(join);
         }
     }
 
@@ -524,10 +560,10 @@ mod tests {
             ));
             let sup = Supervisor::with_cache(provider, "/aisix", SnapshotCache::new(&cache_path));
             sup.load_once().await.unwrap();
-            // Yield so the spawned cache write has a chance to complete
-            // before we drop the supervisor.
-            tokio::task::yield_now().await;
-            tokio::time::sleep(std::time::Duration::from_millis(50)).await;
+            // Deterministically wait for the spawned cache write to
+            // complete before we drop the supervisor. Replaces an
+            // earlier 50ms sleep that flaked on slow CI runners.
+            sup.await_pending_cache_writes().await;
         }
 
         // Second lifecycle: provider returns nothing, but restore_from_cache
@@ -557,15 +593,15 @@ mod tests {
 
         sup.apply_put(&entry("/aisix/models/m-1", VALID_MODEL, 5));
         sup.apply_put(&entry("/aisix/models/m-2", VALID_MODEL, 6));
-        // Yield so the spawned cache writes have a chance to complete.
-        tokio::time::sleep(std::time::Duration::from_millis(50)).await;
+        // Wait for both spawned cache writes to flush before reading.
+        sup.await_pending_cache_writes().await;
 
         let cache = SnapshotCache::new(&cache_path);
         let (entries, _) = cache.load().expect("cache file present");
         assert_eq!(entries.len(), 2);
 
         sup.apply_delete("/aisix/models/m-1");
-        tokio::time::sleep(std::time::Duration::from_millis(50)).await;
+        sup.await_pending_cache_writes().await;
 
         let (entries, _) = cache.load().expect("cache file present");
         assert_eq!(entries.len(), 1);

docs/testing.md

@@ -71,14 +71,14 @@ Used when a test needs a real external service.
 Current integration suites:
 
 - `crates/aisix-cache/tests/redis_integration.rs` — runs against a
-  real Redis. Requires `AISIX_REDIS_URL`; tests no-op silently if the
+  real Redis. Requires `CACHE_TEST_REDIS_URL`; tests no-op silently if the
   env var is unset, so local `cargo test` stays hermetic.
 
 CI exposes the env vars these tests need:
 
 | Test | Service | Env var |
 |---|---|---|
-| `redis_integration` | `redis:7-alpine` | `AISIX_REDIS_URL=redis://127.0.0.1:6379` |
+| `redis_integration` | `redis:7-alpine` | `CACHE_TEST_REDIS_URL=redis://127.0.0.1:6379` |
 
 Future suites that need etcd, an OTLP collector, or a Langfuse mock
 will follow the same pattern: a service container in CI, a no-op
@@ -226,7 +226,7 @@ Job descriptions:
 | Job | Purpose |
 |---|---|
 | `lint` | `cargo fmt --check`, `cargo clippy -D warnings`, `pnpm typecheck` |
-| `rust-unit` | `cargo llvm-cov --workspace --all-features` → uploads `lcov-unit.info`. Spins a `redis:7-alpine` service so `AISIX_REDIS_URL` integration tests can run |
+| `rust-unit` | `cargo llvm-cov --workspace --all-features` → uploads `lcov-unit.info`. Spins a `redis:7-alpine` service so `CACHE_TEST_REDIS_URL` integration tests can run |
 | `build-ui` | `pnpm build` in `ui/` → uploads `ui-dist` artifact for the next job |
 | `build-bin` | `cargo build` of `aisix-server` with `RUSTFLAGS=-C instrument-coverage` → uploads the binary artifact for the e2e job |
 | `e2e` | Spins etcd + redis services, downloads the binary artifact, runs Vitest. Currently `continue-on-error: true` while the harness stabilises across CI runners |