diff --git a/ReleaseNotes.md b/ReleaseNotes.md index 00964c6efbd81e77352ec2b9bf188f0321e16678..93a433951e16218f847247b5f3c5d5bfd59c5ffc 100644 --- a/ReleaseNotes.md +++ b/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 diff --git a/cmdline/standalone_cli_tools/change_storage_class/ChangeStorageClass.cpp b/cmdline/standalone_cli_tools/change_storage_class/ChangeStorageClass.cpp index c73764603816f199999390977451c9c47429d0e9..c926924e5d3f17f3701f802e5cd942529cf7e676 100644 --- a/cmdline/standalone_cli_tools/change_storage_class/ChangeStorageClass.cpp +++ b/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 diff --git a/cmdline/standalone_cli_tools/change_storage_class/ChangeStorageClass.hpp b/cmdline/standalone_cli_tools/change_storage_class/ChangeStorageClass.hpp index 053d7ef42d14fb8238b5f83b5df01e5805aba6f4..1484270d46248884a3e0469faea19408ec077c05 100644 --- a/cmdline/standalone_cli_tools/change_storage_class/ChangeStorageClass.hpp +++ b/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 diff --git a/cmdline/standalone_cli_tools/change_storage_class/JsonFileData.cpp b/cmdline/standalone_cli_tools/change_storage_class/JsonFileData.cpp index 6b5b70335380e323af93955cecbacea53245b166..e2599015abfa2eeee5fd79bfaca407a0f7e18bd4 100644 --- a/cmdline/standalone_cli_tools/change_storage_class/JsonFileData.cpp +++ b/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); } diff --git a/cmdline/standalone_cli_tools/change_storage_class/JsonFileData.hpp b/cmdline/standalone_cli_tools/change_storage_class/JsonFileData.hpp index 3d58e62d1e1fab505ff27edaa8a7b6656183329f..0e87d44e46ff7be625f24f2e832a255866822152 100644 --- a/cmdline/standalone_cli_tools/change_storage_class/JsonFileData.hpp +++ b/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{ diff --git a/cmdline/standalone_cli_tools/common/CmdLineArgs.cpp b/cmdline/standalone_cli_tools/common/CmdLineArgs.cpp index d12ba8186a8b5b0e57bd534a04e7205459902236..5bc9e9064b7fa277a135279948466e5f1ffe0827 100644 --- a/cmdline/standalone_cli_tools/common/CmdLineArgs.cpp +++ b/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:"}, }; diff --git a/continuousintegration/orchestration/tests/changeStorageClass.sh b/continuousintegration/orchestration/tests/changeStorageClass.sh index c0f77de91c808bd45355f9109d05c8b90e35d1d2..05d72f72ae2ae95ab83fe4b1f1a2f6cb1000ad12 100755 --- a/continuousintegration/orchestration/tests/changeStorageClass.sh +++ b/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