Remove legacy storage node root directory IO in unit tests

Once upon a time, VDS roamed the lands. It used real disk IO as
part of tests. Then came the meteor and in-memory dummy persistence
took over. Now it is time for the fossils to be moved into a museum
where they belong.

Also make PID file writing conditional on a config that is set to
`false` during unit testing (but `true` as default).

9 files changed, 34 insertions, 61 deletions

diff --git a/storage/src/tests/bucketdb/bucketmanagertest.cpp b/storage/src/tests/bucketdb/bucketmanagertest.cpp
index 92547a83d25..b5501d38241 100644
--- a/storage/src/tests/bucketdb/bucketmanagertest.cpp
+++ b/storage/src/tests/bucketdb/bucketmanagertest.cpp
@@ -69,8 +69,7 @@ public:
 
     ~BucketManagerTest() override;
 
-    void setupTestEnvironment(bool fakePersistenceLayer = true,
-                              bool noDelete = false);
+    void setupTestEnvironment();
     void addBucketsToDB(uint32_t count);
     bool wasBlockedDueToLastModified(api::StorageMessage* msg, uint64_t lastModified);
     void insertSingleBucket(const document::BucketId& bucket, const api::BucketInfo& info);
@@ -131,15 +130,9 @@ std::string getMkDirDisk(const std::string & rootFolder, int disk) {
     return os.str();
 }
 
