Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: VHDS: on-demand updates #6552

Closed
wants to merge 30 commits into from
Closed
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
f66b55a
Base implementation of VHDS
Jan 22, 2019
0e6c97e
additional formatting fixes
Apr 3, 2019
b447f33
More formatting fixes
Apr 3, 2019
2da973f
more formatting fixes
Apr 3, 2019
21bb7af
Fixing test failures
Apr 3, 2019
2a6c77f
wrapped update details into a dedicated class
Apr 3, 2019
dbafd58
fixing build failures
Apr 3, 2019
200aa3d
Fixing build failures
Apr 4, 2019
68d7c52
Added more vhds tests
Apr 4, 2019
e692f0e
Fixed formatting errors
Apr 4, 2019
38da6ab
fixes based on the PR feedback
Apr 10, 2019
9a14662
fixed a spelling mistake
Apr 10, 2019
33f795b
moved out VhdsSubscription class into its own file
Apr 10, 2019
7a40f59
VHDS: Filter-based on-demand RDS updates
Jan 22, 2019
78acd5c
Merged changes from master
May 14, 2019
96a6c90
Fixes after merging latest changes
May 16, 2019
fe73bbd
Reponded to feedback
May 16, 2019
374de2c
Moved on-demand filter to extensions/filters/http/on_demand dir
May 16, 2019
1c8c52f
Merge branch 'master' into vhds-on-demand
May 16, 2019
c5e314a
fix to build kafka extension under python3
May 17, 2019
28e6ad3
Merge branch 'master' into vhds-on-demand
May 29, 2019
bd11db6
Post-merge fixes
May 29, 2019
bf63b6d
Renamed requestRouteConfigUpdate to requestVirtualHostsUpdate, update…
May 29, 2019
bbc38b4
Added comments re: stream restart after route config update
May 29, 2019
01eca26
Fixing build issues
May 29, 2019
72896bb
Fixing build issues
May 29, 2019
2592cce
Fixing build issues
May 30, 2019
e5d9d94
Fixed formatting issues
May 30, 2019
7bab936
Fixing build issues
May 30, 2019
852c51b
Fixing build issues
May 30, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions include/envoy/config/subscription.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "envoy/common/pure.h"
#include "envoy/stats/stats_macros.h"

#include "common/common/assert.h"
#include "common/protobuf/protobuf.h"

namespace Envoy {
Expand Down Expand Up @@ -81,6 +82,10 @@ class Subscription {
* @param resources vector of resource names to fetch.
*/
virtual void updateResources(const std::vector<std::string>& resources) PURE;

virtual void updateResourcesViaAliases(const std::vector<std::string>&) {
NOT_IMPLEMENTED_GCOVR_EXCL_LINE;
}
};

/**
Expand Down
2 changes: 2 additions & 0 deletions include/envoy/http/filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,8 @@ class StreamFilterCallbacks {
*/
virtual Router::RouteConstSharedPtr route() PURE;

virtual bool requestRouteConfigUpdate(std::function<void()> cb) PURE;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add Doxygen docs for this?


/**
* Returns the clusterInfo for the cached route.
* This method is to avoid multiple look ups in the filter chain, it also provides a consistent
Expand Down
10 changes: 10 additions & 0 deletions include/envoy/router/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,16 @@ envoy_cc_library(
],
)

envoy_cc_library(
name = "route_config_update_info_interface",
hdrs = ["route_config_update_info.h"],
external_deps = ["abseil_optional"],
deps = [
":router_interface",
"//source/common/protobuf",
],
)

envoy_cc_library(
name = "router_interface",
hdrs = ["router.h"],
Expand Down
3 changes: 3 additions & 0 deletions include/envoy/router/rds.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ class RouteConfigProvider {
* @return the last time this RouteConfigProvider was updated. Used for config dumps.
*/
virtual SystemTime lastUpdated() const PURE;

virtual void onConfigUpdate() PURE;
virtual bool requestConfigUpdate(const std::string for_domain, std::function<void()> cb) PURE;
dmitri-d marked this conversation as resolved.
Show resolved Hide resolved
};

typedef std::unique_ptr<RouteConfigProvider> RouteConfigProviderPtr;
Expand Down
25 changes: 25 additions & 0 deletions include/envoy/router/route_config_update_info.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#pragma once

#include <memory>

#include "envoy/api/v2/rds.pb.h"

namespace Envoy {
namespace Router {

struct LastConfigInfo {
uint64_t last_config_hash_;
std::string last_config_version_;
};

class RouteConfigUpdateInfo {
public:
virtual ~RouteConfigUpdateInfo() = default;

virtual absl::optional<LastConfigInfo> configInfo() const PURE;
virtual envoy::api::v2::RouteConfiguration& routeConfiguration() PURE;
virtual SystemTime lastUpdated() const PURE;
};

} // namespace Router
} // namespace Envoy
2 changes: 2 additions & 0 deletions include/envoy/router/router.h
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,8 @@ class Config {
* @return const std::string the RouteConfiguration name.
*/
virtual const std::string& name() const PURE;

virtual bool usesVhds() const PURE;
};

