fix(test-cluster): manage containers via testcontainers' Docker client so start/stop share one runtime by beryllw · Pull Request #624 · apache/fluss-rust

beryllw · 2026-06-15T14:28:42Z

Purpose

Linked issue: close #623

Brief change log

stop_cluster, FlussTestingCluster::stop, and the start pre-clean now force-remove via the shared client. A 404 (already gone) is treated as success; other errors warn instead of being silently swallowed.
all_containers_exist lists containers through the client with an anchored name filter (now async).
New run_blocking helper drives the async teardown from the synchronous stop() callers, working both inside a tokio runtime (async integration tests) and from the atexit cleanup handler.
Imports the Docker types via the testcontainers::bollard re-export, so the crate takes no direct bollard dependency (Cargo.toml/Cargo.lock unchanged on balance).

Tests

Verified against a live Podman cluster:

start            -> zookeeper / coordinator / tablet containers Up
start (again)    -> idempotent, no recreation, no name conflict
stop             -> all three containers removed

cargo build / cargo clippy clean for the crate (the 3 pre-existing main.rs readiness-poll lints are untouched and out of scope).

API and Format

Documentation

…t so start/stop share one runtime

leekeiabstraction

Thank you for the improvements. Left some small comments.

leekeiabstraction · 2026-06-15T19:19:33Z

+        for name in self.container_names() {
+            // Anchored exact-name match; `all(false)` = running only, so a stopped
+            // leftover counts as absent and gets recreated.
+            let mut filters = HashMap::new();
+            filters.insert("name".to_string(), vec![format!("^{name}$")]);
+            let options = ListContainersOptionsBuilder::default()
+                .all(false)
+                .filters(&filters)
+                .build();
+            match docker.list_containers(Some(options)).await {
+                Ok(list) if !list.is_empty() => continue,
+                _ => return false,
+            }
+        }


nit: Seems like we're making a list_containers per-container. Anyway to make this more efficient?

Sorry for the late reply, +1. I've already addressed it in a follow-up PR (#627).

leekeiabstraction · 2026-06-15T19:36:28Z

+    let mut filters = HashMap::new();
+    filters.insert(
+        "name".to_string(),
+        vec![
+            format!("zookeeper-{name}"),
+            format!("coordinator-server-{name}"),
+            format!("tablet-server-{name}-"),
+        ],
+    );
+    let options = ListContainersOptionsBuilder::default()
+        .all(true)
+        .filters(&filters)
+        .build();
+
+    let containers = match docker.list_containers(Some(options)).await {
+        Ok(containers) => containers,
+        Err(e) => {
+            eprintln!("warning: failed to list cluster containers: {e}");
+            return;
+        }
+    };


nit: This looks similar to parts of all_containers_exist, can these be refactored?

I left it inline for now since the error handling differs slightly.

charlesdong1991

Great improvement! 👍

charlesdong1991 · 2026-06-17T18:59:57Z

+    let mut filters = HashMap::new();
+    filters.insert(
+        "name".to_string(),
+        vec![


not a concern, nor it will cause any issues here

but just add a note: when using prefix here, if two different clusters share the prefix of name (e.g. one cluster has name of test and there other has test2 or something), i think when stopping, it can accidentally stop both of them although purpose might be only stopping containers of test cluster

Thanks for review, good catch — fixed in a follow-up PR (#627). The prefix is only used for narrowing, and removal is guarded by an exact local name check, so test and test2 won't interfere.

leekeiabstraction

Approving to get this improvement merged. The earlier comments from me were nits.

TY for your contribution!

fix(test-cluster): manage containers via testcontainers' Docker clien…

97b5cbd

…t so start/stop share one runtime

leekeiabstraction reviewed Jun 15, 2026

View reviewed changes

charlesdong1991 approved these changes Jun 17, 2026

View reviewed changes

leekeiabstraction approved these changes Jun 17, 2026

View reviewed changes

leekeiabstraction merged commit b98ab9c into apache:main Jun 17, 2026
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(test-cluster): manage containers via testcontainers' Docker client so start/stop share one runtime#624

fix(test-cluster): manage containers via testcontainers' Docker client so start/stop share one runtime#624
leekeiabstraction merged 1 commit into
apache:mainfrom
beryllw:fix-fluss-cluster-stop

beryllw commented Jun 15, 2026 •

edited

Loading

Uh oh!

leekeiabstraction left a comment

Uh oh!

leekeiabstraction Jun 15, 2026

Uh oh!

beryllw Jun 18, 2026

Uh oh!

leekeiabstraction Jun 15, 2026

Uh oh!

beryllw Jun 18, 2026

Uh oh!

charlesdong1991 left a comment

Uh oh!

charlesdong1991 Jun 17, 2026

Uh oh!

beryllw Jun 18, 2026

Uh oh!

leekeiabstraction left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

beryllw commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Brief change log

Tests

API and Format

Documentation

Uh oh!

leekeiabstraction left a comment

Choose a reason for hiding this comment

Uh oh!

leekeiabstraction Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

beryllw Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

leekeiabstraction Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

beryllw Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

charlesdong1991 left a comment

Choose a reason for hiding this comment

Uh oh!

charlesdong1991 Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

beryllw Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

leekeiabstraction left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

beryllw commented Jun 15, 2026 •

edited

Loading