From a347d2b6ac22f3ab30297b1237f3d16ef682c118 Mon Sep 17 00:00:00 2001 From: Anastasia Lubennikova Date: Thu, 20 Oct 2022 20:27:26 +0300 Subject: [PATCH] #2616 handle 'Unsupported pg_version' error properly --- control_plane/src/compute.rs | 12 +++++------ control_plane/src/local_env.rs | 30 +++++++++++++------------- libs/postgres_ffi/wal_craft/src/lib.rs | 22 +++++++++---------- pageserver/src/config.rs | 24 ++++++++++----------- pageserver/src/tenant.rs | 4 ++-- pageserver/src/walredo.rs | 25 +++++++++++++++------ 6 files changed, 65 insertions(+), 52 deletions(-) diff --git a/control_plane/src/compute.rs b/control_plane/src/compute.rs index 89994c5647..9f32ad31c1 100644 --- a/control_plane/src/compute.rs +++ b/control_plane/src/compute.rs @@ -183,18 +183,18 @@ impl PostgresNode { } fn sync_safekeepers(&self, auth_token: &Option, pg_version: u32) -> Result { - let pg_path = self.env.pg_bin_dir(pg_version).join("postgres"); + let pg_path = self.env.pg_bin_dir(pg_version)?.join("postgres"); let mut cmd = Command::new(&pg_path); cmd.arg("--sync-safekeepers") .env_clear() .env( "LD_LIBRARY_PATH", - self.env.pg_lib_dir(pg_version).to_str().unwrap(), + self.env.pg_lib_dir(pg_version)?.to_str().unwrap(), ) .env( "DYLD_LIBRARY_PATH", - self.env.pg_lib_dir(pg_version).to_str().unwrap(), + self.env.pg_lib_dir(pg_version)?.to_str().unwrap(), ) .env("PGDATA", self.pgdata().to_str().unwrap()) .stdout(Stdio::piped()) @@ -422,7 +422,7 @@ impl PostgresNode { } fn pg_ctl(&self, args: &[&str], auth_token: &Option) -> Result<()> { - let pg_ctl_path = self.env.pg_bin_dir(self.pg_version).join("pg_ctl"); + let pg_ctl_path = self.env.pg_bin_dir(self.pg_version)?.join("pg_ctl"); let mut cmd = Command::new(pg_ctl_path); cmd.args( [ @@ -440,11 +440,11 @@ impl PostgresNode { .env_clear() .env( "LD_LIBRARY_PATH", - self.env.pg_lib_dir(self.pg_version).to_str().unwrap(), + self.env.pg_lib_dir(self.pg_version)?.to_str().unwrap(), ) .env( "DYLD_LIBRARY_PATH", - self.env.pg_lib_dir(self.pg_version).to_str().unwrap(), + self.env.pg_lib_dir(self.pg_version)?.to_str().unwrap(), ); if let Some(token) = auth_token { cmd.env("ZENITH_AUTH_TOKEN", token); diff --git a/control_plane/src/local_env.rs b/control_plane/src/local_env.rs index f4fbc99420..34ddb41f32 100644 --- a/control_plane/src/local_env.rs +++ b/control_plane/src/local_env.rs @@ -3,7 +3,7 @@ //! Now it also provides init method which acts like a stub for proper installation //! script which will use local paths. -use anyhow::{bail, ensure, Context}; +use anyhow::{bail, ensure, Context, Result}; use reqwest::Url; use serde::{Deserialize, Serialize}; use serde_with::{serde_as, DisplayFromStr}; @@ -201,28 +201,28 @@ impl LocalEnv { self.pg_distrib_dir.clone() } - pub fn pg_distrib_dir(&self, pg_version: u32) -> PathBuf { + pub fn pg_distrib_dir(&self, pg_version: u32) -> Result { let path = self.pg_distrib_dir.clone(); match pg_version { - 14 => path.join(format!("v{pg_version}")), - 15 => path.join(format!("v{pg_version}")), - _ => panic!("Unsupported postgres version: {}", pg_version), + 14 => Ok(path.join(format!("v{pg_version}"))), + 15 => Ok(path.join(format!("v{pg_version}"))), + _ => bail!("Unsupported postgres version: {}", pg_version), } } - pub fn pg_bin_dir(&self, pg_version: u32) -> PathBuf { + pub fn pg_bin_dir(&self, pg_version: u32) -> Result { match pg_version { - 14 => self.pg_distrib_dir(pg_version).join("bin"), - 15 => self.pg_distrib_dir(pg_version).join("bin"), - _ => panic!("Unsupported postgres version: {}", pg_version), + 14 => Ok(self.pg_distrib_dir(pg_version)?.join("bin")), + 15 => Ok(self.pg_distrib_dir(pg_version)?.join("bin")), + _ => bail!("Unsupported postgres version: {}", pg_version), } } - pub fn pg_lib_dir(&self, pg_version: u32) -> PathBuf { + pub fn pg_lib_dir(&self, pg_version: u32) -> Result { match pg_version { - 14 => self.pg_distrib_dir(pg_version).join("lib"), - 15 => self.pg_distrib_dir(pg_version).join("lib"), - _ => panic!("Unsupported postgres version: {}", pg_version), + 14 => Ok(self.pg_distrib_dir(pg_version)?.join("lib")), + 15 => Ok(self.pg_distrib_dir(pg_version)?.join("lib")), + _ => bail!("Unsupported postgres version: {}", pg_version), } } @@ -422,10 +422,10 @@ impl LocalEnv { "directory '{}' already exists. Perhaps already initialized?", base_path.display() ); - if !self.pg_bin_dir(pg_version).join("postgres").exists() { + if !self.pg_bin_dir(pg_version)?.join("postgres").exists() { bail!( "Can't find postgres binary at {}", - self.pg_bin_dir(pg_version).display() + self.pg_bin_dir(pg_version)?.display() ); } for binary in ["pageserver", "safekeeper"] { diff --git a/libs/postgres_ffi/wal_craft/src/lib.rs b/libs/postgres_ffi/wal_craft/src/lib.rs index 7ffe19e209..f0203ce322 100644 --- a/libs/postgres_ffi/wal_craft/src/lib.rs +++ b/libs/postgres_ffi/wal_craft/src/lib.rs @@ -37,22 +37,22 @@ pub static REQUIRED_POSTGRES_CONFIG: Lazy> = Lazy::new(|| { }); impl Conf { - pub fn pg_distrib_dir(&self) -> PathBuf { + pub fn pg_distrib_dir(&self) -> Result { let path = self.pg_distrib_dir.clone(); match self.pg_version { - 14 => path.join(format!("v{}", self.pg_version)), - 15 => path.join(format!("v{}", self.pg_version)), - _ => panic!("Unsupported postgres version: {}", self.pg_version), + 14 => Ok(path.join(format!("v{}", self.pg_version))), + 15 => Ok(path.join(format!("v{}", self.pg_version))), + _ => bail!("Unsupported postgres version: {}", self.pg_version), } } - fn pg_bin_dir(&self) -> PathBuf { - self.pg_distrib_dir().join("bin") + fn pg_bin_dir(&self) -> Result { + Ok(self.pg_distrib_dir()?.join("bin")) } - fn pg_lib_dir(&self) -> PathBuf { - self.pg_distrib_dir().join("lib") + fn pg_lib_dir(&self) -> Result { + Ok(self.pg_distrib_dir()?.join("lib")) } pub fn wal_dir(&self) -> PathBuf { @@ -60,12 +60,12 @@ impl Conf { } fn new_pg_command(&self, command: impl AsRef) -> Result { - let path = self.pg_bin_dir().join(command); + let path = self.pg_bin_dir()?.join(command); ensure!(path.exists(), "Command {:?} does not exist", path); let mut cmd = Command::new(path); cmd.env_clear() - .env("LD_LIBRARY_PATH", self.pg_lib_dir()) - .env("DYLD_LIBRARY_PATH", self.pg_lib_dir()); + .env("LD_LIBRARY_PATH", self.pg_lib_dir()?) + .env("DYLD_LIBRARY_PATH", self.pg_lib_dir()?); Ok(cmd) } diff --git a/pageserver/src/config.rs b/pageserver/src/config.rs index b797866e43..2872fc6255 100644 --- a/pageserver/src/config.rs +++ b/pageserver/src/config.rs @@ -387,28 +387,28 @@ impl PageServerConf { // // Postgres distribution paths // - pub fn pg_distrib_dir(&self, pg_version: u32) -> PathBuf { + pub fn pg_distrib_dir(&self, pg_version: u32) -> Result { let path = self.pg_distrib_dir.clone(); match pg_version { - 14 => path.join(format!("v{pg_version}")), - 15 => path.join(format!("v{pg_version}")), - _ => panic!("Unsupported postgres version: {}", pg_version), + 14 => Ok(path.join(format!("v{pg_version}"))), + 15 => Ok(path.join(format!("v{pg_version}"))), + _ => bail!("Unsupported postgres version: {}", pg_version), } } - pub fn pg_bin_dir(&self, pg_version: u32) -> PathBuf { + pub fn pg_bin_dir(&self, pg_version: u32) -> Result { match pg_version { - 14 => self.pg_distrib_dir(pg_version).join("bin"), - 15 => self.pg_distrib_dir(pg_version).join("bin"), - _ => panic!("Unsupported postgres version: {}", pg_version), + 14 => Ok(self.pg_distrib_dir(pg_version)?.join("bin")), + 15 => Ok(self.pg_distrib_dir(pg_version)?.join("bin")), + _ => bail!("Unsupported postgres version: {}", pg_version), } } - pub fn pg_lib_dir(&self, pg_version: u32) -> PathBuf { + pub fn pg_lib_dir(&self, pg_version: u32) -> Result { match pg_version { - 14 => self.pg_distrib_dir(pg_version).join("lib"), - 15 => self.pg_distrib_dir(pg_version).join("lib"), - _ => panic!("Unsupported postgres version: {}", pg_version), + 14 => Ok(self.pg_distrib_dir(pg_version)?.join("lib")), + 15 => Ok(self.pg_distrib_dir(pg_version)?.join("lib")), + _ => bail!("Unsupported postgres version: {}", pg_version), } } diff --git a/pageserver/src/tenant.rs b/pageserver/src/tenant.rs index 6aae740a78..0e9a6ce4ea 100644 --- a/pageserver/src/tenant.rs +++ b/pageserver/src/tenant.rs @@ -1412,8 +1412,8 @@ fn run_initdb( initdb_target_dir: &Path, pg_version: u32, ) -> Result<()> { - let initdb_bin_path = conf.pg_bin_dir(pg_version).join("initdb"); - let initdb_lib_dir = conf.pg_lib_dir(pg_version); + let initdb_bin_path = conf.pg_bin_dir(pg_version)?.join("initdb"); + let initdb_lib_dir = conf.pg_lib_dir(pg_version)?; info!( "running {} in {}, libdir: {}", initdb_bin_path.display(), diff --git a/pageserver/src/walredo.rs b/pageserver/src/walredo.rs index b8874a0223..e683c301d8 100644 --- a/pageserver/src/walredo.rs +++ b/pageserver/src/walredo.rs @@ -610,13 +610,26 @@ impl PostgresRedoProcess { ); fs::remove_dir_all(&datadir)?; } + let pg_bin_dir_path = conf.pg_bin_dir(pg_version).map_err(|e| { + Error::new( + ErrorKind::Other, + format!("incorrect pg_bin_dir path: {}", e), + ) + })?; + let pg_lib_dir_path = conf.pg_lib_dir(pg_version).map_err(|e| { + Error::new( + ErrorKind::Other, + format!("incorrect pg_lib_dir path: {}", e), + ) + })?; + info!("running initdb in {}", datadir.display()); - let initdb = Command::new(conf.pg_bin_dir(pg_version).join("initdb")) + let initdb = Command::new(pg_bin_dir_path.join("initdb")) .args(&["-D", &datadir.to_string_lossy()]) .arg("-N") .env_clear() - .env("LD_LIBRARY_PATH", conf.pg_lib_dir(pg_version)) - .env("DYLD_LIBRARY_PATH", conf.pg_lib_dir(pg_version)) + .env("LD_LIBRARY_PATH", &pg_lib_dir_path) + .env("DYLD_LIBRARY_PATH", &pg_lib_dir_path) // macOS .close_fds() .output() .map_err(|e| Error::new(e.kind(), format!("failed to execute initdb: {e}")))?; @@ -642,14 +655,14 @@ impl PostgresRedoProcess { } // Start postgres itself - let mut child = Command::new(conf.pg_bin_dir(pg_version).join("postgres")) + let mut child = Command::new(pg_bin_dir_path.join("postgres")) .arg("--wal-redo") .stdin(Stdio::piped()) .stderr(Stdio::piped()) .stdout(Stdio::piped()) .env_clear() - .env("LD_LIBRARY_PATH", conf.pg_lib_dir(pg_version)) - .env("DYLD_LIBRARY_PATH", conf.pg_lib_dir(pg_version)) + .env("LD_LIBRARY_PATH", &pg_lib_dir_path) + .env("DYLD_LIBRARY_PATH", &pg_lib_dir_path) .env("PGDATA", &datadir) // The redo process is not trusted, so it runs in seccomp mode // (see seccomp in zenith_wal_redo.c). We have to make sure it doesn't