feat: inbound only graceful draining (envoyproxy#38430)

Trying envoyproxy#37873 again to get to the bottom of the flakes that caused us to
need to revert. This got a lot more complex after envoyproxy#38333, so I may need
@yanavlasov to help me out with the proper semantics

Commit Message: 
```
Fixes envoyproxy#35020. The inbound_only query param for the drain_listeners admin
endpoint doesn't work with the graceful query parameter. As a result,
outbound listeners will send connection: close headers to upstreams
which is undesired. This PR adds the ability for drain_manager to drain
in a single direction.
```
Additional Description:
Risk Level: Medium
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional [API
Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):]

---------

Signed-off-by: Keith Mattix II <[email protected]>

Author: keithmattix

.bazelrc

@@ -8,6 +8,7 @@
 # leave room for compiler/linker.
 # The number 3G is chosen heuristically to both support large VM and small VM with RBE.
 # Startup options cannot be selected via config.
+# TODO: Adding just to test android
 startup --host_jvm_args=-Xmx3g
 
 common --noenable_bzlmod

changelogs/current.yaml

@@ -146,6 +146,10 @@ new_features:
   change: |
     Added support for outlier detection in UDP proxy. This change can be temporarily reverted by setting runtime guard
     ``envoy.reloadable_features.enable_udp_proxy_outlier_detection`` to ``false``.
+- area: admin
+  change: |
+    Add support for the inbound_only and graceful query params of /drain_listeners to be used together by
+    implementing directional draining in DrainManager.
 - area: http
   change: |
     Added alpha support for asynchronous load balancing. See

docs/root/operations/admin.rst

@@ -363,7 +363,8 @@ modify different aspects of the server:
 
    :ref:`Drains <arch_overview_draining>` all inbound listeners. ``traffic_direction`` field in
    :ref:`Listener <envoy_v3_api_msg_config.listener.v3.Listener>` is used to determine whether a listener
-   is inbound or outbound.
+   is inbound or outbound. May not be effective for network filters like :ref:`Redis <config_network_filters_redis_proxy>`,
+   :ref:`Mongo <config_network_filters_mongo_proxy>`, or :ref:`Thrift <config_network_filters_thrift_proxy>`.
 
    .. http:post:: /drain_listeners?graceful
 

envoy/network/drain_decision.h

