From 571e88733a6201fb4fcb9d63494bd50c891d958e Mon Sep 17 00:00:00 2001
From: Lasse Tjernaes Wardenaer <lasse.tjernaes.wardenaer@cern.ch>
Date: Fri, 17 Feb 2023 13:31:36 +0100
Subject: [PATCH] Resolve "Simple safe tool for EOS NS injection"

---
 ReleaseNotes.md                               |  2 ++
 .../common/CmdLineArgs.cpp                    |  4 +--
 .../EosNamespaceInjection.cpp                 | 29 +++++++++++--------
 3 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/ReleaseNotes.md b/ReleaseNotes.md
index 6c6b3d603d..73e1c852c1 100644
--- a/ReleaseNotes.md
+++ b/ReleaseNotes.md
@@ -1,5 +1,7 @@
 # v4.Next
 ### Features
+- cta/CTA#213 - Improve error messages for `cta-eos-namespace-inject`
+### Bug Fixes
 - cta/CTA#305 - AllCatalogueSchema file not created when updating schema
 
 # v4.8.5-1
diff --git a/cmdline/standalone_cli_tools/common/CmdLineArgs.cpp b/cmdline/standalone_cli_tools/common/CmdLineArgs.cpp
index 0ce6bc5459..d12ba8186a 100644
--- a/cmdline/standalone_cli_tools/common/CmdLineArgs.cpp
+++ b/cmdline/standalone_cli_tools/common/CmdLineArgs.cpp
@@ -65,7 +65,7 @@ static struct option changeStorageClassLongOption[] = {
   {"id", required_argument, nullptr, 'I'},
   {"json", required_argument, nullptr, 'j'},
   {"storageclassname", required_argument, nullptr, 'n'},
-  {"frequenzy", required_argument, nullptr, 't'},
+  {"frequency", required_argument, nullptr, 't'},
   {"help", no_argument, nullptr, 'h'},
   {nullptr, 0, nullptr, 0}
 };
