diff --git a/.github/actions/install-native-demo-deps/action.yml b/.github/actions/install-native-demo-deps/action.yml index e0eb40060bf..ff523310865 100644 --- a/.github/actions/install-native-demo-deps/action.yml +++ b/.github/actions/install-native-demo-deps/action.yml @@ -5,16 +5,11 @@ runs: using: composite steps: - uses: ./.github/actions/install-foundry + - uses: ./.github/actions/install-postgres - shell: bash run: | set -euo pipefail sudo apt-get update - sudo apt-get install -y postgresql - # stop the auto-started cluster so the demo can bind 5432/5433 - sudo systemctl stop postgresql || true - # postgres server binaries (initdb, postgres) are not on PATH on Debian - echo "$(ls -d /usr/lib/postgresql/*/bin | sort -V | tail -1)" >> "$GITHUB_PATH" - # keydb: extract the prebuilt server (no apt repo for 24.04) + its runtime libs sudo apt-get install -y libsnappy1v5 libzstd1 libuuid1 libcurl4t64 libssl3t64 curl -fsSL "https://download.keydb.dev/pkg/open_source/deb/ubuntu22.04_jammy/amd64/keydb-latest/keydb-tools_6.3.4-1~jammy1_amd64.deb" -o /tmp/keydb-tools.deb diff --git a/.github/actions/install-postgres/action.yml b/.github/actions/install-postgres/action.yml new file mode 100644 index 00000000000..b77f6ee41e7 --- /dev/null +++ b/.github/actions/install-postgres/action.yml @@ -0,0 +1,15 @@ +name: Install postgres +description: Install the postgres server binaries (initdb, postgres, pg_isready) and put them on PATH. + +runs: + using: composite + steps: + - shell: bash + run: | + set -euo pipefail + sudo apt-get update + sudo apt-get install -y postgresql + # stop the auto-started cluster; tests run their own instances on ephemeral ports + sudo systemctl stop postgresql || true + # server binaries (initdb, postgres) are not on PATH on Debian + echo "$(ls -d /usr/lib/postgresql/*/bin | sort -V | tail -1)" >> "$GITHUB_PATH" diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 87ba229480b..3f4b79d1c4b 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -37,6 +37,9 @@ jobs: sudo apt-get update sudo apt-get install -y protobuf-compiler + - uses: ./.github/actions/install-postgres + - uses: ./.github/actions/install-foundry + - uses: dtolnay/rust-toolchain@nightly - name: Enable Rust Caching diff --git a/.github/workflows/slowtest.yaml b/.github/workflows/slowtest.yaml index 6fd02706b5d..0a045d315c1 100644 --- a/.github/workflows/slowtest.yaml +++ b/.github/workflows/slowtest.yaml @@ -53,6 +53,7 @@ jobs: sudo apt-get install -y protobuf-compiler - uses: ./.github/actions/install-foundry + - uses: ./.github/actions/install-postgres - uses: taiki-e/install-action@just - uses: taiki-e/install-action@nextest - uses: rui314/setup-mold@v1 diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6c5c95434b4..e76bbe504ec 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -259,6 +259,7 @@ jobs: - uses: actions/checkout@v6 - uses: ./.github/actions/free-disk-space - uses: ./.github/actions/install-foundry + - uses: ./.github/actions/install-postgres - uses: taiki-e/install-action@nextest - name: Download archive diff --git a/doc/ubuntu.md b/doc/ubuntu.md index 434f7442529..0e909af3102 100644 --- a/doc/ubuntu.md +++ b/doc/ubuntu.md @@ -4,13 +4,16 @@ ## Install system dependencies +The `postgresql` package provides the server binaries (`initdb`, `postgres`, `pg_ctl`, `pg_isready`) the SQL tests use; +docker is not required. + sudo apt-get update - sudo apt-get install -y curl cmake pkg-config libssl-dev protobuf-compiler git postgresql-client lsb-release gpg nodejs npm + sudo apt-get install -y curl cmake pkg-config libssl-dev protobuf-compiler git postgresql lsb-release gpg nodejs npm sudo npm install -g yarn -## Install docker +The postgres server binaries are not on `PATH` on Debian/Ubuntu; add them. -Refer to https://docs.docker.com/engine/install/ubuntu + export "PATH=$(ls -d /usr/lib/postgresql/*/bin | sort -V | tail -1):$PATH" ## Install just @@ -47,13 +50,15 @@ Just is outdated in the official ubuntu repos. forge build -## Run the rust tests +## Build and smoke-test the rust tests + +Compiling the test binaries verifies the toolchain and system dependencies. The SQL tests run against a native postgres +server (installed above), so a single migration test smoke-tests that path without docker. -To run the SQL tests docker needs to be installed and running. + just nextest --no-run + just nextest --no-fail-fast test_migrations - export "PATH=$PWD/target/release:$PATH" - cargo build --release --bin diff-test - just test --no-fail-fast +To run the full suite, use `just test` (slow) or `just test-all`. ## Run the foundry tests diff --git a/hotshot-query-service/src/data_source/storage/sql.rs b/hotshot-query-service/src/data_source/storage/sql.rs index 8fd97309943..410b3a64e8b 100644 --- a/hotshot-query-service/src/data_source/storage/sql.rs +++ b/hotshot-query-service/src/data_source/storage/sql.rs @@ -1491,14 +1491,13 @@ pub mod testing { #![allow(unused_imports)] use std::{ env, - process::{Command, Stdio}, - str::{self, FromStr}, + process::{Child, Command, Stdio}, time::Duration, }; use refinery::Migration; use test_utils::reserve_tcp_port; - use tokio::{net::TcpStream, time::timeout}; + use tokio::time::timeout; use super::Config; use crate::testing::sleep; @@ -1509,7 +1508,9 @@ pub mod testing { #[cfg(not(feature = "embedded-db"))] port: u16, #[cfg(not(feature = "embedded-db"))] - container_id: String, + data_dir: std::path::PathBuf, + #[cfg(not(feature = "embedded-db"))] + postgres: Option, #[cfg(feature = "embedded-db")] db_path: std::path::PathBuf, #[allow(dead_code)] @@ -1549,44 +1550,35 @@ pub mod testing { #[cfg(not(feature = "embedded-db"))] async fn init_postgres(persistent: bool) -> Self { - let docker_hostname = env::var("DOCKER_HOSTNAME"); - // This picks an unused port on the current system. If docker is - // configured to run on a different host then this may not find a - // "free" port on that system. - // We *might* be able to get away with this as any remote docker - // host should hopefully be pretty open with it's port space. let port = reserve_tcp_port().unwrap(); - let host = docker_hostname.unwrap_or("localhost".to_string()); - - let mut cmd = Command::new("docker"); - cmd.arg("run") - .arg("-d") - .args(["-p", &format!("{port}:5432")]) - .args(["-e", "POSTGRES_PASSWORD=password"]); + let host = "127.0.0.1".to_string(); - if !persistent { - cmd.arg("--rm"); - } + // initdb requires an empty target; clear any dir left by a crashed run + // that reused this (recycled) ephemeral port. + let data_dir = env::temp_dir().join(format!("espresso-tmpdb-{port}")); + let _ = std::fs::remove_dir_all(&data_dir); - let output = cmd.arg("postgres").output().unwrap(); - let stdout = str::from_utf8(&output.stdout).unwrap(); - let stderr = str::from_utf8(&output.stderr).unwrap(); - if !output.status.success() { - panic!("failed to start postgres docker: {stderr}"); - } + let output = Command::new("initdb") + .arg("-D") + .arg(&data_dir) + .args(["-U", "postgres", "--auth=trust"]) + .output() + .expect("initdb failed to run; is postgres installed and on PATH?"); + assert!( + output.status.success(), + "initdb failed for {data_dir:?}: {}", + String::from_utf8_lossy(&output.stderr) + ); - // Create the TmpDb object immediately after starting the Docker container, so if - // anything panics after this `drop` will be called and we will clean up. - let container_id = stdout.trim().to_owned(); - tracing::info!("launched postgres docker {container_id}"); - let db = Self { + let mut db = Self { host, port, - container_id: container_id.clone(), + data_dir, + postgres: None, persistent, }; - db.wait_for_ready().await; + db.start_postgres().await; db } @@ -1629,32 +1621,44 @@ pub mod testing { #[cfg(not(feature = "embedded-db"))] pub fn stop_postgres(&mut self) { - tracing::info!(container = self.container_id, "stopping postgres"); - let output = Command::new("docker") - .args(["stop", self.container_id.as_str()]) - .output() - .unwrap(); - assert!( - output.status.success(), - "error killing postgres docker {}: {}", - self.container_id, - str::from_utf8(&output.stderr).unwrap() - ); + let Some(mut postgres) = self.postgres.take() else { + return; + }; + tracing::info!(port = self.port, "stopping postgres"); + // Fast shutdown (SIGINT to the postmaster) releases the datadir before + // Drop removes it; fall back to SIGKILL if pg_ctl is unavailable. + let stopped = Command::new("pg_ctl") + .arg("-D") + .arg(&self.data_dir) + .args(["stop", "-m", "fast", "-w"]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + .map(|status| status.success()) + .unwrap_or(false); + if !stopped { + let _ = postgres.kill(); + } + let _ = postgres.wait(); } #[cfg(not(feature = "embedded-db"))] pub async fn start_postgres(&mut self) { - tracing::info!(container = self.container_id, "resuming postgres"); - let output = Command::new("docker") - .args(["start", self.container_id.as_str()]) - .output() - .unwrap(); - assert!( - output.status.success(), - "error starting postgres docker {}: {}", - self.container_id, - str::from_utf8(&output.stderr).unwrap() - ); + self.stop_postgres(); + tracing::info!(port = self.port, "starting postgres"); + let postgres = Command::new("postgres") + .arg("-D") + .arg(&self.data_dir) + .args(["-p", &self.port.to_string()]) + .args(["-h", &self.host]) + // Keep the unix socket inside the data dir instead of a shared /tmp. + .arg("-k") + .arg(&self.data_dir) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .expect("failed to start postgres; is it installed and on PATH?"); + self.postgres = Some(postgres); self.wait_for_ready().await; } @@ -1669,50 +1673,19 @@ pub mod testing { ); if let Err(err) = timeout(timeout_duration, async { - while Command::new("docker") - .args([ - "exec", - &self.container_id, - "pg_isready", - "-h", - "localhost", - "-U", - "postgres", - ]) - .env("PGPASSWORD", "password") - // Null input so the command terminates as soon as it manages to connect. - .stdin(Stdio::null()) - // Discard command output. + while !Command::new("pg_isready") + .args(["-h", &self.host]) + .args(["-p", &self.port.to_string()]) + .args(["-U", "postgres"]) .stdout(Stdio::null()) .stderr(Stdio::null()) .status() - // We should ensure the exit status. A simple `unwrap` - // would panic on unrelated errors (such as network - // connection failures) - .and_then(|status| { - status - .success() - .then_some(true) - // Any ol' Error will do - .ok_or(std::io::Error::from_raw_os_error(666)) - }) - .is_err() + .map(|status| status.success()) + .unwrap_or(false) { tracing::warn!("database is not ready"); sleep(Duration::from_secs(1)).await; } - - // The above command ensures the database is ready inside the Docker container. - // However, on some systems, there is a slight delay before the port is exposed via - // host networking. We don't need to check again that the database is ready on the - // host (and maybe can't, because the host might not have pg_isready installed), but - // we can ensure the port is open by just establishing a TCP connection. - while let Err(err) = - TcpStream::connect(format!("{}:{}", self.host, self.port)).await - { - tracing::warn!("database is ready, but port is not available to host: {err:#}"); - sleep(Duration::from_millis(100)).await; - } }) .await { @@ -1729,6 +1702,9 @@ pub mod testing { impl Drop for TmpDb { fn drop(&mut self) { self.stop_postgres(); + if !self.persistent { + let _ = std::fs::remove_dir_all(&self.data_dir); + } } }