From 0f065864c47b39612e49acc63bce22a9031bae8a Mon Sep 17 00:00:00 2001 From: Patrick LeBlanc Date: Wed, 28 Aug 2019 12:59:09 -0500 Subject: [PATCH] Started writing unit tests to verify SM does the right thing when given short data (sender crashed). --- storage-manager/src/Config.cpp | 23 ++++++ storage-manager/src/Config.h | 4 + storage-manager/src/ListDirectoryTask.cpp | 2 +- storage-manager/src/unit_tests.cpp | 89 ++++++++++++++++++++++- 4 files changed, 114 insertions(+), 4 deletions(-) diff --git a/storage-manager/src/Config.cpp b/storage-manager/src/Config.cpp index cfd7dfcd1..dca61f938 100644 --- a/storage-manager/src/Config.cpp +++ b/storage-manager/src/Config.cpp @@ -31,6 +31,7 @@ #include "SMLogging.h" +namespace bf = boost::filesystem; using namespace std; namespace @@ -53,6 +54,17 @@ Config * Config::get() return inst; } +Config * Config::get(const string &configFile) +{ + if (inst) + return inst; + boost::mutex::scoped_lock s(m); + if (inst) + return inst; + inst = new Config(configFile); + return inst; +} + Config::Config() : die(false) { /* This will search the current directory, @@ -91,6 +103,17 @@ Config::Config() : die(false) reloader = boost::thread([this] { this->reloadThreadFcn(); }); } +Config::Config(const string &configFile) : filename(configFile), die(false) +{ + if (!bf::is_regular_file(configFile)) + throw runtime_error("Config: Could not find the config file for StorageManager"); + + reloadInterval = boost::posix_time::seconds(60); + last_mtime = {0, 0}; + reload(); + reloader = boost::thread([this] { this->reloadThreadFcn(); }); +} + Config::~Config() { die = true; diff --git a/storage-manager/src/Config.h b/storage-manager/src/Config.h index 869347e72..f746f72b2 100644 --- a/storage-manager/src/Config.h +++ b/storage-manager/src/Config.h @@ -37,8 +37,12 @@ class Config : public boost::noncopyable std::string getValue(const std::string §ion, const std::string &key) const; + // for testing, lets caller specify a config file to use + static Config *get(const std::string &); + private: Config(); + Config(const std::string &); void reload(); void reloadThreadFcn(); diff --git a/storage-manager/src/ListDirectoryTask.cpp b/storage-manager/src/ListDirectoryTask.cpp index 96e3be250..3bfee2f22 100644 --- a/storage-manager/src/ListDirectoryTask.cpp +++ b/storage-manager/src/ListDirectoryTask.cpp @@ -116,7 +116,7 @@ bool ListDirectoryTask::run() sm_response *resp = (sm_response *) buf; resp->header.type = SM_MSG_START; - resp->header.payloadLen = payloadLen + sizeof(sm_response) - sizeof(sm_msg_header); // the +4 is for the length of the return code + resp->header.payloadLen = payloadLen + sizeof(sm_response) - sizeof(sm_msg_header); resp->header.flags = 0; resp->returnCode = 0; listdir_resp *r = (listdir_resp *) resp->payload; diff --git a/storage-manager/src/unit_tests.cpp b/storage-manager/src/unit_tests.cpp index 27d010bf5..e2ecdbef0 100644 --- a/storage-manager/src/unit_tests.cpp +++ b/storage-manager/src/unit_tests.cpp @@ -1582,6 +1582,68 @@ void bigMergeJournal1() assert(buf); } + +// This should write an incomplete msg(s) to make sure SM does the right thing. Not +// done yet, handing this off to Ben. +void shortWriteMsg() +{ + // copy/modified/pasted from writetask(). + bf::path fullPath = homepath / prefix / "writetest1"; + const char *filename = fullPath.string().c_str(); + ::unlink(filename); + int fd = ::open(filename, O_CREAT | O_RDWR, 0666); + assert(fd > 0); + scoped_closer f(fd); + + uint8_t buf[1024]; + sm_msg_header *hdr = (sm_msg_header *) buf; + write_cmd *cmd = (write_cmd *) &hdr[1]; + uint8_t *data; + + cmd->opcode = WRITE; + cmd->offset = 0; + cmd->count = 9; + cmd->flen = 10; + memcpy(&cmd->filename, filename, cmd->flen); + data = (uint8_t *) &cmd->filename[cmd->flen]; + memcpy(data, "123456789", cmd->count); + + hdr->type = SM_MSG_START; + hdr->payloadLen = sizeof(*cmd) + cmd->flen + cmd->count; + + WriteTask w(clientSock, hdr->payloadLen); + ssize_t result = ::write(sessionSock, cmd, hdr->payloadLen); + assert(result==(hdr->payloadLen)); + + w.run(); + + // verify response + int err = ::recv(sessionSock, buf, 1024, MSG_DONTWAIT); + sm_response *resp = (sm_response *) buf; + assert(err == sizeof(*resp)); + assert(resp->header.type == SM_MSG_START); + assert(resp->header.payloadLen == sizeof(ssize_t)); + assert(resp->header.flags == 0); + assert(resp->returnCode == 9); + + //check file contents + err = ::read(fd, buf, 1024); + assert(err == 9); + buf[9] = 0; + assert(!strcmp("123456789", (const char *) buf)); + ::unlink(filename); + cout << "write task OK" << endl; +} + +// write and append are the biggest vulnerabilities here b/c those msgs could be sent in multiple +// pieces, are much larger, and thus if there is a crash mid-message it's most likely to happen +// during a call to write/append(). +// it may not even be possible for CS to write a partial open/stat/read/etc msg, but that should be +// tested as well. +void shortMsgTests() +{ + shortWriteMsg(); +} int main(int argc, char* argv[]) { @@ -1612,6 +1674,18 @@ int main(int argc, char* argv[]) } } + if (!bf::is_regular_file("test_data/storagemanager.cnf")) + { + cerr << "This should be run in a dir where ./test_data/storagemanager.cnf exists" << endl; + exit(1); + } + Config *config = Config::get("test_data/storagemanager.cnf"); + cout << "Cleaning out debris from previous runs" << endl; + bf::remove_all(config->getValue("ObjectStorage", "metadata_path")); + bf::remove_all(config->getValue("ObjectStorage", "journal_path")); + bf::remove_all(config->getValue("LocalStorage", "path")); + bf::remove_all(config->getValue("Cache", "path")); + cout << "connecting" << endl; makeConnection(); cout << "connected" << endl; @@ -1639,12 +1713,11 @@ int main(int argc, char* argv[]) // this starts in one object and crosses into new object metadataJournalTest_append((7*sizeKB)); - //writetask(); //appendtask(); unlinktask(); stattask(); - truncatetask(); // currently waiting on IOC::write() to be completed. + truncatetask(); listdirtask(); pingtask(); copytask(); @@ -1655,16 +1728,26 @@ int main(int argc, char* argv[]) replicatorTest(); syncTest1(); - s3storageTest1(); IOCReadTest1(); IOCTruncate(); IOCUnlink(); IOCCopyFile(); + //shortMsgTests(); // For the moment, this next one just verifies no error happens as reported by the fcns called. // It doesn't verify the result yet. bigMergeJournal1(); + // skip the s3 test if s3 is not configured + if (config->getValue("S3", "region") != "") + { + s3storageTest1(); + } + else + cout << "To run the S3Storage unit tests, configure the S3 section of test-data/storagemanager.cnf" + << endl; + + metadataJournalTestCleanup(); (Cache::get())->shutdown();