Verify socket passing in hot restart test

docs/operations/cli.rst

@@ -10,6 +10,10 @@ following are the command line options that Envoy supports.
 
   *(required)* The path to the :ref:`JSON configuration file <config>`.
 
+.. option:: --admin-address-path <path string>
+
+  *(optional)* The output file path where the admin address and port will be written.
+
 .. option:: --base-id <integer>
 
   *(optional)* The base ID to use when allocating shared memory regions. Envoy uses shared memory

include/envoy/server/options.h

@@ -40,6 +40,11 @@ class Options {
    */
   virtual const std::string& configPath() PURE;
 
+  /**
+   * @return const std::string& the admin address output file.
+   */
+  virtual const std::string& adminAddressPath() PURE;
+
   /**
    * @return spdlog::level::level_enum the default log level for the server.
    */

source/common/json/json_loader.cc

@@ -239,4 +239,17 @@ ObjectPtr Factory::LoadFromString(const std::string& json) {
   return ObjectPtr{new ObjectImplRoot(std::move(document))};
 }
 
+const std::string Factory::listAsJsonString(const std::list<std::string>& items) {
+  rapidjson::StringBuffer writer_string_buffer;
+  rapidjson::Writer<rapidjson::StringBuffer> writer(writer_string_buffer);
+
+  writer.StartArray();
+  for (const std::string& item : items) {
+    writer.String(item.c_str());
+  }
+  writer.EndArray();
+
+  return writer_string_buffer.GetString();
+}
+
 } // Json

source/common/json/json_loader.h

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <list>
 #include <string>
 
 #include "envoy/json/json_object.h"
