Do not use unresolvable configservers.

10 files changed, 51 insertions, 23 deletions

diff --git a/filedistribution/src/apps/filedistributor/filedistributor.cpp b/filedistribution/src/apps/filedistributor/filedistributor.cpp
index 954c2bb246c..ee6663e05f4 100644
--- a/filedistribution/src/apps/filedistributor/filedistributor.cpp
+++ b/filedistribution/src/apps/filedistributor/filedistributor.cpp
@@ -93,7 +93,7 @@ class FileDistributor : public config::IFetcherCallback<ZookeepersConfig>,
                    const ZookeepersConfig& zooKeepersConfig,
                    const FiledistributorConfig& fileDistributorConfig,
                    const FiledistributorrpcConfig& rpcConfig)
-            :_zk(track(new ZKFacade(zooKeepersConfig.zookeeperserverlist))),
+            :_zk(track(new ZKFacade(zooKeepersConfig.zookeeperserverlist, false))),
              _model(track(new FileDistributionModelImpl(fileDistributorConfig.hostname, fileDistributorConfig.torrentport, _zk))),
              _tracker(track(new FileDistributorTrackerImpl(_model))),
              _downloader(track(new FileDownloader(_tracker, fileDistributorConfig.hostname, fileDistributorConfig.torrentport, Path(fileDistributorConfig.filedbpath)))),
