config: Scoped RDS (PR 1/4): Introduce {static,dynamic} config provider framework #5243

AndresGuedez · 2018-12-07T16:34:19Z

This PR introduces a framework for creating static, inline and dynamic (xDS) config providers.

The intent is to create a shared foundation that simplifies implementation of Envoy configuration, in particular xDS config handlers with shared ownership semantics such that the underlying subscription, config proto and config implementation (i.e., the resulting data structures and business logic) are shared across providers and Envoy worker threads.

This PR generalizes the RouteConfigProvider, RouteConfigProviderManager and associated *Impls currently used for implementing HTTP route configuration and the RDS API. Note that static route configuration and RDS are not being transitioned to the generalized framework as part of this PR (see note below for details).

NOTE: This is PR 1 (out of 4) to implement the scoped HTTP routing design introduced in #4704. The planned PR chain is as follows:

Introduce generalized config provider framework
Implement static and dynamic scoped routing configuration based on framework introduced in PR 1; the scoped routing business logic will not be part of PR 2
Implement scoped routing business logic
Refactor RDS to use generalized config provider

Risk Level: Low (this code is not yet in use; it will be enabled in a follow up PR)
Testing: New tests added.
Docs Changes: N/A
Release Notes: N/A

This commit introduces a framework for implementing configuration providers by generalizing the RouteConfigProvider{,Manager} which currently implement HttpConnectionManager routing configuration and the RDS API. This is the first deliverable (out of a planned 4 or 5 PRs) of the Scoped RDS implementation (envoyproxy#4704). Signed-off-by: Andres Guedez <aguedez@google.com>

Signed-off-by: Andres Guedez <aguedez@google.com>

junr03 · 2018-12-07T17:02:59Z

@htuch I am assigning you as a first pass reviewer as you were involved in the design process. I am also assigning myself, but I will not be able to do a full review until next week.

htuch · 2018-12-12T06:48:52Z

Apologies for the review delay; will take a pass in the next 1-2 days.

htuch · 2018-12-13T08:41:23Z

include/envoy/config/config_provider.h

+namespace Config {
+
+/**
+ * A provider for configuration obtained ether statically or dynamically via xDS APIs.


One high level question to kick this off with. Is the plan to also replace the static/dynamic treatment of SDS resoureces (e.g. see https://github.com/envoyproxy/envoy/blob/master/source/common/ssl/context_config_impl.cc#L57) with this class framework? It would be helpful to have in mind the concrete types that this will be instantiated with and places used when reading.

Yes, SDS looks like a good candidate as well, although I was not planning to transition it to the new API as part of this PR chain so there are likely a few gaps that will need to be addressed.

One of those gaps is that the ConfigProvider currently only exposes a std::shared_ptr<const Config> config() const function, but the SDS config impls require allowing the user/consumer of the API to set a callback (see ContextConfigImpl::setSecretUpdateCallback). From an API standpoint, I think the solution is trivial by exposing a std::shared_ptr<Config> config() overload, and having the config impl enforce thread safety itself by leveraging any synchronization mechanisms required.

I think it would be ideal from a tech debt perspective if we could at least file an issue for this if not do the conversion at the end of the PR chain. There is a lot of commonality here between RDS and SDS, having multiple methods of managing static/dynamic resources seems redundant. Anyway, I'll do a deep dive now.

Agreed, I'll file an issue to keep track of SDS. I plan to take a deeper look at it once I get through these PRs and will likely take on the work myself.

Issue filed: #5329.

I think I might be being dense here. However, this discussion and @lizan's comment here leave me with a few questions:

The advantage of this framework is that it simplifies managing config providers that provide configuration that have shared semantics. In the PR comment this is clear, but I think it would be clarifying to spell that out in the interface here.

Given the fact that listeners, and clusters have similar (between them), but different (w.r.t SDS/RDS) ownership models, I wonder if it is worth it to spell out the fact that this framework, as is, is not really intended for config objects that are not shared. Or is the plan long term to use this framework to manage "singly owned" config providers?

About @lizan's comment. Should we put some effort in unifying what we mean when we say static vs inline? And how is the framework going to deal with the third origin model in SDS (what they call static)?

Yeah, more emphasis on the shared aspect in this comment would make sense; a short sentence or two explaining where this fits into the taxonomy of configuration management in Envoy would be rad.

Answering @junr03 's questions below:

The advantage of this framework is that it simplifies managing config providers that provide configuration that have shared semantics. In the PR comment this is clear, but I think it would be clarifying to spell that out in the interface here.

Done.

Given the fact that listeners, and clusters have similar (between them), but different (w.r.t SDS/RDS) ownership models, I wonder if it is worth it to spell out the fact that this framework, as is, is not really intended for config objects that are not shared. Or is the plan long term to use this framework to manage "singly owned" config providers?

This framework can also be leveraged for use cases where there is no expected sharing of dynamic resources, since that use case is a simplification of the target use case and the publicly exposed ConfigProvider API should suffice as well. However, I have avoided introducing this framework as the solution for config management, since it's likely that some of these "singly owned" use cases could make due with simpler implementations than what is imposed by this framework.

About @lizan's comment. Should we put some effort in unifying what we mean when we say static vs inline? And how is the framework going to deal with the third origin model in SDS (what they call static)?

I have changed the naming of the *ImplBase classes and added comments to clarify how this framework can be leveraged for SDS as well and the static/inline distinction that it makes. Essentially, the ImmutableConfigProviderImplBase can satisfy both config types, and I've introduced a ConfigProviderInstanceType parameter to the constructor to distinguish between them.

Yeah, more emphasis on the shared aspect in this comment would make sense; a short sentence or two explaining where this fits into the taxonomy of configuration management in Envoy would be rad.

Done.

Signed-off-by: Andres Guedez <aguedez@google.com>

htuch

Looks like a very clean and understandable approach to structuring things, a bunch of nits and deeper comments/questions to get started with.

htuch · 2018-12-14T03:36:52Z

include/envoy/config/config_provider.h

+    if (!config_proto) {
+      return {};
+    }
+    return ConfigInfo<P>{*config_proto, getConfigVersion()};


I'm struck by the similarity to

envoy/include/envoy/config/typed_metadata.h

Line 16 in 13789ee

class TypedMetadata {

. Could this be structure in term of config providers? Could there be some common pattern refactored here?

This is a pattern that repeats several times across the codebase; other classes that implement it are ThreadLocal::Slot and Tcp::ConnectionPool::ConnectionData.

I'll experiment with/prototype an abstraction to figure out whether it makes sense to generalize it into a common class or whether it's better left as a design pattern separately implemented by each class.

Yeah, no need to do gymnastics or anything here, just maybe there is an opportunity for some lightweight refactor. Thanks for taking a look.

include/envoy/config/config_provider.h

include/envoy/config/config_provider_manager.h

source/common/config/config_provider_impl.h

htuch · 2018-12-14T03:54:32Z

source/common/config/config_provider_impl.h

+   */
+  template <typename T>
+  std::shared_ptr<T>
+  getSubscription(const Protobuf::Message& config_source_proto, Init::Manager& init_manager,


I'm wondering what, if any, relationship this bears with what is being discussed in #5270?

I need to dive deeper into the various subscription classes' internals, but based on my current understanding of that issue, I don't see the relationship with my use here. This function is simply looking for an existing ConfigSubscriptionInstance (if any) that can be shared with a newly allocated dynamic config provider.

Any changes introduced by #5270 would be transparent to this function, although the ConfigSubscriptionInstance may have to be modified at to reflect API level changes.

source/common/config/config_provider_impl.h

htuch · 2018-12-14T03:59:32Z

source/common/config/config_provider_impl.cc

+  ASSERT(!dynamic_config_providers_.empty());
+  ConfigProvider::ConfigConstSharedPtr new_config;
+  for (auto* provider : dynamic_config_providers_) {
+    if (new_config == nullptr) {


This loop looks wrong based on a quick glance; the new_config nullness gets shared across dynamic providers... unless onConfigProtoUpdate is identical across them all? If so, please add comments and maybe ASSERTs.

These dynamic providers are all instances of the same class, and they share the underlying ConfigSubscriptionInstance on which this function is taking effect.

I will add the comments and assert()s.

Added comments and an ASSERT to bindConfigProvider(). The assertion is tricky due to the lack of concrete type information in these functions, so PTAL and let me know your thoughts on my approach using RTTI.

htuch · 2018-12-14T04:00:18Z

source/common/config/config_provider_impl.cc

+
+  ASSERT(!dynamic_config_providers_.empty());
+  ConfigProvider::ConfigConstSharedPtr new_config;
+  for (auto* provider : dynamic_config_providers_) {


I'm scratching my head and trying to remember why we have/need multiple dynamic config providers for a single config proto update?

This is another carryover from the existing RDS implementation of config providers. Currently, each HttpConnectionManager instantiates its own RdsRouteConfigProviderImpl (if RDS is configured).

I found some context on the decision to avoid sharing the RouteConfigProvider in #3960 and #3967. Based on those PRs, it appears the goal was to avoid use of shared_ptr outside the *ProviderImpl.

Yeah, anything you could do to document this design history would be great. I feel this is something I will forget in 6 months again.

Added class level comment explaining this.

The goal was to avoid use of shared_ptr outside the *ProviderImpl.

This is not quite true, see #3960 description. FactoryContext is tied to Listener (some of refs in it), and they will be dangling after they are updated by LDS, while the config provider may outlive them.

Signed-off-by: Andres Guedez <aguedez@google.com>

htuch

Yeah, the progression from the existing config provider implementation is a bit clearer, thanks. @lizan can you take a look as well, since this was your old stomping ground?

htuch · 2018-12-17T08:53:19Z

source/common/common/assert.h

+// type checks using typeid can trigger it.
+#define ASSERT_WITH_PRECOND(PRECOND, COND)                                                         \
+  do {                                                                                             \
+    if (PRECOND) {                                                                                 \


Could we just structure this as ASSERT(!PRECOND || COND)? Might simplify if that is possible.

Done. I'm still using a wrapper macro to hide the verbose #pragmas required to silence the potentially evaluated statement warnings from clang.

htuch · 2018-12-17T08:58:52Z

source/common/config/config_provider_impl.cc

+
+  ASSERT(!dynamic_config_providers_.empty());
+  ConfigProvider::ConfigConstSharedPtr new_config;
+  for (auto* provider : dynamic_config_providers_) {


Yeah, anything you could do to document this design history would be great. I feel this is something I will forget in 6 months again.

source/common/config/config_provider_impl.cc

test/common/config/dummy_config.proto

lizan · 2018-12-17T09:45:42Z

Sure, I will take a pass tomorrow.

Signed-off-by: Andres Guedez <aguedez@google.com>

lizan · 2018-12-18T01:06:48Z

source/common/config/config_provider_impl.cc

+
+  ASSERT(!dynamic_config_providers_.empty());
+  ConfigProvider::ConfigConstSharedPtr new_config;
+  for (auto* provider : dynamic_config_providers_) {


The goal was to avoid use of shared_ptr outside the *ProviderImpl.

This is not quite true, see #3960 description. FactoryContext is tied to Listener (some of refs in it), and they will be dangling after they are updated by LDS, while the config provider may outlive them.

lizan · 2018-12-18T01:13:52Z

source/common/config/config_provider_impl.h

+                           const std::string& version_info);
+
+  const std::unordered_set<const DynamicConfigProviderImpl*> dynamic_config_providers() const {
+    return std::unordered_set<const DynamicConfigProviderImpl*>(dynamic_config_providers_.begin(),


Why do you need copy here? Seems you're only checking the size and the first element when you use it.

Removed this accessor and replaced it getAnyBoundDynamicConfigProvider().

include/envoy/config/config_provider_manager.h

source/common/config/config_provider_impl.h

…-provider-refactor Signed-off-by: Andres Guedez <aguedez@google.com>

Signed-off-by: Andres Guedez <aguedez@google.com>

Also, formalizes the concept of types of ConfigProvider instances via the introduction of a ConfigProviderInstanceType. Signed-off-by: Andres Guedez <aguedez@google.com>

AndresGuedez · 2018-12-20T23:03:35Z

142d9c1 renames the {Static,Dynamic}ConfigProviderImplBase classes to {Immutable,Mutable}* respectively. This enables the introduction of a ConfigProviderInstanceType that allows users of the framework to specify the type of config associated with a provider.

See #5243 (comment) for context on this commit. These changes should allow a clean transition to this framework for both RDS and SDS.

lizan · 2018-12-20T23:10:25Z

source/common/config/config_provider_impl.cc

+namespace Envoy {
+namespace Config {
+
+std::ostream& operator<<(std::ostream& os, ConfigProviderInstanceType type) {


where is this used?

It was needed for an fmt::format() statement, but I have now removed it.

lizan · 2018-12-20T23:19:18Z

LGTM re: naming, seems build is broken though.

…-provider-refactor Signed-off-by: Andres Guedez <aguedez@google.com>

Also, removes thread safety assertions due to changes introduced in (envoyproxy#5072); it is no longer possible to statically call functions to obtain/check the current thread ID. I will reintroduce these checks once static accessors are available again. Signed-off-by: Andres Guedez <aguedez@google.com>

Signed-off-by: Andres Guedez <aguedez@google.com>

…-provider-refactor Signed-off-by: Andres Guedez <aguedez@google.com>

AndresGuedez · 2018-12-21T19:28:59Z

LGTM re: naming, seems build is broken though.

Build is fixed now. Just merged master to fix the mac build as well.

htuch · 2018-12-24T11:36:17Z

include/envoy/config/config_provider.h

+namespace Config {
+
+/**
+ * A provider for configuration obtained ether statically or dynamically via xDS APIs.


Yeah, more emphasis on the shared aspect in this comment would make sense; a short sentence or two explaining where this fits into the taxonomy of configuration management in Envoy would be rad.

htuch · 2018-12-24T11:45:35Z

source/common/common/assert.h

+#define ASSERT_IGNORE_POTENTIALLY_EVALUATED(COND)                                                  \
+  do {                                                                                             \
+    __CLANG_PRAGMA("clang diagnostic push")                                                        \
+    __CLANG_PRAGMA("clang diagnostic ignored \"-Wpotentially-evaluated-expression\"")              \


Searching around for what this does, it seems to involve side-effects, which I don't see at the sites where it is used. Can you add some comments on when you need to use this?

Alternatively, can we just fold these pragmas into ASSERT? Fewer knobs here seems better, we could potentially have an unbounded number of these ASSERTs, one for each clang warning :D

The comment in https://github.com/envoyproxy/envoy/pull/5243/files#diff-0bbc1b7ef8d5cfd41d41b2296caf7f1fR65 attempts to address the reason for this assert: the typeid(**mutable_config_providers_.begin()) statement triggers the potentially evaluated warning in clang.

This seems like a useful enough warning that I would rather not hide it by default (by folding it into the ASSERT() macro), but if others feel strongly about this I am certainly open to doing so.

Well, I guess I'm still not a huge fan of adding compiler specific ASSERTs into the implementation here. @envoyproxy/maintainers WDYT?

Hmm, if we do assert like this (evaluate the expression outside of typeid) then we don't need the pragma at all, since it is no longer potentially evaluated expression, do you think this is cleaner?

ASSERT([&]() { if (!mutable_config_providers_.empty()) { const auto& first_provider = **mutable_config_providers_.begin(); return typeid(*provider) == typeid(first_provider); } return true; }());

This is a good solution; I definitely think it's cleaner overall even though it makes the ASSERT() slightly harder to parse. I just pushed this change and removed my changes to assert.h. Thanks!

htuch · 2018-12-24T11:53:06Z

source/common/config/config_provider_impl.h

+    return tls_->getTyped<ThreadLocalConfig>().config_;
+  }
+
+  virtual ConfigConstSharedPtr onConfigProtoUpdate(const Protobuf::Message& config_proto) PURE;


Can you add a comment on what the semantics of this are? It's still unclear why it's safe to only call this once in checkAndApplyConfig(). I get that it returns identical objects for the same type, but doesn't the config provider need to latch the proto for configProtoInfo for all providers?

Added comments.

The latching of the config proto happens in the onConfigUpdate() override that must be implemented by the concrete subscription class that derives from ConfigSubscriptionInstanceBase and Envoy::Config::SubscriptionCallbacks<ConfigProto>. See here for the test source that does this.

htuch · 2018-12-24T12:02:14Z

source/common/config/config_provider_impl.cc

+  return *it->second;
+}
+
+void ConfigProviderManagerImplBase::bindImmutableConfigProvider(


FWIW, the new naming scheme is also a bit confusing, since the config object that is provided is always immutable, but the fact that a new config can be swapped in is being assigned the mutability characteristic. IDK if it's worth bike shedding further on this, but just raising this as a potential reader confusion.

I added to the file level comment more details regarding the mutability property and how it applies in this context.

I am open to considering other names but this is the best I have come up with given the constraints pointed out by @lizan related to naming conventions already in place for the concrete classes.

htuch · 2018-12-24T12:04:53Z

source/common/config/config_provider_impl.h

+  // xDS.
+  Inline,
+  // Configuration obtained from an xDS subscription.
+  xDS


Nit: prefer Xds for consistency here for the enum class value.

include/envoy/config/config_provider_manager.h

htuch · 2018-12-24T12:09:46Z

source/common/config/config_provider_impl.h

+ *
+ * All config processing is done on the main thread, so instantiation of *ConfigProvider* objects
+ * via createStaticConfigProvider() and createXdsConfigProvider() is naturally thread safe. Care
+ * must be taken with regards to destruction of these objects, since it must also happen on the main


Also, relative ordering of object destruction on the main thread.. We've had a bunch of use-after-frees by getting this wrong.. would love to have a structural approach to dealing with enforcement of object graph destruction ordering.

Added note to comment regarding destruction order.

would love to have a structural approach to dealing with enforcement of object graph destruction ordering.

I agree this would be great to have. Is there a pattern already in place elsewhere in the codebase that I should be following?

No, this is something we would benefit from adding.

Signed-off-by: Andres Guedez <aguedez@google.com>

htuch

LGTM modulo the ASSERT comment/nit. @lizan since I'm going to be off-grid until Fri, can I hand you the baton for final discussion on that topic and merge? Thanks!

htuch · 2018-12-30T04:59:07Z

source/common/common/assert.h

+#define ASSERT_IGNORE_POTENTIALLY_EVALUATED(COND)                                                  \
+  do {                                                                                             \
+    __CLANG_PRAGMA("clang diagnostic push")                                                        \
+    __CLANG_PRAGMA("clang diagnostic ignored \"-Wpotentially-evaluated-expression\"")              \


Well, I guess I'm still not a huge fan of adding compiler specific ASSERTs into the implementation here. @envoyproxy/maintainers WDYT?

- Removed ASSERT_IGNORE_POTENTIALLY_EVALUATED() and replaced with an ASSERT(lambda) that bypasses the issue. Signed-off-by: Andres Guedez <aguedez@google.com>

lizan · 2018-12-31T23:33:26Z

include/envoy/config/config_provider.h

+
+    const auto* config_proto = dynamic_cast<const P*>(getConfigProto());
+    if (config_proto == nullptr) {
+      return {};


nit: absl::nullopt

source/common/config/config_provider_impl.cc

source/common/config/config_provider_impl.h

Signed-off-by: Andres Guedez <aguedez@google.com>

Comments addressed.

AndresGuedez · 2019-01-03T14:00:24Z

Thanks to all involved for the reviews!

…er framework (envoyproxy#5243) This PR introduces a framework for creating static, inline and dynamic (xDS) config providers. The intent is to create a shared foundation that simplifies implementation of Envoy configuration, in particular xDS config handlers with shared ownership semantics such that the underlying subscription, config proto and config implementation (i.e., the resulting data structures and business logic) are shared across providers and Envoy worker threads. This PR generalizes the RouteConfigProvider, RouteConfigProviderManager and associated *Impls currently used for implementing HTTP route configuration and the RDS API. Note that static route configuration and RDS are not being transitioned to the generalized framework as part of this PR (see note below for details). NOTE: This is PR 1 (out of 4) to implement the scoped HTTP routing design introduced in envoyproxy#4704. The planned PR chain is as follows: 1. Introduce generalized config provider framework 2. Implement static and dynamic scoped routing configuration based on framework introduced in PR 1; the scoped routing business logic will _not_ be part of PR 2 3. Implement scoped routing business logic 4. Refactor RDS to use generalized config provider *Risk Level*: Low (this code is not yet in use; it will be enabled in a follow up PR) *Testing*: New tests added. *Docs Changes*: N/A *Release Notes*: N/A Signed-off-by: Andres Guedez <aguedez@google.com> Signed-off-by: Fred Douglas <fredlas@google.com>

Implement the scoped RDS (SRDS) API config subscription and provider based on the config protos introduced in #6675 and the ConfigProvider framework introduced in #5243 and #6781. NOTES: See parent PR #5839 for full context into these changes. PRs 2a (#6675) and 2b (#6781) have already been merged. The API is not yet fully implemented. This PR introduces static and dynamic (xDS config subscription) handling of scoped routing configuration, but the new L7 multi tenant routing logic (see #4704) has not yet been introduced. The API is not yet plumbed into the HttpConnectionManager, that will be done in the next PR. This PR includes unit tests only; integration tests will follow in the next PR. Risk Level: Low (this DS API is not yet integrated into the HCM and can not be enabled via config). Testing: Unit tests added. Docs Changes: N/A. Signed-off-by: Andres Guedez <aguedez@google.com>

AndresGuedez added 5 commits December 6, 2018 17:50

Add comments.

1bccfaf

Signed-off-by: Andres Guedez <aguedez@google.com>

Minor cleanup.

66e4c45

Signed-off-by: Andres Guedez <aguedez@google.com>

More comments.

02b5e97

Signed-off-by: Andres Guedez <aguedez@google.com>

Comments.

09a0846

Signed-off-by: Andres Guedez <aguedez@google.com>

AndresGuedez changed the title ~~Scoped RDS (PR 1/4): Introduce {static,dynamic} config provider framework~~ router: Scoped RDS (PR 1/4): Introduce {static,dynamic} config provider framework Dec 7, 2018

AndresGuedez changed the title ~~router: Scoped RDS (PR 1/4): Introduce {static,dynamic} config provider framework~~ config: Scoped RDS (PR 1/4): Introduce {static,dynamic} config provider framework Dec 7, 2018

junr03 assigned junr03 and htuch Dec 7, 2018

mattklein123 self-assigned this Dec 7, 2018

htuch reviewed Dec 13, 2018

View reviewed changes

AndresGuedez added 2 commits December 13, 2018 13:17

Minor const correctness fixes.

acf19cf

Signed-off-by: Andres Guedez <aguedez@google.com>

Add test.

c0366a7

Signed-off-by: Andres Guedez <aguedez@google.com>

htuch suggested changes Dec 14, 2018

View reviewed changes

AndresGuedez added 2 commits December 14, 2018 16:06

Comments, general cleanup and renaming.

4b1bef4

Signed-off-by: Andres Guedez <aguedez@google.com>

Minor cleanup.

550723c

Signed-off-by: Andres Guedez <aguedez@google.com>

htuch suggested changes Dec 17, 2018

View reviewed changes

htuch requested a review from lizan December 17, 2018 09:04

htuch assigned lizan Dec 17, 2018

AndresGuedez mentioned this pull request Dec 17, 2018

Migrate SDS to ConfigProvider framework introduced in #5243 #5329

Closed

Cleanup and comments based on reviewer feedback.

2c79e9c

Signed-off-by: Andres Guedez <aguedez@google.com>

lizan reviewed Dec 18, 2018

View reviewed changes

lizan added the waiting label Dec 19, 2018

AndresGuedez added 3 commits December 19, 2018 10:52

Merge remote-tracking branch 'upstream/master' into scoped-rds-config…

854c2f2

…-provider-refactor Signed-off-by: Andres Guedez <aguedez@google.com>

Rename *Impl -> *ImplBase.

e67b5ee

Signed-off-by: Andres Guedez <aguedez@google.com>

Cleanup based on review feedback.

821aab9

Signed-off-by: Andres Guedez <aguedez@google.com>

repokitteh-read-only bot removed the waiting label Dec 19, 2018

AndresGuedez added 4 commits December 20, 2018 12:02

Comments and cleanup based on reviewer comments.

15c9d19

Signed-off-by: Andres Guedez <aguedez@google.com>

Header include order fix.

f69eb74

Signed-off-by: Andres Guedez <aguedez@google.com>

Remove superfluous constructor deletes.

ba44ad9

Signed-off-by: Andres Guedez <aguedez@google.com>

Rename ({Static,Dynamic} -> {Immutable,Mutable})ConfigProviderImplBase.

142d9c1

Also, formalizes the concept of types of ConfigProvider instances via the introduction of a ConfigProviderInstanceType. Signed-off-by: Andres Guedez <aguedez@google.com>

lizan reviewed Dec 20, 2018

View reviewed changes

AndresGuedez added 4 commits December 20, 2018 21:59

Merge remote-tracking branch 'upstream/master' into scoped-rds-config…

5c71955

…-provider-refactor Signed-off-by: Andres Guedez <aguedez@google.com>

clang-tidy cleanup.

a36a3e8

Signed-off-by: Andres Guedez <aguedez@google.com>

Remove unused function.

bb34fdd

Signed-off-by: Andres Guedez <aguedez@google.com>

AndresGuedez mentioned this pull request Dec 21, 2018

WIP: config: Add FilterChain Discovery Service. #5385

Closed

Merge remote-tracking branch 'upstream/master' into scoped-rds-config…

ccc244a

…-provider-refactor Signed-off-by: Andres Guedez <aguedez@google.com>

htuch previously requested changes Dec 24, 2018

View reviewed changes

AndresGuedez added 3 commits December 27, 2018 15:10

Add comments.

6d7cba8

Signed-off-by: Andres Guedez <aguedez@google.com>

clang-tidy cleanup.

9cf7f27

Signed-off-by: Andres Guedez <aguedez@google.com>

Comments.

58ac876

Signed-off-by: Andres Guedez <aguedez@google.com>

htuch reviewed Dec 30, 2018

View reviewed changes

Cleanup.

bb990fa

- Removed ASSERT_IGNORE_POTENTIALLY_EVALUATED() and replaced with an ASSERT(lambda) that bypasses the issue. Signed-off-by: Andres Guedez <aguedez@google.com>

lizan reviewed Dec 31, 2018

View reviewed changes

Minor cleanup.

b49127f

Signed-off-by: Andres Guedez <aguedez@google.com>

lizan approved these changes Jan 2, 2019

View reviewed changes

lizan merged commit b71bd22 into envoyproxy:master Jan 2, 2019

AndresGuedez mentioned this pull request Feb 5, 2019

router: scoped RDS (2/4): support scoped routing configuration #5839

Closed

AndresGuedez mentioned this pull request May 14, 2019

router: scoped rds (2c): implement scoped rds API #6932

Merged

config: Scoped RDS (PR 1/4): Introduce {static,dynamic} config provider framework #5243

config: Scoped RDS (PR 1/4): Introduce {static,dynamic} config provider framework #5243

Conversation

AndresGuedez commented Dec 7, 2018 • edited Loading

junr03 commented Dec 7, 2018

htuch commented Dec 12, 2018

Choose a reason for hiding this comment

AndresGuedez Dec 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

htuch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndresGuedez Dec 14, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

htuch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lizan commented Dec 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndresGuedez commented Dec 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lizan commented Dec 20, 2018

AndresGuedez commented Dec 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndresGuedez Dec 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

htuch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndresGuedez commented Jan 3, 2019

AndresGuedez commented Dec 7, 2018 •

edited

Loading

AndresGuedez Dec 13, 2018 •

edited

Loading

AndresGuedez Dec 14, 2018 •

edited

Loading

AndresGuedez commented Dec 20, 2018 •

edited

Loading

AndresGuedez Dec 27, 2018 •

edited

Loading