be more careful about splunk start/stop

* validate the configurable splunk-home
* do stopping via C++ program instead of shell command
* some refactoring of C++ code as well

10 files changed, 208 insertions, 90 deletions

diff --git a/logforwarder/src/apps/vespa-logforwarder-start/CMakeLists.txt b/logforwarder/src/apps/vespa-logforwarder-start/CMakeLists.txt
index 74ca0ff6abc..eb19ed3e45c 100644
--- a/logforwarder/src/apps/vespa-logforwarder-start/CMakeLists.txt
+++ b/logforwarder/src/apps/vespa-logforwarder-start/CMakeLists.txt
@@ -2,6 +2,8 @@
 vespa_add_executable(logforwarder-start_app
     SOURCES
     main.cpp
+    splunk-stopper.cpp
+    splunk-starter.cpp
     cf-handler.cpp
     child-handler.cpp
     OUTPUT_NAME vespa-logforwarder-start
diff --git a/logforwarder/src/apps/vespa-logforwarder-start/cf-handler.cpp b/logforwarder/src/apps/vespa-logforwarder-start/cf-handler.cpp
index b8e2a95783e..5d17357e74e 100644
--- a/logforwarder/src/apps/vespa-logforwarder-start/cf-handler.cpp
+++ b/logforwarder/src/apps/vespa-logforwarder-start/cf-handler.cpp
@@ -4,100 +4,44 @@
 #include <vespa/config/common/configsystem.h>
 #include <vespa/config/common/exceptions.h>
 #include <vespa/config/subscription/configsubscriber.hpp>
-#include <dirent.h>
+
+#include <sys/types.h>
 #include <sys/stat.h>
+#include <unistd.h>
 
 #include <vespa/log/log.h>
 LOG_SETUP(".cf-handler");
 
-
 CfHandler::CfHandler() = default;
 
 CfHandler::~CfHandler() = default;
 
-void
-CfHandler::subscribe(const std::string & configId, std::chrono::milliseconds timeout)
-{
+void CfHandler::subscribe(const std::string & configId, std::chrono::milliseconds timeout) {
     _handle = _subscriber.subscribe<LogforwarderConfig>(configId, timeout);
 }
 
 namespace {
-
-bool fixDir(const vespalib::string &path) {
-    if (path.size() == 0) return true;
-    size_t lastSlash = path.rfind('/');
-    if (lastSlash != vespalib::string::npos) {
-        vespalib::string parent = path.substr(0, lastSlash);
-        if (!fixDir(parent)) return false;
+bool isExecutable(const char *path) {
+    struct stat statbuf;
+    if (stat(path, &statbuf) != 0) {
+        return false;
     }
-    DIR *dp = opendir(path.c_str());
-    if (dp == NULL) {
-        if (errno != ENOENT || mkdir(path.c_str(), 0755) != 0) {
-            perror(path.c_str());
-            return false;
-        }
-    } else {
-        closedir(dp);
+    if (! S_ISREG(statbuf.st_mode)) {
+        return false;
     }
-    return true;
+    return ((statbuf.st_mode & S_IXOTH) != 0);
 }
-
-vespalib::string
-cfFilePath(const vespalib::string &parent, const vespalib::string &filename) {
-    vespalib::string path = parent + "/etc/system/local";
-    fixDir(path);
-    path += "/";
-    path += filename;
-    return path;
 }
 
-} // namespace <unnamed>
-
-void
-CfHandler::doConfigure()
-{
+void CfHandler::doConfigure() {
     std::unique_ptr<LogforwarderConfig> cfg(_handle->getConfig());
     const LogforwarderConfig& config(*cfg);
-
-    vespalib::string path = cfFilePath(config.splunkHome, "deploymentclient.conf");
-    vespalib::string tmpPath = path + ".new";
-    FILE *fp = fopen(tmpPath.c_str(), "w");
-    if (fp == NULL) return;
-
-    fprintf(fp, "[deployment-client]\n");
-    fprintf(fp, "clientName = %s\n", config.clientName.c_str());
-    fprintf(fp, "phoneHomeIntervalInSecs = %i\n", config.phoneHomeInterval);
-    fprintf(fp, "\n");
-    fprintf(fp, "[target-broker:deploymentServer]\n");
-    fprintf(fp, "targetUri = %s\n", config.deploymentServer.c_str());
-
-    fclose(fp);
-    rename(tmpPath.c_str(), path.c_str());
-
-    if (getenv("VESPA_HOSTNAME") != NULL &&
-        getenv("VESPA_TENANT") != NULL &&
-        getenv("VESPA_APPLICATION")!= NULL &&
-        getenv("VESPA_INSTANCE") != NULL &&
-        getenv("VESPA_ENVIRONMENT") != NULL &&
-        getenv("VESPA_REGION") != NULL)
-    {
-        path = cfFilePath(config.splunkHome, "inputs.conf");
-        tmpPath = path + ".new";
-        fp = fopen(tmpPath.c_str(), "w");
-        if (fp != NULL) {
-            fprintf(fp, "[default]\n");
-            fprintf(fp, "host = %s\n", getenv("VESPA_HOSTNAME"));
-            fprintf(fp, "_meta = vespa_tenant::%s vespa_app::%s.%s vespa_zone::%s.%s\n", getenv("VESPA_TENANT"), getenv("VESPA_APPLICATION"), getenv("VESPA_INSTANCE"), getenv("VESPA_ENVIRONMENT"), getenv("VESPA_REGION"));
-            fclose(fp);
-            rename(tmpPath.c_str(), path.c_str());
-        }
-    }
-    if (config.clientName.size() == 0 ||
-        config.deploymentServer.size() == 0)
-    {
-        _childHandler.stopChild();
+    LOG(debug, "validating splunk home '%s'", config.splunkHome.c_str());
+    auto program = config.splunkHome + "/bin/splunk";
+    if (isExecutable(program.c_str())) {
+        gotConfig(config);
     } else {
-        _childHandler.startChild(config.splunkHome);
+        LOG(warning, "invalid splunk home, '%s' is not an executable", program.c_str());
     }
 }
 
@@ -109,10 +53,6 @@ CfHandler::check()
     }
 }
 
-void CfHandler::stop() {
-    _childHandler.stopChild();
-}
-
 constexpr std::chrono::milliseconds CONFIG_TIMEOUT_MS(30 * 1000);
 
 void
diff --git a/logforwarder/src/apps/vespa-logforwarder-start/cf-handler.h b/logforwarder/src/apps/vespa-logforwarder-start/cf-handler.h
index 651b8d22fa8..c66be0e2099 100644
--- a/logforwarder/src/apps/vespa-logforwarder-start/cf-handler.h
+++ b/logforwarder/src/apps/vespa-logforwarder-start/cf-handler.h
@@ -1,7 +1,6 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 #pragma once
 
-#include "child-handler.h"
 #include <vespa/config-logforwarder.h>
 #include <vespa/config/subscription/configsubscriber.h>
 
@@ -9,7 +8,6 @@ using cloud::config::LogforwarderConfig;
 
 class CfHandler {
 private:
-    ChildHandler _childHandler;
     config::ConfigSubscriber _subscriber;
     config::ConfigHandle<LogforwarderConfig>::UP _handle;
     void subscribe(const std::string & configId, std::chrono::milliseconds timeout);
@@ -18,6 +16,7 @@ public:
     CfHandler();
     virtual ~CfHandler();
     void start(const char *configId);
-    void stop();
     void check();
+
+    virtual void gotConfig(const LogforwarderConfig&) = 0;
 };
diff --git a/logforwarder/src/apps/vespa-logforwarder-start/child-handler.cpp b/logforwarder/src/apps/vespa-logforwarder-start/child-handler.cpp
index a478e0bbd01..301c5055272 100644
--- a/logforwarder/src/apps/vespa-logforwarder-start/child-handler.cpp
+++ b/logforwarder/src/apps/vespa-logforwarder-start/child-handler.cpp
@@ -76,6 +76,7 @@ runSplunk(const vespalib::string &prefix, std::vector<const char *> args)
 void
 ChildHandler::startChild(const vespalib::string &prefix)
 {
+    LOG(debug, "startChild '%s'", prefix.c_str());
     if (_childRunning && prefix == _runningPrefix) {
         runSplunk(prefix, {"restart"});
     } else {
@@ -93,9 +94,17 @@ ChildHandler::startChild(const vespalib::string &prefix)
     }
 }
 
-void
-ChildHandler::stopChild()
-{
-    runSplunk(_runningPrefix, {"stop"});
+void ChildHandler::stopChild() {
+    if (_runningPrefix != "") {
+        LOG(debug, "stopChild '%s'", _runningPrefix.c_str());
+        runSplunk(_runningPrefix, {"stop"});
+    }
     _childRunning = false;
 }
+
+void
+ChildHandler::stopChild(const vespalib::string &prefix) {
+    stopChild();
+    _runningPrefix = prefix;
+    stopChild();
+}
diff --git a/logforwarder/src/apps/vespa-logforwarder-start/child-handler.h b/logforwarder/src/apps/vespa-logforwarder-start/child-handler.h
index 22396a0e448..f512d241c71 100644
--- a/logforwarder/src/apps/vespa-logforwarder-start/child-handler.h
+++ b/logforwarder/src/apps/vespa-logforwarder-start/child-handler.h
@@ -10,5 +10,6 @@ private:
 public:
     void startChild(const vespalib::string &prefix);
     void stopChild();
+    void stopChild(const vespalib::string &prefix);
     ChildHandler();
 };
diff --git a/logforwarder/src/apps/vespa-logforwarder-start/main.cpp b/logforwarder/src/apps/vespa-logforwarder-start/main.cpp
index 33fe4489811..d24c9c982a1 100644
--- a/logforwarder/src/apps/vespa-logforwarder-start/main.cpp
+++ b/logforwarder/src/apps/vespa-logforwarder-start/main.cpp
@@ -6,7 +6,8 @@
 #include <vespa/log/log.h>
 LOG_SETUP("vespa-logforwarder-start");
 
-#include "cf-handler.h"
+#include "splunk-starter.h"
+#include "splunk-stopper.h"
 #include <vespa/vespalib/util/sig_catch.h>
 
 class Wrapper {
@@ -15,7 +16,7 @@ public:
     Wrapper(const char *cfid) : _configId(cfid) {}
     void run() {
         vespalib::SigCatch catcher;
-        CfHandler handler;
+        SplunkStarter handler;
         handler.start(_configId);
         while (! catcher.receivedStopSignal()) {
             handler.check();
@@ -28,12 +29,32 @@ public:
 int
 main(int argc, char** argv)
 {
-    int c = getopt(argc, argv, "c:");
-    if (c != 'c') {
+    int c = -1;
+    bool stopMode = false;
+    const char *cfid = nullptr;
+    while ((c = getopt(argc, argv, "Sc:")) != -1) {
+        switch (c) {
+        case 'S':
+            stopMode = true;
+            break;
+        case 'c':
+            cfid = optarg;
+            break;
+        default:
+            cfid = nullptr;
+            break;
+        }
+    }
+    if (cfid == nullptr) {
         LOG(error, "Usage: %s -c <config-id>", argv[0]);
         return EXIT_FAILURE;
     }
-    Wrapper wrapper(optarg);
-    wrapper.run();
+    if (stopMode) {
+        SplunkStopper stopper(cfid);
+        stopper.check();
+    } else {
+        Wrapper wrapper(cfid);
+        wrapper.run();
+    }
     return 0;
 }
diff --git a/logforwarder/src/apps/vespa-logforwarder-start/splunk-starter.cpp b/logforwarder/src/apps/vespa-logforwarder-start/splunk-starter.cpp
new file mode 100644
index 00000000000..905f4640c92
--- /dev/null
+++ b/logforwarder/src/apps/vespa-logforwarder-start/splunk-starter.cpp
@@ -0,0 +1,95 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#include "splunk-starter.h"
+#include <dirent.h>
+#include <sys/stat.h>
+
+#include <vespa/log/log.h>
+LOG_SETUP(".splunk-starter");
+
+SplunkStarter::SplunkStarter() = default;
+
+SplunkStarter::~SplunkStarter() = default;
+
+namespace {
+
+vespalib::string fixDir(const vespalib::string &parent, const vespalib::string &subdir) {
+    auto dirname = parent + "/" + subdir;
+    DIR *dp = opendir(dirname.c_str());
+    if (dp == NULL) {
+        if (errno != ENOENT || mkdir(dirname.c_str(), 0755) != 0) {
+            LOG(warning, "Could not create directory '%s'", dirname.c_str());
+            perror(dirname.c_str());
+        }
+    } else {
+        closedir(dp);
+    }
+    return dirname;
+}
+
+vespalib::string
+cfFilePath(const vespalib::string &parent, const vespalib::string &filename) {
+    vespalib::string path = parent;
+    path = fixDir(path, "etc");
+    path = fixDir(path, "system");
+    path = fixDir(path, "local");
+    return path + "/" + filename;
+}
+
+} // namespace <unnamed>
+
+void SplunkStarter::gotConfig(const LogforwarderConfig& config) {
+    vespalib::string path = cfFilePath(config.splunkHome, "deploymentclient.conf");
+    LOG(debug, "got config, writing %s", path.c_str());
+    vespalib::string tmpPath = path + ".new";
+    FILE *fp = fopen(tmpPath.c_str(), "w");
+    if (fp == NULL) {
+        LOG(warning, "could not open '%s' for write", tmpPath.c_str());
+        return;
+    }
+
+    fprintf(fp, "[deployment-client]\n");
+    fprintf(fp, "clientName = %s\n", config.clientName.c_str());
+    fprintf(fp, "phoneHomeIntervalInSecs = %i\n", config.phoneHomeInterval);
+    fprintf(fp, "\n");
+    fprintf(fp, "[target-broker:deploymentServer]\n");
+    fprintf(fp, "targetUri = %s\n", config.deploymentServer.c_str());
+
+    fclose(fp);
+    rename(tmpPath.c_str(), path.c_str());
+
+    if (getenv("VESPA_HOSTNAME") != NULL &&
+        getenv("VESPA_TENANT") != NULL &&
+        getenv("VESPA_APPLICATION")!= NULL &&
+        getenv("VESPA_INSTANCE") != NULL &&
+        getenv("VESPA_ENVIRONMENT") != NULL &&
+        getenv("VESPA_REGION") != NULL)
+    {
+        path = cfFilePath(config.splunkHome, "inputs.conf");
+        tmpPath = path + ".new";
+        fp = fopen(tmpPath.c_str(), "w");
+        if (fp != NULL) {
+            fprintf(fp, "[default]\n");
+            fprintf(fp, "host = %s\n", getenv("VESPA_HOSTNAME"));
+            fprintf(fp, "_meta = vespa_tenant::%s vespa_app::%s.%s vespa_zone::%s.%s\n",
+                    getenv("VESPA_TENANT"),
+                    getenv("VESPA_APPLICATION"),
+                    getenv("VESPA_INSTANCE"),
+                    getenv("VESPA_ENVIRONMENT"),
+                    getenv("VESPA_REGION"));
+            fclose(fp);
+            rename(tmpPath.c_str(), path.c_str());
+        }
+    }
+    if (config.clientName.size() == 0 ||
+        config.deploymentServer.size() == 0)
+    {
+        _childHandler.stopChild();
+    } else {
+        _childHandler.startChild(config.splunkHome);
+    }
+}
+
+void SplunkStarter::stop() {
+    _childHandler.stopChild();
+}
diff --git a/logforwarder/src/apps/vespa-logforwarder-start/splunk-starter.h b/logforwarder/src/apps/vespa-logforwarder-start/splunk-starter.h
new file mode 100644
index 00000000000..39cb8ca0efe
--- /dev/null
+++ b/logforwarder/src/apps/vespa-logforwarder-start/splunk-starter.h
@@ -0,0 +1,19 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+#pragma once
+
+#include "cf-handler.h"
+#include "child-handler.h"
+#include <vespa/config-logforwarder.h>
+
+using cloud::config::LogforwarderConfig;
+
+class SplunkStarter : public CfHandler {
+private:
+    ChildHandler _childHandler;
+public:
+    SplunkStarter();
+    virtual ~SplunkStarter();
+    void stop();
+    void gotConfig(const LogforwarderConfig& config) override;
+};
+
diff --git a/logforwarder/src/apps/vespa-logforwarder-start/splunk-stopper.cpp b/logforwarder/src/apps/vespa-logforwarder-start/splunk-stopper.cpp
new file mode 100644
index 00000000000..2da8a1acd5c
--- /dev/null
+++ b/logforwarder/src/apps/vespa-logforwarder-start/splunk-stopper.cpp
@@ -0,0 +1,18 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#include "splunk-stopper.h"
+#include "child-handler.h"
+
+#include <vespa/log/log.h>
+LOG_SETUP(".splunk-stopper");
+
+SplunkStopper::SplunkStopper(const char *configId) {
+    start(configId);
+}
+
+SplunkStopper::~SplunkStopper() = default;
+
+void SplunkStopper::gotConfig(const LogforwarderConfig& config) {
+    LOG(debug, "got config with splunk home '%s'", config.splunkHome.c_str());
+    ChildHandler().stopChild(config.splunkHome);
+}
diff --git a/logforwarder/src/apps/vespa-logforwarder-start/splunk-stopper.h b/logforwarder/src/apps/vespa-logforwarder-start/splunk-stopper.h
new file mode 100644
index 00000000000..8ff520fcc4d
--- /dev/null
+++ b/logforwarder/src/apps/vespa-logforwarder-start/splunk-stopper.h
@@ -0,0 +1,14 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+#pragma once
+
+#include "cf-handler.h"
+#include <vespa/config-logforwarder.h>
+
+using cloud::config::LogforwarderConfig;
+
+class SplunkStopper : public CfHandler {
+public:
+    SplunkStopper(const char *cfid);
+    virtual ~SplunkStopper();
+    void gotConfig(const LogforwarderConfig& config) override;
+};