refactor child startup

5 files changed, 90 insertions, 35 deletions

diff --git a/logforwarder/src/apps/vespa-logforwarder-start/CMakeLists.txt b/logforwarder/src/apps/vespa-logforwarder-start/CMakeLists.txt
index 9a402bb58da..aa5584ec108 100644
--- a/logforwarder/src/apps/vespa-logforwarder-start/CMakeLists.txt
+++ b/logforwarder/src/apps/vespa-logforwarder-start/CMakeLists.txt
@@ -3,6 +3,7 @@ vespa_add_executable(logforwarder-start_app
     SOURCES
     main.cpp
     cf-handler.cpp
+    child-handler.cpp
     OUTPUT_NAME vespa-logforwarder-start
     INSTALL bin
     DEPENDS
diff --git a/logforwarder/src/apps/vespa-logforwarder-start/cf-handler.cpp b/logforwarder/src/apps/vespa-logforwarder-start/cf-handler.cpp
index cd0df574cd1..18b2c1d255b 100644
--- a/logforwarder/src/apps/vespa-logforwarder-start/cf-handler.cpp
+++ b/logforwarder/src/apps/vespa-logforwarder-start/cf-handler.cpp
@@ -13,7 +13,7 @@
 #include <vespa/log/log.h>
 LOG_SETUP(".cf-handler");
 
-CfHandler::CfHandler() : _childRunning(false), _subscriber() {}
+CfHandler::CfHandler() : _subscriber() {}
 
 CfHandler::~CfHandler()
 {
@@ -54,26 +54,6 @@ cfFilePath(const vespalib::string &parent) {
     return path;
 }
 
-void
-runSplunk(const vespalib::string &prefix, const char *a1, const char *a2 = 0)
-{
-    const char *argv[] = { 0, a1, a2, 0 };
-    vespalib::string path = prefix + "/bin/splunk";
-    argv[0] = path.c_str();
-    fprintf(stdout, "starting splunk forwarder with command: '%s' '%s'\n",
-            argv[0], argv[1]);
-    if (fork() == 0) {
-        vespalib::string env = "SPLUNK_HOME=" + prefix;
-        char *cenv = const_cast<char *>(env.c_str());
-        putenv(cenv);
-        char **cargv = const_cast<char **>(argv);
-        execv(argv[0], cargv);
-        // if execv fails:
-        perror(argv[0]);
-        exit(1);
-    }
-}
-
 } // namespace <unnamed>
 
 void
@@ -96,18 +76,7 @@ CfHandler::doConfigure()
     fclose(fp);
     rename(tmpPath.c_str(), path.c_str());
 
-    startChild(config.splunkPath);
-}
-
-void
-CfHandler::startChild(const vespalib::string &prefix)
-{
-    if (_childRunning) {
-        runSplunk(prefix, "restart");
-    } else {
-        runSplunk(prefix, "start", "--accept-license");
-        _childRunning = true;
-    }
+    childHandler.startChild(config.splunkPath);
 }
 
 void
diff --git a/logforwarder/src/apps/vespa-logforwarder-start/cf-handler.h b/logforwarder/src/apps/vespa-logforwarder-start/cf-handler.h
index d6c7f2e2076..0dcf2be7058 100644
--- a/logforwarder/src/apps/vespa-logforwarder-start/cf-handler.h
+++ b/logforwarder/src/apps/vespa-logforwarder-start/cf-handler.h
@@ -3,13 +3,13 @@
 
 #include <vespa/config/config.h>
 #include <vespa/config-logforwarder.h>
+#include "child-handler.h"
 
 using cloud::config::LogforwarderConfig;
 
 class CfHandler {
 private:
-    bool _childRunning;
-    void startChild(const vespalib::string &prefix);
+    ChildHandler childHandler;
     config::ConfigSubscriber _subscriber;
     config::ConfigHandle<LogforwarderConfig>::UP _handle;
     void subscribe(const std::string & configId, uint64_t timeoutMS);
diff --git a/logforwarder/src/apps/vespa-logforwarder-start/child-handler.cpp b/logforwarder/src/apps/vespa-logforwarder-start/child-handler.cpp
new file mode 100644
index 00000000000..d7c7a923ddb
--- /dev/null
+++ b/logforwarder/src/apps/vespa-logforwarder-start/child-handler.cpp
@@ -0,0 +1,73 @@
+// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#include "child-handler.h"
+
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+#include <vespa/log/log.h>
+LOG_SETUP(".child-handler");
+
+ChildHandler::ChildHandler() : _childRunning(false) {}
+
+namespace {
+
+void
+runSplunk(const vespalib::string &prefix, const char *a1, const char *a2 = 0)
+{
+    const char *argv[] = { 0, a1, a2, 0 };
+    vespalib::string path = prefix + "/bin/splunk";
+    argv[0] = path.c_str();
+    fprintf(stdout, "starting splunk forwarder with command: '%s' '%s'\n",
+            argv[0], argv[1]);
+    pid_t child = fork();
+    if (child == -1) {
+        perror("fork()");
+        return;
+    }
+    if (child == 0) {
+        vespalib::string env = "SPLUNK_HOME=" + prefix;
+        char *cenv = const_cast<char *>(env.c_str()); // safe cast
+        putenv(cenv);
+        char **cargv = const_cast<char **>(argv); // safe cast
+        execv(argv[0], cargv);
+        // if execv fails:
+        perror(argv[0]);
+        exit(1);
+    }
+    int waitStatus = 0;
+    if (waitpid(child, &waitStatus, 0) == -1) {
+        perror("waitpid()");
+        return;
+    }
+    if (WIFEXITED(waitStatus) && WEXITSTATUS(waitStatus) == 0) {
+        // all OK
+        return;
+    }
+    if (WIFEXITED(waitStatus)) {
+        fprintf(stderr, "failed starting splunk forwarder (exit status %d)\n",
+                WEXITSTATUS(waitStatus));
+    } else if (WIFSIGNALED(waitStatus)) {
+        fprintf(stderr, "failed starting splunk forwarder (exit on signal %d)\n",
+                WTERMSIG(waitStatus));
+    } else {
+        fprintf(stderr, "failed starting splunk forwarder (abnormal exit status %d)\n",
+                waitStatus);
+    }
+}
+
+} // namespace <unnamed>
+
+
+void
+ChildHandler::startChild(const vespalib::string &prefix)
+{
+    if (_childRunning) {
+        runSplunk(prefix, "restart");
+    } else {
+        runSplunk(prefix, "start", "--accept-license");
+        _childRunning = true;
+    }
+}
diff --git a/logforwarder/src/apps/vespa-logforwarder-start/child-handler.h b/logforwarder/src/apps/vespa-logforwarder-start/child-handler.h
new file mode 100644
index 00000000000..619dc1e40aa
--- /dev/null
+++ b/logforwarder/src/apps/vespa-logforwarder-start/child-handler.h
@@ -0,0 +1,12 @@
+// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+#pragma once
+
+#include <vespa/vespalib/stllike/string.h>
+
+class ChildHandler {
+private:
+    bool _childRunning;
+public:
+    void startChild(const vespalib::string &prefix);
+    ChildHandler();
+};