Skip to content

Wait for PostgreSQL readiness before local start success - #498

Open
sdairs wants to merge 8 commits into
issue-460-postgres-preflight-validationfrom
issue-457-postgres-readiness-v2
Open

Wait for PostgreSQL readiness before local start success#498
sdairs wants to merge 8 commits into
issue-460-postgres-preflight-validationfrom
issue-457-postgres-readiness-v2

Conversation

@sdairs

@sdairs sdairs commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Wait for pg_isready inside the managed container before fresh and resumed local Postgres starts print credentials or return success.
  • Bound readiness at 30 seconds, keep the probe detached from CLI stdio, and include the last 50 container log lines for immediate exits and timeouts.
  • Return readiness failures through the normal command error path so telemetry records failure; clean up failed fresh starts and stop failed resumed starts for retry.
  • Add deterministic fake-Docker unit and subprocess coverage, and make the Docker integration script rely on the start readiness guarantee.

Tests

  • cargo test -p clickhousectl (754 passed)
  • cargo test -p clickhousectl postgres_readiness (focused readiness coverage passed)
  • cargo test -p clickhousectl --test local_postgres_readiness_test (3 passed)
  • cargo clippy -p clickhousectl --all-targets -- -D warnings (passed)
  • cargo build -p clickhousectl (passed)
  • cargo fmt --all -- --check (passed)
  • CI local postgres edge cases Docker integration (passed)
  • Local Docker integration not run: configured OrbStack socket was unavailable

Stack

Position 2 of 2 (top), based on #486.

Closes #457

@sdairs sdairs changed the title issue 457 postgres readiness v2 Wait for PostgreSQL readiness before local start success Aug 24, 2026
Comment thread crates/clickhousectl/src/local/postgres.rs Outdated
Comment thread crates/clickhousectl/src/local/postgres.rs
@sdairs
sdairs force-pushed the issue-457-postgres-readiness-v2 branch from 606d87f to 047c2c1 Compare August 25, 2026 12:11
)
.await
{
let _ = docker::stop_container(&docker, &container_id).await;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High local/postgres.rs:257

A readiness failure removes the container and metadata but leaves server::pg_data_dir intact, so a timeout after PostgreSQL initializes the directory causes the next fresh start to reuse that database while reporting newly generated credentials that do not apply. Clear the data directory during this rollback (or preserve the existing metadata/container for retry) before allowing a fresh initialization.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @crates/clickhousectl/src/local/postgres.rs around line 257:

A readiness failure removes the container and metadata but leaves `server::pg_data_dir` intact, so a timeout after PostgreSQL initializes the directory causes the next fresh start to reuse that database while reporting newly generated credentials that do not apply. Clear the data directory during this rollback (or preserve the existing metadata/container for retry) before allowing a fresh initialization.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 047c2c1. Configure here.

"Postgres container '{}' did not start.\n--- container logs ---\n{}",
user_name, logs
)));
return Err(error);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incomplete fresh-start failure cleanup

High Severity

When a fresh start times out waiting for readiness, the container and metadata are removed but the bind-mounted data directory is left behind. A retry takes the fresh-create path, may mint a new POSTGRES_PASSWORD, and remounts that existing PGDATA. The official image then ignores the new password, so printed credentials can fail, and remove cannot clean up because metadata is already gone. That conflicts with the orphaned-metadata path, which refuses silent re-init against leftover data.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 047c2c1. Configure here.

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.

Wait for PostgreSQL readiness before reporting local start success

1 participant