Add host filtering option to config status util (#350)

* Let vespa-config-status take in an optional host filter set

If the argument is not specified, services on all hosts in the model
will be queried as before. Iff the filter argument is specified, only
services on the hosts explicitly given will be queried.

* Make variable name easier on the eyes

9 files changed, 154 insertions, 36 deletions

diff --git a/configutil/CMakeLists.txt b/configutil/CMakeLists.txt
index 081e74ac46f..b308a63aa8a 100644
--- a/configutil/CMakeLists.txt
+++ b/configutil/CMakeLists.txt
@@ -19,4 +19,5 @@ vespa_define_module(
     src/tests/config_status
     src/tests/model_inspect
     src/tests/tags
+    src/tests/host_filter
     )
diff --git a/configutil/src/apps/configstatus/main.cpp b/configutil/src/apps/configstatus/main.cpp
index 6cc1b8f1240..e17ee658cc2 100644
--- a/configutil/src/apps/configstatus/main.cpp
+++ b/configutil/src/apps/configstatus/main.cpp
@@ -3,17 +3,19 @@
 #include <vespa/fastos/fastos.h>
 #include <vespa/defaults.h>
 #include <vespa/log/log.h>
+#include <vespa/vespalib/text/stringtokenizer.h>
 LOG_SETUP("vespa-config-status");
 #include <iostream>
 #include <lib/configstatus.h>
+#include <lib/hostfilter.h>
 
-class Application : public FastOS_Application
-{
+class Application : public FastOS_Application {
     ConfigStatus::Flags _flags;
     vespalib::string _cfgId;
     vespalib::string _specString;
     int parseOpts();
     vespalib::string getSources();
+    HostFilter parse_host_set(vespalib::stringref raw_arg) const;
 public:
     void usage(void);
     int Main(void);
@@ -21,13 +23,11 @@ public:
     Application() : _flags(), _cfgId("admin/model"), _specString("") {}
 };
 
-int
-Application::parseOpts()
-{
+int Application::parseOpts() {
     char c = '?';
     const char *optArg = NULL;
     int optInd = 0;
-    while ((c = GetOpt("c:s:vC:", optArg, optInd)) != -1) {
+    while ((c = GetOpt("c:s:vC:f:", optArg, optInd)) != -1) {
         switch (c) {
         case 'v':
             _flags.verbose = true;
@@ -41,6 +41,9 @@ Application::parseOpts()
         case 'h':
             usage();
             exit(0);
+        case 'f':
+            _flags.host_filter = parse_host_set(optArg);
+            break;
         default:
             usage();
             exit(1);
@@ -52,21 +55,28 @@ Application::parseOpts()
     return optInd;
 }
 
+HostFilter Application::parse_host_set(vespalib::stringref raw_arg) const {
+    vespalib::StringTokenizer tokenizer(raw_arg, ",");
+    tokenizer.removeEmptyTokens();
+
+    HostFilter::HostSet hosts;
+    for (auto& host : tokenizer) {
+        hosts.emplace(host);
+    }
+    return HostFilter(std::move(hosts));
+}
 
-void
-Application::usage(void)
-{
-    std::cerr <<
-        "vespa-config-status version 1.0"                                  << std::endl <<
-        "Usage: " << _argv[0] << " [options] "                             << std::endl <<
-        "options: [-v] for verbose"                                        << std::endl <<
-        "         [-c host] or [-c host:port] to specify config server"    << std::endl <<
-        std::endl;
+void Application::usage() {
+    std::cerr << "vespa-config-status version 1.0\n"
+              << "Usage: " << _argv[0] << " [options]\n"
+              << "options: [-v] for verbose\n"
+              << "         [-c host] or [-c host:port] to specify config server\n"
+              << "         [-f host0,...,hostN] filter to only query config\n"
+                 "         status for the given comma-separated set of hosts\n"
+              << std::endl;
 }
 
-int
-Application::Main(void)
-{
+int Application::Main() {
     parseOpts();
 
     config::ServerSpec spec(_specString);
@@ -76,9 +86,7 @@ Application::Main(void)
     return status.action();
 }
 
-vespalib::string
-Application::getSources(void)
-{
+vespalib::string Application::getSources() {
     vespalib::string specs;
     for (std::string v : vespa::Defaults::vespaConfigSourcesRpcAddrs()) {
         if (! specs.empty()) specs += ",";
@@ -87,9 +95,7 @@ Application::getSources(void)
     return specs;
 }
 
-int
-main(int argc, char **argv)
-{
+int main(int argc, char **argv) {
     Application app;
     return app.Entry(argc, argv);
 }
diff --git a/configutil/src/lib/configstatus.cpp b/configutil/src/lib/configstatus.cpp
index f889c436a97..76a1faec625 100644
--- a/configutil/src/lib/configstatus.cpp
+++ b/configutil/src/lib/configstatus.cpp
@@ -139,10 +139,14 @@ ConfigStatus::action()
 
     for (size_t i = 0; i < _cfg->hosts.size(); i++) {
         const cloud::config::ModelConfig::Hosts &hconf = _cfg->hosts[i];
+        // TODO PERF: don't fetch entire model when we're only looking for
+        // a subset of hosts.
+        if (!_flags.host_filter.includes(hconf.name)) {
+            continue;
+        }
 
         for (size_t j = 0; j < hconf.services.size(); j++) {
             const cloud::config::ModelConfig::Hosts::Services &svc = hconf.services[j];
-
             if (svc.type == "configserver") {
                 continue;
             }
diff --git a/configutil/src/lib/configstatus.h b/configutil/src/lib/configstatus.h
index 41e3d2fe782..0b168605d49 100644
--- a/configutil/src/lib/configstatus.h
+++ b/configutil/src/lib/configstatus.h
@@ -1,6 +1,7 @@
 // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 #pragma once
 
+#include "hostfilter.h"
 #include <vespa/config-model.h>
 #include <vespa/vespalib/stllike/string.h>
 #include <vespa/config/config.h>
@@ -9,9 +10,15 @@ class ConfigStatus
 {
 public:
     struct Flags {
+        HostFilter host_filter;
         bool verbose;
         Flags()
-            : verbose(false)
+            : host_filter(), verbose(false)
+        {}
+
+        explicit Flags(const HostFilter& filter)
+            : host_filter(filter),
+              verbose(false)
         {}
     };
 
diff --git a/configutil/src/lib/hostfilter.h b/configutil/src/lib/hostfilter.h
new file mode 100644
index 00000000000..8c255c930cc
--- /dev/null
+++ b/configutil/src/lib/hostfilter.h
@@ -0,0 +1,49 @@
+// Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+#pragma once
+
+#include <unordered_set>
+#include <string>
+
+/**
+ * Simple host filter which in its default empty state implicitly includes all
+ * hosts, or only an explicit subset iff at least one host has been provided
+ * to the filter as part of construction.
+ */
+class HostFilter {
+public:
+    using HostSet = std::unordered_set<std::string>;
+private:
+    HostSet _hosts;
+public:
+    /**
+     * Empty host filter; all hosts are implicitly included.
+     */
+    HostFilter() : _hosts() {}
+
+    /**
+     * Explicitly given host set; only the hosts whose name exactly match
+     * one of the provided names will pass the includes(name) check.
+     */
+    explicit HostFilter(const std::unordered_set<std::string>& hosts)
+        : _hosts(hosts)
+    {
+    }
+
+    explicit HostFilter(std::unordered_set<std::string>&& hosts)
+        : _hosts(std::move(hosts))
+    {
+    }
+
+    HostFilter(HostFilter&&) = default;
+    HostFilter& operator=(HostFilter&&) = default;
+
+    HostFilter(const HostFilter&) = default;
+    HostFilter& operator=(const HostFilter&) = default;
+
+    bool includes(const std::string& candidate) const {
+        if (_hosts.empty()) {
+            return true;
+        }
+        return (_hosts.find(candidate) != _hosts.end());
+    }
+};
diff --git a/configutil/src/testlist.txt b/configutil/src/testlist.txt
index 79108b8c618..a85c4f988b8 100644
--- a/configutil/src/testlist.txt
+++ b/configutil/src/testlist.txt
@@ -1,3 +1,4 @@
 tests/config_status
 tests/model_inspect
 tests/tags
+tests/host_filter
diff --git a/configutil/src/tests/config_status/config_status_test.cpp b/configutil/src/tests/config_status/config_status_test.cpp
index af3f343a0b7..ac672b3a367 100644
--- a/configutil/src/tests/config_status/config_status_test.cpp
+++ b/configutil/src/tests/config_status/config_status_test.cpp
@@ -7,7 +7,8 @@
 #include <vespa/config-model.h>
 #include <vespa/config/config.h>
 #include <vespa/config/subscription/sourcespec.h>
-#include <vespa/vespalib/stllike/string.h>
+#include <vector>
+#include <string>
 
 using namespace config;
 
@@ -56,14 +57,18 @@ public:
     ConfigStatus::Flags flags;
     std::unique_ptr<ConfigStatus> status;
 
-    Status(int httpport) : flags() {
+    Status(int http_port,
+           const ConfigStatus::Flags& cfg_flags,
+           const std::vector<std::string>& model_hosts)
+        : flags(cfg_flags)
+    {
         flags.verbose = true;
         ConfigSet set;
         ConfigContext::SP ctx(new ConfigContext(set));
         cloud::config::ModelConfigBuilder builder;
         
         cloud::config::ModelConfigBuilder::Hosts::Services::Ports port;
-        port.number = httpport;
+        port.number = http_port;
         port.tags = "http state";
 
         cloud::config::ModelConfigBuilder::Hosts::Services service;
@@ -74,23 +79,33 @@ public:
         service.clustername = "default";
         service.ports.push_back(port);
 
-        cloud::config::ModelConfigBuilder::Hosts host;
-        host.services.push_back(service);
-        host.name = "localhost";
+        for (auto& mhost : model_hosts) {
+            cloud::config::ModelConfigBuilder::Hosts host;
+            host.services.push_back(service);
+            host.name = mhost;
 
-        builder.hosts.push_back(host);
+            builder.hosts.push_back(host);
+        }
 
         set.addBuilder("admin/model", &builder);
         config::ConfigUri uri("admin/model", ctx);
         std::unique_ptr<ConfigStatus> s(new ConfigStatus(flags, uri));
         status = std::move(s);
-    };
+    }
+
+    Status(int http_port)
+        : Status(http_port, ConfigStatus::Flags(), {{"localhost"}})
+    {}
 
     ~Status() {
-    };
+    }
 };
 
-TEST_FF("all ok", HTTPStatus(std::string("{\"config\": { \"all\": { \"generation\": 1 } }}")), Status(f1.getListenPort())) {
+std::string ok_json_at_gen_1() {
+    return "{\"config\": { \"all\": { \"generation\": 1 } }}";
+}
+
+TEST_FF("all ok", HTTPStatus(ok_json_at_gen_1()), Status(f1.getListenPort())) {
     ASSERT_EQUAL(0, f2.status->action());
 }
 
@@ -106,4 +121,13 @@ TEST_FF("http failure", HTTPStatus(true), Status(f1.getListenPort())) {
     ASSERT_EQUAL(1, f2.status->action());
 }
 
+TEST_F("queried host set can be constrained", HTTPStatus(ok_json_at_gen_1())) {
+    HostFilter filter({"localhost"});
+    std::vector<std::string> hosts(
+            {"localhost", "no-such-host.foo.yahoo.com"});
+    Status status(f1.getListenPort(), ConfigStatus::Flags(filter), hosts);
+    // Non-existing host should never be contacted.
+    ASSERT_EQUAL(0, status.status->action());
+}
+
 TEST_MAIN() { TEST_RUN_ALL(); }
diff --git a/configutil/src/tests/host_filter/CMakeLists.txt b/configutil/src/tests/host_filter/CMakeLists.txt
new file mode 100644
index 00000000000..964e036ce1d
--- /dev/null
+++ b/configutil/src/tests/host_filter/CMakeLists.txt
@@ -0,0 +1,8 @@
+# Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+vespa_add_executable(configutil_host_filter_test_app TEST
+    SOURCES
+    host_filter_test.cpp
+    DEPENDS
+    configutil_util
+)
+vespa_add_test(NAME configutil_host_filter_test_app COMMAND configutil_host_filter_test_app)
diff --git a/configutil/src/tests/host_filter/host_filter_test.cpp b/configutil/src/tests/host_filter/host_filter_test.cpp
new file mode 100644
index 00000000000..66c0418bb46
--- /dev/null
+++ b/configutil/src/tests/host_filter/host_filter_test.cpp
@@ -0,0 +1,18 @@
+// Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+#include <vespa/vespalib/testkit/testapp.h>
+#include <lib/hostfilter.h>
+
+TEST("empty hostfilter includes any and all hosts") {
+    HostFilter filter;
+    EXPECT_TRUE(filter.includes("foo.yahoo.com"));
+}
+
+TEST("explicit host set limits to provided hosts only") {
+    HostFilter::HostSet hosts({"bar.yahoo.com", "zoidberg.yahoo.com"});
+    HostFilter filter(std::move(hosts));
+    EXPECT_TRUE(filter.includes("bar.yahoo.com"));
+    EXPECT_TRUE(filter.includes("zoidberg.yahoo.com"));
+    EXPECT_FALSE(filter.includes("foo.yahoo.com"));
+}
+
+TEST_MAIN() { TEST_RUN_ALL(); }