external service registry for registry aggregator by GregHanson · Pull Request #4108 · istio/istio

GregHanson · 2018-03-08T22:07:11Z

during xDS api generation, pilot code needs to know whether a service is external or not. treating v1alpha3.ExternalService objects as rules (as done in the first pass implementation) does not allow pilot to make this distinction. This PR creates a new external service registry which will be added to the service registry aggregator in a followup PR.

istio-merge-robot · 2018-03-08T22:07:49Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
We suggest the following additional approver: nmittler

Assign the PR to them by writing /assign @nmittler in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these OWNERS Files:

pilot/OWNERS

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

ijsnellf

Some quick comments.

ijsnellf · 2018-03-08T22:12:34Z

pilot/pkg/serviceregistry/external/controller.go

+			newRecord := configs(externalServiceConfigs)
+			sort.Sort(newRecord)
+
+			if !reflect.DeepEqual(newRecord, cachedConfigs) {


We shouldn't be checking for changes this way. We should be able to leverage CRUD events for external services.

Why can't you do a dumb SHA of the entire externalservices config? If the SHA is different, then fire off the handlers.. if its same, skip.

Could do that, but we have existing CRUD events that we can leverage instead.

ijsnellf · 2018-03-08T22:13:56Z

pilot/pkg/serviceregistry/external/conversion.go

+	out := make([]*model.Service, 0)
+
+	ports := make(map[int]*model.Port)
+


style nit: unnecessary whitespaces here and elsewhere in this file

ijsnellf · 2018-03-08T22:16:08Z

pilot/pkg/serviceregistry/external/serviceaccounts.go

+// GetIstioServiceAccounts implements model.ServiceAccounts operation TODO
+func (sa *serviceAccounts) GetIstioServiceAccounts(hostname string, ports []string) []string {
+	// Need to get service account of service registered with consul
+	// Currently Consul does not have service account or equivalent concept


Nix the references to Consul and Eureka all over this file.

ijsnellf · 2018-03-08T22:18:07Z

pilot/pkg/serviceregistry/external/serviceaccounts.go

+}
+
+// GetIstioServiceAccounts implements model.ServiceAccounts operation TODO
+func (sa *serviceAccounts) GetIstioServiceAccounts(hostname string, ports []string) []string {


This needs thought. Needs clear TODOs at least.

ijsnellf · 2018-03-08T22:19:20Z

pilot/pkg/serviceregistry/external/servicediscovery.go

+
+// Controller communicates with Consul and monitors for changes
+type externalDiscovery struct {
+	config model.IstioConfigStore


config -> store so you have less name conflicts.
Stuff like configs := c.config.ExternalServices() is confusing.

ijsnellf · 2018-03-08T22:22:31Z

pilot/pkg/serviceregistry/external/servicediscovery.go

+	}
+
+	return false
+}


could be a single line:
return len(portMap) == 0 || portMap[instance.Endpoint.ServicePort.Name]

ijsnellf · 2018-03-08T22:23:32Z

pilot/pkg/serviceregistry/external/servicediscovery.go

+
+
+// ManagementPorts retries set of health check ports by instance IP.
+// This does not apply to Consul service registry, as Consul does not


More references to Consul...

ijsnellf · 2018-03-08T22:24:05Z

pilot/pkg/serviceregistry/external/servicediscovery.go

+}
+
+func (c *externalDiscovery) getServices() ([]*model.Service) {
+	configs := c.config.ExternalServices()


Confusing naming. See above ^^^

ijsnellf · 2018-03-08T22:27:51Z

#3901

rshriram · 2018-03-09T01:01:08Z

pilot/pkg/serviceregistry/external/controller.go

+	config           model.IstioConfigStore
+}
+
+// NewController instantiates a new Eureka controller


fix comments

rshriram · 2018-03-09T01:04:49Z

pilot/pkg/serviceregistry/external/conversion.go

+	// TODO: https://github.com/istio/istio/issues/3338
+	// Check for the label - auth.istio.io/<port> and return auth policy respectively
+
+	return meshconfig.AuthenticationPolicy_INHERIT


You cannot have any istio auth policy here. Just return auth policy none. The user would most probably have their certs, etc.

rshriram · 2018-03-09T01:07:55Z

pilot/pkg/serviceregistry/external/serviceaccounts.go

+	return []string{
+		"spiffe://cluster.local/ns/default/sa/default",
+	}
+}


This should be empty. Basically for external services, there is no istio auth, no service accounts, and such. Its just a service, with service instances, and dns stuff.

rshriram · 2018-03-09T01:10:35Z

pilot/pkg/serviceregistry/external/servicediscovery.go

