fix for #573: corrected error message in case of deployment mismatch

data directory creation should not happen in case of deployment mismatch
staging should be overwritten in case of new staging

Author: nikhilsinhaparseable

server/src/migration.rs

@@ -35,9 +35,7 @@ use crate::{
 pub async fn run_metadata_migration(config: &Config) -> anyhow::Result<()> {
     let object_store = config.storage().get_object_store();
     let storage_metadata = get_storage_metadata(&*object_store).await?;
-    println!("storage_metadata in run_metadata_migration: {:?}", storage_metadata);
     let staging_metadata = get_staging_metadata(config)?;
-    println!("staging_metadata in run_metadata_migration: {:?}", staging_metadata);
 
     fn get_version(metadata: &serde_json::Value) -> Option<&str> {
         metadata

server/src/option.rs

@@ -48,15 +48,13 @@ impl Config {
                 let server = match Server::from_arg_matches(m) {
 
                     Ok(server) => {
-                        println!("server: {:?}", server);
                         server
                     },
                     Err(err) => err.exit(),
                 };
                 let storage = match FSConfig::from_arg_matches(m) {
 
                     Ok(storage) => {
-                        println!("storage: {:?}", storage);
                         storage
                     },
                     Err(err) => err.exit(),
@@ -89,14 +87,12 @@ impl Config {
             Some(("s3-store", m)) => {
                 let server = match Server::from_arg_matches(m) {
                     Ok(server) => {
-                        println!("server: {:?}", server);
                         server
                     },
                     Err(err) => err.exit(),
                 };
                 let storage = match S3Config::from_arg_matches(m) {
                     Ok(storage) => {
-                        println!("storage: {:?}", storage);
                         storage
                     },
                     Err(err) => err.exit(),

server/src/storage/localfs.rs

@@ -32,7 +32,7 @@ use tokio::fs::{self, DirEntry};
 use tokio_stream::wrappers::ReadDirStream;
 
 use crate::metrics::storage::{localfs::REQUEST_RESPONSE_TIME, StorageMetrics};
-use crate::{option::validation, utils::validate_path_is_writeable};
+use crate::utils::validate_path_is_writeable;
 
 use super::{object_storage, LogStream, ObjectStorage, ObjectStorageError, ObjectStorageProvider};
 
@@ -49,8 +49,7 @@ pub struct FSConfig {
     #[arg(
         env = "P_FS_DIR",
         value_name = "filesystem path",
-        default_value = "./data",
-        value_parser = validation::canonicalize_path
+        default_value = "./data"
     )]
     pub root: PathBuf,
 }
@@ -120,7 +119,6 @@ impl ObjectStorage for LocalFS {
         let path = self.path_in_root(path);
         if let Some(parent) = path.parent() {
             fs::create_dir_all(parent).await?;
-            println!("created dir {:?}", parent);
         }
         let res = fs::write(path, resource).await;
 
@@ -141,7 +139,6 @@ impl ObjectStorage for LocalFS {
 
     async fn check(&self) -> Result<(), ObjectStorageError> {
         fs::create_dir_all(&self.root).await?;
-        println!("created dir self.root {:?}", &self.root);
         validate_path_is_writeable(&self.root)
             .map_err(|e| ObjectStorageError::UnhandledError(e.into()))
     }
@@ -189,9 +186,9 @@ impl ObjectStorage for LocalFS {
         };
         let to_path = self.root.join(key);
         if let Some(path) = to_path.parent() {
-            println!("created dir path {:?}", &path);
-            fs::create_dir_all(path).await?
 
+            fs::create_dir_all(path).await?;
+
         }
         let _ = fs_extra::file::copy(path, to_path, &op)?;
         Ok(())

server/src/storage/store_metadata.rs

@@ -96,19 +96,11 @@ impl StorageMetadata {
 // overwrites staging metadata while updating storage info
 pub async fn resolve_parseable_metadata() -> Result<StorageMetadata, ObjectStorageError> {
     let staging_metadata = get_staging_metadata()?;
-    println!("staging metadata in resolve_parseable_metadata: {:?}", staging_metadata);
     let storage = CONFIG.storage().get_object_store();
     let remote_metadata = storage.get_metadata().await?;
-    println!("remote metadata in resolve_parseable_metadata: {:?}", remote_metadata);
 
     let check = match (staging_metadata, remote_metadata) {
-        (Some(staging), Some(remote)) => {
-            if staging.deployment_id == remote.deployment_id {
-                EnvChange::None(remote)
-            } else {
-                EnvChange::DeploymentMismatch
-            }
-        }
+        (Some(_staging), Some(remote)) => EnvChange::None(remote),
         (None, Some(remote)) => EnvChange::NewStaging(remote),
         (Some(_), None) => EnvChange::NewRemote,
         (None, None) => EnvChange::CreateBoth,
@@ -118,16 +110,14 @@ pub async fn resolve_parseable_metadata() -> Result<StorageMetadata, ObjectStora
     let mut overwrite_staging = false;
     let mut overwrite_remote = false;
 
-    const MISMATCH: &str = "Could not start the server because metadata file found in staging directory does not match one in the storage";
     let res = match check {
         EnvChange::None(metadata) => {
             // overwrite staging anyways so that it matches remote in case of any divergence
             overwrite_staging = true;
             Ok(metadata)
-        }
-        EnvChange::DeploymentMismatch => Err(MISMATCH),
+        },
         EnvChange::NewRemote => {
-            Err("Could not start the server because metadata not found in storage")
+            Err("Could not start the server because staging directory indicates stale data from previous deployment, please choose an empty staging directory and restart the server")
         }
         EnvChange::NewStaging(mut metadata) => {
             create_dir_all(CONFIG.staging_dir())?;
@@ -173,9 +163,8 @@ pub async fn resolve_parseable_metadata() -> Result<StorageMetadata, ObjectStora
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum EnvChange {
     /// No change in env i.e both staging and remote have same id  
+    /// or deployment id of staging is not matching with that of remote
     None(StorageMetadata),
-    /// Mismatch in deployment id. Cannot use this staging for this remote
-    DeploymentMismatch,
     /// Metadata not found in storage. Treated as possible misconfiguration on user side.
     NewRemote,
     /// If a new staging is found then we just copy remote metadata to this staging.