Resolve "Add extra arguments to cta-change-storage-class tool to make sure the...

Resolve "Add extra arguments to cta-change-storage-class tool to make sure the right file is being changed"

ReleaseNotes.md

@@ -6,6 +6,7 @@
 - cta/CTA#294 - Improve error messages for 'Exec' in the gRPC client
 - cta/CTA#245 - Implements cta-admin functions in cta-frontend-grpc
 - cta/CTA#213 - Skip files with paths that have valid metadata
+- cta/CTA#321 - Add arguments to cta-change-storage-class to validate that the correct files are being changed
 ### Bug Fixes
 - cta/CTA#305 - AllCatalogueSchema file not created when updating schema
 - cta/CTA#309 - Ignore 'NoSuchObject' exceptions thrown by non-existing objects during retrieve job requeuing

cmdline/standalone_cli_tools/change_storage_class/ChangeStorageClass.cpp

@@ -77,12 +77,13 @@ ChangeStorageClass::~ChangeStorageClass() = default;
 //------------------------------------------------------------------------------
 int ChangeStorageClass::exceptionThrowingMain(const int argc, char *const *const argv) {
   CmdLineArgs cmdLineArgs(argc, argv, StandaloneCliTool::CTA_CHANGE_STORAGE_CLASS);
-  handleArguments(cmdLineArgs);
 
   auto [serviceProvider, endpointmap] = ConnConfiguration::readAndSetConfiguration(m_log, getUsername(), cmdLineArgs);
   m_serviceProviderPtr = std::move(serviceProvider);
   m_endpointMapPtr = std::move(endpointmap);
 
+  handleArguments(cmdLineArgs);
+
   storageClassExists();
   updateStorageClassInEosNamespace();
   updateStorageClassInCatalogue();
@@ -102,24 +103,30 @@ void ChangeStorageClass::handleArguments(const CmdLineArgs &cmdLineArgs) {
   if(cmdLineArgs.m_json) {
     JsonFileData jsonFilaData(cmdLineArgs.m_json.value());
     for(const auto &jsonFileDataObject : jsonFilaData.m_jsonArgumentsCollection) {
+      if(!validateUserInputFileMetadata(jsonFileDataObject.archiveId, jsonFileDataObject.fid, jsonFileDataObject.instance)) {
+        throw exception::UserError("Archive id does not match with disk file id or disk instance, are you sure the correct file metadata was provided?");
+      }
       m_archiveFileIds.push_back(jsonFileDataObject.archiveId);
     }
   }
 
   if (!cmdLineArgs.m_storageClassName) {
     cmdLineArgs.printUsage(std::cout);
-    throw exception::UserError("Missing requried option: storage.class.name");
+    throw exception::UserError("Missing required option: storage.class.name");
   }
 
-  if (!cmdLineArgs.m_archiveFileId && !cmdLineArgs.m_json) {
+  if ((!cmdLineArgs.m_archiveFileId || !cmdLineArgs.m_fids || !cmdLineArgs.m_diskInstance) && !cmdLineArgs.m_json) {
     cmdLineArgs.printUsage(std::cout);
-    throw exception::UserError("filename or id must be provided");
+    throw exception::UserError("Archive id, eos file id and disk instance must be provided must be provided");
   }
 
   m_storageClassName = cmdLineArgs.m_storageClassName.value();
 
   if (cmdLineArgs.m_archiveFileId) {
     m_archiveFileIds.push_back(cmdLineArgs.m_archiveFileId.value());
+    if(!validateUserInputFileMetadata(cmdLineArgs.m_archiveFileId.value(), cmdLineArgs.m_fids.value().front(), cmdLineArgs.m_diskInstance.value())) {
+      throw exception::UserError("Archive id does not match with disk file id or disk instance, are you sure the correct file metadata was provided?");
+    }
   }
 
   if (cmdLineArgs.m_frequency) {
@@ -137,7 +144,6 @@ bool ChangeStorageClass::fileInFlight(const google::protobuf::RepeatedField<uint
   return std::all_of(std::begin(locations), std::end(locations), [](const int &location) { return location != 65535; });
 }
 
-
 //------------------------------------------------------------------------------
 // updateStorageClassInEosNamespace
 //------------------------------------------------------------------------------
@@ -305,7 +311,7 @@ void ChangeStorageClass::writeSkippedArchiveIdsToFile() const {
 
   if (archiveIdFile.is_open()) {
     for (const auto& archiveId : m_archiveIdsNotUpdatedInEos) {
-      archiveIdFile << "{ archiveId : " << archiveId << " }" << std::endl;
+      archiveIdFile << "{ \"archiveId\" : " << archiveId << " }" << std::endl;
     }
     archiveIdFile.close();
     std::cout << m_archiveIdsNotUpdatedInEos.size() << " files did not update the storage class." << std::endl;
@@ -313,5 +319,10 @@ void ChangeStorageClass::writeSkippedArchiveIdsToFile() const {
   }
 }
 
+bool ChangeStorageClass::validateUserInputFileMetadata(const std::string& archiveId, const std::string& operatorProvidedFid, const std::string& operatorProvidedInstance) {
+  const auto [diskDiskInstance, diskFileId] = CatalogueFetch::getInstanceAndFid(archiveId, m_serviceProviderPtr, m_log);
+  return ((operatorProvidedInstance == diskDiskInstance) && (operatorProvidedFid == diskFileId));
+}
+
 } // namespace admin
 } // namespace cta

cmdline/standalone_cli_tools/change_storage_class/ChangeStorageClass.hpp

@@ -127,6 +127,14 @@ private:
   */
   void handleArguments(const CmdLineArgs &cmdLineArgs);
 
+  /**
+  * Checks that the provided archive id is related to the correct disk file id and disk instance
+  * @param archiveId the archive file id to check
+  * @param operatorProvidedFid The disk file id provided by the operator, either from a json file or an command line argument
+  * @param operatorProvidedInstance The disk instance provided by the operator, either from a json file or an command line argument
+  */
+  bool validateUserInputFileMetadata(const std::string& archiveId, const std::string& operatorProvidedFid, const std::string& operatorProvidedInstance);
+
   /**
    * An exception throwing version of main().
    *
@@ -137,6 +145,7 @@ private:
   int exceptionThrowingMain(const int argc, char *const *const argv) override;
 
 
+
 } ; // class CtaChangeStorageClass
 
 } // namespace cta::admin

cmdline/standalone_cli_tools/change_storage_class/JsonFileData.cpp

@@ -42,6 +42,8 @@ void JsonFileData::readJson(const std::filesystem::path &jsonPath) {
 
       JsonFileDataObject JsonFileDataObject;
       JsonFileDataObject.archiveId = jsonGetValue<std::string>("archiveId");
+      JsonFileDataObject.fid       = std::to_string(jsonGetValue<uint64_t>("fid"));
+      JsonFileDataObject.instance  = jsonGetValue<std::string>("instance");
 
       m_jsonArgumentsCollection.push_back(JsonFileDataObject);
     }

cmdline/standalone_cli_tools/change_storage_class/JsonFileData.hpp

@@ -27,6 +27,8 @@ namespace cta::cliTool {
 
 struct JsonFileDataObject {
   std::string archiveId;
+  std::string fid;
+  std::string instance;
 };
 
 class JsonFileData : public cta::utils::json::object::JSONCObject{

cmdline/standalone_cli_tools/common/CmdLineArgs.cpp

@@ -65,6 +65,8 @@ static struct option changeStorageClassLongOption[] = {
   {"id", required_argument, nullptr, 'I'},
   {"json", required_argument, nullptr, 'j'},
   {"storageclassname", required_argument, nullptr, 'n'},
+  {"fid", required_argument, nullptr, 'f'},
+  {"instance", required_argument, nullptr, 'i'},
   {"frequency", required_argument, nullptr, 't'},
   {"help", no_argument, nullptr, 'h'},
   {nullptr, 0, nullptr, 0}
@@ -88,7 +90,7 @@ std::map<StandaloneCliTool, const char*> shortopts = {
   {StandaloneCliTool::RESTORE_FILES, "I:i:f:F:v:c:hd:"},
   {StandaloneCliTool::CTA_SEND_EVENT, "i:e:u:g:"},
   {StandaloneCliTool::CTA_VERIFY_FILE, "I:F:i:u:g:v:h:"},
-  {StandaloneCliTool::CTA_CHANGE_STORAGE_CLASS, "I:j:n:t:h:"},
+  {StandaloneCliTool::CTA_CHANGE_STORAGE_CLASS, "I:f:i:j:n:t:h:"},
   {StandaloneCliTool::EOS_NAMESPACE_INJECTION, "j:h:"},
 };
 

continuousintegration/orchestration/tests/changeStorageClass.sh

@@ -77,20 +77,23 @@ echo "SEND EOS METADATA TO JSON FILE: ${EOS_METADATA_PATH}"
 touch ${EOS_METADATA_PATH_1}
 kubectl -n ${NAMESPACE} exec client -- eos -j root://${EOSINSTANCE} file info /eos/ctaeos/cta/${FILE_1} | jq . | tee ${EOS_METADATA_PATH_1}
 EOS_ARCHIVE_ID_1=$(jq -r '.xattr | .["sys.archive.file_id"]' ${EOS_METADATA_PATH_1})
+EOS_FILE_ID_1=$(jq -r '.id' ${EOS_METADATA_PATH_1})
+
 
 EOS_METADATA_PATH_2=$(mktemp -d).json
 echo "SEND EOS METADATA TO JSON FILE: ${EOS_METADATA_PATH_2}"
 touch ${EOS_METADATA_PATH_2}
 kubectl -n ${NAMESPACE} exec client -- eos -j root://${EOSINSTANCE} file info /eos/ctaeos/cta/${FILE_2} | jq . | tee ${EOS_METADATA_PATH_2}
 EOS_ARCHIVE_ID_2=$(jq -r '.xattr | .["sys.archive.file_id"]' ${EOS_METADATA_PATH_2})
+EOS_FILE_ID_2=$(jq -r '.id' ${EOS_METADATA_PATH_2})
 
 echo
 echo "CHANGE FILES WITH IDS"
 cd ~
 IDS_FILEPATH=${TMP_DIR}/archiveFileIds.txt
 touch ${IDS_FILEPATH}
-echo '{"archiveId": '${EOS_ARCHIVE_ID_1}'}' >> ${IDS_FILEPATH}
-echo '{"archiveId": '${EOS_ARCHIVE_ID_2}'}' >> ${IDS_FILEPATH}
+echo '{"archiveId": '${EOS_ARCHIVE_ID_1}', "fid": '${EOS_FILE_ID_1}', "instance": "'${EOSINSTANCE}'"}' >> ${IDS_FILEPATH}
+echo '{"archiveId": '${EOS_ARCHIVE_ID_2}', "fid": '${EOS_FILE_ID_2}', "instance": "'${EOSINSTANCE}'"}' >> ${IDS_FILEPATH}
 sleep 1
 
 echo