From f5c5a3adf0244f63dfb2f4b339437ce762f546bd Mon Sep 17 00:00:00 2001
From: Lasse Tjernaes Wardenaer <lasse.tjernaes.wardenaer@cern.ch>
Date: Fri, 4 Nov 2022 15:24:33 +0100
Subject: [PATCH] Resolve "Update usage message format for cta-verify-file and
fix "Unknown command line option -V""
---
ReleaseNotes.md | 3 +-
catalogue/RdbmsCatalogue.cpp | 2 +-
cmdline/standalone_cli_tools/CMakeLists.txt | 2 +-
.../standalone_cli_tools/CtaVerifyFile.cpp | 95 +++++++++++--------
.../common/CatalogueFetch.cpp | 34 +++++--
.../common/CatalogueFetch.hpp | 22 +++--
.../common/CmdLineArgs.cpp | 28 +++---
7 files changed, 113 insertions(+), 73 deletions(-)
diff --git a/ReleaseNotes.md b/ReleaseNotes.md
index 8453f32ae7..1462528e43 100644
--- a/ReleaseNotes.md
+++ b/ReleaseNotes.md
@@ -9,15 +9,16 @@
- cta/CTA#173 - Update release notes and small changes to refactoring of operation tools cmd line parsing - Compatible with operations 0.4-95 or later
- cta/CTA#180 - Allow to submit multiple files for verification
- cta/CTA#94 - Remove tape session error codes
+- cta/CTA#198 - Add vid existence check and update usage message for cta-verify-file
### Continuous Integration
- cta/CTA#118 - Add unit tests for OSM label
- cta/CTA#191 - Block merge until cta_valgrind success
### Bug fixes
- cta/CTA#48 - Catch tape server exception and log an error instead
+- cta/CTA#80 - Fix tape thread complete success/failure message parameter
- cta/CTA#123 - Change some tape server errors into warnings
- cta/CTA#161 - Fix bug when using temporary tables with PostgreSQL
- cta/CTA#197 - Include order in XrdSsiCtaRequestMessage.cpp
-- cta/CTA#80 - Fix tape thread complete success/failure message parameter
# v4.7.12-1
diff --git a/catalogue/RdbmsCatalogue.cpp b/catalogue/RdbmsCatalogue.cpp
index 55c929e164..bf003e6d1b 100644
--- a/catalogue/RdbmsCatalogue.cpp
+++ b/catalogue/RdbmsCatalogue.cpp
@@ -9042,7 +9042,7 @@ common::dataStructures::RetrieveFileQueueCriteria RdbmsCatalogue::prepareToRetri
exception::UserError ex;
auto tapeFileStateList = getTapeFileStateListForArchiveFileId(conn, archiveFileId);
if (tapeFileStateList.empty()) {
- ex.getMessage() << "CRITICAL: File with archive file ID " << archiveFileId << " does not exist in CTA namespace";
+ ex.getMessage() << "File with archive file ID " << archiveFileId << " does not exist in CTA namespace";
throw ex;
}
const auto nonBrokenState = std::find_if(std::begin(tapeFileStateList), std::end(tapeFileStateList),
diff --git a/cmdline/standalone_cli_tools/CMakeLists.txt b/cmdline/standalone_cli_tools/CMakeLists.txt
index 18f3c324c4..930aa0f403 100644
--- a/cmdline/standalone_cli_tools/CMakeLists.txt
+++ b/cmdline/standalone_cli_tools/CMakeLists.txt
@@ -42,7 +42,7 @@ set_property(TARGET cta-send-event APPEND PROPERTY INSTALL_RPATH ${PROTOBUF3_RPA
# verify-file <archiveFileId> <vid>
# recalls a file from tape without writing a disk copy
#
-add_executable(cta-verify-file CtaVerifyFile.cpp common/CmdLineArgs.cpp)
+add_executable(cta-verify-file CtaVerifyFile.cpp common/CmdLineArgs.cpp common/CatalogueFetch.cpp)
target_link_libraries(cta-verify-file ctacommon XrdSsiPbEosCta XrdSsiLib XrdUtils)
set_property (TARGET cta-verify-file APPEND PROPERTY INSTALL_RPATH ${PROTOBUF3_RPATH})
diff --git a/cmdline/standalone_cli_tools/CtaVerifyFile.cpp b/cmdline/standalone_cli_tools/CtaVerifyFile.cpp
index f799306ae0..499b0d1659 100644
--- a/cmdline/standalone_cli_tools/CtaVerifyFile.cpp
+++ b/cmdline/standalone_cli_tools/CtaVerifyFile.cpp
@@ -19,32 +19,17 @@
#include <iostream>
#include <map>
+#include "cmdline/standalone_cli_tools/common/CatalogueFetch.hpp"
+#include "cmdline/standalone_cli_tools/common/ConnectionConfiguration.hpp"
#include "common/CmdLineArgs.hpp"
+#include "common/exception/CommandLineNotParsed.hpp"
#include "common/utils/utils.hpp"
#include "CtaFrontendApi.hpp"
#include "version.h"
using namespace cta::cliTool;
-const std::string config_file = "/etc/cta/cta-cli.conf";
-
-// Define XRootD SSI Alert message callback
-namespace XrdSsiPb {
-/*
- * Alert callback.
- *
- * Defines how Alert messages should be logged by EOS (or directed to the User)
- */
-template<>
-void RequestCallback<cta::xrd::Alert>::operator()(const cta::xrd::Alert &alert)
-{
- std::cout << "AlertCallback():" << std::endl;
- XrdSsiPb::Log::DumpProtobuf(XrdSsiPb::Log::PROTOBUF, &alert);
-}
-} // namespace XrdSsiPb
-
-// Attribute map type
-typedef std::map<std::string, std::string> AttrMap;
+const std::string g_config_file = "/etc/cta/cta-cli.conf";
/*
* Fill a Notification message from the command-line parameters and stdin
@@ -54,10 +39,10 @@ typedef std::map<std::string, std::string> AttrMap;
* @param[in] archiveFileId Archive file id to verify
*/
void fillNotification(cta::eos::Notification ¬ification, const CmdLineArgs &cmdLineArgs, const std::string &archiveFileId) {
- XrdSsiPb::Config config(config_file, "eos");
+ XrdSsiPb::Config config(g_config_file, "eos");
for (const auto &conf_option : std::vector<std::string>({ "instance", "requester.user", "requester.group" })) {
if (!config.getOptionValueStr(conf_option).first) {
- throw std::runtime_error(conf_option + " must be specified in " + config_file);
+ throw std::runtime_error(conf_option + " must be specified in " + g_config_file);
}
}
notification.mutable_wf()->mutable_instance()->set_name(config.getOptionValueStr("instance").second);
@@ -87,7 +72,8 @@ void fillNotification(cta::eos::Notification ¬ification, const CmdLineArgs &c
// File
notification.mutable_file()->set_lpath("dummy");
- // eXtended attributes
+ // Attribute map type
+ using AttrMap= std::map<std::string, std::string>;
AttrMap xattrs;
xattrs["sys.archive.file_id"] = archiveFileId;
@@ -97,21 +83,27 @@ void fillNotification(cta::eos::Notification ¬ification, const CmdLineArgs &c
}
}
-void sendVerifyRequest(const CmdLineArgs &cmdLineArgs, const std::string &archiveFileId) {
- std::string vid;
-
- // Verify that the Google Protocol Buffer header and linked library versions are compatible
- GOOGLE_PROTOBUF_VERIFY_VERSION;
-
- cta::xrd::Request request;
- cta::eos::Notification ¬ification = *(request.mutable_notification());
+/*
+ * Checks if the provided vid exists
+ */
+void vidExists(cta::cliTool::CmdLineArgs cmdLineArgs, const XrdSsiPb::Config &config) {
+ std::string hostName = std::getenv("HOSTNAME");
+ if(hostName.empty()) {
+ hostName = "UNKNOWN";
+ }
+ cta::log::StdoutLogger log(hostName, "cta-verify-file");
+ auto serviceProviderPtr = std::make_unique<XrdSsiPbServiceType>(config);
+ auto vidsInCatalogue = CatalogueFetch::getVids(serviceProviderPtr, log);
- // Set client version
- request.set_client_cta_version(CTA_VERSION);
- request.set_client_xrootd_ssi_protobuf_interface_version(XROOTD_SSI_PROTOBUF_INTERFACE_VERSION);
+ std::list<std::string>::iterator findIter = std::find(vidsInCatalogue.begin(), vidsInCatalogue.end(), cmdLineArgs.m_vid);
+ if(vidsInCatalogue.end() == findIter) {
+ throw std::runtime_error("The provided --vid does not exist in the Catalogue.");
+ }
+}
+XrdSsiPb::Config getConfig() {
// Set configuration options
- XrdSsiPb::Config config(config_file, "cta");
+ XrdSsiPb::Config config(g_config_file, "cta");
config.set("resource", "/ctafrontend");
// Allow environment variables to override config file
@@ -124,6 +116,22 @@ void sendVerifyRequest(const CmdLineArgs &cmdLineArgs, const std::string &archiv
// If fine-grained control over log level is required, use XrdSsiPbLogLevel
config.getEnv("log", "XrdSsiPbLogLevel");
+ return config;
+}
+
+void sendVerifyRequest(const CmdLineArgs &cmdLineArgs, const std::string &archiveFileId, const XrdSsiPb::Config &config) {
+ std::string vid;
+
+ // Verify that the Google Protocol Buffer header and linked library versions are compatible
+ GOOGLE_PROTOBUF_VERIFY_VERSION;
+
+ cta::xrd::Request request;
+ cta::eos::Notification ¬ification = *(request.mutable_notification());
+
+ // Set client version
+ request.set_client_cta_version(CTA_VERSION);
+ request.set_client_xrootd_ssi_protobuf_interface_version(XROOTD_SSI_PROTOBUF_INTERFACE_VERSION);
+
// Parse the command line arguments: fill the Notification fields
fillNotification(notification, cmdLineArgs, archiveFileId);
@@ -150,6 +158,7 @@ void sendVerifyRequest(const CmdLineArgs &cmdLineArgs, const std::string &archiv
google::protobuf::ShutdownProtobufLibrary();
}
+
/*
* Sends a Notification to the CTA XRootD SSI server
*/
@@ -163,9 +172,16 @@ int exceptionThrowingMain(int argc, char *const *const argv)
std::vector<std::string> archiveFileIds;
- if((!cmdLineArgs.m_archiveFileId && !cmdLineArgs.m_archiveFileIds) || !cmdLineArgs.m_vid) {
+ if((!cmdLineArgs.m_archiveFileId && !cmdLineArgs.m_archiveFileIds)) {
+ cmdLineArgs.printUsage(std::cout);
+ std::cout << "Missing command-line option: --id or --filename must be provided" << std::endl;
+ throw std::runtime_error("");
+ }
+
+ if(!cmdLineArgs.m_vid) {
cmdLineArgs.printUsage(std::cout);
- throw std::runtime_error("Error: Usage");
+ std::cout << "Missing command-line option: --vid must be provided" << std::endl;
+ throw std::runtime_error("");
}
if(cmdLineArgs.m_archiveFileId) {
@@ -181,14 +197,17 @@ int exceptionThrowingMain(int argc, char *const *const argv)
}
}
+ const XrdSsiPb::Config config = getConfig();
+
+ vidExists(cmdLineArgs, config);
+
for(const auto &archiveFileId : archiveFileIds) {
- sendVerifyRequest(cmdLineArgs, archiveFileId);
+ sendVerifyRequest(cmdLineArgs, archiveFileId, config);
}
return 0;
}
-
/*
* Start here
*/
diff --git a/cmdline/standalone_cli_tools/common/CatalogueFetch.cpp b/cmdline/standalone_cli_tools/common/CatalogueFetch.cpp
index f9c87a8b22..dca8689878 100644
--- a/cmdline/standalone_cli_tools/common/CatalogueFetch.cpp
+++ b/cmdline/standalone_cli_tools/common/CatalogueFetch.cpp
@@ -38,6 +38,7 @@ std::atomic<bool> isHeaderSent = false;
std::list<cta::admin::RecycleTapeFileLsItem> deletedTapeFiles;
std::list<std::pair<std::string,std::string>> listedTapeFiles;
std::list<std::string> g_storageClasses;
+std::list<std::string> g_listedVids;
namespace XrdSsiPb {
@@ -49,7 +50,7 @@ namespace XrdSsiPb {
template<>
void RequestCallback<cta::xrd::Alert>::operator()(const cta::xrd::Alert &alert)
{
- Log::DumpProtobuf(Log::PROTOBUF, &alert);
+ Log::DumpProtobuf(Log::PROTOBUF, &alert);
}
/*!
@@ -86,6 +87,12 @@ void IStreamBuffer<cta::xrd::Data>::DataCallback(cta::xrd::Data record) const
g_storageClasses.push_back(item.name());
}
break;
+ case Data::kTalsItem:
+ {
+ const auto item = record.tals_item();
+ g_listedVids.push_back(item.vid());
+ }
+ break;
default:
throw std::runtime_error("Received invalid stream data from CTA Frontend for the cta-restore-deleted-files command.");
}
@@ -96,13 +103,6 @@ void IStreamBuffer<cta::xrd::Data>::DataCallback(cta::xrd::Data record) const
namespace cta {
namespace cliTool {
- /**
- * Fetches the instance and fid from the CTA catalogue
- *
- * @param archiveFileId The arhive file id.
- * @param serviceProviderPtr Service provider for communication with the catalogue.
- * @return a pair with the instance and the fid.
- */
std::tuple<std::string,std::string> CatalogueFetch::getInstanceAndFid(const std::string& archiveFileId, std::unique_ptr<XrdSsiPbServiceType> &serviceProviderPtr, cta::log::StdoutLogger &log) {
{
std::list<cta::log::Param> params;
@@ -137,6 +137,24 @@ std::tuple<std::string,std::string> CatalogueFetch::getInstanceAndFid(const std:
return listedTapeFile;
}
+std::list<std::string> CatalogueFetch::getVids(std::unique_ptr<XrdSsiPbServiceType> &serviceProviderPtr, cta::log::StdoutLogger &log) {
+ cta::xrd::Request request;
+ auto admincmd = request.mutable_admincmd();
+
+ request.set_client_cta_version(CTA_VERSION);
+ request.set_client_xrootd_ssi_protobuf_interface_version(XROOTD_SSI_PROTOBUF_INTERFACE_VERSION);
+ admincmd->set_cmd(cta::admin::AdminCmd::CMD_TAPE);
+ admincmd->set_subcmd(cta::admin::AdminCmd::SUBCMD_LS);
+
+ auto new_opt = admincmd->add_option_bool();
+ new_opt->set_key(cta::admin::OptionBoolean::ALL);
+ new_opt->set_value(true);
+
+ handleResponse(request, serviceProviderPtr);
+
+ return g_listedVids;
+}
+
void CatalogueFetch::handleResponse(const cta::xrd::Request &request, std::unique_ptr<XrdSsiPbServiceType> &serviceProviderPtr) {
// Send the Request to the Service and get a Response
cta::xrd::Response response;
diff --git a/cmdline/standalone_cli_tools/common/CatalogueFetch.hpp b/cmdline/standalone_cli_tools/common/CatalogueFetch.hpp
index bfd7d6fdd2..112711246b 100644
--- a/cmdline/standalone_cli_tools/common/CatalogueFetch.hpp
+++ b/cmdline/standalone_cli_tools/common/CatalogueFetch.hpp
@@ -37,15 +37,23 @@ namespace cliTool {
class CatalogueFetch {
public:
- /**
- * Fetches the instance and fid from the CTA catalogue
- *
- * @param archiveFileId The arhive file id.
- * @param serviceProviderPtr Service provider for communication with the catalogue.
- * @return a pair with the instance and the fid.
- */
+ /**
+ * Fetches the instance and fid from the CTA catalogue
+ *
+ * @param archiveFileId The arhive file id.
+ * @param serviceProviderPtr Service provider for communication with the catalogue.
+ * @return a pair with the instance and the fid.
+ */
static std::tuple<std::string,std::string> getInstanceAndFid(const std::string& archiveFileId, std::unique_ptr<XrdSsiPbServiceType> &serviceProviderPtr, cta::log::StdoutLogger &log);
+ /**
+ * Fetches the vids form the CTA catalogue
+ *
+ * @param serviceProviderPtr Service provider for communication with the catalogue.
+ * @return A list of vids.
+ */
+ static std::list<std::string> getVids(std::unique_ptr<XrdSsiPbServiceType> &serviceProviderPtr, cta::log::StdoutLogger &log);
+
private:
static void handleResponse(const cta::xrd::Request &request, std::unique_ptr<XrdSsiPbServiceType> &serviceProviderPtr);
};
diff --git a/cmdline/standalone_cli_tools/common/CmdLineArgs.cpp b/cmdline/standalone_cli_tools/common/CmdLineArgs.cpp
index 22bbb28be7..4413ea365c 100644
--- a/cmdline/standalone_cli_tools/common/CmdLineArgs.cpp
+++ b/cmdline/standalone_cli_tools/common/CmdLineArgs.cpp
@@ -21,6 +21,7 @@
#include <climits>
#include <fstream>
#include <getopt.h>
+#include <iostream>
#include <map>
#include <string>
@@ -175,18 +176,18 @@ m_help(false), m_debug(false), m_standaloneCliTool{standaloneCliTool} {
}
case '?': // Unknown option
{
- exception::CommandLineNotParsed ex;
+ exception::CommandLineNotParsed ex("", false);
if(0 == optopt) {
ex.getMessage() << "Unknown command-line option" << std::endl;
} else {
ex.getMessage() << "Unknown command-line option: -" << static_cast<char>(optopt) << std::endl;
}
- printUsage(ex.getMessage());
+ printUsage(std::cout);
throw ex;
}
default:
{
- exception::CommandLineNotParsed ex;
+ exception::CommandLineNotParsed ex("", false);
ex.getMessage() <<
"getopt_long returned the following unknown value: 0x" <<
std::hex << static_cast<int>(optopt);
@@ -239,30 +240,23 @@ void CmdLineArgs::printUsage(std::ostream &os) const {
os << " Usage:" << std::endl <<
" cta-restore-deleted-files [--id/-I <archive_file_id>] [--instance/-i <disk_instance>]" << std::endl <<
" [--fxid/-f <eos_fxid>] [--fxidfile/-F <filename>]" << std::endl <<
- " [--vid/-v <vid>] [--copynb/-c <copy_number>] [--debug/-d]" << std::endl;
+ " [--vid/-v <vid>] [--copynb/-c <copy_number>] [--debug/-d]" << std::endl << std::endl;
break;
case StandaloneCliTool::CTA_SEND_EVENT:
os << " Usage:" << std::endl <<
" eos --json fileinfo /eos/path | cta-send-event CLOSEW|PREPARE " << std::endl <<
" -i/--eos.instance <instance> [-e/--eos.endpoint <url>]" << std::endl <<
- " -u/--request.user <user> -g/--request.group <group>" << std::endl;
+ " -u/--request.user <user> -g/--request.group <group>" << std::endl << std::endl;
break;
case StandaloneCliTool::CTA_VERIFY_FILE :
- os << " Usage:" << std::endl <<
- " cta-verify-file --id/-I <archiveFileID,archiveFileID,...,archiveFileID> | --filename/-F <filename> " << std::endl <<
- " --vid/-v <vid>" << std::endl <<
- " [--instance/-i <instance>]" << std::endl <<
- " [--request.user/-u <user>]" << std::endl <<
- " [request.group/-g <group>]" << std::endl << std::endl <<
- " If a filename is used to provide archive file ids, it should be following the format:" << std::endl << std::endl <<
- " <archiveFileId_1>" << std::endl <<
- " <archiveFileId_2>" << std::endl <<
- " ..." << std::endl <<
- " <archiveFileId_n>" << std::endl;
+ os <<
+ "cta-verify-file --vid/-v <vid> [--id/-I <archiveFileID,...>] [--filename/-F <filename>] [--instance/-i <instance>] [--request.user/-u <user>] [request.group/-g <group>]" << std::endl << std::endl <<
+ " Submit a verification request for a given list of archive file IDs on a given tape." << std::endl <<
+ " * If the --filename option is used, each line in the provided file should contain exactly one <archiveFileID>." << std::endl << std::endl;
break;
case StandaloneCliTool::CTA_CHANGE_STORAGE_CLASS :
os << " Usage:" << std::endl <<
- " cta-change-storage-class --id/-I <archiveFileID> | --filename/-F <filename> --storage.class.name/-n <storageClassName> [--frequenzy/-t <eosRequestFrequency>]" << std::endl;
+ " cta-change-storage-class --id/-I <archiveFileID> | --filename/-F <filename> --storage.class.name/-n <storageClassName> [--frequenzy/-t <eosRequestFrequency>]" << std::endl << std::endl;
break;
default:
break;
--
GitLab