typedef std::shared_ptr<const Config> ConfigConstSharedPtr;
Expand Down
8 changes: 8 additions & 0 deletions source/common/config/delta_subscription_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,14 @@ class DeltaSubscriptionImpl
stats_.update_attempt_.inc();
}

void updateResourcesViaAliases(const std::vector<std::string>& aliases) override {
ResourceNameDiff diff;
std::copy(aliases.begin(), aliases.end(), std::inserter(diff.added_, diff.added_.begin()));
queueDiscoveryRequest(diff);
// sendDiscoveryRequest(diff);
stats_.update_attempt_.inc();
}

private:
void disableInitFetchTimeoutTimer() {
if (init_fetch_timeout_timer_) {
Expand Down
1 change: 1 addition & 0 deletions source/common/config/resources.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class TypeUrlValues {
const std::string ClusterLoadAssignment{"type.googleapis.com/envoy.api.v2.ClusterLoadAssignment"};
const std::string Secret{"type.googleapis.com/envoy.api.v2.auth.Secret"};
const std::string RouteConfiguration{"type.googleapis.com/envoy.api.v2.RouteConfiguration"};
const std::string VirtualHost{"type.googleapis.com/envoy.api.v2.route.VirtualHost"};
};

typedef ConstSingleton<TypeUrlValues> TypeUrl;
Expand Down
5 changes: 5 additions & 0 deletions source/common/http/async_client_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ class AsyncStreamImpl : public AsyncClient::Stream,
AsyncStreamImpl(AsyncClientImpl& parent, AsyncClient::StreamCallbacks& callbacks,
const AsyncClient::StreamOptions& options);

bool requestRouteConfigUpdate(std::function<void()>) override { return false; }

// Http::AsyncClient::Stream
void sendHeaders(HeaderMap& headers, bool end_stream) override;
void sendData(Buffer::Instance& data, bool end_stream) override;
Expand Down Expand Up @@ -172,6 +174,7 @@ class AsyncStreamImpl : public AsyncClient::Stream,
}

const std::string& name() const override { return EMPTY_STRING; }
bool usesVhds() const override { return false; }

static const std::list<LowerCaseString> internal_only_headers_;
};
Expand Down Expand Up @@ -370,6 +373,8 @@ class AsyncRequestImpl final : public AsyncClient::Request,
AsyncRequestImpl(MessagePtr&& request, AsyncClientImpl& parent, AsyncClient::Callbacks& callbacks,
const AsyncClient::RequestOptions& options);

bool requestRouteConfigUpdate(std::function<void()>) override { return false; }

// AsyncClient::Request
virtual void cancel() override;

Expand Down
15 changes: 14 additions & 1 deletion source/common/http/conn_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,7 @@ void ConnectionManagerImpl::chargeTracingStats(const Tracing::Reason& tracing_re

ConnectionManagerImpl::ActiveStream::ActiveStream(ConnectionManagerImpl& connection_manager)
: connection_manager_(connection_manager),
route_config_provider_(connection_manager.config_.routeConfigProvider()),
snapped_route_config_(connection_manager.config_.routeConfigProvider().config()),
stream_id_(connection_manager.random_generator_.random()),
request_response_timespan_(new Stats::Timespan(
Expand Down Expand Up @@ -1064,6 +1065,7 @@ void ConnectionManagerImpl::ActiveStream::refreshCachedRoute() {
Router::RouteConstSharedPtr route;
if (request_headers_ != nullptr) {
route = snapped_route_config_->route(*request_headers_, stream_id_);
// route = route_config_provider_.config()->route(*request_headers_, stream_id_);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

}
stream_info_.route_entry_ = route ? route->routeEntry() : nullptr;
cached_route_ = std::move(route);
Expand All @@ -1076,6 +1078,12 @@ void ConnectionManagerImpl::ActiveStream::refreshCachedRoute() {
}
}

bool ConnectionManagerImpl::ActiveStream::requestRouteConfigUpdate(std::function<void()> cb) {
// TODO check for an empty header?
auto host_header = Http::LowerCaseString(request_headers_->Host()->value().c_str()).get();
return route_config_provider_.requestConfigUpdate(host_header, cb);
}

void ConnectionManagerImpl::ActiveStream::sendLocalReply(
bool is_grpc_request, Code code, absl::string_view body,
const std::function<void(HeaderMap& headers)>& modify_headers, bool is_head_request,
Expand Down Expand Up @@ -1710,13 +1718,18 @@ Upstream::ClusterInfoConstSharedPtr ConnectionManagerImpl::ActiveStreamFilterBas
}

Router::RouteConstSharedPtr ConnectionManagerImpl::ActiveStreamFilterBase::route() {
if (!parent_.cached_route_.has_value()) {
if (!parent_.cached_route_.has_value() || parent_.cached_route_.value() == nullptr) {
parent_.refreshCachedRoute();
}

return parent_.cached_route_.value();
}

bool ConnectionManagerImpl::ActiveStreamFilterBase::requestRouteConfigUpdate(
std::function<void()> cb) {
return parent_.requestRouteConfigUpdate(cb);
}

void ConnectionManagerImpl::ActiveStreamFilterBase::clearRouteCache() {
parent_.cached_route_ = absl::optional<Router::RouteConstSharedPtr>();
parent_.cached_cluster_info_ = absl::optional<Upstream::ClusterInfoConstSharedPtr>();
Expand Down
4 changes: 4 additions & 0 deletions source/common/http/conn_manager_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
Event::Dispatcher& dispatcher() override;
void resetStream() override;
Router::RouteConstSharedPtr route() override;
bool requestRouteConfigUpdate(std::function<void()> cb) override;
Upstream::ClusterInfoConstSharedPtr clusterInfo() override;
void clearRouteCache() override;
uint64_t streamId() override;
Expand Down Expand Up @@ -271,6 +272,7 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,

void responseDataTooLarge();
void responseDataDrained();
bool requestRouteConfigUpdate(std::function<void()>) { return false; }

StreamEncoderFilterSharedPtr handle_;
};
Expand Down Expand Up @@ -351,6 +353,7 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
void traceRequest();

void refreshCachedRoute();
bool requestRouteConfigUpdate(std::function<void()> cb);

// Pass on watermark callbacks to watermark subscribers. This boils down to passing watermark
// events for this stream and the downstream connection to the router filter.
Expand Down Expand Up @@ -420,6 +423,7 @@ class ConnectionManagerImpl : Logger::Loggable<Logger::Id::http>,
void onRequestTimeout();

ConnectionManagerImpl& connection_manager_;
Router::RouteConfigProvider& route_config_provider_;
Router::ConfigConstSharedPtr snapped_route_config_;
Tracing::SpanPtr active_span_;
const uint64_t stream_id_;
Expand Down
55 changes: 55 additions & 0 deletions source/common/router/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,31 @@ envoy_cc_library(
],
)

envoy_cc_library(
name = "vhds_lib",
srcs = ["vhds.cc"],
hdrs = ["vhds.h"],
deps = [
":config_lib",
"//include/envoy/config:subscription_interface",
"//include/envoy/http:codes_interface",
"//include/envoy/local_info:local_info_interface",
"//include/envoy/router:rds_interface",
"//include/envoy/router:route_config_provider_manager_interface",
"//include/envoy/router:route_config_update_info_interface",
"//include/envoy/singleton:instance_interface",
"//include/envoy/thread_local:thread_local_interface",
"//source/common/common:assert_lib",
"//source/common/common:minimal_logger_lib",
"//source/common/config:rds_json_lib",
"//source/common/config:subscription_factory_lib",
"//source/common/config:utility_lib",
"//source/common/init:target_lib",
"//source/common/protobuf:utility_lib",
"@envoy_api//envoy/config/filter/network/http_connection_manager/v2:http_connection_manager_cc",
],
)

envoy_cc_library(
name = "rds_lib",
srcs = ["rds_impl.cc"],
Expand All @@ -77,6 +102,7 @@ envoy_cc_library(
"//include/envoy/local_info:local_info_interface",
"//include/envoy/router:rds_interface",
"//include/envoy/router:route_config_provider_manager_interface",
"//include/envoy/router:route_config_update_info_interface",
"//include/envoy/server:admin_interface",
"//include/envoy/singleton:instance_interface",
"//include/envoy/thread_local:thread_local_interface",
Expand All @@ -87,6 +113,7 @@ envoy_cc_library(
"//source/common/config:utility_lib",
"//source/common/init:target_lib",
"//source/common/protobuf:utility_lib",
"//source/common/router:vhds_lib",
"@envoy_api//envoy/admin/v2alpha:config_dump_cc",
"@envoy_api//envoy/api/v2:rds_cc",
"@envoy_api//envoy/config/filter/network/http_connection_manager/v2:http_connection_manager_cc",
Expand Down Expand Up @@ -160,6 +187,34 @@ envoy_cc_library(
],
)

envoy_cc_library(
name = "on_demand_update_lib",
srcs = ["on_demand_update.cc"],
hdrs = ["on_demand_update.h"],
deps = [
"//include/envoy/event:dispatcher_interface",
"//include/envoy/event:timer_interface",
"//include/envoy/http:filter_interface",
"//include/envoy/server:filter_config_interface",
"//source/common/access_log:access_log_lib",
"//source/common/buffer:watermark_buffer_lib",
"//source/common/common:assert_lib",
"//source/common/common:empty_string",
"//source/common/common:enum_to_int",
"//source/common/common:hash_lib",
"//source/common/common:hex_lib",
"//source/common/common:minimal_logger_lib",
"//source/common/common:utility_lib",
"//source/common/grpc:common_lib",
"//source/common/http:codes_lib",
"//source/common/http:header_map_lib",
"//source/common/http:headers_lib",
"//source/common/http:message_lib",
"//source/common/http:utility_lib",
"@envoy_api//envoy/config/filter/http/router/v2:router_cc",
],
)

envoy_cc_library(
name = "router_ratelimit_lib",
srcs = ["router_ratelimit.cc"],
Expand Down
2 changes: 1 addition & 1 deletion source/common/router/config_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1086,7 +1086,7 @@ VirtualHostImpl::virtualClusterFromEntries(const Http::HeaderMap& headers) const
ConfigImpl::ConfigImpl(const envoy::api::v2::RouteConfiguration& config,
Server::Configuration::FactoryContext& factory_context,
bool validate_clusters_default)
: name_(config.name()) {
: name_(config.name()), uses_vhds_(config.has_vhds()) {
route_matcher_ = std::make_unique<RouteMatcher>(
config, *this, factory_context,
PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, validate_clusters, validate_clusters_default));
Expand Down
4 changes: 4 additions & 0 deletions source/common/router/config_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -768,12 +768,15 @@ class ConfigImpl : public Config {

const std::string& name() const override { return name_; }

bool usesVhds() const override { return uses_vhds_; }

private:
std::unique_ptr<RouteMatcher> route_matcher_;
std::list<Http::LowerCaseString> internal_only_headers_;
HeaderParserPtr request_headers_parser_;
HeaderParserPtr response_headers_parser_;
const std::string name_;
const bool uses_vhds_;
};

/**
Expand All @@ -789,6 +792,7 @@ class NullConfigImpl : public Config {
}

const std::string& name() const override { return name_; }
bool usesVhds() const override { return false; }

private:
std::list<Http::LowerCaseString> internal_only_headers_;
Expand Down
58 changes: 58 additions & 0 deletions source/common/router/on_demand_update.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
#include "common/router/on_demand_update.h"
dmitri-d marked this conversation as resolved.
Show resolved Hide resolved

#include "common/common/assert.h"
#include "common/common/enum_to_int.h"
#include "common/http/codes.h"

#include "extensions/filters/http/well_known_names.h"

namespace Envoy {
namespace Router {

void OnDemandRouteUpdate::requestRouteConfigUpdate() {
if (callbacks_->route() != nullptr) {
filter_return_ = FilterReturn::ContinueDecoding;
} else {
auto configUpdateScheduled =
dmitri-d marked this conversation as resolved.
Show resolved Hide resolved
callbacks_->requestRouteConfigUpdate([this]() -> void { onComplete(); });
filter_return_ =
configUpdateScheduled ? FilterReturn::StopDecoding : FilterReturn::ContinueDecoding;
}
}

Http::FilterHeadersStatus OnDemandRouteUpdate::decodeHeaders(Http::HeaderMap&, bool) {
requestRouteConfigUpdate();
return filter_return_ == FilterReturn::StopDecoding ? Http::FilterHeadersStatus::StopIteration
: Http::FilterHeadersStatus::Continue;
}

Http::FilterDataStatus OnDemandRouteUpdate::decodeData(Buffer::Instance&, bool) {
return filter_return_ == FilterReturn::StopDecoding
? Http::FilterDataStatus::StopIterationAndWatermark
: Http::FilterDataStatus::Continue;
}

Http::FilterTrailersStatus OnDemandRouteUpdate::decodeTrailers(Http::HeaderMap&) {
return filter_return_ == FilterReturn::StopDecoding ? Http::FilterTrailersStatus::StopIteration
dmitri-d marked this conversation as resolved.
Show resolved Hide resolved
: Http::FilterTrailersStatus::Continue;
}

void OnDemandRouteUpdate::setDecoderFilterCallbacks(Http::StreamDecoderFilterCallbacks& callbacks) {
callbacks_ = &callbacks;
}

void OnDemandRouteUpdate::onComplete() {
filter_return_ = FilterReturn::ContinueDecoding;

if (!callbacks_->decodingBuffer() && // Redirects with body not yet supported.
dmitri-d marked this conversation as resolved.
Show resolved Hide resolved
callbacks_->recreateStream()) {
// cluster_->stats().upstream_internal_redirect_succeeded_total_.inc();
return;
}

// recreating stream failed, continue the filter-chain
callbacks_->continueDecoding();
}

} // namespace Router
} // namespace Envoy
Loading