Add a flag to allow for continued use of the deprecated v2 api

api/client/options.proto

@@ -105,7 +105,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
@@ -219,4 +219,6 @@ 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). Default is false.
+  google.protobuf.BoolValue allow_api_v2 = 38 [deprecated = true];
 }

include/nighthawk/client/options.h

@@ -74,6 +74,7 @@ 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 allowApiV2() const PURE;
 
   /**
    * Converts an Options instance to an equivalent CommandLineOptions instance in terms of option

source/client/options_impl.cc

@@ -315,6 +315,10 @@ OptionsImpl::OptionsImpl(int argc, const char* const* argv) {
       "Default: \"\"",
       false, "", "string", cmd);
 
+  TCLAP::SwitchArg allow_api_v2(
+      "", "allow-v2-api", "Set to allow usage of the v2 api. (Not recommended). Default: false",
+      cmd);
+
   Utility::parseCommand(cmd, argc, argv);
 
   // --duration and --no-duration are mutually exclusive
@@ -447,6 +451,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_api_v2, allow_api_v2_);
 
   // CLI-specific tests.
   // TODO(oschaaf): as per mergconflicts's remark, it would be nice to aggregate
@@ -652,7 +657,7 @@ 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_api_v2_ = PROTOBUF_GET_WRAPPED_OR_DEFAULT(options, allow_api_v2, allow_api_v2_);
   validate();
 }
 
@@ -829,6 +834,7 @@ 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_api_v2()->set_value(allow_api_v2_);
   return command_line_options;
 }
 

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 allowApiV2() const override { return allow_api_v2_; }
 
 private:
   void parsePredicates(const TCLAP::MultiArg<std::string>& arg,
@@ -149,6 +150,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_api_v2_{false};
 };
 
 } // namespace Client

source/client/process_impl.cc

@@ -270,10 +270,31 @@ ProcessImpl::mergeWorkerStatistics(const std::vector<ClientWorkerPtr>& workers)
   return merged_statistics;
 }
 
+void ProcessImpl::allowApiV2(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(std::string("true"));
+  (*runtime_layer->mutable_static_layer()
+        ->mutable_fields())[std::string("envoy.reloadable_features.enable_deprecated_v2_api")] =
+      proto_true;
+  (*runtime_layer->mutable_static_layer()
+        ->mutable_fields())[std::string("envoy.features.enable_all_deprecated_features")] =
+      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_api_v2) const {
+  if (allow_api_v2) {
+    allowApiV2(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");
@@ -454,7 +475,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_.allowApiV2());
     // Needs to happen as early as possible (before createWorkers()) in the instantiation to preempt
     // the objects that require stats.
     if (!options_.statsSinks().empty()) {
@@ -466,7 +488,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>(

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 allowApiV2(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_api_v2) 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

test/integration/BUILD

@@ -13,6 +13,7 @@ py_library(
     name = "integration_test_base",
     data = [
         "configurations/nighthawk_http_origin.yaml",
+        "configurations/nighthawk_http_origin_v2_api.yaml",
         "configurations/nighthawk_https_origin.yaml",
         "configurations/nighthawk_track_timings.yaml",
         "configurations/sni_origin.yaml",

test/integration/configurations/nighthawk_http_origin_v2_api.yaml

@@ -0,0 +1,39 @@
+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
+        envoy.features.enable_all_deprecated_features: true

test/integration/integration_test_fixtures.py

@@ -56,13 +56,14 @@ 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: number of Nighthawk Test Server backends to run, to allow testing MultiTarget mode
     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 +91,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 +143,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)
@@ -297,6 +300,25 @@ def getTestServerRootUri(self):
     return super(HttpIntegrationTestBase, self).getTestServerRootUri(False)
 
 
+class HttpIntegrationTestBaseWithV2Bootstrap(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
+  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(HttpIntegrationTestBaseWithV2Bootstrap, self).__init__(ip_version,
+                                                                 server_config,
+                                                                 bootstrap_version_arg="2")
+
+  def getTestServerRootUri(self):
+    """See base class."""
+    return super(HttpIntegrationTestBaseWithV2Bootstrap, self).getTestServerRootUri(False)
+
+
 class MultiServerHttpIntegrationTestBase(IntegrationTestBase):
   """Base for running plain http tests against multiple Nighthawk test servers."""
 
@@ -378,6 +400,19 @@ def http_test_server_fixture(request, server_config):
   f.tearDown()
 
 
+@pytest.fixture(params=determineIpVersionsFromEnvironment())
+def http_test_server_fixture_v2_api(request, server_config):
+  """Fixture for setting up a test environment with http server configuration that uses v2 configuration.
+
+  Yields:
+      HttpIntegrationTestBaseWithV2Bootstrap: A fully set up instance. Tear down will happen automatically.
+  """
+  f = HttpIntegrationTestBaseWithV2Bootstrap(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.

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 (str, 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,7 +130,10 @@ def _serverThreadRunner(self):
         self._parameterized_config_path, "-l", "debug", "--base-id", self._instance_id,
         "--admin-address-path", self._admin_address_path, "--concurrency", "1"
     ]
-    logging.info("Test server popen() args: %s" % str.join(" ", args))
+    if not self._bootstrap_version_arg is None:
+      args = args + ["--bootstrap-version", self._bootstrap_version_arg]
+
+    logging.error("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()
     logging.info("Process stdout: %s", stdout.decode("utf-8"))
@@ -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."""