@@ -8,6 +9,7 @@ namespace Json {
 
 class Factory {
 public:
+  // TODO(hennna): Cleanup function names - i.e. s/LoadFromFile/loadFromFile/.
   /*
    * Constructs a Json Object from a File.
    */
@@ -17,6 +19,8 @@ class Factory {
    * Constructs a Json Object from a String.
    */
   static ObjectPtr LoadFromString(const std::string& json);
+
+  static const std::string listAsJsonString(const std::list<std::string>& items);
 };
 
 } // Json

source/server/http/admin.cc

@@ -1,6 +1,7 @@
 #include "server/http/admin.h"
 
 #include <cstdint>
+#include <fstream>
 #include <string>
 #include <unordered_set>
 
@@ -23,6 +24,7 @@
 #include "common/http/header_map_impl.h"
 #include "common/http/headers.h"
 #include "common/http/http1/codec_impl.h"
+#include "common/json/json_loader.h"
 #include "common/network/listen_socket_impl.h"
 #include "common/profiler/profiler.h"
 #include "common/router/config_impl.h"
@@ -284,6 +286,16 @@ Http::Code AdminImpl::handlerQuitQuitQuit(const std::string&, Buffer::Instance&
   return Http::Code::OK;
 }
 
+Http::Code AdminImpl::handlerListenerInfo(const std::string&, Buffer::Instance& response) {
+  std::list<std::string> listeners;
+  int listener_index = 0;
+  while (auto listen_socket = server_.getListenSocketByIndex(listener_index++)) {
+    listeners.push_back(listen_socket->localAddress()->asString());
+  }
+  response.add(Json::Factory::listAsJsonString(listeners));
+  return Http::Code::OK;
+}
+
 Http::Code AdminImpl::handlerCerts(const std::string&, Buffer::Instance& response) {
   // This set is used to track distinct certificates. We may have multiple listeners, upstreams, etc
   // using the same cert.
@@ -322,6 +334,7 @@ AdminImpl::NullRouteConfigProvider::NullRouteConfigProvider()
     : config_(new Router::NullConfigImpl()) {}
 
 AdminImpl::AdminImpl(const std::string& access_log_path, const std::string& profile_path,
+                     const std::string& address_out_path,
                      Network::Address::InstanceConstSharedPtr address, Server::Instance& server)
     : server_(server), profile_path_(profile_path),
       socket_(new Network::TcpListenSocket(address, true)),
@@ -343,7 +356,17 @@ AdminImpl::AdminImpl(const std::string& access_log_path, const std::string& prof
           {"/reset_counters", "reset all counters to zero", MAKE_HANDLER(handlerResetCounters)},
           {"/server_info", "print server version/status information",
            MAKE_HANDLER(handlerServerInfo)},
-          {"/stats", "print server stats", MAKE_HANDLER(handlerStats)}} {
+          {"/stats", "print server stats", MAKE_HANDLER(handlerStats)},
+          {"/listeners", "print listener addresses", MAKE_HANDLER(handlerListenerInfo)}} {
+
+  if (!address_out_path.empty()) {
+    std::ofstream address_out_file(address_out_path);
+    if (!address_out_file) {
+      log().critical("cannot open admin address output file {} for writing.", address_out_path);
+    } else {
+      address_out_file << socket_->localAddress()->asString();
+    }
+  }
 
   access_logs_.emplace_back(new Http::AccessLog::InstanceImpl(
       access_log_path, {}, Http::AccessLog::AccessLogFormatUtils::defaultAccessLogFormatter(),

source/server/http/admin.h

@@ -29,7 +29,8 @@ class AdminImpl : public Admin,
                   Logger::Loggable<Logger::Id::admin> {
 public:
   AdminImpl(const std::string& access_log_path, const std::string& profiler_path,
-            Network::Address::InstanceConstSharedPtr address, Server::Instance& server);
+            const std::string& address_out_path, Network::Address::InstanceConstSharedPtr address,
+            Server::Instance& server);
 
   Http::Code runCallback(const std::string& path, Buffer::Instance& response);
   const Network::ListenSocket& socket() override { return *socket_; }
@@ -118,6 +119,7 @@ class AdminImpl : public Admin,
   Http::Code handlerServerInfo(const std::string& url, Buffer::Instance& response);
   Http::Code handlerStats(const std::string& url, Buffer::Instance& response);
   Http::Code handlerQuitQuitQuit(const std::string& url, Buffer::Instance& response);
+  Http::Code handlerListenerInfo(const std::string& url, Buffer::Instance& response);
 
   Server::Instance& server_;
   std::list<Http::AccessLog::InstanceSharedPtr> access_logs_;

source/server/options_impl.cc

@@ -29,6 +29,8 @@ OptionsImpl::OptionsImpl(int argc, char** argv, const std::string& hot_restart_v
                                         std::thread::hardware_concurrency(), "uint32_t", cmd);
   TCLAP::ValueArg<std::string> config_path("c", "config-path", "Path to configuration file", false,
                                            "", "string", cmd);
+  TCLAP::ValueArg<std::string> admin_address_path("", "admin-address-path", "Admin address path",
+                                                  false, "", "string", cmd);
   TCLAP::ValueArg<std::string> log_level("l", "log-level", log_levels_string, false,
                                          spdlog::level::level_names[default_log_level], "string",
                                          cmd);
@@ -74,6 +76,7 @@ OptionsImpl::OptionsImpl(int argc, char** argv, const std::string& hot_restart_v
   base_id_ = base_id.getValue() * 10;
   concurrency_ = concurrency.getValue();
   config_path_ = config_path.getValue();
+  admin_address_path_ = admin_address_path.getValue();
   restart_epoch_ = restart_epoch.getValue();
   service_cluster_ = service_cluster.getValue();
   service_node_ = service_node.getValue();

source/server/options_impl.h

@@ -24,6 +24,7 @@ class OptionsImpl : public Server::Options {
   uint64_t baseId() override { return base_id_; }
   uint32_t concurrency() override { return concurrency_; }
   const std::string& configPath() override { return config_path_; }
+  const std::string& adminAddressPath() override { return admin_address_path_; }
   std::chrono::seconds drainTime() override { return drain_time_; }
   spdlog::level::level_enum logLevel() override { return log_level_; }
   std::chrono::seconds parentShutdownTime() override { return parent_shutdown_time_; }
@@ -34,6 +35,7 @@ class OptionsImpl : public Server::Options {
   uint64_t base_id_;
   uint32_t concurrency_;
   std::string config_path_;
+  std::string admin_address_path_;
   spdlog::level::level_enum log_level_;
   uint64_t restart_epoch_;
   std::string service_cluster_;

source/server/server.cc

@@ -189,8 +189,9 @@ void InstanceImpl::initialize(Options& options, TestHooks& hooks,
   restarter_.shutdownParentAdmin(info);
   original_start_time_ = info.original_start_time_;
   admin_.reset(new AdminImpl(initial_config.admin().accessLogPath(),
-                             initial_config.admin().profilePath(), initial_config.admin().address(),
-                             *this));
+                             initial_config.admin().profilePath(), options.adminAddressPath(),
+                             initial_config.admin().address(), *this));
+
   admin_scope_ = stats_store_.createScope("listener.admin.");
   handler_.addListener(*admin_, admin_->mutable_socket(), *admin_scope_,
                        Network::ListenerOptions::listenerOptionsWithBindToPort());

test/common/json/json_loader_test.cc

@@ -96,6 +96,7 @@ TEST(JsonLoaderTest, Basic) {
 
     ObjectPtr config = Factory::LoadFromString(json);
     EXPECT_EQ(2U, config->getObjectArray("descriptors")[0]->asObjectArray().size());
+
     EXPECT_EQ(1U, config->getObjectArray("descriptors")[1]->asObjectArray().size());
   }
 
@@ -227,4 +228,42 @@ TEST(JsonLoaderTest, AsString) {
   });
 }
 
+TEST(JsonLoaderTest, ListAsString) {
+  {
+    std::list<std::string> list = {};
+    Json::ObjectPtr json = Json::Factory::LoadFromString(Json::Factory::listAsJsonString(list));
+    std::vector<Json::ObjectPtr> output = json->asObjectArray();
+    EXPECT_TRUE(output.empty());
+  }
+
+  {
+    std::list<std::string> list = {"one"};
+    Json::ObjectPtr json = Json::Factory::LoadFromString(Json::Factory::listAsJsonString(list));
+    std::vector<Json::ObjectPtr> output = json->asObjectArray();
+    EXPECT_EQ(1, output.size());
+    EXPECT_EQ("one", output[0]->asString());
+  }
+
+  {
+    std::list<std::string> list = {"one", "two", "three", "four"};
+    Json::ObjectPtr json = Json::Factory::LoadFromString(Json::Factory::listAsJsonString(list));
+    std::vector<Json::ObjectPtr> output = json->asObjectArray();
+    EXPECT_EQ(4, output.size());
+    EXPECT_EQ("one", output[0]->asString());
+    EXPECT_EQ("two", output[1]->asString());
+    EXPECT_EQ("three", output[2]->asString());
+    EXPECT_EQ("four", output[3]->asString());
+  }
+
+  {
+    std::list<std::string> list = {"127.0.0.1:46465", "127.0.0.1:52211", "127.0.0.1:58941"};
+    Json::ObjectPtr json = Json::Factory::LoadFromString(Json::Factory::listAsJsonString(list));
+    std::vector<Json::ObjectPtr> output = json->asObjectArray();
+    EXPECT_EQ(3, output.size());
+    EXPECT_EQ("127.0.0.1:46465", output[0]->asString());
+    EXPECT_EQ("127.0.0.1:52211", output[1]->asString());
+    EXPECT_EQ("127.0.0.1:58941", output[2]->asString());
+  }
+}
+
 } // Json

test/integration/BUILD

@@ -16,6 +16,7 @@ envoy_sh_test(
         "//test/common/runtime:filesystem_setup.sh",
         "//test/common/runtime:filesystem_test_data",
         "//test/config/integration:server_config_files",
+        "//tools:socket_passing",
     ],
     # Need to disable test execution sandboxing for this test, since it uses shmem.
     # TODO(htuch): When most folks are using a Bazel with the recent

test/integration/hotrestart_test.sh

@@ -16,11 +16,19 @@ cat "${TEST_RUNDIR}"/test/config/integration/server.json |
 # Now start the real server, hot restart it twice, and shut it all down as a basic hot restart
 # sanity test.
 echo "Starting epoch 0"
+ADMIN_ADDRESS_PATH_0="${TEST_TMPDIR}"/admin_0.address
 "${ENVOY_BIN}" -c "${HOT_RESTART_JSON}" \
-    --restart-epoch 0 --base-id 1 --service-cluster cluster --service-node node &
+    --restart-epoch 0 --base-id 1 --service-cluster cluster --service-node node \
+    --admin-address-path "${ADMIN_ADDRESS_PATH_0}" &
 
 FIRST_SERVER_PID=$!
 sleep 3
+
+echo "Updating original config json listener addresses"
+UPDATED_HOT_RESTART_JSON="${TEST_TMPDIR}"/hot_restart_updated.json
+"${TEST_RUNDIR}"/tools/socket_passing.py "-o" "${HOT_RESTART_JSON}" "-a" "${ADMIN_ADDRESS_PATH_0}" \
+  "-u" "${UPDATED_HOT_RESTART_JSON}"
+
 # Send SIGUSR1 signal to the first server, this should not kill it. Also send SIGHUP which should
 # get eaten.
 echo "Sending SIGUSR1/SIGHUP to first server"
@@ -29,20 +37,38 @@ kill -SIGHUP ${FIRST_SERVER_PID}
 sleep 3
 
 echo "Starting epoch 1"
-"${ENVOY_BIN}" -c "${HOT_RESTART_JSON}" \
-    --restart-epoch 1 --base-id 1 --service-cluster cluster --service-node node &
+ADMIN_ADDRESS_PATH_1="${TEST_TMPDIR}"/admin_1.address
+"${ENVOY_BIN}" -c "${UPDATED_HOT_RESTART_JSON}" \
+    --restart-epoch 1 --base-id 1 --service-cluster cluster --service-node node \
+    --admin-address-path "${ADMIN_ADDRESS_PATH_1}" &
 
 SECOND_SERVER_PID=$!
 # Wait for stat flushing
 sleep 7
 
+echo "Checking that listener addresses have not changed"
+HOT_RESTART_JSON_1="${TEST_TMPDIR}"/hot_restart_1.json
+"${TEST_RUNDIR}"/tools/socket_passing.py "-o" "${UPDATED_HOT_RESTART_JSON}" "-a" "${ADMIN_ADDRESS_PATH_1}" \
+  "-u" "${HOT_RESTART_JSON_1}"
+CONFIG_DIFF=$(diff -Z "${UPDATED_HOT_RESTART_JSON}" "${HOT_RESTART_JSON_1}")
+[[ -z "${CONFIG_DIFF}" ]]
+
+ADMIN_ADDRESS_PATH_2="${TEST_TMPDIR}"/admin_2.address
 echo "Starting epoch 2"
-"${ENVOY_BIN}" -c "${HOT_RESTART_JSON}" \
-    --restart-epoch 2 --base-id 1 --service-cluster cluster --service-node node &
+"${ENVOY_BIN}" -c "${UPDATED_HOT_RESTART_JSON}" \
+    --restart-epoch 2 --base-id 1 --service-cluster cluster --service-node node \
+    --admin-address-path "${ADMIN_ADDRESS_PATH_2}" &
 
 THIRD_SERVER_PID=$!
 sleep 3
 
+echo "Checking that listener addresses have not changed"
+HOT_RESTART_JSON_2="${TEST_TMPDIR}"/hot_restart_2.json
+"${TEST_RUNDIR}"/tools/socket_passing.py "-o" "${UPDATED_HOT_RESTART_JSON}" "-a" "${ADMIN_ADDRESS_PATH_2}" \
+  "-u" "${HOT_RESTART_JSON_2}"
+CONFIG_DIFF=$(diff -Z "${UPDATED_HOT_RESTART_JSON}" "${HOT_RESTART_JSON_2}")
+[[ -z "${CONFIG_DIFF}" ]]
+
 # First server should already be gone.
 echo "Waiting for epoch 0"
 wait ${FIRST_SERVER_PID}

test/integration/integration_admin_test.cc

@@ -1,5 +1,7 @@
 #include "envoy/http/header_map.h"
 
+#include "common/json/json_loader.h"
+
 #include "test/integration/integration_test.h"
 #include "test/integration/utility.h"
 
@@ -122,6 +124,18 @@ TEST_F(IntegrationTest, Admin) {
                                                 Http::CodecClient::Type::HTTP1);
   EXPECT_TRUE(response->complete());
   EXPECT_STREQ("200", response->headers().Status()->value().c_str());
+
+  response = IntegrationUtil::makeSingleRequest(lookupPort("admin"), "GET", "/listeners", "",
+                                                Http::CodecClient::Type::HTTP1);
+  EXPECT_TRUE(response->complete());
+  EXPECT_STREQ("200", response->headers().Status()->value().c_str());
+
+  Json::ObjectPtr json = Json::Factory::LoadFromString(response->body());
+  std::vector<Json::ObjectPtr> listener_info = json->asObjectArray();
+  for (std::size_t index = 0; index < listener_info.size(); index++) {
+    EXPECT_EQ(test_server_->server().getListenSocketByIndex(index)->localAddress()->asString(),
+              listener_info[index]->asString());
+  }
 }
 
 // Successful call to startProfiler requires tcmalloc.

test/integration/server.h

@@ -34,6 +34,7 @@ class TestOptionsImpl : public Options {
   uint64_t baseId() override { return 0; }
   uint32_t concurrency() override { return 1; }
   const std::string& configPath() override { return config_path_; }
+  const std::string& adminAddressPath() override { return admin_address_path_; }
   std::chrono::seconds drainTime() override { return std::chrono::seconds(0); }
   spdlog::level::level_enum logLevel() override { NOT_IMPLEMENTED; }
   std::chrono::seconds parentShutdownTime() override { return std::chrono::seconds(0); }
@@ -44,6 +45,7 @@ class TestOptionsImpl : public Options {
 
 private:
   const std::string config_path_;
+  const std::string admin_address_path_;
 };
 
 class TestDrainManager : public DrainManager {

test/mocks/server/mocks.cc

@@ -13,8 +13,10 @@ using testing::SaveArg;
 
 namespace Server {
 
-MockOptions::MockOptions(const std::string& path) : path_(path) {
-  ON_CALL(*this, configPath()).WillByDefault(ReturnRef(path_));
+MockOptions::MockOptions(const std::string& config_path)
+    : config_path_(config_path), admin_address_path_("") {
+  ON_CALL(*this, configPath()).WillByDefault(ReturnRef(config_path_));
+  ON_CALL(*this, adminAddressPath()).WillByDefault(ReturnRef(admin_address_path_));
 }
 MockOptions::~MockOptions() {}
 

test/mocks/server/mocks.h

@@ -45,13 +45,15 @@ class MockOptions : public Options {
   MOCK_METHOD0(baseId, uint64_t());
   MOCK_METHOD0(concurrency, uint32_t());
   MOCK_METHOD0(configPath, const std::string&());
+  MOCK_METHOD0(adminAddressPath, const std::string&());
   MOCK_METHOD0(drainTime, std::chrono::seconds());
   MOCK_METHOD0(logLevel, spdlog::level::level_enum());
   MOCK_METHOD0(parentShutdownTime, std::chrono::seconds());
   MOCK_METHOD0(restartEpoch, uint64_t());
   MOCK_METHOD0(fileFlushIntervalMsec, std::chrono::milliseconds());
 
-  std::string path_;
+  std::string config_path_;
+  std::string admin_address_path_;
 };
 
 class MockAdmin : public Admin {

test/server/http/BUILD

@@ -15,6 +15,7 @@ envoy_cc_test(
         "//source/server/http:admin_lib",
         "//test/mocks/server:server_mocks",
         "//test/test_common:environment_lib",
+        "//test/test_common:network_utility_lib",
         "//test/test_common:utility_lib",
     ],
 )