Skip to content

config: de-templatize source/common/config#6391

Merged
htuch merged 8 commits intoenvoyproxy:masterfrom
fredlas:TPL_remove_templates
Mar 29, 2019
Merged

config: de-templatize source/common/config#6391
htuch merged 8 commits intoenvoyproxy:masterfrom
fredlas:TPL_remove_templates

Conversation

@fredlas
Copy link
Copy Markdown
Contributor

@fredlas fredlas commented Mar 26, 2019

A refactor to remove a bunch of templating from the xDS implementation, all the way down to the Subscription and SubscriptionCallbacks interfaces. Until now, the subscription implementations have all been templated on resource proto type, but that template type is really only used to convert Any protobuf blobs to the appropriate type before handing them off to the RDS, CDS, etc code. This PR has those Any blobs remain unconverted until they reach e.g. rds_impl.cc, which knows to MessageUtil::anyConvert() them.

Removing understanding of type from the xDS protocol implementation brings things more into line with discovery.proto, which view xDS resources as Any blobs, and generally knows nothing about resource type.

Contributes to #5270.

Risk Level: low
Testing: existing tests

fredlas added 2 commits March 26, 2019 17:55
Signed-off-by: Fred Douglas <fredlas@google.com>
Signed-off-by: Fred Douglas <fredlas@google.com>
@fredlas
Copy link
Copy Markdown
Contributor Author

fredlas commented Mar 26, 2019

All the classes that were detemplatized should be split out into .cc files, but I figured I would save that until after the initial review to make the diff easier to review.

Motivation: my aggregated delta implementation takes the approach of a "context" (playing a similar role to GrpcMux), which has a map of type_url -> DeltaSubscriptionState, where DeltaSubscriptionState is the logic that tracks our interest in+version of resources, builds request protos, and handles response protos after they are received. Here is a quick WIP snapshot of that work:
https://github.com/fredlas/envoy/blob/ADS_incremental/source/common/config/delta_subscription_impl.h
https://github.com/fredlas/envoy/blob/ADS_incremental/source/common/config/grpc_delta_xds_context.h
https://github.com/fredlas/envoy/blob/ADS_incremental/source/common/config/delta_subscription_state.h

fredlas added 2 commits March 26, 2019 21:12
Signed-off-by: Fred Douglas <fredlas@google.com>
Signed-off-by: Fred Douglas <fredlas@google.com>
@htuch htuch self-assigned this Mar 27, 2019
fredlas added 2 commits March 27, 2019 14:56
Signed-off-by: Fred Douglas <fredlas@google.com>
Signed-off-by: Fred Douglas <fredlas@google.com>
Copy link
Copy Markdown
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

Yeah, this is a good move. Thinking back on the history, I originally used templating as it seemed cleaner to have the subscription code figure out type URL and do unpacking from Any using C++ types. As we added more complexity around ADS and now delta xDS, this has outlived its usefulness and what you have here is a nice improvement that should make a bunch of things easier to handle generically.

/wait

fredlas added 2 commits March 28, 2019 10:26
Signed-off-by: Fred Douglas <fredlas@google.com>
Signed-off-by: Fred Douglas <fredlas@google.com>
Copy link
Copy Markdown
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

Great cleanup, thanks!

@htuch htuch merged commit af26857 into envoyproxy:master Mar 29, 2019
spenceral added a commit to spenceral/envoy that referenced this pull request Mar 29, 2019
* master:
  router: support prefix wildcards in virtual hosts domains (envoyproxy#6303)
  security: update distributor application example to include e-mail. (envoyproxy#6425)
  Examples: Update gen script of grpc example service (envoyproxy#6372)
  config: de-templatize source/common/config (envoyproxy#6391)
  docs: adds information about the Envoy tracer from Instana envoyproxy#6371 (envoyproxy#6416)
  test: convert ratelimit test configs to v2 YAML (envoyproxy#6411)
  include required python and go dependencies for grpc-bridge example (envoyproxy#6402)
  docs: more snapping fixes (envoyproxy#6404)
  runtime: switching from unordered_map to absl::flat_hash_map (envoyproxy#6389)
htuch pushed a commit that referenced this pull request May 7, 2019
Followup of #6391

Risk Level: none, cleanup only

Signed-off-by: Fred Douglas <fredlas@google.com>
htuch pushed a commit that referenced this pull request May 28, 2019
Another .h split enabled by xDS detemplatization. (Followup to #6391)

Risk Level: none

Signed-off-by: Fred Douglas <fredlas@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants