Skip to content

Make local Postgres startup transactional - #501

Open
sdairs wants to merge 1 commit into
issue-457-postgres-readiness-v2from
issue-464-postgres-start-rollback
Open

Make local Postgres startup transactional#501
sdairs wants to merge 1 commit into
issue-457-postgres-readiness-v2from
issue-464-postgres-start-rollback

Conversation

@sdairs

@sdairs sdairs commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Split fresh Postgres container creation from start so every error after creation has a container ID and crosses one rollback boundary.
  • On start, metadata, or readiness failure, force-remove the container and metadata, then remove PGDATA only when the instance directory was absent or empty before the attempt. Preserve pre-existing and resumed data, and retain data when container removal fails.
  • Preserve the primary startup error while appending actionable cleanup diagnostics.
  • Add deterministic fake-Docker and temporary-filesystem coverage for start failure, initialization timeout, metadata failure, clean retry, cleanup failure, pre-existing data, and resume failure.

Tests

  • cargo test -p clickhousectl (762 passed)
  • cargo test -p clickhousectl --test local_postgres_readiness_test (9 passed)
  • cargo test -p clickhousectl local::postgres::tests::fresh_ (2 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: the configured OrbStack daemon socket was unavailable

Stack

Position 3 of 3 (top), based on #498.

Closes #464

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Critical

if let Err(error) = docker::remove_host_dir_blocking(&rollback.instance_dir) {

Rollback can execute shell metacharacters from a user-controlled server name and delete other Postgres instances under the mounted servers directory. instance_dir reaches remove_host_dir_blocking, whose sh -c "rm -rf /work/{basename}" fallback does not quote the basename; use a non-shell deletion path or safely pass the directory as an argument.

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

Rollback can execute shell metacharacters from a user-controlled server name and delete other Postgres instances under the mounted `servers` directory. `instance_dir` reaches `remove_host_dir_blocking`, whose `sh -c "rm -rf /work/{basename}"` fallback does not quote the basename; use a non-shell deletion path or safely pass the directory as an argument.

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.

1 participant