Merge pull request #18013 from vespa-engine/arnej/refactor-sentinel-startup

Arnej/refactor sentinel startup

8 files changed, 275 insertions, 104 deletions

diff --git a/configd/src/apps/sentinel/CMakeLists.txt b/configd/src/apps/sentinel/CMakeLists.txt
index b4fca4a3285..f6b4c3a2184 100644
--- a/configd/src/apps/sentinel/CMakeLists.txt
+++ b/configd/src/apps/sentinel/CMakeLists.txt
@@ -1,16 +1,18 @@
 # Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 vespa_add_executable(configd_config-sentinel_app
     SOURCES
-    sentinel.cpp
-    service.cpp
-    config-handler.cpp
+    cmdq.cpp
+    config-owner.cpp
+    env.cpp
     line-splitter.cpp
-    output-connection.cpp
+    manager.cpp
     metrics.cpp
-    state-api.cpp
-    cmdq.cpp
+    output-connection.cpp
     rpchooks.cpp
     rpcserver.cpp
+    sentinel.cpp
+    service.cpp
+    state-api.cpp
     OUTPUT_NAME vespa-config-sentinel
     INSTALL sbin
     DEPENDS
diff --git a/configd/src/apps/sentinel/config-owner.cpp b/configd/src/apps/sentinel/config-owner.cpp
new file mode 100644
index 00000000000..3feec70d2e5
--- /dev/null
+++ b/configd/src/apps/sentinel/config-owner.cpp
@@ -0,0 +1,44 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#include "config-owner.h"
+#include <vespa/vespalib/util/exceptions.h>
+#include <string>
+
+#include <vespa/log/log.h>
+LOG_SETUP(".config-owner");
+
+namespace config::sentinel {
+
+ConfigOwner::ConfigOwner() : _subscriber() {}
+
+ConfigOwner::~ConfigOwner() = default;
+
+void
+ConfigOwner::subscribe(const std::string & configId, std::chrono::milliseconds timeout) {
+    _sentinelHandle = _subscriber.subscribe<SentinelConfig>(configId, timeout);
+}
+
+void
+ConfigOwner::doConfigure()
+{
+    _currConfig = _sentinelHandle->getConfig();
+    LOG_ASSERT(_currConfig);
+    _currGeneration = _subscriber.getGeneration();
+    const SentinelConfig& config(*_currConfig);
+    const auto & app = config.application;
+    LOG(config, "Sentinel got %zd service elements [tenant(%s), application(%s), instance(%s)] for config generation %zd",
+        config.service.size(), app.tenant.c_str(), app.name.c_str(), app.instance.c_str(), _currGeneration);
+}
+
+
+// Return true if there was a config generation change
+bool
+ConfigOwner::checkForConfigUpdate() {
+    if (_subscriber.nextGenerationNow()) {
+        doConfigure();
+        return true;
+    }
+    return false;
+}
+
+}
diff --git a/configd/src/apps/sentinel/config-owner.h b/configd/src/apps/sentinel/config-owner.h
new file mode 100644
index 00000000000..612db7fe9a1
--- /dev/null
+++ b/configd/src/apps/sentinel/config-owner.h
@@ -0,0 +1,40 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#pragma once
+
+#include <vespa/config-sentinel.h>
+#include <vespa/config/config.h>
+
+using cloud::config::SentinelConfig;
+
+using config::ConfigSubscriber;
+using config::ConfigHandle;
+
+namespace config::sentinel {
+
+/**
+ * Handles config subscription and has a snapshot of current config.
+ **/
+class ConfigOwner {
+private:
+    ConfigSubscriber _subscriber;
+    ConfigHandle<SentinelConfig>::UP _sentinelHandle;
+
+    int64_t _currGeneration = -1;
+    std::unique_ptr<SentinelConfig> _currConfig;
+
+    ConfigOwner(const ConfigOwner&) = delete;
+    ConfigOwner& operator =(const ConfigOwner&) = delete;
+
+    void doConfigure();
+public:
+    ConfigOwner();
+    virtual ~ConfigOwner();
+    void subscribe(const std::string & configId, std::chrono::milliseconds timeout);
+    bool checkForConfigUpdate();
+    bool hasConfig() const { return _currConfig.get() != nullptr; }
+    const SentinelConfig& getConfig() const { return *_currConfig; }
+    int64_t getGeneration() const { return _currGeneration; }
+};
+
+}
diff --git a/configd/src/apps/sentinel/env.cpp b/configd/src/apps/sentinel/env.cpp
new file mode 100644
index 00000000000..02df292a423
--- /dev/null
+++ b/configd/src/apps/sentinel/env.cpp
@@ -0,0 +1,84 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#include "env.h"
+#include <vespa/log/log.h>
+#include <vespa/config/common/exceptions.h>
+#include <vespa/vespalib/util/exceptions.h>
+#include <thread>
+#include <chrono>
+
+LOG_SETUP(".env");
+
+namespace config::sentinel {
+
+constexpr std::chrono::milliseconds CONFIG_TIMEOUT_MS(3 * 60 * 1000);
+
+Env::Env()
+  : _cfgOwner(),
+    _rpcCommandQueue(),
+    _rpcServer(),
+    _stateApi(),
+    _startMetrics(),
+    _stateServer(),
+    _statePort(0)
+{
+    _startMetrics.startedTime = vespalib::steady_clock::now();
+}
+
+Env::~Env() = default;
+
+void Env::boot(const std::string &configId) {
+    LOG(debug, "Reading configuration for ID: %s", configId.c_str());
+    _cfgOwner.subscribe(configId, CONFIG_TIMEOUT_MS);
+    if (_cfgOwner.checkForConfigUpdate()) {
+        LOG_ASSERT(_cfgOwner.hasConfig());
+    } else {
+        throw InvalidConfigException("checkForConfigUpdate() failed");
+    }
+    const auto & cfg = _cfgOwner.getConfig();
+    LOG(config, "Booting sentinel '%s' with [stateserver port %d] and [rpc port %d]",
+        configId.c_str(), cfg.port.telnet, cfg.port.rpc);
+    rpcPort(cfg.port.rpc);
+    statePort(cfg.port.telnet);
+}
+
+void Env::rpcPort(int port) {
+    if (port < 0 || port > 65535) {
+        throw vespalib::FatalException("Bad port " + std::to_string(port) + ", expected range [1, 65535]", VESPA_STRLOC);
+    }
+    if (port == 0) {
+        port = 19097; // default in config
+    }
+    if (_rpcServer && port == _rpcServer->getPort()) {
+        return; // ok already
+    }
+    _rpcServer = std::make_unique<RpcServer>(port, _rpcCommandQueue);
+}
+
+void Env::statePort(int port) {
+    if (port < 0 || port > 65535) {
+        throw vespalib::FatalException("Bad port " + std::to_string(port) + ", expected range [1, 65535]", VESPA_STRLOC);
+    }
+    if (port == 0) {
+        port = 19098;
+        const char *portString = getenv("VESPA_SENTINEL_PORT");
+        if (portString) {
+            port = strtoul(portString, nullptr, 10);
+        }
+    }
+    if (_stateServer && port == _statePort) {
+        return; // ok already
+    }
+    LOG(debug, "Config-sentinel accepts connections on port %d", port);
+    _stateServer = std::make_unique<vespalib::StateServer>(
+        port, _stateApi.myHealth, _startMetrics.producer, _stateApi.myComponents);
+    _statePort = port;
+}
+
+void Env::notifyConfigUpdated() {
+    vespalib::ComponentConfigProducer::Config current("sentinel", _cfgOwner.getGeneration(), "ok");
+    _stateApi.myComponents.addConfig(current);
+
+}
+
+}
diff --git a/configd/src/apps/sentinel/env.h b/configd/src/apps/sentinel/env.h
new file mode 100644
index 00000000000..da04a328c41
--- /dev/null
+++ b/configd/src/apps/sentinel/env.h
@@ -0,0 +1,44 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#pragma once
+
+#include "cmdq.h"
+#include "config-owner.h"
+#include "metrics.h"
+#include "rpcserver.h"
+#include "state-api.h"
+#include <vespa/vespalib/net/state_server.h>
+
+namespace config::sentinel {
+
+/**
+ * Environment for config sentinel, with config
+ * subscription, rpc server, state server, and
+ * metrics.
+ **/
+class Env {
+public:
+    Env();
+    ~Env();
+
+    ConfigOwner &configOwner() { return _cfgOwner; }
+    CommandQueue &commandQueue() { return _rpcCommandQueue; }
+    StartMetrics &metrics() { return _startMetrics; }
+
+    void boot(const std::string &configId);
+    void rpcPort(int portnum);
+    void statePort(int portnum);
+
+    void notifyConfigUpdated();
+
+private:
+    ConfigOwner _cfgOwner;
+    CommandQueue _rpcCommandQueue;
+    std::unique_ptr<RpcServer> _rpcServer;
+    StateApi _stateApi;
+    StartMetrics _startMetrics;
+    std::unique_ptr<vespalib::StateServer> _stateServer;
+    int _statePort;
+};
+
+}
diff --git a/configd/src/apps/sentinel/config-handler.cpp b/configd/src/apps/sentinel/manager.cpp
index 9c39b13ba4c..d1d06c6039a 100644
--- a/configd/src/apps/sentinel/config-handler.cpp
+++ b/configd/src/apps/sentinel/manager.cpp
@@ -1,6 +1,6 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
-#include "config-handler.h"
+#include "manager.h"
 #include "output-connection.h"
 
 #include <vespa/vespalib/net/socket_address.h>
@@ -11,43 +11,18 @@
 #include <sys/wait.h>
 
 #include <vespa/log/log.h>
-LOG_SETUP(".config-handler");
+LOG_SETUP(".manager");
 
 namespace config::sentinel {
 
-void
-ConfigHandler::configure_port(int port)
-{
-    if (port == 0) {
-        port = 19098;
-        const char *portString = getenv("VESPA_SENTINEL_PORT");
-        if (portString) {
-            port = strtoul(portString, nullptr, 10);
-        }
-    }
-    if (port <= 0 || port > 65535) {
-        throw vespalib::FatalException("Bad port " + std::to_string(port) + ", expected range [1, 65535]", VESPA_STRLOC);
-    }
-    if (port != _boundPort) {
-        LOG(debug, "Config-sentinel accepts connections on port %d", port);
-        _stateServer = std::make_unique<vespalib::StateServer>(
-            port, _stateApi.myHealth, _startMetrics.producer, _stateApi.myComponents);
-        _boundPort = port;
-    }
-}
-
-ConfigHandler::ConfigHandler()
-    : _subscriber(),
-      _services(),
-      _outputConnections(),
-      _boundPort(0),
-      _startMetrics(),
-      _stateApi()
+Manager::Manager(Env &env)
+  : _env(env),
+    _services(),
+    _outputConnections()
 {
-    _startMetrics.startedTime = vespalib::steady_clock::now();
 }
 
-ConfigHandler::~ConfigHandler()
+Manager::~Manager()
 {
     terminateServices(false);
     for (OutputConnection * conn : _outputConnections) {
@@ -56,7 +31,7 @@ ConfigHandler::~ConfigHandler()
 }
 
 void
-ConfigHandler::terminateServices(bool catchable, bool printDebug)
+Manager::terminateServices(bool catchable, bool printDebug)
 {
     for (const auto & entry : _services) {
         Service *service = entry.second.get();
@@ -74,7 +49,7 @@ ConfigHandler::terminateServices(bool catchable, bool printDebug)
 
 
 bool
-ConfigHandler::terminate()
+Manager::terminate()
 {
     // Call terminate(true) for all services.
     // Give them 58 seconds to exit cleanly, then terminate(false) all
@@ -98,26 +73,15 @@ ConfigHandler::terminate()
 }
 
 void
-ConfigHandler::subscribe(const std::string & configId, std::chrono::milliseconds timeout)
+Manager::doConfigure()
 {
-    _sentinelHandle = _subscriber.subscribe<SentinelConfig>(configId, timeout);
-}
-
-void
-ConfigHandler::doConfigure()
-{
-    std::unique_ptr<SentinelConfig> cfg(_sentinelHandle->getConfig());
-    const SentinelConfig& config(*cfg);
+    LOG_ASSERT(_env.configOwner().hasConfig());
+    const SentinelConfig& config(_env.configOwner().getConfig());
 
-    if (config.port.telnet != _boundPort) {
-        configure_port(config.port.telnet);
-    }
-
-    if (!_rpcServer || config.port.rpc != _rpcServer->getPort()) {
-        _rpcServer = std::make_unique<RpcServer>(config.port.rpc, _cmdQ);
-    }
+    _env.rpcPort(config.port.rpc);
+    _env.statePort(config.port.telnet);
 
-    LOG(debug, "ConfigHandler::configure() %d config elements, tenant(%s), application(%s), instance(%s)",
+    LOG(debug, "Manager::configure() %d config elements, tenant(%s), application(%s), instance(%s)",
         (int)config.service.size(), config.application.tenant.c_str(), config.application.name.c_str(),
         config.application.instance.c_str());
     ServiceMap services;
@@ -126,7 +90,7 @@ ConfigHandler::doConfigure()
         const vespalib::string name(serviceConfig.name);
         auto found(_services.find(name));
         if (found == _services.end()) {
-            services[name] = std::make_unique<Service>(serviceConfig, config.application, _outputConnections, _startMetrics);
+            services[name] = std::make_unique<Service>(serviceConfig, config.application, _outputConnections, _env.metrics());
         } else {
             found->second->reconfigure(serviceConfig);
             services[name] = std::move(found->second);
@@ -140,24 +104,22 @@ ConfigHandler::doConfigure()
             _orphans[entry.first] = std::move(svc);
         }
     }
-    vespalib::ComponentConfigProducer::Config current("sentinel", _subscriber.getGeneration(), "ok");
-    _stateApi.myComponents.addConfig(current);
+    _env.notifyConfigUpdated();
 }
 
 
 int
-ConfigHandler::doWork()
+Manager::doWork()
 {
     // Return true if there are any running services, false if not.
-
-    if (_subscriber.nextGenerationNow()) {
+    if (_env.configOwner().checkForConfigUpdate()) {
         doConfigure();
     }
     handleRestarts();
     handleCommands();
     handleOutputs();
     handleChildDeaths();
-    _startMetrics.maybeLog();
+    _env.metrics().maybeLog();
 
     // Check for active services.
     for (const auto & service : _services) {
@@ -169,7 +131,7 @@ ConfigHandler::doWork()
 }
 
 void
-ConfigHandler::handleRestarts()
+Manager::handleRestarts()
 {
     for (const auto & entry : _services) {
         Service & svc = *(entry.second);
@@ -180,7 +142,7 @@ ConfigHandler::handleRestarts()
 }
 
 void
-ConfigHandler::handleChildDeaths()
+Manager::handleChildDeaths()
 {
     // See if any of our child processes have exited, and take
     // the appropriate action.
@@ -203,7 +165,7 @@ ConfigHandler::handleChildDeaths()
 }
 
 void
-ConfigHandler::updateActiveFdset(fd_set *fds, int *maxNum)
+Manager::updateActiveFdset(fd_set *fds, int *maxNum)
 {
     // ### _Possibly put an assert here if fd is > 1023???
     for (OutputConnection *c : _outputConnections) {
@@ -218,7 +180,7 @@ ConfigHandler::updateActiveFdset(fd_set *fds, int *maxNum)
 }
 
 void
-ConfigHandler::handleOutputs()
+Manager::handleOutputs()
 {
     std::list<OutputConnection *>::iterator dst;
     std::list<OutputConnection *>::const_iterator src;
@@ -241,10 +203,10 @@ ConfigHandler::handleOutputs()
 }
 
 void
-ConfigHandler::handleCommands()
+Manager::handleCommands()
 {
     // handle RPC commands
-    std::vector<Cmd::UP> got = _cmdQ.drain();
+    std::vector<Cmd::UP> got = _env.commandQueue().drain();
     for (const Cmd::UP & cmd : got) {
         handleCmd(*cmd);
     }
@@ -252,7 +214,7 @@ ConfigHandler::handleCommands()
 }
 
 Service *
-ConfigHandler::serviceByPid(pid_t pid)
+Manager::serviceByPid(pid_t pid)
 {
     for (const auto & service : _services) {
         if (service.second->pid() == pid) {
@@ -269,7 +231,7 @@ ConfigHandler::serviceByPid(pid_t pid)
 }
 
 Service *
-ConfigHandler::serviceByName(const vespalib::string & name)
+Manager::serviceByName(const vespalib::string & name)
 {
     auto found(_services.find(name));
     if (found != _services.end()) {
@@ -280,7 +242,7 @@ ConfigHandler::serviceByName(const vespalib::string & name)
 
 
 void
-ConfigHandler::handleCmd(const Cmd& cmd)
+Manager::handleCmd(const Cmd& cmd)
 {
     switch (cmd.type()) {
     case Cmd::LIST:
@@ -353,9 +315,9 @@ ConfigHandler::handleCmd(const Cmd& cmd)
 }
 
 void
-ConfigHandler::updateMetrics()
+Manager::updateMetrics()
 {
-    _startMetrics.maybeLog();
+    _env.metrics().maybeLog();
 }
 
 }
diff --git a/configd/src/apps/sentinel/config-handler.h b/configd/src/apps/sentinel/manager.h
index 463e9e7ce6a..dabe1590d20 100644
--- a/configd/src/apps/sentinel/config-handler.h
+++ b/configd/src/apps/sentinel/manager.h
@@ -1,11 +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 "service.h"
-#include "metrics.h"
-#include "state-api.h"
 #include "cmdq.h"
+#include "env.h"
+#include "metrics.h"
 #include "rpcserver.h"
+#include "service.h"
+#include "state-api.h"
 #include <vespa/config-sentinel.h>
 #include <vespa/config/config.h>
 #include <vespa/vespalib/net/state_server.h>
@@ -22,24 +23,22 @@ namespace config::sentinel {
 
 class OutputConnection;
 
-class ConfigHandler {
+/**
+ *  Management of services.
+ *  Handles requests from RPC, service events,
+ *  and service configuration updates.
+ **/
+class Manager {
 private:
     typedef std::map<vespalib::string, Service::UP> ServiceMap;
 
-    ConfigSubscriber _subscriber;
-    ConfigHandle<SentinelConfig>::UP _sentinelHandle;
+    Env &_env;
     ServiceMap _services;
     ServiceMap _orphans;
     std::list<OutputConnection *> _outputConnections;
-    CommandQueue _cmdQ;
-    std::unique_ptr<RpcServer> _rpcServer;
-    int _boundPort;
-    StartMetrics _startMetrics;
-    StateApi _stateApi;
-    std::unique_ptr<vespalib::StateServer> _stateServer;
 
-    ConfigHandler(const ConfigHandler&);
-    ConfigHandler& operator =(const ConfigHandler&);
+    Manager(const Manager&) = delete;
+    Manager& operator =(const Manager&) = delete;
 
     Service *serviceByPid(pid_t pid);
     Service *serviceByName(const vespalib::string & name);
@@ -49,19 +48,14 @@ private:
     void handleChildDeaths();
     void handleRestarts();
 
-    static int listen(int port);
-    void configure_port(int port);
-
     void updateMetrics();
 
     void terminateServices(bool catchable, bool printDebug = false);
 
-    void doConfigure();
-
 public:
-    ConfigHandler();
-    virtual ~ConfigHandler();
-    void subscribe(const std::string & configId, std::chrono::milliseconds timeout);
+    Manager(Env &env);
+    virtual ~Manager();
+    void doConfigure();
     bool terminate();
     int doWork();
     void updateActiveFdset(fd_set *fds, int *maxNum);
diff --git a/configd/src/apps/sentinel/sentinel.cpp b/configd/src/apps/sentinel/sentinel.cpp
index 1bca6f72d10..53c9ae7fbc2 100644
--- a/configd/src/apps/sentinel/sentinel.cpp
+++ b/configd/src/apps/sentinel/sentinel.cpp
@@ -1,6 +1,6 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
-#include "config-handler.h"
+#include "manager.h"
 #include <vespa/config/common/exceptions.h>
 #include <vespa/vespalib/util/signalhandler.h>
 #include <vespa/vespalib/util/exceptions.h>
@@ -61,11 +61,10 @@ main(int argc, char **argv)
     }
     setlocale(LC_ALL, "C");
 
-    sentinel::ConfigHandler handler;
-
+    sentinel::Env environment;
     LOG(debug, "Reading configuration");
     try {
-        handler.subscribe(configId, CONFIG_TIMEOUT_MS);
+        environment.boot(configId);
     } catch (ConfigTimeoutException & ex) {
         LOG(warning, "Timeout getting config, please check your setup. Will exit and restart: %s", ex.getMessage().c_str());
         EV_STOPPING("config-sentinel", ex.what());
@@ -80,11 +79,13 @@ main(int argc, char **argv)
         return EXIT_FAILURE;
     }
 
+    sentinel::Manager manager(environment);
+    manager.doConfigure();
     vespalib::steady_time lastTime = vespalib::steady_clock::now();
     while (!stop()) {
         try {
             vespalib::SignalHandler::CHLD.clear();
-            handler.doWork();       // Check for child procs & commands
+            manager.doWork();       // Check for child procs & commands
         } catch (InvalidConfigException& ex) {
             LOG(warning, "Configuration problem: (ignoring): %s", ex.what());
         } catch (vespalib::PortListenException& ex) {
@@ -102,7 +103,7 @@ main(int argc, char **argv)
         int maxNum = 0;
         fd_set fds;
         FD_ZERO(&fds);
-        handler.updateActiveFdset(&fds, &maxNum);
+        manager.updateActiveFdset(&fds, &maxNum);
 
         struct timeval tv;
         tv.tv_sec = 0;
@@ -118,6 +119,6 @@ main(int argc, char **argv)
     }
 
     EV_STOPPING("config-sentinel", "normal exit");
-    int rv = handler.terminate();
+    int rv = manager.terminate();
     return rv;
 }