@@ -270,7 +270,7 @@ void CmdLineArgs::printUsage(std::ostream &os) const {
     break;
   case StandaloneCliTool::CTA_CHANGE_STORAGE_CLASS :
     os << "    Usage:" << std::endl <<
-    "    cta-change-storage-class --id/-I <archiveFileID> | --json/-j <path> --storageclassname/-n <storageClassName> [--frequenzy/-t <eosRequestFrequency>]" << std::endl << std::endl;
+    "    cta-change-storage-class --id/-I <archiveFileID> | --json/-j <path> --storageclassname/-n <storageClassName> [--frequency/-t <eosRequestFrequency>]" << std::endl << std::endl;
     break;
   case StandaloneCliTool::EOS_NAMESPACE_INJECTION :
     os << "    Usage:" << std::endl <<
diff --git a/cmdline/standalone_cli_tools/eos_namespace_injection/EosNamespaceInjection.cpp b/cmdline/standalone_cli_tools/eos_namespace_injection/EosNamespaceInjection.cpp
index f3710795a1..e58dbe0093 100644
--- a/cmdline/standalone_cli_tools/eos_namespace_injection/EosNamespaceInjection.cpp
+++ b/cmdline/standalone_cli_tools/eos_namespace_injection/EosNamespaceInjection.cpp
@@ -153,12 +153,16 @@ int EosNamespaceInjection::exceptionThrowingMain(const int argc, char *const *co
     const auto enclosingPath = cta::utils::getEnclosingPath(metaDataObject.eosPath);
     const auto [parentId, uid, gid] = getContainerIdsEos(metaDataObject.diskInstance, enclosingPath);
     if(!parentId) {
-      throw exception::UserError("Path: " + enclosingPath + " does not exist.");
+      throw exception::UserError("Could not find: " + enclosingPath + ". Check that gRPC authentication is set up correctly, and that the path exists");
+    }
+
+    if(const auto fid = getFileIdEos(metaDataObject.diskInstance, metaDataObject.eosPath); fid != 0) {
+      throw std::runtime_error("The file with path " + metaDataObject.eosPath + " already exists for instance " + metaDataObject.diskInstance + ". This tool does not overwrite existing files");
     }
-    const auto newFid = createFileInEos(metaDataObject, parentId, uid, gid);
 
+    const auto newFid = createFileInEos(metaDataObject, parentId, uid, gid);
     if(newFid == 0) {
-      throw std::runtime_error("The file was with path " + metaDataObject.eosPath + "was not created");
+      throw std::runtime_error("The file with path " + metaDataObject.eosPath + " was not created");
     }
 
     auto decimalToHexadecimal = [](const std::string &decimalNumber) {
@@ -177,15 +181,16 @@ int EosNamespaceInjection::exceptionThrowingMain(const int argc, char *const *co
     const auto& ctaChecksum = g_metaDataObjectCatalogue.checksumValue;
     std::list<cta::log::Param> params;
     params.push_back(cta::log::Param("archiveFileId", archiveId));
-    params.push_back(cta::log::Param("diskInstance", g_metaDataObjectCatalogue.diskInstance));
-    params.push_back(cta::log::Param("diskFileId", g_metaDataObjectCatalogue.fxId));
+    params.push_back(cta::log::Param("diskFileId in EOS for new file", fxId));
+    params.push_back(cta::log::Param("diskFileId in Catalogue", g_metaDataObjectCatalogue.fxId));
+    params.push_back(cta::log::Param("diskInstance in Catalogue", g_metaDataObjectCatalogue.diskInstance));
     params.push_back(cta::log::Param("checksum", ctaChecksum));
-    if(eosArchiveFileId ==  archiveId && eosChecksum == ctaChecksum) {
+    if(eosArchiveFileId == archiveId && eosChecksum == ctaChecksum && g_metaDataObjectCatalogue.fxId == fxId) {
       m_log(cta::log::INFO, "File metadata in EOS and CTA matches", params);
     } else {
       params.push_back(cta::log::Param("eosArchiveFileId", eosArchiveFileId));
       params.push_back(cta::log::Param("eosChecksum", eosChecksum));
-      m_log(cta::log::INFO, "File metadata in EOS and CTA does not match", params);
+      m_log(cta::log::WARNING, "File metadata in EOS and CTA does not match", params);
       throw std::runtime_error("Sanity check failed.");
     }
   }
@@ -323,9 +328,9 @@ bool EosNamespaceInjection::getMetaDataFromCatalogue(const uint64_t &archiveId)
   std::list<cta::log::Param> params;
   params.push_back(cta::log::Param("containerId", cid));
   if (cid != 0) {
-    m_log(cta::log::DEBUG, "Container exists in the EOS namespace", params);
+    m_log(cta::log::INFO, "Container exists in the EOS namespace", params);
   } else {
-    m_log(cta::log::DEBUG, "Container does not exist in the EOS namespace", params);
+    m_log(cta::log::WARNING, "Container does not exist in the EOS namespace", params);
   }
   return std::make_tuple(cid, uid, gid);
 }
@@ -344,9 +349,9 @@ uint64_t EosNamespaceInjection::getFileIdEos(const std::string &diskInstance, co
   const auto fid = md_response.fmd().id();
   params.push_back(cta::log::Param("diskFileId", fid));
   if (fid != 0) {
-    m_log(cta::log::DEBUG, "File path exists in the EOS namespace", params);
+    m_log(cta::log::INFO, "File path exists in the EOS namespace", params);
   } else {
-    m_log(cta::log::DEBUG, "File path does not exist in the EOS namespace", params);
+    m_log(cta::log::WARNING, "Could not find path in the EOS namespace. Check that gRPC authentication is set up correctly, and that the path exists", params);
   }
   return fid;
 }
@@ -442,7 +447,7 @@ std::pair<ArchiveId, Checksum> EosNamespaceInjection::getArchiveFileIdAndChecksu
     std::list<cta::log::Param> params;
     params.push_back(cta::log::Param("archiveFileId", archiveFileId));
     params.push_back(cta::log::Param("checksumValue", checksumValue));
-    m_log(cta::log::DEBUG, "Response from EOS nameserver", params);
+    m_log(cta::log::INFO, "Response from EOS nameserver", params);
   }
 
   return std::make_pair(archiveFileId,checksumValue);
-- 
GitLab