@@ -12,6 +12,23 @@
 namespace Envoy {
 namespace Network {
 
+enum class DrainDirection {
+  /**
+   * Not draining yet. Default value, should not be externally set.
+   */
+  None = 0,
+
+  /**
+   * Drain inbound connections only.
+   */
+  InboundOnly,
+
+  /**
+   * Drain both inbound and outbound connections.
+   */
+  All,
+};
+
 class DrainDecision {
 public:
   using DrainCloseCb = std::function<absl::Status(std::chrono::milliseconds)>;
@@ -21,8 +38,9 @@ class DrainDecision {
   /**
    * @return TRUE if a connection should be drained and closed. It is up to individual network
    *         filters to determine when this should be called for the least impact possible.
+   * @param direction supplies the direction for which the caller is checking drain close.
    */
-  virtual bool drainClose() const PURE;
+  virtual bool drainClose(DrainDirection scope) const PURE;
 
   /**
    * @brief Register a callback to be called proactively when a drain decision enters into a
@@ -34,7 +52,8 @@ class DrainDecision {
    * @return handle to remove callback
    */
   ABSL_MUST_USE_RESULT
-  virtual Common::CallbackHandlePtr addOnDrainCloseCb(DrainCloseCb cb) const PURE;
+  virtual Common::CallbackHandlePtr addOnDrainCloseCb(DrainDirection scope,
+                                                      DrainCloseCb cb) const PURE;
 };
 
 } // namespace Network

envoy/server/drain_manager.h

@@ -42,15 +42,17 @@ class DrainManager : public Network::DrainDecision, public ThreadLocal::ThreadLo
 
   /**
    * Invoked to begin the drain procedure. (Making drain close operations more likely).
+   * @param direction is the direction of the drain.
    * @param drain_complete_cb will be invoked once the drain sequence is finished. The parameter is
    * optional and can be an unassigned function.
    */
-  virtual void startDrainSequence(std::function<void()> drain_complete_cb) PURE;
+  virtual void startDrainSequence(Network::DrainDirection direction,
+                                  std::function<void()> drain_complete_cb) PURE;
 
   /**
-   * @return whether the drain sequence has started.
+   * @return whether the drain sequence has started for this direction.
    */
-  virtual bool draining() const PURE;
+  virtual bool draining(Network::DrainDirection) const PURE;
 
   /**
    * Invoked in the newly launched primary process to begin the parent shutdown sequence. At the end

mobile/library/jni/BUILD

@@ -141,6 +141,7 @@ cc_binary(
         "-lm",
         # See libpthread below.
         "-L$(GENDIR)/{}".format(package_name()),
+        "-latomic",
     ] + select({
         "@envoy//bazel:dbg_build": ["-Wl,--build-id=sha1"],
         "//conditions:default": [],

mobile/test/jni/BUILD

@@ -64,6 +64,9 @@ cc_library(
 cc_binary(
     name = "libenvoy_jni_http_test_server_factory.so",
     testonly = True,
+    linkopts = [
+        "-latomic",
+    ],
     linkshared = True,
     deps = [
         ":jni_http_test_server_factory_lib",
@@ -98,6 +101,9 @@ cc_library(
 cc_binary(
     name = "libenvoy_jni_http_proxy_test_server_factory.so",
     testonly = True,
+    linkopts = [
+        "-latomic",
+    ],
     linkshared = True,
     deps = [
         ":jni_http_proxy_test_server_factory_lib",
@@ -109,6 +115,9 @@ cc_binary(
 cc_binary(
     name = "libenvoy_jni_with_test_extensions.so",
     testonly = True,
+    linkopts = [
+        "-latomic",
+    ],
     linkshared = True,
     deps = [
         ":envoy_jni_with_test_extensions_lib",
@@ -140,6 +149,9 @@ cc_library(
 cc_binary(
     name = "libenvoy_jni_with_test_and_listener_extensions.so",
     testonly = True,
+    linkopts = [
+        "-latomic",
+    ],
     linkshared = True,
     deps = [
         ":envoy_jni_with_test_and_listener_extensions_lib",

mobile/test/performance/BUILD

@@ -7,6 +7,9 @@ envoy_mobile_package()
 envoy_cc_binary(
     name = "test_binary_size",
     srcs = ["test_binary_size.cc"],
+    linkopts = [
+        "-latomic",
+    ],
     repository = "@envoy",
     stamped = True,
     deps = ["//library/common:internal_engine_lib"],

source/common/http/BUILD

@@ -400,6 +400,7 @@ envoy_cc_library(
         "//source/common/stats:timespan_lib",
         "//source/common/stream_info:stream_info_lib",
         "//source/common/tracing:http_tracer_lib",
+        "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
         "@envoy_api//envoy/extensions/filters/network/http_connection_manager/v3:pkg_cc_proto",
         "@envoy_api//envoy/type/v3:pkg_cc_proto",
     ],

source/common/http/conn_manager_impl.cc

@@ -11,6 +11,7 @@
 
 #include "envoy/buffer/buffer.h"
 #include "envoy/common/time.h"
+#include "envoy/config/core/v3/base.pb.h"
 #include "envoy/event/dispatcher.h"
 #include "envoy/event/scaled_range_timer_manager.h"
 #include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.h"
@@ -98,14 +99,12 @@ ConnectionManagerImpl::generateListenerStats(const std::string& prefix, Stats::S
   return {CONN_MAN_LISTENER_STATS(POOL_COUNTER_PREFIX(scope, prefix))};
 }
 
-ConnectionManagerImpl::ConnectionManagerImpl(ConnectionManagerConfigSharedPtr config,
-                                             const Network::DrainDecision& drain_close,
-                                             Random::RandomGenerator& random_generator,
-                                             Http::Context& http_context, Runtime::Loader& runtime,
-                                             const LocalInfo::LocalInfo& local_info,
-                                             Upstream::ClusterManager& cluster_manager,
-                                             Server::OverloadManager& overload_manager,
-                                             TimeSource& time_source)
+ConnectionManagerImpl::ConnectionManagerImpl(
+    ConnectionManagerConfigSharedPtr config, const Network::DrainDecision& drain_close,
+    Random::RandomGenerator& random_generator, Http::Context& http_context,
+    Runtime::Loader& runtime, const LocalInfo::LocalInfo& local_info,
+    Upstream::ClusterManager& cluster_manager, Server::OverloadManager& overload_manager,
+    TimeSource& time_source, envoy::config::core::v3::TrafficDirection direction)
     : config_(std::move(config)), stats_(config_->stats()),
       conn_length_(new Stats::HistogramCompletableTimespanImpl(
           stats_.named_.downstream_cx_length_ms_, time_source)),
@@ -128,6 +127,7 @@ ConnectionManagerImpl::ConnectionManagerImpl(ConnectionManagerConfigSharedPtr co
                                      /*proxy_status_config=*/config_->proxyStatusConfig())),
       max_requests_during_dispatch_(
           runtime_.snapshot().getInteger(ConnectionManagerImpl::MaxRequestsPerIoCycle, UINT32_MAX)),
+      direction_(direction),
       allow_upstream_half_close_(Runtime::runtimeFeatureEnabled(
           "envoy.reloadable_features.allow_multiplexed_upstream_half_close")) {
   ENVOY_LOG_ONCE_IF(
@@ -1799,10 +1799,19 @@ void ConnectionManagerImpl::ActiveStream::encodeHeaders(ResponseHeaderMap& heade
     connection_manager_.stats_.named_.downstream_cx_overload_disable_keepalive_.inc();
   }
 
+  // If we're an inbound listener, then we should drain even if the drain direction is inbound only.
+  // If not, then we only drain if the drain direction is all.
+  Network::DrainDirection drain_scope =
+      connection_manager_.direction_ == envoy::config::core::v3::TrafficDirection::INBOUND
+          ? Network::DrainDirection::InboundOnly
+          : Network::DrainDirection::All;
+
   // See if we want to drain/close the connection. Send the go away frame prior to encoding the
-  // header block.
+  // header block. Only drain if the drain direction is not inbound only or the connection is
+  // inbound.
   if (connection_manager_.drain_state_ == DrainState::NotDraining &&
-      (connection_manager_.drain_close_.drainClose() || drain_connection_due_to_overload)) {
+      (connection_manager_.drain_close_.drainClose(drain_scope) ||
+       drain_connection_due_to_overload)) {
 
     // This doesn't really do anything for HTTP/1.1 other then give the connection another boost
     // of time to race with incoming requests. For HTTP/2 connections, send a GOAWAY frame to