diff --git a/filedistribution/src/apps/status/status-filedistribution.cpp b/filedistribution/src/apps/status/status-filedistribution.cpp
index 87fa04e503b..697c7b504af 100644
--- a/filedistribution/src/apps/status/status-filedistribution.cpp
+++ b/filedistribution/src/apps/status/status-filedistribution.cpp
@@ -60,7 +60,7 @@ printWaitingForHosts(const StatusByHostName& notFinishedHosts)
 //TODO:refactor
 int printStatus(const std::string& zkservers)
 {
-    std::shared_ptr<ZKFacade> zk(new ZKFacade(zkservers));
+    std::shared_ptr<ZKFacade> zk(new ZKFacade(zkservers, true));
 
     std::shared_ptr<FileDBModel> model(new ZKFileDBModel(zk));
 
diff --git a/filedistribution/src/tests/filedbmodelimpl/test-filedistributionmodelimpl.cpp b/filedistribution/src/tests/filedbmodelimpl/test-filedistributionmodelimpl.cpp
index 3cf12722d86..31dd267366c 100644
--- a/filedistribution/src/tests/filedbmodelimpl/test-filedistributionmodelimpl.cpp
+++ b/filedistribution/src/tests/filedbmodelimpl/test-filedistributionmodelimpl.cpp
@@ -24,7 +24,7 @@ struct Fixture {
     std::shared_ptr<ZKFacade> _zk;
     std::shared_ptr<FileDistributionModelImpl> _distModel;
     Fixture() {
-        _zk = _componentsDeleter.track(new ZKFacade("test1-tonyv:2181"));  
+        _zk = _componentsDeleter.track(new ZKFacade("test1-tonyv:2181", false));
         _distModel.reset(new FileDistributionModelImpl("hostname", 12345, _zk));       
      }
     ~Fixture() { }
diff --git a/filedistribution/src/tests/zkfacade/test-zkfacade.cpp b/filedistribution/src/tests/zkfacade/test-zkfacade.cpp
index 8589cb95d10..c20e7058933 100644
--- a/filedistribution/src/tests/zkfacade/test-zkfacade.cpp
+++ b/filedistribution/src/tests/zkfacade/test-zkfacade.cpp
@@ -40,7 +40,7 @@ struct Fixture {
 
     Fixture() {
         zoo_set_debug_level(ZOO_LOG_LEVEL_WARN);
-        zk = _componentsDeleter.track(new ZKFacade("test1-tonyv:2181"));
+        zk = _componentsDeleter.track(new ZKFacade("test1-tonyv:2181", false));
 
         testNode = "/test-node";
         zk->removeIfExists(testNode);
@@ -67,6 +67,17 @@ BOOST_AUTO_TEST_CASE(hasNode)
     BOOST_CHECK(!zk->hasNode(testNode));
 }
 
+BOOST_AUTO_TEST_CASE(getValidZKServers)
+{
+    BOOST_CHECK_EQUAL("localhost:22", ZKFacade::getValidZKServers("localhost:22", false));
+    BOOST_CHECK_EQUAL("localhost:22", ZKFacade::getValidZKServers("localhost:22", true));
+    BOOST_CHECK_EQUAL("idonotexist:22", ZKFacade::getValidZKServers("idonotexist:22", false));
+    BOOST_CHECK_EQUAL("", ZKFacade::getValidZKServers("idonotexist:22", true));
+    BOOST_CHECK_EQUAL("localhost:22,idonotexist:22", ZKFacade::getValidZKServers("localhost:22,idonotexist:22", false));
+    BOOST_CHECK_EQUAL("localhost:22", ZKFacade::getValidZKServers("localhost:22,idonotexist:22", true));
+    BOOST_CHECK_EQUAL("idonotexist:22,localhost:22", ZKFacade::getValidZKServers("idonotexist:22,localhost:22", false));
+    BOOST_CHECK_EQUAL("localhost:22", ZKFacade::getValidZKServers("idonotexist:22,localhost:22", true));
+}
 
 BOOST_AUTO_TEST_CASE(hasNodeNotification)
 {
@@ -152,7 +163,7 @@ BOOST_AUTO_TEST_CASE(addEphemeralNode)
     zk->removeIfExists(ephemeralNode);
 
     //Checked deleter is ok here since we're not installing any watchers
-    ZKFacade::SP zk2(new ZKFacade("test1-tonyv:2181"), boost::checked_deleter<ZKFacade>());
+    ZKFacade::SP zk2(new ZKFacade("test1-tonyv:2181", false), boost::checked_deleter<ZKFacade>());
     zk2->addEphemeralNode(ephemeralNode);
 
     BOOST_CHECK(zk->hasNode(ephemeralNode));
diff --git a/filedistribution/src/tests/zkfiledbmodel/test-zkfiledbmodel.cpp b/filedistribution/src/tests/zkfiledbmodel/test-zkfiledbmodel.cpp
index c102a235603..690cb73da11 100644
--- a/filedistribution/src/tests/zkfiledbmodel/test-zkfiledbmodel.cpp
+++ b/filedistribution/src/tests/zkfiledbmodel/test-zkfiledbmodel.cpp
@@ -25,7 +25,7 @@ struct Fixture {
 
     Fixture() {
         zoo_set_debug_level(ZOO_LOG_LEVEL_WARN);
-        zk = _componentsDeleter.track(new ZKFacade("test1-tonyv:2181"));
+        zk = _componentsDeleter.track(new ZKFacade("test1-tonyv:2181", false));
         zk->setData("/vespa", "", 0);
 
         model = _componentsDeleter.track(new ZKFileDBModel(zk));
diff --git a/filedistribution/src/vespa/filedistribution/common/logfwd.h b/filedistribution/src/vespa/filedistribution/common/logfwd.h
index 2786b9c3ed9..88937d9217e 100644
--- a/filedistribution/src/vespa/filedistribution/common/logfwd.h
+++ b/filedistribution/src/vespa/filedistribution/common/logfwd.h
@@ -1,6 +1,8 @@
 // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 #pragma once
 
+#include <cstdarg>
+
 namespace filedistribution {
 
 /** To avoid requiring vespa log from the jni library*/
diff --git a/filedistribution/src/vespa/filedistribution/common/vespa_logfwd.cpp b/filedistribution/src/vespa/filedistribution/common/vespa_logfwd.cpp
index c5785f9e24e..b199eb80725 100644
--- a/filedistribution/src/vespa/filedistribution/common/vespa_logfwd.cpp
+++ b/filedistribution/src/vespa/filedistribution/common/vespa_logfwd.cpp
@@ -1,9 +1,6 @@
 // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-#include <vespa/fastos/fastos.h>
-#include <stdarg.h>
-
 #include "logfwd.h"
-
+#include <vector>
 #include <vespa/log/log.h>
 
 LOG_SETUP(".common.model");
diff --git a/filedistribution/src/vespa/filedistribution/manager/filedistributionmanager.cpp b/filedistribution/src/vespa/filedistribution/manager/filedistributionmanager.cpp
index c87360f3f67..6af340a0ff6 100644
--- a/filedistribution/src/vespa/filedistribution/manager/filedistributionmanager.cpp
+++ b/filedistribution/src/vespa/filedistribution/manager/filedistributionmanager.cpp
@@ -87,7 +87,7 @@ void initMockFileDBModel(NativeFileDistributionManager& manager)
 void initFileDBModel(NativeFileDistributionManager& manager, const std::string& zkServers)
 {
     //Ignored for now, since we're not installing any watchers.
-    std::shared_ptr<ZKFacade> zk(new ZKFacade(zkServers));
+    std::shared_ptr<ZKFacade> zk(new ZKFacade(zkServers, true));
     manager._fileDBModel.reset(new ZKFileDBModel(zk));
 }
 } //end anonymous namespace
diff --git a/filedistribution/src/vespa/filedistribution/model/zkfacade.cpp b/filedistribution/src/vespa/filedistribution/model/zkfacade.cpp
index be11f4d5519..ea620a37bfd 100644
--- a/filedistribution/src/vespa/filedistribution/model/zkfacade.cpp
+++ b/filedistribution/src/vespa/filedistribution/model/zkfacade.cpp
@@ -1,13 +1,7 @@
 // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-#include <vespa/fastos/fastos.h>
 
 #include "zkfacade.h"
-
-#include <iostream>
-#include <unistd.h>
-#include <signal.h>
-#include <cassert>
-#include <cstdio>
+#include <vespa/vespalib/net/socket_address.h>
 #include <sstream>
 #include <thread>
 #include <boost/function_output_iterator.hpp>
@@ -16,6 +10,7 @@
 #include <vespa/filedistribution/common/logfwd.h>
 #include <vespa/defaults.h>
 #include <vespa/vespalib/util/sync.h>
+#include <vespa/vespalib/text/stringtokenizer.h>
 
 typedef std::unique_lock<std::mutex> UniqueLock;
 
@@ -75,7 +70,7 @@ toErrorMsg(int zkStatus) {
       case ZNOTHING:
         return "Zookeeper: No server responses to process(ZNOTHING)";
       default:
-        std::cerr <<"In ZKGenericException::what(): Invalid error code " << zkStatus <<std::endl;
+        LOGFWD(error, "In ZKGenericException::what(): Invalid error code %d", zkStatus);
         return "Zookeeper: Invalid error code.";
     }
 }
@@ -306,11 +301,32 @@ ZKFacade::invokeWatcher(void* watcherContext) {
 
 /********** End live watchers ***************************************/
 
+std::string
+ZKFacade::getValidZKServers(const std::string &input, bool allowDNSFailure) {
+    if (allowDNSFailure) {
+        vespalib::StringTokenizer tokenizer(input, ",");
+        vespalib::string validServers;
+        for (vespalib::string spec : tokenizer) {
+            vespalib::StringTokenizer addrTokenizer(spec, ":");
+            vespalib::string address = addrTokenizer[0];
+            vespalib::string port = addrTokenizer[1];
+            if ( !vespalib::SocketAddress::resolve(atoi(port.c_str()), address.c_str()).empty()) {
+                if ( !validServers.empty() ) {
+                    validServers += ',';
+                }
+                validServers += spec;
+            }
+        }
+        return validServers;
+    }
+    return input;
+}
+
 
-ZKFacade::ZKFacade(const std::string& zkservers)
+ZKFacade::ZKFacade(const std::string& zkservers, bool allowDNSFailure)
     :_retriesEnabled(true),
      _watchersEnabled(true),
-     _zhandle(zookeeper_init(zkservers.c_str(),
+     _zhandle(zookeeper_init(getValidZKServers(zkservers, allowDNSFailure).c_str(),
                              &ZKFacade::stateWatchingFun,
                              _zkSessionTimeOut,
                              0, //clientid,
@@ -573,7 +589,7 @@ ZKLogging::ZKLogging() :
     filename.append("/tmp/zookeeper.log");
     _file = std::fopen(filename.c_str(), "w");
     if (_file == nullptr) {
-         std::cerr <<"Could not open file " <<filename << std::endl;
+         LOGFWD(error, "Could not open file '%s'", filename.c_str());
     } else {
          zoo_set_log_stream(_file);
     }
diff --git a/filedistribution/src/vespa/filedistribution/model/zkfacade.h b/filedistribution/src/vespa/filedistribution/model/zkfacade.h
index 0e1236974a7..8d7d8cca0eb 100644
--- a/filedistribution/src/vespa/filedistribution/model/zkfacade.h
+++ b/filedistribution/src/vespa/filedistribution/model/zkfacade.h
@@ -73,7 +73,7 @@ public:
 
     ZKFacade(const ZKFacade &) = delete;
     ZKFacade & operator = (const ZKFacade &) = delete;
-    ZKFacade(const std::string& zkservers);
+    ZKFacade(const std::string& zkservers, bool allowDNSFailure);
     ~ZKFacade();
 
     bool hasNode(const Path&);
@@ -105,6 +105,8 @@ public:
         return _retriesEnabled;
     }
 
+    static std::string getValidZKServers(const std::string &input, bool allowDNSFailure);
+
 private:
     class RegistrationGuard {
     public: