ask for reverse connectivity check in OutwardCheck

3 files changed, 73 insertions, 52 deletions

diff --git a/configd/src/apps/sentinel/env.cpp b/configd/src/apps/sentinel/env.cpp
index 9f76df9c05a..cec3157ac61 100644
--- a/configd/src/apps/sentinel/env.cpp
+++ b/configd/src/apps/sentinel/env.cpp
@@ -7,11 +7,13 @@
 #include <vespa/log/log.h>
 #include <vespa/config/common/exceptions.h>
 #include <vespa/vespalib/util/exceptions.h>
+#include <vespa/vespalib/util/stringfmt.h>
 #include <thread>
 #include <chrono>
 
 LOG_SETUP(".env");
 
+using vespalib::make_string_short::fmt;
 using namespace std::chrono_literals;
 
 namespace config::sentinel {
@@ -92,14 +94,15 @@ void Env::respondAsEmpty() {
 }
 
 void Env::waitForConnectivity(const ModelConfig &model) {
-    std::map<std::string, OutwardCheck> connectivityMap;
+    std::map<std::string, std::string> checkSpecs;
     for (const auto & h : model.hosts) {
         bool foundSentinelPort = false;
         for (const auto & s : h.services) {       
             if (s.name == "config-sentinel") { 
                 for (const auto & p : s.ports) {
                     if (p.tags.find("rpc") != p.tags.npos) {
-                        connectivityMap.try_emplace(h.name, h.name, p.number, _rpcServer->orb());
+                        auto spec = fmt("tcp/%s:%d", h.name.c_str(), p.number);
+                        checkSpecs[h.name] = spec;
                         foundSentinelPort = true;
                     }
                 }
@@ -110,25 +113,14 @@ void Env::waitForConnectivity(const ModelConfig &model) {
                 h.name.c_str(), h.services.size());
         }
     }
-    size_t cntOk = 0;
-    size_t cntBad = 0;
-    for (int retry = 1; retry <= 100; ++retry) {
-        cntOk = 0;
-        cntBad = 0;
-        for (const auto & [hostname, check] : connectivityMap) {
-            if (check.ok()) {
-                ++cntOk;
-            } else if (check.bad()) {
-                ++cntBad;
-            }
-        }
-        if (cntOk + cntBad == connectivityMap.size()) break;
-        respondAsEmpty();
-        std::this_thread::sleep_for(15ms);
-        if ((retry % 20) == 0) {
-            LOG(warning, "still waiting for connectivity checks after %d retries", retry);
-        }
+    vespalib::CountDownLatch latch(checkSpecs.size());
+    const char *myName = vespa::Defaults::vespaHostname();
+    int myPort = _rpcServer->getPort();
+    std::map<std::string, OutwardCheck> connectivityMap;
+    for (const auto & [ hn, spec ] : checkSpecs) {
+        connectivityMap.try_emplace(hn, spec, myName, myPort, _rpcServer->orb(), latch);
     }
+    latch.await();
     for (const auto & [hostname, check] : connectivityMap) {
         const char *s = "unknown";
         if (check.ok()) { s = "ok"; }
@@ -136,21 +128,6 @@ void Env::waitForConnectivity(const ModelConfig &model) {
         LOG(info, "outward check status for host %s is: %s",
             hostname.c_str(), s);
     }
-    LOG_ASSERT(cntOk + cntBad == connectivityMap.size());
-    const char *myName = vespa::Defaults::vespaHostname();
-    int myPort = _rpcServer->getPort();
-    OutwardCheck selfCheck(myName, myPort, _rpcServer->orb());
-    for (int retry = 0; retry < 1000; ++retry) {
-        if (selfCheck.bad()) {
-            LOG(error, "Could not connect to '%s' (myself) at port %d", myName, myPort);
-            throw InvalidConfigException("failed to self-connect");
-        }
-        if (selfCheck.ok()) {
-            break;
-        }
-        std::this_thread::sleep_for(5ms);
-    }
-    LOG_ASSERT(selfCheck.ok());
 }
 
 }
diff --git a/configd/src/apps/sentinel/outward-check.cpp b/configd/src/apps/sentinel/outward-check.cpp
index e497f03fe71..07ff7dc90f9 100644
--- a/configd/src/apps/sentinel/outward-check.cpp
+++ b/configd/src/apps/sentinel/outward-check.cpp
@@ -1,10 +1,55 @@
 // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "outward-check.h"
-#include <thread>
+#include <vespa/log/log.h>
+
+LOG_SETUP(".outward-check");
 
 namespace config::sentinel {
 
+OutwardCheck::OutwardCheck(const std::string &spec, const char * myHostname, int myPortnum,
+                           FRT_Supervisor &orb, vespalib::CountDownLatch &latch)
+  : _spec(spec),
+    _countDownLatch(latch)
+{
+    _target = orb.GetTarget(spec.c_str());
+    _req = orb.AllocRPCRequest();
+    _req->SetMethodName("sentinel.check.connectivity");
+    _req->GetParams()->AddString(myHostname);
+    _req->GetParams()->AddInt32(myPortnum);
+    _req->GetParams()->AddInt32(500);
+    _target->InvokeAsync(_req, 1.500, this);
+}
+
 OutwardCheck::~OutwardCheck() = default;
 
+void OutwardCheck::RequestDone(FRT_RPCRequest *req) {
+    LOG_ASSERT(req == _req);
+    if (req->CheckReturnTypes("s")) {
+        std::string answer = _req->GetReturn()->GetValue(0)._string._str;
+        if (answer == "ok") {
+            LOG(info, "ping to %s with reverse connectivity OK", _spec.c_str());
+            _wasOk = true;
+        } else {
+            LOG(warning, "connected to %s, but reverse connectivity fails: %s",
+                _spec.c_str(), answer.c_str());
+            _wasBad = true;
+        }
+    } else if (req->GetErrorCode() == FRTE_RPC_NO_SUCH_METHOD ||
+               req->GetErrorCode() == FRTE_RPC_WRONG_PARAMS)
+    {
+        _wasOk = true;
+        LOG(info, "Connected OK to %s but no reverse connectivity check available", _spec.c_str());
+    } else {
+        _wasBad = true;
+        LOG(warning, "error on ping to %s : %s (%d)", _spec.c_str(),
+            req->GetErrorMessage(), req->GetErrorCode());
+    }
+    _req->SubRef();
+    _req = nullptr;
+    _target->SubRef();
+    _target = nullptr;
+    _countDownLatch.countDown();
+}
+
 }
diff --git a/configd/src/apps/sentinel/outward-check.h b/configd/src/apps/sentinel/outward-check.h
index 563151af47b..8a0b036ac49 100644
--- a/configd/src/apps/sentinel/outward-check.h
+++ b/configd/src/apps/sentinel/outward-check.h
@@ -1,31 +1,30 @@
 // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
-#include "status-callback.h"
-#include "peer-check.h"
+#include <string>
+#include <vespa/vespalib/util/count_down_latch.h>
 #include <vespa/fnet/frt/supervisor.h>
+#include <vespa/fnet/frt/invoker.h>
+#include <vespa/fnet/frt/rpcrequest.h>
+#include <vespa/fnet/frt/supervisor.h>
+#include <vespa/fnet/frt/target.h>
 
 namespace config::sentinel {
 
-class OutwardCheck : public StatusCallback {
+class OutwardCheck  : public FRT_IRequestWait {
+private:
     bool _wasOk = false;
     bool _wasBad = false;
-    PeerCheck _check;
+    FRT_Target *_target = nullptr;
+    FRT_RPCRequest *_req = nullptr;
+    std::string _spec;
+    vespalib::CountDownLatch &_countDownLatch;
 public:
-    OutwardCheck(const std::string &hostname, int portnumber, FRT_Supervisor &orb)
-      : _check(*this, hostname, portnumber, orb)
-    {}
+    OutwardCheck(const std::string &spec, const char * myHostname, int myPortnum,
+                 FRT_Supervisor &orb, vespalib::CountDownLatch &latch);
     virtual ~OutwardCheck();
+    void RequestDone(FRT_RPCRequest *req) override;
     bool ok() const { return _wasOk; }
     bool bad() const { return _wasBad; }
-    void returnStatus(bool ok) override {
-        if (ok) {
-            _wasBad = false;
-            _wasOk = true;
-        } else {
-            _wasOk = false;
-            _wasBad = true;
-        }
-    }
 };
 
 }