diff --git a/control_plane/src/background_process.rs b/control_plane/src/background_process.rs index 94666f2870..3f4ddbdb2b 100644 --- a/control_plane/src/background_process.rs +++ b/control_plane/src/background_process.rs @@ -69,6 +69,9 @@ where // Not generic AsRef, otherwise empty `envs` prevents type inference EI: IntoIterator, { + if !datadir.metadata().context("stat datadir")?.is_dir() { + anyhow::bail!("`datadir` must be a directory when calling this function: {datadir:?}"); + } let log_path = datadir.join(format!("{process_name}.log")); let process_log_file = fs::OpenOptions::new() .create(true) @@ -85,7 +88,13 @@ where let background_command = command .stdout(process_log_file) .stderr(same_file_for_stderr) - .args(args); + .args(args) + // spawn all child processes in their datadir, useful for all kinds of things, + // not least cleaning up child processes e.g. after an unclean exit from the test suite: + // ``` + // lsof -d cwd -a +D Users/cs/src/neon/test_output + // ``` + .current_dir(datadir); let filled_cmd = fill_env_vars_prefixed_neon(fill_remote_storage_secrets_vars( fill_rust_env_vars(background_command), diff --git a/control_plane/src/bin/neon_local.rs b/control_plane/src/bin/neon_local.rs index 18e395e2b5..8fe959792b 100644 --- a/control_plane/src/bin/neon_local.rs +++ b/control_plane/src/bin/neon_local.rs @@ -87,7 +87,8 @@ fn main() -> Result<()> { handle_init(sub_args).map(Some) } else { // all other commands need an existing config - let mut env = LocalEnv::load_config().context("Error loading config")?; + let mut env = + LocalEnv::load_config(&local_env::base_path()).context("Error loading config")?; let original_env = env.clone(); let rt = tokio::runtime::Builder::new_current_thread() @@ -364,7 +365,8 @@ fn handle_init(init_match: &ArgMatches) -> anyhow::Result { LocalEnv::init(init_conf, force) .context("materialize initial neon_local environment on disk")?; - Ok(LocalEnv::load_config().expect("freshly written config should be loadable")) + Ok(LocalEnv::load_config(&local_env::base_path()) + .expect("freshly written config should be loadable")) } /// The default pageserver is the one where CLI tenant/timeline operations are sent by default. diff --git a/control_plane/src/local_env.rs b/control_plane/src/local_env.rs index 0edcf1be4e..6634274d2a 100644 --- a/control_plane/src/local_env.rs +++ b/control_plane/src/local_env.rs @@ -42,8 +42,8 @@ pub struct LocalEnv { // compute endpoints). // // This is not stored in the config file. Rather, this is the path where the - // config file itself is. It is read from the NEON_REPO_DIR env variable or - // '.neon' if not given. + // config file itself is. It is read from the NEON_REPO_DIR env variable which + // must be an absolute path. If the env var is not set, $PWD/.neon is used. pub base_data_dir: PathBuf, // Path to postgres distribution. It's expected that "bin", "include", @@ -431,9 +431,7 @@ impl LocalEnv { } /// Construct `Self` from on-disk state. - pub fn load_config() -> anyhow::Result { - let repopath = base_path(); - + pub fn load_config(repopath: &Path) -> anyhow::Result { if !repopath.exists() { bail!( "Neon config is not found in {}. You need to run 'neon_local init' first", @@ -461,7 +459,7 @@ impl LocalEnv { branch_name_mappings, } = on_disk_config; LocalEnv { - base_data_dir: repopath.clone(), + base_data_dir: repopath.to_owned(), pg_distrib_dir, neon_distrib_dir, default_tenant_id, @@ -482,7 +480,7 @@ impl LocalEnv { "we ensure this during deserialization" ); env.pageservers = { - let iter = std::fs::read_dir(&repopath).context("open dir")?; + let iter = std::fs::read_dir(repopath).context("open dir")?; let mut pageservers = Vec::new(); for res in iter { let dentry = res?; @@ -719,10 +717,25 @@ impl LocalEnv { } pub fn base_path() -> PathBuf { - match std::env::var_os("NEON_REPO_DIR") { - Some(val) => PathBuf::from(val), - None => PathBuf::from(".neon"), - } + let path = match std::env::var_os("NEON_REPO_DIR") { + Some(val) => { + let path = PathBuf::from(val); + if !path.is_absolute() { + // repeat the env var in the error because our default is always absolute + panic!("NEON_REPO_DIR must be an absolute path, got {path:?}"); + } + path + } + None => { + let pwd = std::env::current_dir() + // technically this can fail but it's quite unlikeley + .expect("determine current directory"); + let pwd_abs = pwd.canonicalize().expect("canonicalize current directory"); + pwd_abs.join(".neon") + } + }; + assert!(path.is_absolute()); + path } /// Generate a public/private key pair for JWT authentication diff --git a/control_plane/src/storage_controller.rs b/control_plane/src/storage_controller.rs index 72948e203f..4f9f0ba794 100644 --- a/control_plane/src/storage_controller.rs +++ b/control_plane/src/storage_controller.rs @@ -314,15 +314,17 @@ impl StorageController { args.push(format!("--split-threshold={split_threshold}")) } + args.push(format!( + "--neon-local-repo-dir={}", + self.env.base_data_dir.display() + )); + background_process::start_process( COMMAND, &self.env.base_data_dir, &self.env.storage_controller_bin(), args, - [( - "NEON_REPO_DIR".to_string(), - self.env.base_data_dir.to_string_lossy().to_string(), - )], + [], background_process::InitialPidFile::Create(self.pid_file()), || async { match self.ready().await { diff --git a/storage_controller/src/compute_hook.rs b/storage_controller/src/compute_hook.rs index 9d326ef82d..a1d051f150 100644 --- a/storage_controller/src/compute_hook.rs +++ b/storage_controller/src/compute_hook.rs @@ -283,7 +283,13 @@ impl ComputeHook { // all calls to this function let _locked = self.neon_local_lock.lock().await; - let env = match LocalEnv::load_config() { + let Some(repo_dir) = self.config.neon_local_repo_dir.as_deref() else { + tracing::warn!( + "neon_local_repo_dir not set, likely a bug in neon_local; skipping compute update" + ); + return Ok(()); + }; + let env = match LocalEnv::load_config(repo_dir) { Ok(e) => e, Err(e) => { tracing::warn!("Couldn't load neon_local config, skipping compute update ({e})"); diff --git a/storage_controller/src/main.rs b/storage_controller/src/main.rs index ce8f8d0cdd..f1eb0b30fc 100644 --- a/storage_controller/src/main.rs +++ b/storage_controller/src/main.rs @@ -4,6 +4,7 @@ use clap::Parser; use diesel::Connection; use metrics::launch_timestamp::LaunchTimestamp; use metrics::BuildInfo; +use std::path::PathBuf; use std::sync::Arc; use storage_controller::http::make_router; use storage_controller::metrics::preinitialize_metrics; @@ -77,6 +78,12 @@ struct Cli { /// How long to wait for the initial database connection to be available. #[arg(long, default_value = "5s")] db_connect_timeout: humantime::Duration, + + /// `neon_local` sets this to the path of the neon_local repo dir. + /// Only relevant for testing. + // TODO: make `cfg(feature = "testing")` + #[arg(long)] + neon_local_repo_dir: Option, } enum StrictMode { @@ -260,6 +267,7 @@ async fn async_main() -> anyhow::Result<()> { .reconciler_concurrency .unwrap_or(RECONCILER_CONCURRENCY_DEFAULT), split_threshold: args.split_threshold, + neon_local_repo_dir: args.neon_local_repo_dir, }; // After loading secrets & config, but before starting anything else, apply database migrations diff --git a/storage_controller/src/service.rs b/storage_controller/src/service.rs index 181e262638..8475bf46d2 100644 --- a/storage_controller/src/service.rs +++ b/storage_controller/src/service.rs @@ -2,6 +2,7 @@ use std::{ borrow::Cow, cmp::Ordering, collections::{BTreeMap, HashMap, HashSet}, + path::PathBuf, str::FromStr, sync::Arc, time::{Duration, Instant}, @@ -236,6 +237,9 @@ pub struct Config { /// How large must a shard grow in bytes before we split it? /// None disables auto-splitting. pub split_threshold: Option, + + // TODO: make this cfg(feature = "testing") + pub neon_local_repo_dir: Option, } impl From for ApiError {