-void BucketManagerTest::setupTestEnvironment(bool fakePersistenceLayer, bool noDelete)
+void BucketManagerTest::setupTestEnvironment()
 {
     vdstestlib::DirConfig config(getStandardConfig(true, "bucketmanagertest"));
-    std::string rootFolder = getRootFolder(config);
-    if (!noDelete) {
-        assert(system(("rm -rf " + rootFolder).c_str()) == 0);
-    }
-    assert(system(getMkDirDisk(rootFolder, 0).c_str()) == 0);
-    assert(system(getMkDirDisk(rootFolder, 1).c_str()) == 0);
 
     auto repo = std::make_shared<const DocumentTypeRepo>(
                 *ConfigGetter<DocumenttypesConfig>::getConfig("config-doctypes", FileSpec("../config-doctypes.cfg")));
@@ -153,18 +146,10 @@ void BucketManagerTest::setupTestEnvironment(bool fakePersistenceLayer, bool noD
     auto manager = std::make_unique<BucketManager>(*config_from<StorServerConfig>(config_uri), _node->getComponentRegister());
     _manager = manager.get();
     _top->push_back(std::move(manager));
-    if (fakePersistenceLayer) {
-        auto bottom = std::make_unique<DummyStorageLink>();
-        _bottom = bottom.get();
-        _top->push_back(std::move(bottom));
-    } else {
-        using StorFilestorConfig = vespa::config::content::internal::InternalStorFilestorType;
-        auto bottom = std::make_unique<FileStorManager>(*config_from<StorFilestorConfig>(config_uri),
-                                                        _node->getPersistenceProvider(), _node->getComponentRegister(),
-                                                        *_node, _node->get_host_info());
-        _top->push_back(std::move(bottom));
-    }
-    // Generate a doc to use for testing..
+    auto bottom = std::make_unique<DummyStorageLink>();
+    _bottom = bottom.get();
+    _top->push_back(std::move(bottom));
+
     const DocumentType &type(*_node->getTypeRepo()->getDocumentType("text/html"));
     _document = std::make_shared<document::Document>(*_node->getTypeRepo(), type, document::DocumentId("id:ns:text/html::ntnu"));
 }
diff --git a/storage/src/tests/common/metricstest.cpp b/storage/src/tests/common/metricstest.cpp
index 899c1979e86..7216bef03db 100644
--- a/storage/src/tests/common/metricstest.cpp
+++ b/storage/src/tests/common/metricstest.cpp
@@ -67,16 +67,12 @@ MetricsTest::~MetricsTest() = default;
 
 void MetricsTest::SetUp() {
     _config = std::make_unique<vdstestlib::DirConfig>(getStandardConfig(true, "metricstest"));
-    std::filesystem::remove_all(std::filesystem::path(getRootFolder(*_config)));
-    try {
-        _node = std::make_unique<TestServiceLayerApp>(NodeIndex(0), _config->getConfigId());
-        _node->setupDummyPersistence();
-        _clock = &_node->getClock();
-        _clock->setAbsoluteTimeInSeconds(1000000);
-        _top = std::make_unique<DummyStorageLink>();
-    } catch (config::InvalidConfigException& e) {
-        fprintf(stderr, "%s\n", e.what());
-    }
+    _node = std::make_unique<TestServiceLayerApp>(NodeIndex(0), _config->getConfigId());
+    _node->setupDummyPersistence();
+    _clock = &_node->getClock();
+    _clock->setAbsoluteTimeInSeconds(1000000);
+    _top = std::make_unique<DummyStorageLink>();
+
     _metricManager = std::make_unique<metrics::MetricManager>(std::make_unique<MetricClock>(*_clock));
     _topSet.reset(new metrics::MetricSet("vds", {}, ""));
     {
diff --git a/storage/src/tests/common/testhelper.cpp b/storage/src/tests/common/testhelper.cpp
index f295d955ab2..91758b894b0 100644
--- a/storage/src/tests/common/testhelper.cpp
+++ b/storage/src/tests/common/testhelper.cpp
@@ -68,6 +68,7 @@ vdstestlib::DirConfig getStandardConfig(bool storagenode, const std::string & ro
     rootFolder += (storagenode ? "vdsroot" : "vdsroot.distributor");
     config->set("root_folder", rootFolder);
     config->set("is_distributor", (storagenode ? "false" : "true"));
+    config->set("write_pid_file_on_startup", "false");
     config = &dc.addConfig("stor-devices");
     config->set("root_folder", rootFolder);
     config = &dc.addConfig("stor-status");
diff --git a/storage/src/tests/persistence/persistencetestutils.cpp b/storage/src/tests/persistence/persistencetestutils.cpp
index a599b1d380a..4a2ee987362 100644
--- a/storage/src/tests/persistence/persistencetestutils.cpp
+++ b/storage/src/tests/persistence/persistencetestutils.cpp
@@ -26,11 +26,7 @@ namespace storage {
 namespace {
 
 vdstestlib::DirConfig initialize(const std::string & rootOfRoot) {
-    vdstestlib::DirConfig config(getStandardConfig(true, rootOfRoot));
-    std::string rootFolder = getRootFolder(config);
-    std::filesystem::remove_all(std::filesystem::path(rootFolder));
-    std::filesystem::create_directories(std::filesystem::path(vespalib::make_string("%s/disks/d0", rootFolder.c_str())));
-    return config;
+    return getStandardConfig(true, rootOfRoot);
 }
 
 template<typename T>
diff --git a/storage/src/tests/storageserver/statereportertest.cpp b/storage/src/tests/storageserver/statereportertest.cpp
index 43eb37afe15..c233c6e9314 100644
--- a/storage/src/tests/storageserver/statereportertest.cpp
+++ b/storage/src/tests/storageserver/statereportertest.cpp
@@ -69,7 +69,6 @@ StateReporterTest::~StateReporterTest() = default;
 
 void StateReporterTest::SetUp() {
     _config = std::make_unique<vdstestlib::DirConfig>(getStandardConfig(true, "statereportertest"));
-    assert(system(("rm -rf " + getRootFolder(*_config)).c_str()) == 0);
 
     _node = std::make_unique<TestServiceLayerApp>(NodeIndex(0), _config->getConfigId());
     _node->setupDummyPersistence();
diff --git a/storage/src/tests/visiting/visitortest.cpp b/storage/src/tests/visiting/visitortest.cpp
index f83b6c99d64..29a6c1baeb4 100644
--- a/storage/src/tests/visiting/visitortest.cpp
+++ b/storage/src/tests/visiting/visitortest.cpp
@@ -155,13 +155,6 @@ VisitorTest::initializeTest(const TestParams& params)
             "visitor_memory_usage_limit",
             std::to_string(params._maxVisitorMemoryUsage));
 
-    std::string rootFolder = getRootFolder(config);
-
-    ::chmod(rootFolder.c_str(), 0755);
-    std::filesystem::remove_all(std::filesystem::path(rootFolder));
-    std::filesystem::create_directories(std::filesystem::path(vespalib::make_string("%s/disks/d0", rootFolder.c_str())));
-    std::filesystem::create_directories(std::filesystem::path(vespalib::make_string("%s/disks/d1", rootFolder.c_str())));
-
     _messageSessionFactory = std::make_unique<TestVisitorMessageSessionFactory>();
     if (params._autoReplyError.getCode() != mbus::ErrorCode::NONE) {
         _messageSessionFactory->_autoReplyError = params._autoReplyError;
@@ -217,14 +210,12 @@ VisitorTest::initializeTest(const TestParams& params)
     _documents.clear();
     for (uint32_t i=0; i<docCount; ++i) {
         std::ostringstream uri;
-        uri << "id:test:testdoctype1:n=" << i % 10 << ":http://www.ntnu.no/"
-            << i << ".html";
+        uri << "id:test:testdoctype1:n=" << i % 10 << ":http://www.ntnu.no/" << i << ".html";
 
         _documents.push_back(Document::SP(
                 _node->getTestDocMan().createDocument(content, uri.str())));
         const document::DocumentType& type(_documents.back()->getType());
-        _documents.back()->setValue(type.getField("headerval"),
-                                    document::IntFieldValue(i % 4));
+        _documents.back()->setValue(type.getField("headerval"), document::IntFieldValue(i % 4));
     }
 }
 
diff --git a/storage/src/vespa/storage/config/stor-server.def b/storage/src/vespa/storage/config/stor-server.def
index 8cd204bcf9f..44e4b14eafc 100644
--- a/storage/src/vespa/storage/config/stor-server.def
+++ b/storage/src/vespa/storage/config/stor-server.def
@@ -114,3 +114,7 @@ simulated_bucket_request_latency_msec int default=0
 ## a disjoint subset of the node's buckets, in order to reduce locking contention.
 ## Max value is unspecified, but will be clamped internally.
 content_node_bucket_db_stripe_bits int default=4 restart
+
+## Iff set, a special `pidfile` file is written under the node's root directory upon
+## startup containing the PID of the running process.
+write_pid_file_on_startup bool default=true
diff --git a/storage/src/vespa/storage/storageserver/storagenode.cpp b/storage/src/vespa/storage/storageserver/storagenode.cpp
index f7a426a0527..35b70dd853c 100644
--- a/storage/src/vespa/storage/storageserver/storagenode.cpp
+++ b/storage/src/vespa/storage/storageserver/storagenode.cpp
@@ -140,7 +140,7 @@ StorageNode::initialize(const NodeStateReporter & nodeStateReporter)
     // Initializing state manager early, as others use it init time to
     // update node state according min used bits etc.
     // Needs node type to be set right away. Needs thread pool, index and
-    // dead lock detector too, but not before open()
+    // deadlock detector too, but not before open()
     _stateManager = std::make_unique<StateManager>(
             _context.getComponentRegister(),
             std::move(_hostInfo),
@@ -148,10 +148,10 @@ StorageNode::initialize(const NodeStateReporter & nodeStateReporter)
             _singleThreadedDebugMode);
     _context.getComponentRegister().setNodeStateUpdater(*_stateManager);
 
-    // Create VDS root folder, in case it doesn't already exist.
-    // Maybe better to rather fail if it doesn't exist, but tests
-    // might break if we do that. Might alter later.
-    std::filesystem::create_directories(std::filesystem::path(_rootFolder));
+    // Create storage root folder, in case it doesn't already exist.
+    if (!_rootFolder.empty()) {
+        std::filesystem::create_directories(std::filesystem::path(_rootFolder));
+    } // else: running as part of unit tests
 
     initializeNodeSpecific();
 
@@ -192,13 +192,16 @@ StorageNode::initialize(const NodeStateReporter & nodeStateReporter)
 
     initializeStatusWebServer();
 
+    if (server_config().writePidFileOnStartup) {
+        assert(!_rootFolder.empty());
         // Write pid file as the last thing we do. If we fail initialization
         // due to an exception we won't run shutdown. Thus we won't remove the
         // pid file if something throws after writing it in initialization.
         // Initialize _pidfile here, such that we can know that we didn't create
         // it in shutdown code for shutdown during init.
-    _pidFile = _rootFolder + "/pidfile";
-    writePidFile(_pidFile);
+        _pidFile = _rootFolder + "/pidfile";
+        writePidFile(_pidFile);
+    }
 }
 
 void
diff --git a/storageserver/src/tests/testhelper.cpp b/storageserver/src/tests/testhelper.cpp
index 40e263d4e68..e3b047ddfd1 100644
--- a/storageserver/src/tests/testhelper.cpp
+++ b/storageserver/src/tests/testhelper.cpp
@@ -62,13 +62,11 @@ vdstestlib::DirConfig getStandardConfig(bool storagenode) {
     config->set("enable_dead_lock_detector_warnings", "false");
     config->set("max_merges_per_node", "25");
     config->set("max_merge_queue_size", "20");
-    config->set("root_folder",
-                    (storagenode ? "vdsroot" : "vdsroot.distributor"));
-    config->set("is_distributor",
-                    (storagenode ? "false" : "true"));
+    config->set("root_folder", (storagenode ? "vdsroot" : "vdsroot.distributor"));
+    config->set("is_distributor", (storagenode ? "false" : "true"));
+    config->set("write_pid_file_on_startup", "false");
     config = &dc.addConfig("stor-devices");
-    config->set("root_folder",
-                    (storagenode ? "vdsroot" : "vdsroot.distributor"));
+    config->set("root_folder", (storagenode ? "vdsroot" : "vdsroot.distributor"));
     config = &dc.addConfig("stor-status");
     config->set("httpport", "0");
     config = &dc.addConfig("stor-visitor");