envoyproxy · mum4k · Dec 8, 2020 · Dec 2, 2020 · Dec 2, 2020 · Dec 3, 2020
diff --git a/README.md b/README.md
@@ -43,7 +43,8 @@ bazel build -c opt //:nighthawk
 ```
 USAGE:
 
-bazel-bin/nighthawk_client  [--latency-response-header-name <string>]
+bazel-bin/nighthawk_client  [--allow-envoy-deprecated-v2-api]
+[--latency-response-header-name <string>]
 [--stats-flush-interval <uint32_t>]
 [--stats-sinks <string>] ...
 [--no-duration] [--simple-warmup]
@@ -83,6 +84,10 @@ format>
 
 Where:
 
+--allow-envoy-deprecated-v2-api
+Set to allow usage of the v2 api. (Not recommended, support will stop
+in Q1 2021). Default: false
+
 --latency-response-header-name <string>
 Set an optional header name that will be returned in responses, whose
 values will be tracked in a latency histogram if set. Can be used in

diff --git a/api/client/options.proto b/api/client/options.proto
@@ -106,7 +106,7 @@ message H1ConnectionReuseStrategy {
 
 // TODO(oschaaf): Ultimately this will be a load test specification. The fact that it
 // can arrive via CLI is just a concrete detail. Change this to reflect that.
-// highest unused number is 38
+// highest unused number is 39
 message CommandLineOptions {
   // The target requests-per-second rate. Default: 5.
   google.protobuf.UInt32Value requests_per_second = 1
@@ -220,6 +220,8 @@ message CommandLineOptions {
   // "emit_previous_request_delta_in_response_header" to record elapsed time between request
   // arrivals.
   google.protobuf.StringValue latency_response_header_name = 36;
+  // Set to allow usage of the v2 api. (Not recommended, support will stop in Q1 2021).
+  google.protobuf.BoolValue allow_envoy_deprecated_v2_api = 38 [deprecated = true];
   // Provide an execution starting date and time. Optional, any value specified must be in the
   // future.
   google.protobuf.Timestamp scheduled_start = 105;

diff --git a/include/nighthawk/client/options.h b/include/nighthawk/client/options.h
@@ -75,6 +75,8 @@ class Options {
   virtual std::vector<envoy::config::metrics::v3::StatsSink> statsSinks() const PURE;
   virtual uint32_t statsFlushInterval() const PURE;
   virtual std::string responseHeaderWithLatencyInput() const PURE;
+  virtual bool allowEnvoyDeprecatedV2Api() const PURE;
+
   virtual absl::optional<Envoy::SystemTime> scheduled_start() const PURE;
   /**
    * Converts an Options instance to an equivalent CommandLineOptions instance in terms of option

diff --git a/source/client/options_impl.cc b/source/client/options_impl.cc
@@ -315,6 +315,12 @@ OptionsImpl::OptionsImpl(int argc, const char* const* argv) {
       "Default: \"\"",
       false, "", "string", cmd);
 
+  TCLAP::SwitchArg allow_envoy_deprecated_v2_api(
+      "", "allow-envoy-deprecated-v2-api",
+      "Set to allow usage of the v2 api. (Not recommended, support will stop in Q1 2021). Default: "
+      "false",
+      cmd);
+
   Utility::parseCommand(cmd, argc, argv);
 
   // --duration and --no-duration are mutually exclusive
@@ -447,6 +453,7 @@ OptionsImpl::OptionsImpl(int argc, const char* const* argv) {
   }
   TCLAP_SET_IF_SPECIFIED(stats_flush_interval, stats_flush_interval_);
   TCLAP_SET_IF_SPECIFIED(latency_response_header_name, latency_response_header_name_);
+  TCLAP_SET_IF_SPECIFIED(allow_envoy_deprecated_v2_api, allow_envoy_deprecated_v2_api_);
 
   // CLI-specific tests.
   // TODO(oschaaf): as per mergconflicts's remark, it would be nice to aggregate
@@ -652,6 +659,8 @@ OptionsImpl::OptionsImpl(const nighthawk::client::CommandLineOptions& options) {
   std::copy(options.labels().begin(), options.labels().end(), std::back_inserter(labels_));
   latency_response_header_name_ = PROTOBUF_GET_WRAPPED_OR_DEFAULT(
       options, latency_response_header_name, latency_response_header_name_);
+  allow_envoy_deprecated_v2_api_ = PROTOBUF_GET_WRAPPED_OR_DEFAULT(
+      options, allow_envoy_deprecated_v2_api, allow_envoy_deprecated_v2_api_);
   if (options.has_scheduled_start()) {
     const auto elapsed_since_epoch = std::chrono::nanoseconds(options.scheduled_start().nanos()) +
                                      std::chrono::seconds(options.scheduled_start().seconds());
@@ -834,6 +843,8 @@ CommandLineOptionsPtr OptionsImpl::toCommandLineOptionsInternal() const {
   command_line_options->mutable_stats_flush_interval()->set_value(stats_flush_interval_);
   command_line_options->mutable_latency_response_header_name()->set_value(
       latency_response_header_name_);
+  command_line_options->mutable_allow_envoy_deprecated_v2_api()->set_value(
+      allow_envoy_deprecated_v2_api_);
   if (scheduled_start_.has_value()) {
     *(command_line_options->mutable_scheduled_start()) =
         Envoy::ProtobufUtil::TimeUtil::NanosecondsToTimestamp(

diff --git a/source/client/options_impl.h b/source/client/options_impl.h
@@ -93,6 +93,7 @@ class OptionsImpl : public Options, public Envoy::Logger::Loggable<Envoy::Logger
   std::string responseHeaderWithLatencyInput() const override {
     return latency_response_header_name_;
   };
+  bool allowEnvoyDeprecatedV2Api() const override { return allow_envoy_deprecated_v2_api_; }
   absl::optional<Envoy::SystemTime> scheduled_start() const override { return scheduled_start_; }
 
 private:
@@ -150,6 +151,7 @@ class OptionsImpl : public Options, public Envoy::Logger::Loggable<Envoy::Logger
   std::vector<envoy::config::metrics::v3::StatsSink> stats_sinks_;
   uint32_t stats_flush_interval_{5};
   std::string latency_response_header_name_;
+  bool allow_envoy_deprecated_v2_api_{false};
   absl::optional<Envoy::SystemTime> scheduled_start_;
 };
 

diff --git a/source/client/process_impl.cc b/source/client/process_impl.cc
@@ -276,10 +276,28 @@ ProcessImpl::mergeWorkerStatistics(const std::vector<ClientWorkerPtr>& workers)
   return merged_statistics;
 }
 
+void ProcessImpl::allowEnvoyDeprecatedV2Api(envoy::config::bootstrap::v3::Bootstrap& bootstrap) {
+  auto* admin_layer = bootstrap.mutable_layered_runtime()->add_layers();
+  admin_layer->set_name("admin layer");
+  admin_layer->mutable_admin_layer();
+  envoy::config::bootstrap::v3::RuntimeLayer* runtime_layer =
+      bootstrap.mutable_layered_runtime()->add_layers();
+  runtime_layer->set_name("static_layer");
+  Envoy::ProtobufWkt::Value proto_true;
+  proto_true.set_string_value("true");
+  (*runtime_layer->mutable_static_layer()
+        ->mutable_fields())["envoy.reloadable_features.enable_deprecated_v2_api"] = proto_true;
+}
+
 void ProcessImpl::createBootstrapConfiguration(envoy::config::bootstrap::v3::Bootstrap& bootstrap,
                                                const std::vector<UriPtr>& uris,
                                                const UriPtr& request_source_uri,
-                                               int number_of_clusters) const {
+                                               int number_of_clusters,
+                                               bool allow_envoy_deprecated_v2_api) const {
+  if (allow_envoy_deprecated_v2_api) {
+    allowEnvoyDeprecatedV2Api(bootstrap);
+  }
+
   for (int i = 0; i < number_of_clusters; i++) {
     auto* cluster = bootstrap.mutable_static_resources()->add_clusters();
     RELEASE_ASSERT(!uris.empty(), "illegal configuration with zero endpoints");
@@ -466,7 +484,8 @@ bool ProcessImpl::runInternal(OutputCollector& collector, const std::vector<UriP
     int number_of_workers = determineConcurrency();
     shutdown_ = false;
     envoy::config::bootstrap::v3::Bootstrap bootstrap;
-    createBootstrapConfiguration(bootstrap, uris, request_source_uri, number_of_workers);
+    createBootstrapConfiguration(bootstrap, uris, request_source_uri, number_of_workers,
+                                 options_.allowEnvoyDeprecatedV2Api());
     // Needs to happen as early as possible (before createWorkers()) in the instantiation to preempt
     // the objects that require stats.
     if (!options_.statsSinks().empty()) {
@@ -478,7 +497,7 @@ bool ProcessImpl::runInternal(OutputCollector& collector, const std::vector<UriP
     store_root_.initializeThreading(*dispatcher_, tls_);
     runtime_singleton_ = std::make_unique<Envoy::Runtime::ScopedLoaderSingleton>(
         Envoy::Runtime::LoaderPtr{new Envoy::Runtime::LoaderImpl(
-            *dispatcher_, tls_, {}, *local_info_, store_root_, generator_,
+            *dispatcher_, tls_, bootstrap.layered_runtime(), *local_info_, store_root_, generator_,
             Envoy::ProtobufMessage::getStrictValidationVisitor(), *api_)});
     ssl_context_manager_ =
         std::make_unique<Envoy::Extensions::TransportSockets::Tls::ContextManagerImpl>(

diff --git a/source/client/process_impl.h b/source/client/process_impl.h
@@ -30,6 +30,7 @@
 #include "external/envoy/source/exe/process_wide.h"
 #include "external/envoy/source/extensions/transport_sockets/tls/context_manager_impl.h"
 #include "external/envoy/source/server/config_validation/admin.h"
+#include "external/envoy_api/envoy/config/bootstrap/v3/bootstrap.pb.h"
 
 #include "client/benchmark_client_impl.h"
 #include "client/factories_impl.h"
@@ -84,6 +85,12 @@ class ProcessImpl : public Process, public Envoy::Logger::Loggable<Envoy::Logger
 
   bool requestExecutionCancellation() override;
 
+  /**
+   * @param bootstrap The bootstrap that should have it's runtime configuration
+   * modified to allow for api v2 usage.
+   */
+  static void allowEnvoyDeprecatedV2Api(envoy::config::bootstrap::v3::Bootstrap& bootstrap);
+
 private:
   /**
    * @brief Creates a cluster for usage by a remote request source.
@@ -99,9 +106,9 @@ class ProcessImpl : public Process, public Envoy::Logger::Loggable<Envoy::Logger
                                   const Uri& uri) const;
   void createBootstrapConfiguration(envoy::config::bootstrap::v3::Bootstrap& bootstrap,
                                     const std::vector<UriPtr>& uris,
-                                    const UriPtr& request_source_uri, int number_of_workers) const;
+                                    const UriPtr& request_source_uri, int number_of_workers,
+                                    bool allow_envoy_deprecated_v2_api) const;
   void maybeCreateTracingDriver(const envoy::config::trace::v3::Tracing& configuration);
-
   void configureComponentLogLevels(spdlog::level::level_enum level);
   /**
    * Prepare the ProcessImpl instance by creating and configuring the workers it needs for execution

diff --git a/test/integration/BUILD b/test/integration/BUILD
@@ -13,6 +13,7 @@ py_library(
     name = "integration_test_base",
     data = [
         "configurations/nighthawk_http_origin.yaml",
+        "configurations/nighthawk_http_origin_envoy_deprecated_v2_api.yaml",
         "configurations/nighthawk_https_origin.yaml",
         "configurations/nighthawk_track_timings.yaml",
         "configurations/sni_origin.yaml",

diff --git a/test/integration/configurations/nighthawk_http_origin_envoy_deprecated_v2_api.yaml b/test/integration/configurations/nighthawk_http_origin_envoy_deprecated_v2_api.yaml
@@ -0,0 +1,39 @@
+# This file is intentionally using the v2 api: it is used to test support for that.
+admin:
+  access_log_path: $tmpdir/nighthawk-test-server-admin-access.log
+  profile_path: $tmpdir/nighthawk-test-server.prof
+  address:
+    socket_address: { address: $server_ip, port_value: 0 }
+static_resources:
+  listeners:
+  - address:
+      socket_address:
+        address: $server_ip
+        port_value: 0
+    filter_chains:
+    - filters:
+      - name: envoy.http_connection_manager
+        config:
+          generate_request_id: false
+          codec_type: auto
+          stat_prefix: ingress_http
+          route_config:
+            name: local_route
+            virtual_hosts:
+            - name: service
+              domains:
+              - "*"
+          http_filters:
+          - name: test-server
+            config:
+              response_body_size: 10
+              response_headers:
+              - { header: { key: "x-nh", value: "1"}}
+          - name: envoy.router
+            config:
+              dynamic_stats: false
+layered_runtime:
+  layers:
+    - name: static_layer
+      static_layer:
+        envoy.reloadable_features.enable_deprecated_v2_api: true
diff --git a/test/integration/integration_test_fixtures.py b/test/integration/integration_test_fixtures.py
@@ -56,13 +56,15 @@ class IntegrationTestBase():
   This class will be refactored (https://github.com/envoyproxy/nighthawk/issues/258).
   """
 
-  def __init__(self, ip_version, server_config, backend_count=1):
+  def __init__(self, ip_version, server_config, backend_count=1, bootstrap_version_arg=None):
     """Initialize the IntegrationTestBase instance.
 
     Args:
       ip_version: a single IP mode that this instance will test: IpVersion.IPV4 or IpVersion.IPV6
       server_config: path to the server configuration
       backend_count: number of Nighthawk Test Server backends to run, to allow testing MultiTarget mode
+      bootstrap_version_arg: An optional int, specify a bootstrap cli argument value for the test server binary. If None is specified, no bootstrap cli argment will be passed.
+
     Attributes:
       ip_version: IP version that the proxy should use when listening.
       server_ip: string containing the server ip that will be used to listen
@@ -90,6 +92,7 @@ def __init__(self, ip_version, server_config, backend_count=1):
     self._test_servers = []
     self._backend_count = backend_count
     self._test_id = ""
+    self._bootstrap_version_arg = bootstrap_version_arg
 
   # TODO(oschaaf): For the NH test server, add a way to let it determine a port by itself and pull that
   # out.
@@ -141,7 +144,8 @@ def _tryStartTestServers(self):
                                         self.server_ip,
                                         self.ip_version,
                                         parameters=self.parameters,
-                                        tag=self.tag)
+                                        tag=self.tag,
+                                        bootstrap_version_arg=self._bootstrap_version_arg)
       if not test_server.start():
         return False
       self._test_servers.append(test_server)
@@ -283,7 +287,7 @@ def startNighthawkGrpcService(self, service_name="traffic-generator-service"):
 class HttpIntegrationTestBase(IntegrationTestBase):
   """Base for running plain http tests against the Nighthawk test server.
 
-  NOTE: any script that consumes derivations of this, needs to needs also explictly
+  NOTE: any script that consumes derivations of this, needs to also explicitly
   import server_config, to avoid errors caused by the server_config not being found
   by pytest.
   """
@@ -297,6 +301,25 @@ def getTestServerRootUri(self):
     return super(HttpIntegrationTestBase, self).getTestServerRootUri(False)
 
 
+class HttpIntegrationTestBaseWithEnvoyDeprecatedV2Bootstrap(IntegrationTestBase):
+  """Base for running plain http tests against the Nighthawk test server.
+
+  NOTE: any script that consumes derivations of this, needs to also explicitly
+  import server_config, to avoid errors caused by the server_config not being found
+  by pytest.
+  """
+
+  def __init__(self, ip_version, server_config):
+    """See base class."""
+    super(HttpIntegrationTestBaseWithEnvoyDeprecatedV2Bootstrap,
+          self).__init__(ip_version, server_config, bootstrap_version_arg=2)
+
+  def getTestServerRootUri(self):
+    """See base class."""
+    return super(HttpIntegrationTestBaseWithEnvoyDeprecatedV2Bootstrap,
+                 self).getTestServerRootUri(False)
+
+
 class MultiServerHttpIntegrationTestBase(IntegrationTestBase):
   """Base for running plain http tests against multiple Nighthawk test servers."""
 
@@ -378,6 +401,19 @@ def http_test_server_fixture(request, server_config):
   f.tearDown()
 
 
+@pytest.fixture(params=determineIpVersionsFromEnvironment())
+def http_test_server_fixture_envoy_deprecated_v2_api(request, server_config):
+  """Fixture for setting up a test environment with http server configuration that uses v2 configuration.
+
+  Yields:
+      HttpIntegrationTestBaseWithEnvoyDeprecatedV2Bootstrap: A fully set up instance. Tear down will happen automatically.
+  """
+  f = HttpIntegrationTestBaseWithEnvoyDeprecatedV2Bootstrap(request.param, server_config)
+  f.setUp()
+  yield f
+  f.tearDown()
+
+
 @pytest.fixture(params=determineIpVersionsFromEnvironment())
 def https_test_server_fixture(request, server_config):
   """Fixture for setting up a test environment with the stock https server configuration.

diff --git a/test/integration/nighthawk_test_server.py b/test/integration/nighthawk_test_server.py
@@ -51,8 +51,15 @@ class TestServerBase(object):
     tmpdir: String, indicates the location used to store outputs like logs.
   """
 
-  def __init__(self, server_binary_path, config_template_path, server_ip, ip_version,
-               server_binary_config_path_arg, parameters, tag):
+  def __init__(self,
+               server_binary_path,
+               config_template_path,
+               server_ip,
+               ip_version,
+               server_binary_config_path_arg,
+               parameters,
+               tag,
+               bootstrap_version_arg=None):
     """Initialize a TestServerBase instance.
 
     Args:
@@ -63,6 +70,7 @@ def __init__(self, server_binary_path, config_template_path, server_ip, ip_versi
         server_binary_config_path_arg (str): Specify the name of the CLI argument the test server binary uses to accept a configuration path.
         parameters (dict): Supply to provide configuration template parameter replacement values.
         tag (str): Supply to get recognizeable output locations.
+        bootstrap_version_arg (int, optional): specify a bootstrap cli argument value for the test server binary.
     """
     assert ip_version != IpVersion.UNKNOWN
     self.ip_version = ip_version
@@ -82,6 +90,7 @@ def __init__(self, server_binary_path, config_template_path, server_ip, ip_versi
     self._parameterized_config_path = ""
     self._instance_id = str(random.randint(1, 1024 * 1024 * 1024))
     self._server_binary_config_path_arg = server_binary_config_path_arg
+    self._bootstrap_version_arg = bootstrap_version_arg
     self._prepareForExecution()
 
   def _prepareForExecution(self):
@@ -121,6 +130,9 @@ def _serverThreadRunner(self):
         self._parameterized_config_path, "-l", "debug", "--base-id", self._instance_id,
         "--admin-address-path", self._admin_address_path, "--concurrency", "1"
     ]
+    if self._bootstrap_version_arg is not None:
+      args = args + ["--bootstrap-version", str(self._bootstrap_version_arg)]
+
     logging.info("Test server popen() args: %s" % str.join(" ", args))
     self._server_process = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
     stdout, stderr = self._server_process.communicate()
@@ -228,7 +240,8 @@ def __init__(self,
                server_ip,
                ip_version,
                parameters=dict(),
-               tag=""):
+               tag="",
+               bootstrap_version_arg=None):
     """Initialize a NighthawkTestServer instance.
 
     Args:
@@ -238,9 +251,16 @@ def __init__(self,
         ip_version (IPVersion): IPVersion enum member indicating the ip version that the server should use when listening.
         parameters (dictionary, optional): Directionary with replacement values for substition purposes in the server configuration template. Defaults to dict().
         tag (str, optional): Tags. Supply this to get recognizeable output locations. Defaults to "".
+        bootstrap_version_arg (String, optional): Specify a cli argument value for --bootstrap-version when running the server.
     """
-    super(NighthawkTestServer, self).__init__(server_binary_path, config_template_path, server_ip,
-                                              ip_version, "--config-path", parameters, tag)
+    super(NighthawkTestServer, self).__init__(server_binary_path,
+                                              config_template_path,
+                                              server_ip,
+                                              ip_version,
+                                              "--config-path",
+                                              parameters,
+                                              tag,
+                                              bootstrap_version_arg=bootstrap_version_arg)
 
   def getCliVersionString(self):
     """Get the version string as written to the output by the CLI."""