+
+// GetProxyServiceInstances lists service instances co-located with a given proxy
+func (c *externalDiscovery) GetProxyServiceInstances(node model.Proxy) ([]*model.ServiceInstance, error) {
+	configs := c.config.ExternalServices()


This should be nil. There is no proxy sitting next to google.com.
If you supply this, we end up generating a full envoy configuration with routes to internal services, (listeners, etc.) for the external service (which does not exist in the cluster).

codecov · 2018-03-09T15:19:33Z

Codecov Report

Merging #4108 into master will increase coverage by 6%.
The diff coverage is 88%.

@@           Coverage Diff            @@
##           master   #4108     +/-   ##
========================================
+ Coverage      71%     77%     +6%     
========================================
  Files         315     301     -14     
  Lines       29079   26802   -2277     
========================================
- Hits        20435   20376     -59     
+ Misses       7422    5163   -2259     
- Partials     1222    1263     +41

Impacted Files	Coverage Δ
pilot/pkg/model/service.go	`100% <ø> (+3%)`	⬆️
...ot/pkg/serviceregistry/external/serviceaccounts.go	`0% <0%> (ø)`
pilot/pkg/serviceregistry/external/controller.go	`100% <100%> (ø)`
...t/pkg/serviceregistry/external/servicediscovery.go	`81% <81%> (ø)`
pilot/pkg/serviceregistry/external/conversion.go	`95% <95%> (ø)`
security/pkg/workload/secretserver.go	`0% <0%> (-100%)`	⬇️
security/pkg/workload/config.go	`0% <0%> (-100%)`	⬇️
pkg/version/cobra.go	`0% <0%> (-100%)`	⬇️
security/pkg/cmd/probe.go	`0% <0%> (-62%)`	⬇️
pkg/version/version.go	`54% <0%> (-46%)`	⬇️
... and 105 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44b4d8f...9848311. Read the comment docs.

ijsnellf · 2018-03-09T16:57:31Z

pilot/pkg/serviceregistry/external/servicediscovery.go

+}
+
+// Services list declarations of all services in the system
+func (c *externalDiscovery) Services() ([]*model.Service, error) {


nit: here and elsewhere c->d or similar. c doesn't make any sense

GregHanson · 2018-03-09T21:12:01Z

/test istio-unit-tests

GregHanson · 2018-03-09T21:21:09Z

/test istio-unit-tests

ijsnellf · 2018-03-12T14:20:57Z

@ZackButcher @frankbu: I would like feedback before I merge this.

frankbu · 2018-03-12T16:21:17Z

pilot/pkg/model/service.go


+	// MeshExternal (if true) indicates that the service is external to the mesh.
+	// These services are defined using Istio's ExternalService spec.
+	MeshExternal bool


For what, other than the two additional Resolution methods does the runtime need to know that a service is external?

I tend to agree with Frank here that'd prefer us not surface the notion of "internal"/"external" in our representation of services. We have a name with endpoints, we shouldn't have to care beyond that, other than wiring up how to talk to the service as described in the external service config (but that should happen when we build this service out of an external service config).

We might need to know about the external service property. I put this in because today we have the super clunky model where we expect external service accesses to be http only while we generate https upstream.

We could do this another way: ask people to explicitly create external service as http port only. And then tell them to create a destination rule that asks for https to the external service.

frankbu · 2018-03-12T16:21:22Z

pilot/pkg/model/service.go

+	// could either use DNS load balancing (i.e. proxy will query DNS server for the IP of the service)
+	// or use the passthrough model (i.e. proxy will forward the traffic to the network endpoint requested
+	// by the caller)
+	Resolution Resolution


Are Resolution values other than ClientSideLB (i.e. 0) applicable for internal services? If not, would it make sense to combine the MeshExternal field with Resolution, or maybe just delete MeshExternal?

Headless services in k8s shouldn't be load balanced by Istio, so they would use Passthrough resolution.

frankbu · 2018-03-12T16:23:58Z

@ijsnellf If I'm not mistaken, merging this PR would be harmless because it's not being called yet. Is that right?

ijsnellf · 2018-03-12T20:55:28Z

Yes, it shouldn't interfere with any existing logic.

ZackButcher

Overall shape of the PR LGTM, only big complaint ATM is what @frankbu already called out: I feel like the need for the MeshExternal flag is a bad smell and we should be able to get by without it.

ZackButcher · 2018-03-15T19:19:12Z

pilot/pkg/model/service.go


+	// MeshExternal (if true) indicates that the service is external to the mesh.
+	// These services are defined using Istio's ExternalService spec.
+	MeshExternal bool


I tend to agree with Frank here that'd prefer us not surface the notion of "internal"/"external" in our representation of services. We have a name with endpoints, we shouldn't have to care beyond that, other than wiring up how to talk to the service as described in the external service config (but that should happen when we build this service out of an external service config).

ZackButcher · 2018-03-15T19:25:58Z

pilot/pkg/serviceregistry/external/conversion.go

+	ports := make(map[int]*model.Port)
+
+	for _, host := range externalService.Hosts {
+		service := &model.Service{}


nit: do the work to construct the port list and resolution type up front, then just merge all of this into a literal; I think it's a lot cleaner to read:

var res Resolution switch externalService.Discovery { ... } svcPorts := make(model.PortList, 0, len(ports) for ... range externalService.Ports { ... } out = append(out, &model.Service { MeshExternal: true, Hostname: host, Resolution: res, Ports: svcPorst, })

ijsnellf · 2018-03-16T16:25:05Z

@ZackButcher @frankbu: makes sense. An alternative is to have an additional Resolution type SDS (or EDS). I think that would be more logical. Most internal services would be SDS except for a few exceptions like headless services which would be passthrough. Thoughts?

ijsnellf · 2018-03-16T16:25:15Z

cc @rshriram

rshriram · 2018-03-16T20:37:35Z

@ZackButcher / @frankbu repeating my earlier comment
We might need to know about the external service property. I put this in because today we have the super clunky model where we expect external service accesses to be http only while we generate https upstream.

We could do this another way: ask people to explicitly create external service as http port only. And then tell them to create a destination rule that asks for https to the external service.
If we choose the latter approach, we can get rid of the external service property.

But keep in mind that the code is already doing a bunch of checks (if service.External() that checked if hostname was unset) .. I was merely trying to codify that into a boolean field to get more flexibility.

ZackButcher · 2018-03-16T21:01:02Z

That's fair; I'm super opposed to it, just would've liked to do without if we could.

frankbu · 2018-03-16T21:08:32Z

I'm not following the alternative suggestion:

We could do this another way: ask people to explicitly create external service as http port only. And then tell them to create a destination rule that asks for https to the external service. If we choose the latter approach, we can get rid of the external service property.

I thought that's already what we tell people to do, for example:

apiVersion: networking.istio.io/v1alpha3
kind: ExternalService
metadata:
  name: google-external-svc
spec:
  hosts:
  - www.google.com
  ports:
  - number: 443
    name: https
    protocol: http
---
apiVersion: networking.istio.io/v1alpha3
kind: DestinationRule
metadata:
  name: google-destination
spec:
  name: www.google.com
  trafficPolicy:
    tls:
      mode: SIMPLE

rshriram · 2018-03-17T22:49:00Z

@frankbu / @ZackButcher thinking through this more, I think you guys are right.. Lets have a resolution type EDS or something, that will allow us to get rid of this special classification.

There is an associated piece of work that needs to be done in Envoy (allowing ports to be rewritten for original dst clusters). Barring that, the rest should work.

I am merging this PR with the guarantee that I will get rid of MeshExternal in the subsequent PR. I just need this code in place, to unblock other refactoring I am doing. Besides, this code is not activated yet.

rshriram · 2018-03-19T03:53:29Z

@frankbu / @ZackButcher I think we now have a use case for MeshExternal.. We need to know if the service is an external one in order to setup the MixerFilter config for external services. Resolution/other fields will not be able to indicate that this is a special class of services that need the same mixer config as the one we setup on the inbound path.

Pulling the following changes from github.com/istio/proxy: 7a0fca9 Update Envoy SHA to latest with LcTrie optimizations (release-1.0). (istio#1919) d93f0fe Fix macOS build on CircleCI (release-1.0). (istio#1921) Pulling the following changes from github.com/envoyproxy/envoy: 73bd3d95c http_filter: add addEncodedTrailers and addDecodedTrailers (istio#3980) c3652aad5 rbac/fuzz: fix build (istio#4150) 07bc27c05 fix flaky RBAC integration test. (istio#4147) b150d61a9 header_map: copy constructor for HeaderMapImpl. (istio#4129) f345c8b23 test: moving websocket tests to using HTTP codec. (istio#4143) da500d20f upstream: init host hc value based on hc value from other priorities (istio#3959) da6194b94 test: add tests for corner-cases around sending requests before run() starts or after run() ends. (istio#4114) 3527f7799 perf: reduce the memory usage of LC Trie construction (istio#4117) b538e46d8 test: moving redundant code in websocket_integration_test to utilities (istio#4127) a3c55bf7b test: make YamlLoadFromStringFail less picky about error msg. (istio#4141) c283439b6 rbac: add rbac network filter. (istio#4083) 5a7152d21 fuzz: route lookup and header finalization fuzzer. (istio#4116) 589467360 Set content-type and content-length (istio#4113) 714ae130a fault: use FractionalPercent for percent (istio#3978) fde378705 test: Fix inverted exact match logic in IntegrationTcpClient::waitForData() (istio#4134) 794a00126 Added cluster_name to load assignment config for static cluster (istio#4123) 19f51e5e1 ssl: refactor ContextConfig to use TlsCertificateConfig (istio#4115) 0a4bffc5a syscall: refactor OsSysCalls for deeper errno latching (istio#4111) ec0d98e5e thrift_proxy: fix oneway bugs (istio#4025) 1381673ad Do not crash when converting YAML to JSON fails (istio#4110) 2662bf1f2 config: allow unknown fields flag (take 2) (istio#4096) 1ab839c1f Use a jittered backoff strategy for handling HdsDelegate stream/connection failures (istio#4108) 7309c14cf bazel: use GCS remote cache (istio#4050) 5fe4e14f0 Add thread local cache of overload action states (istio#4090) 3bb7fbc5f Added TCP healthcheck capabilities to the HdsDelegate (istio#4079) 98037ed37 secret: add secret provider interface and use it for TlsCertificates (istio#4086) 3e15c9490 upstream: allow custom extension protocol options (istio#4098) 9b33c49d1 Rename message types in hds.proto to improve readability (istio#4109) bb70b42bb fuzz: router header formatter/parser fuzz test. (istio#4105) fe57f6b33 fuzz: http parsing utility fuzzer. (istio#4107) 73dfedc95 ci: link ninja-buid to ninja for centos (istio#4106) 1cd509ef1 docs: add curl to Ubuntu deps (istio#4104) 45b900829 Handling updates from the management server on HDS (istio#4077) 510994c6a Don't use SIGTERM for admin /quitquitquit, just shut down directly. (istio#4099) 29b60291e fuzz: access log formatter fuzz test. (istio#4102) 765cac42f Destroy pending updates when updating a cluster (istio#4084) aafdf6037 authz_client_fix: fixed ext_authz http client when request contains content-length greater than 0 (istio#3888) 22ae0ab93 HttpConnectionManager and upstream counters for total completed requests (istio#3995) 04616d676 tcp_proxy: convert TCP proxy to use TCP connection pool (istio#4067) e759eab17 buffer: add prepend functions to Buffer::Instance (istio#4064) 14baa40ea fuzz: h1_capture_fuzz with direct response (istio#3787) d47365a9a Per endpoint load report (istio#4044) 70e9878ed Fix bug in `HostSetImpl::chooseLocality()` (istio#4061) 797e82484 deps: update gRPC to 1.14.0 (istio#4047) 628730666 Remove std::string cast in upstream impl lib and tests. (istio#4080) 33ab6ddac bot: exempt label "no stalebot" for PRs (istio#4081) 699c008d6 Absl string view to std string in dynamic metadata (istio#4078) e9dc1090e collect metrics for RBAC shadow policy (istio#4062) e9d81e179 Combine query-params into admin API's path, with API access from MainCommon sinking to main thread (istio#4059) fccaeade9 Revert "Revert "Basic Implementation of HDS (istio#3973)" (istio#4063)" (istio#4068) e96d4a6c4 http: fix upstream_rq stat increment (istio#4055) 14140ad83 Add overload manager to bootstrap config (istio#4038) b14dee5ee thrift_proxy: introduce MessageMetadata to track message headers and other metadata (istio#3991) 9ee2b2759 authz: correct stat names (istio#4074) c68063c05 Stats interface atomization (istio#4071) 82e3541b0 docs: fix incorrect doc about cluster warming in CDS (istio#4040) 3868326bd Support ListValue for metadata matcher (istio#3964) 4e5258953 Revert "Basic Implementation of HDS (istio#3973)" (istio#4063) f3b0f8580 Basic Implementation of HDS (istio#3973) 7b03f2ef5 tracing: Fixes issue with small LightStep reports. (istio#3989) fd517b356 request_info: initial implementation of dynamic metadata object (istio#3918) d5bbd1e0c Ability to specify a test or a test group when building with docker release (istio#4030) a1c646102 Remove stats_impl.h (istio#4057) 7bf713a93 fuzz: H2 codec fuzzer. (istio#4017) a614808b9 upstream: fix typo (s/lb_type/lb_policy/g) in previous commit. (istio#4051) 346059548 upstream: require opt-in for the x-envoy-original-dst-host header. (istio#4046) f2c9652a9 owners: add Dhi is maintainer (istio#4042) 6a1868dff Revert "tcp_proxy: convert TCP proxy to use TCP connection pool (istio#3938)" (istio#4043) cc3657797 docs: document request_timeout in version_history (istio#4041) a3364380a rest-api: make request timeout configurable (istio#4006) fa628c44e logging: optional details for ASSERT (istio#3934) 55606ec3f bump abseil-cpp commit (istio#4034) 4c3219c0c owners: promote Stephan and Greg to senior maintainer! (istio#4039) ddd661ac0 hot restarter: Log errno for 'panic: cannot open shared memory' error (istio#4032) cb3356fc5 Sds: Ssl socket factory owns ContextConfig (istio#4028) 9bc047226 Refactor TransportSocketFactoryContext and Cluster interfaces. (istio#4026) f8f21c26d Rename duplicated ads integration test case name (istio#4035) 02281809b fix duplicate listeners in lds response (istio#4029) 61421bddf upstream: fix duplicate clusters (istio#4012) 1f1166167 split up stats_impl_test to match the *impl.h and and *impl.cc files. (istio#4024) 5ec8b37da Remove "DO NOT SUBMIT" comment. (istio#4020) 882c49832 Add more information to errors about rejected cipher suite configuration. (istio#4019) ffc8258e5 Rename common/stats/stats_impl.* to common/stats/source_impl.* and fix refs (istio#4021) 891135e38 Fix overload manager unit test build (istio#4022) c2f204cc7 Add stats for overload manager (istio#4001) aec92237a remove unused variables (istio#4013) e999cfacc Re-order functions in stats_impl to group classes together (istio#4004) d5805b171 typos (istio#4009) aeb3f2875 Fix perf_annotation_test compilation under gcc 8.1.1 (istio#4000) da3c1eaf8 test/mock: Add 3 new gmock matchers (istio#3972) 6a8b84384 test: Add timeouts to methods that could wait forever in test/integration/fake_upstream.h. (istio#3936) d0f10faff HeapStatData with a distinct allocation mechanism for RawStatData (istio#3710) 2012c3e4c rds: make RouteConfigProvider unique_ptr (istio#3967) 62441f9fe Add option for merging cluster updates (istio#3941) eb5ea98ff fuzz: fixes oss-fuzz: 9599, 9600 (istio#3979) b27068bd0 listener: add socket api in os sys calls for additional tests (istio#3968) 83b9e2da8 Add overload manager for Envoy (istio#3954) f0ca75415 Fix prometheus typo. (istio#3999) 028387a3b tcp_proxy: convert TCP proxy to use TCP connection pool (istio#3938) f882e74dc syscall: use Api::SysCallResult in buffer impl (istio#3976) 7d61b0017 fuzz: fixes oss-fuzz: 9621 (istio#3988) dc03a9a41 docs: fix grammar errors (istio#3983) ed131cfa9 docs: minor typo and grammar fixups (istio#3984) 08fadcc41 http: fix segfault when idle timer fires before request headers received. (istio#3970) 8b9fd9aa7 Refactor setSocketOption for better errno latching (istio#3915) 6b65dbe3a Change drop_percentage to FractionalPercent (istio#3974) f28dc53f4 Remove deprecated handling of mutating admin requests from GET. (istio#3975) 324e628b7 syscall: refactor address APIs for deeper errno latching (istio#3897) Fixes istio#7710, fixes istio#7817, and hopefully fixes istio#7759. Signed-off-by: Piotr Sikora <piotrsikora@google.com>

* Update Envoy SHA to latest (release-1.0). Pulling the following changes from github.com/istio/proxy: 7a0fca9 Update Envoy SHA to latest with LcTrie optimizations (release-1.0). (#1919) d93f0fe Fix macOS build on CircleCI (release-1.0). (#1921) Pulling the following changes from github.com/envoyproxy/envoy: 73bd3d95c http_filter: add addEncodedTrailers and addDecodedTrailers (#3980) c3652aad5 rbac/fuzz: fix build (#4150) 07bc27c05 fix flaky RBAC integration test. (#4147) b150d61a9 header_map: copy constructor for HeaderMapImpl. (#4129) f345c8b23 test: moving websocket tests to using HTTP codec. (#4143) da500d20f upstream: init host hc value based on hc value from other priorities (#3959) da6194b94 test: add tests for corner-cases around sending requests before run() starts or after run() ends. (#4114) 3527f7799 perf: reduce the memory usage of LC Trie construction (#4117) b538e46d8 test: moving redundant code in websocket_integration_test to utilities (#4127) a3c55bf7b test: make YamlLoadFromStringFail less picky about error msg. (#4141) c283439b6 rbac: add rbac network filter. (#4083) 5a7152d21 fuzz: route lookup and header finalization fuzzer. (#4116) 589467360 Set content-type and content-length (#4113) 714ae130a fault: use FractionalPercent for percent (#3978) fde378705 test: Fix inverted exact match logic in IntegrationTcpClient::waitForData() (#4134) 794a00126 Added cluster_name to load assignment config for static cluster (#4123) 19f51e5e1 ssl: refactor ContextConfig to use TlsCertificateConfig (#4115) 0a4bffc5a syscall: refactor OsSysCalls for deeper errno latching (#4111) ec0d98e5e thrift_proxy: fix oneway bugs (#4025) 1381673ad Do not crash when converting YAML to JSON fails (#4110) 2662bf1f2 config: allow unknown fields flag (take 2) (#4096) 1ab839c1f Use a jittered backoff strategy for handling HdsDelegate stream/connection failures (#4108) 7309c14cf bazel: use GCS remote cache (#4050) 5fe4e14f0 Add thread local cache of overload action states (#4090) 3bb7fbc5f Added TCP healthcheck capabilities to the HdsDelegate (#4079) 98037ed37 secret: add secret provider interface and use it for TlsCertificates (#4086) 3e15c9490 upstream: allow custom extension protocol options (#4098) 9b33c49d1 Rename message types in hds.proto to improve readability (#4109) bb70b42bb fuzz: router header formatter/parser fuzz test. (#4105) fe57f6b33 fuzz: http parsing utility fuzzer. (#4107) 73dfedc95 ci: link ninja-buid to ninja for centos (#4106) 1cd509ef1 docs: add curl to Ubuntu deps (#4104) 45b900829 Handling updates from the management server on HDS (#4077) 510994c6a Don't use SIGTERM for admin /quitquitquit, just shut down directly. (#4099) 29b60291e fuzz: access log formatter fuzz test. (#4102) 765cac42f Destroy pending updates when updating a cluster (#4084) aafdf6037 authz_client_fix: fixed ext_authz http client when request contains content-length greater than 0 (#3888) 22ae0ab93 HttpConnectionManager and upstream counters for total completed requests (#3995) 04616d676 tcp_proxy: convert TCP proxy to use TCP connection pool (#4067) e759eab17 buffer: add prepend functions to Buffer::Instance (#4064) 14baa40ea fuzz: h1_capture_fuzz with direct response (#3787) d47365a9a Per endpoint load report (#4044) 70e9878ed Fix bug in `HostSetImpl::chooseLocality()` (#4061) 797e82484 deps: update gRPC to 1.14.0 (#4047) 628730666 Remove std::string cast in upstream impl lib and tests. (#4080) 33ab6ddac bot: exempt label "no stalebot" for PRs (#4081) 699c008d6 Absl string view to std string in dynamic metadata (#4078) e9dc1090e collect metrics for RBAC shadow policy (#4062) e9d81e179 Combine query-params into admin API's path, with API access from MainCommon sinking to main thread (#4059) fccaeade9 Revert "Revert "Basic Implementation of HDS (#3973)" (#4063)" (#4068) e96d4a6c4 http: fix upstream_rq stat increment (#4055) 14140ad83 Add overload manager to bootstrap config (#4038) b14dee5ee thrift_proxy: introduce MessageMetadata to track message headers and other metadata (#3991) 9ee2b2759 authz: correct stat names (#4074) c68063c05 Stats interface atomization (#4071) 82e3541b0 docs: fix incorrect doc about cluster warming in CDS (#4040) 3868326bd Support ListValue for metadata matcher (#3964) 4e5258953 Revert "Basic Implementation of HDS (#3973)" (#4063) f3b0f8580 Basic Implementation of HDS (#3973) 7b03f2ef5 tracing: Fixes issue with small LightStep reports. (#3989) fd517b356 request_info: initial implementation of dynamic metadata object (#3918) d5bbd1e0c Ability to specify a test or a test group when building with docker release (#4030) a1c646102 Remove stats_impl.h (#4057) 7bf713a93 fuzz: H2 codec fuzzer. (#4017) a614808b9 upstream: fix typo (s/lb_type/lb_policy/g) in previous commit. (#4051) 346059548 upstream: require opt-in for the x-envoy-original-dst-host header. (#4046) f2c9652a9 owners: add Dhi is maintainer (#4042) 6a1868dff Revert "tcp_proxy: convert TCP proxy to use TCP connection pool (#3938)" (#4043) cc3657797 docs: document request_timeout in version_history (#4041) a3364380a rest-api: make request timeout configurable (#4006) fa628c44e logging: optional details for ASSERT (#3934) 55606ec3f bump abseil-cpp commit (#4034) 4c3219c0c owners: promote Stephan and Greg to senior maintainer! (#4039) ddd661ac0 hot restarter: Log errno for 'panic: cannot open shared memory' error (#4032) cb3356fc5 Sds: Ssl socket factory owns ContextConfig (#4028) 9bc047226 Refactor TransportSocketFactoryContext and Cluster interfaces. (#4026) f8f21c26d Rename duplicated ads integration test case name (#4035) 02281809b fix duplicate listeners in lds response (#4029) 61421bddf upstream: fix duplicate clusters (#4012) 1f1166167 split up stats_impl_test to match the *impl.h and and *impl.cc files. (#4024) 5ec8b37da Remove "DO NOT SUBMIT" comment. (#4020) 882c49832 Add more information to errors about rejected cipher suite configuration. (#4019) ffc8258e5 Rename common/stats/stats_impl.* to common/stats/source_impl.* and fix refs (#4021) 891135e38 Fix overload manager unit test build (#4022) c2f204cc7 Add stats for overload manager (#4001) aec92237a remove unused variables (#4013) e999cfacc Re-order functions in stats_impl to group classes together (#4004) d5805b171 typos (#4009) aeb3f2875 Fix perf_annotation_test compilation under gcc 8.1.1 (#4000) da3c1eaf8 test/mock: Add 3 new gmock matchers (#3972) 6a8b84384 test: Add timeouts to methods that could wait forever in test/integration/fake_upstream.h. (#3936) d0f10faff HeapStatData with a distinct allocation mechanism for RawStatData (#3710) 2012c3e4c rds: make RouteConfigProvider unique_ptr (#3967) 62441f9fe Add option for merging cluster updates (#3941) eb5ea98ff fuzz: fixes oss-fuzz: 9599, 9600 (#3979) b27068bd0 listener: add socket api in os sys calls for additional tests (#3968) 83b9e2da8 Add overload manager for Envoy (#3954) f0ca75415 Fix prometheus typo. (#3999) 028387a3b tcp_proxy: convert TCP proxy to use TCP connection pool (#3938) f882e74dc syscall: use Api::SysCallResult in buffer impl (#3976) 7d61b0017 fuzz: fixes oss-fuzz: 9621 (#3988) dc03a9a41 docs: fix grammar errors (#3983) ed131cfa9 docs: minor typo and grammar fixups (#3984) 08fadcc41 http: fix segfault when idle timer fires before request headers received. (#3970) 8b9fd9aa7 Refactor setSocketOption for better errno latching (#3915) 6b65dbe3a Change drop_percentage to FractionalPercent (#3974) f28dc53f4 Remove deprecated handling of mutating admin requests from GET. (#3975) 324e628b7 syscall: refactor address APIs for deeper errno latching (#3897) Fixes #7710, fixes #7817, and hopefully fixes #7759. Signed-off-by: Piotr Sikora <piotrsikora@google.com> * reivew: fix for duplicate clusters (backported from master). Signed-off-by: Piotr Sikora <piotrsikora@google.com> * review: disable broken tests (backported from master). Signed-off-by: Piotr Sikora <piotrsikora@google.com>

Pulling the following changes from github.com/istio/proxy: 1fc6253 add debug logs for collecting rbac attributes (istio#1922) c5282b6 Update Envoy SHA to latest with LcTrie optimizations. (istio#1918) 4ced9e7 Update clang to 6.0 and use it for release binaries. (istio#1914) 585abec fixed broken links to dev guide and contribution guide (istio#1913) c63d841 Provide source version information in the binary. (istio#1915) b49589a Install clang-format in the build image used by CircleCI. (istio#1917) 5d42471 Fix macOS build on CircleCI. (istio#1916) b1f4e7e add rbac filter to istio http integration test. (istio#1907) Pulling the following changes from github.com/envoyproxy/envoy: 73bd3d95c http_filter: add addEncodedTrailers and addDecodedTrailers (istio#3980) c3652aad5 rbac/fuzz: fix build (istio#4150) 07bc27c05 fix flaky RBAC integration test. (istio#4147) b150d61a9 header_map: copy constructor for HeaderMapImpl. (istio#4129) f345c8b23 test: moving websocket tests to using HTTP codec. (istio#4143) da500d20f upstream: init host hc value based on hc value from other priorities (istio#3959) da6194b94 test: add tests for corner-cases around sending requests before run() starts or after run() ends. (istio#4114) 3527f7799 perf: reduce the memory usage of LC Trie construction (istio#4117) b538e46d8 test: moving redundant code in websocket_integration_test to utilities (istio#4127) a3c55bf7b test: make YamlLoadFromStringFail less picky about error msg. (istio#4141) c283439b6 rbac: add rbac network filter. (istio#4083) 5a7152d21 fuzz: route lookup and header finalization fuzzer. (istio#4116) 589467360 Set content-type and content-length (istio#4113) 714ae130a fault: use FractionalPercent for percent (istio#3978) fde378705 test: Fix inverted exact match logic in IntegrationTcpClient::waitForData() (istio#4134) 794a00126 Added cluster_name to load assignment config for static cluster (istio#4123) 19f51e5e1 ssl: refactor ContextConfig to use TlsCertificateConfig (istio#4115) 0a4bffc5a syscall: refactor OsSysCalls for deeper errno latching (istio#4111) ec0d98e5e thrift_proxy: fix oneway bugs (istio#4025) 1381673ad Do not crash when converting YAML to JSON fails (istio#4110) 2662bf1f2 config: allow unknown fields flag (take 2) (istio#4096) 1ab839c1f Use a jittered backoff strategy for handling HdsDelegate stream/connection failures (istio#4108) 7309c14cf bazel: use GCS remote cache (istio#4050) 5fe4e14f0 Add thread local cache of overload action states (istio#4090) 3bb7fbc5f Added TCP healthcheck capabilities to the HdsDelegate (istio#4079) 98037ed37 secret: add secret provider interface and use it for TlsCertificates (istio#4086) 3e15c9490 upstream: allow custom extension protocol options (istio#4098) 9b33c49d1 Rename message types in hds.proto to improve readability (istio#4109) bb70b42bb fuzz: router header formatter/parser fuzz test. (istio#4105) fe57f6b33 fuzz: http parsing utility fuzzer. (istio#4107) 73dfedc95 ci: link ninja-buid to ninja for centos (istio#4106) 1cd509ef1 docs: add curl to Ubuntu deps (istio#4104) 45b900829 Handling updates from the management server on HDS (istio#4077) 510994c6a Don't use SIGTERM for admin /quitquitquit, just shut down directly. (istio#4099) 29b60291e fuzz: access log formatter fuzz test. (istio#4102) 765cac42f Destroy pending updates when updating a cluster (istio#4084) aafdf6037 authz_client_fix: fixed ext_authz http client when request contains content-length greater than 0 (istio#3888) 22ae0ab93 HttpConnectionManager and upstream counters for total completed requests (istio#3995) 04616d676 tcp_proxy: convert TCP proxy to use TCP connection pool (istio#4067) e759eab17 buffer: add prepend functions to Buffer::Instance (istio#4064) Fixes istio#7710, fixes istio#7817, and hopefully fixes istio#7759. Signed-off-by: Piotr Sikora <piotrsikora@google.com>

Pulling the following changes from github.com/istio/proxy: 1fc6253 add debug logs for collecting rbac attributes (#1922) c5282b6 Update Envoy SHA to latest with LcTrie optimizations. (#1918) 4ced9e7 Update clang to 6.0 and use it for release binaries. (#1914) 585abec fixed broken links to dev guide and contribution guide (#1913) c63d841 Provide source version information in the binary. (#1915) b49589a Install clang-format in the build image used by CircleCI. (#1917) 5d42471 Fix macOS build on CircleCI. (#1916) b1f4e7e add rbac filter to istio http integration test. (#1907) Pulling the following changes from github.com/envoyproxy/envoy: 73bd3d95c http_filter: add addEncodedTrailers and addDecodedTrailers (#3980) c3652aad5 rbac/fuzz: fix build (#4150) 07bc27c05 fix flaky RBAC integration test. (#4147) b150d61a9 header_map: copy constructor for HeaderMapImpl. (#4129) f345c8b23 test: moving websocket tests to using HTTP codec. (#4143) da500d20f upstream: init host hc value based on hc value from other priorities (#3959) da6194b94 test: add tests for corner-cases around sending requests before run() starts or after run() ends. (#4114) 3527f7799 perf: reduce the memory usage of LC Trie construction (#4117) b538e46d8 test: moving redundant code in websocket_integration_test to utilities (#4127) a3c55bf7b test: make YamlLoadFromStringFail less picky about error msg. (#4141) c283439b6 rbac: add rbac network filter. (#4083) 5a7152d21 fuzz: route lookup and header finalization fuzzer. (#4116) 589467360 Set content-type and content-length (#4113) 714ae130a fault: use FractionalPercent for percent (#3978) fde378705 test: Fix inverted exact match logic in IntegrationTcpClient::waitForData() (#4134) 794a00126 Added cluster_name to load assignment config for static cluster (#4123) 19f51e5e1 ssl: refactor ContextConfig to use TlsCertificateConfig (#4115) 0a4bffc5a syscall: refactor OsSysCalls for deeper errno latching (#4111) ec0d98e5e thrift_proxy: fix oneway bugs (#4025) 1381673ad Do not crash when converting YAML to JSON fails (#4110) 2662bf1f2 config: allow unknown fields flag (take 2) (#4096) 1ab839c1f Use a jittered backoff strategy for handling HdsDelegate stream/connection failures (#4108) 7309c14cf bazel: use GCS remote cache (#4050) 5fe4e14f0 Add thread local cache of overload action states (#4090) 3bb7fbc5f Added TCP healthcheck capabilities to the HdsDelegate (#4079) 98037ed37 secret: add secret provider interface and use it for TlsCertificates (#4086) 3e15c9490 upstream: allow custom extension protocol options (#4098) 9b33c49d1 Rename message types in hds.proto to improve readability (#4109) bb70b42bb fuzz: router header formatter/parser fuzz test. (#4105) fe57f6b33 fuzz: http parsing utility fuzzer. (#4107) 73dfedc95 ci: link ninja-buid to ninja for centos (#4106) 1cd509ef1 docs: add curl to Ubuntu deps (#4104) 45b900829 Handling updates from the management server on HDS (#4077) 510994c6a Don't use SIGTERM for admin /quitquitquit, just shut down directly. (#4099) 29b60291e fuzz: access log formatter fuzz test. (#4102) 765cac42f Destroy pending updates when updating a cluster (#4084) aafdf6037 authz_client_fix: fixed ext_authz http client when request contains content-length greater than 0 (#3888) 22ae0ab93 HttpConnectionManager and upstream counters for total completed requests (#3995) 04616d676 tcp_proxy: convert TCP proxy to use TCP connection pool (#4067) e759eab17 buffer: add prepend functions to Buffer::Instance (#4064) Fixes #7710, fixes #7817, and hopefully fixes #7759. Signed-off-by: Piotr Sikora <piotrsikora@google.com>

first pass external service registry for registry aggregator

dba0768

GregHanson added the area/networking label Mar 8, 2018

GregHanson requested review from a team, ijsnellf and rshriram March 8, 2018 22:07

googlebot added the cla: yes label Mar 8, 2018

GregHanson changed the title ~~first pass external service registry for registry aggregator~~ [WIP] external service registry for registry aggregator Mar 8, 2018

ijsnellf reviewed Mar 8, 2018

View reviewed changes

ijsnellf requested a review from frankbu March 8, 2018 22:27

rshriram reviewed Mar 9, 2018

View reviewed changes

review comments

24f38e5

GregHanson added 2 commits March 9, 2018 09:25

bug fix

04a550b

switched to event handler for detecting changes to ExternalServices crds

39370a6

ijsnellf reviewed Mar 9, 2018

View reviewed changes

GregHanson added 4 commits March 9, 2018 13:35

bug fix in tests

d01b957

include Resolution in conversion

6ea2235

lint fixes

b9af577

lint fixes

d8f227e

GregHanson changed the title ~~[WIP] external service registry for registry aggregator~~ external service registry for registry aggregator Mar 9, 2018

ijsnellf requested a review from ZackButcher March 10, 2018 00:03

frankbu reviewed Mar 12, 2018

View reviewed changes

ijsnellf requested a review from costinm March 13, 2018 20:39

rshriram approved these changes Mar 13, 2018

View reviewed changes

Merge branch 'master' into external-service-registry

ec1c7d9

ZackButcher reviewed Mar 16, 2018

View reviewed changes

GregHanson added 2 commits March 16, 2018 08:41

Merge branch 'master' into external-service-registry

3a45c9c

minor refactors

9848311

rshriram merged commit 1c763c1 into istio:master Mar 17, 2018

GregHanson deleted the external-service-registry branch March 19, 2018 13:38

PiotrSikora mentioned this pull request Aug 15, 2018

Update Envoy SHA to latest. #7922

Merged

PiotrSikora mentioned this pull request Aug 15, 2018

Update Envoy SHA to latest (release-1.0). #7954

Merged

		out := make([]*model.Service, 0)

		ports := make(map[int]*model.Port)



		// ManagementPorts retries set of health check ports by instance IP.
		// This does not apply to Consul service registry, as Consul does not

Conversation

GregHanson commented Mar 8, 2018

Uh oh!

istio-merge-robot commented Mar 8, 2018

Uh oh!

ijsnellf left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ijsnellf commented Mar 8, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Mar 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GregHanson commented Mar 9, 2018

Uh oh!

GregHanson commented Mar 9, 2018

Uh oh!

ijsnellf commented Mar 12, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

frankbu commented Mar 12, 2018

Uh oh!

ijsnellf commented Mar 12, 2018

Uh oh!

ZackButcher left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ijsnellf commented Mar 16, 2018

Uh oh!

ijsnellf commented Mar 16, 2018

Uh oh!

rshriram commented Mar 16, 2018

Uh oh!

ZackButcher commented Mar 16, 2018

codecov bot commented Mar 9, 2018 •

edited

Loading