From bef693df9c662a1dbe180bbdbaf6bdc2a3f1322f Mon Sep 17 00:00:00 2001 From: Kim Ngo Date: Tue, 17 Mar 2020 14:50:14 -0500 Subject: [PATCH] agent/xds: Update mesh gateway to use service router timeout (#7444) * website/connect/proxy/envoy: specify timeout precedence for services behind mesh gateway --- agent/xds/clusters.go | 30 +++++-- agent/xds/clusters_test.go | 23 +++++ .../mesh-gateway-service-timeouts.golden | 87 +++++++++++++++++++ website/source/docs/connect/proxies/envoy.md | 7 +- 4 files changed, 140 insertions(+), 7 deletions(-) create mode 100644 agent/xds/testdata/clusters/mesh-gateway-service-timeouts.golden diff --git a/agent/xds/clusters.go b/agent/xds/clusters.go index a8e3938c9f..6274d2eaaa 100644 --- a/agent/xds/clusters.go +++ b/agent/xds/clusters.go @@ -159,20 +159,28 @@ func (s *Server) clustersFromSnapshotMeshGateway(cfgSnap *proxycfg.ConfigSnapsho // generate the per-service clusters for svc, _ := range cfgSnap.MeshGateway.ServiceGroups { clusterName := connect.ServiceSNI(svc.ID, "", svc.NamespaceOrDefault(), cfgSnap.Datacenter, cfgSnap.Roots.TrustDomain) + resolver, hasResolver := cfgSnap.MeshGateway.ServiceResolvers[svc] - cluster, err := s.makeMeshGatewayCluster(clusterName, cfgSnap) + // Create the cluster for default/unnamed services + var cluster *envoy.Cluster + var err error + if hasResolver { + cluster, err = s.makeMeshGatewayClusterWithConnectTimeout(clusterName, cfgSnap, resolver.ConnectTimeout) + } else { + cluster, err = s.makeMeshGatewayCluster(clusterName, cfgSnap) + } if err != nil { return nil, err } clusters = append(clusters, cluster) // if there is a service-resolver for this service then also setup subset clusters for it - if resolver, ok := cfgSnap.MeshGateway.ServiceResolvers[svc]; ok { + if hasResolver { // generate 1 cluster for each service subset - for subsetName, _ := range resolver.Subsets { + for subsetName := range resolver.Subsets { clusterName := connect.ServiceSNI(svc.ID, subsetName, svc.NamespaceOrDefault(), cfgSnap.Datacenter, cfgSnap.Roots.TrustDomain) - cluster, err := s.makeMeshGatewayCluster(clusterName, cfgSnap) + cluster, err := s.makeMeshGatewayClusterWithConnectTimeout(clusterName, cfgSnap, resolver.ConnectTimeout) if err != nil { return nil, err } @@ -464,6 +472,14 @@ func makeClusterFromUserConfig(configJSON string) (*envoy.Cluster, error) { } func (s *Server) makeMeshGatewayCluster(clusterName string, cfgSnap *proxycfg.ConfigSnapshot) (*envoy.Cluster, error) { + return s.makeMeshGatewayClusterWithConnectTimeout(clusterName, cfgSnap, 0) +} + +// makeMeshGatewayClusterWithConnectTimeout initializes a mesh gateway cluster +// with the specified connect timeout. If the timeout is 0, the connect timeout +// defaults to use the mesh gateway timeout. +func (s *Server) makeMeshGatewayClusterWithConnectTimeout(clusterName string, cfgSnap *proxycfg.ConfigSnapshot, + connectTimeout time.Duration) (*envoy.Cluster, error) { cfg, err := ParseMeshGatewayConfig(cfgSnap.Proxy.Config) if err != nil { // Don't hard fail on a config typo, just warn. The parse func returns @@ -471,9 +487,13 @@ func (s *Server) makeMeshGatewayCluster(clusterName string, cfgSnap *proxycfg.Co s.Logger.Warn("failed to parse mesh gateway config", "error", err) } + if connectTimeout <= 0 { + connectTimeout = time.Duration(cfg.ConnectTimeoutMs) * time.Millisecond + } + return &envoy.Cluster{ Name: clusterName, - ConnectTimeout: time.Duration(cfg.ConnectTimeoutMs) * time.Millisecond, + ConnectTimeout: connectTimeout, ClusterDiscoveryType: &envoy.Cluster_Type{Type: envoy.Cluster_EDS}, EdsClusterConfig: &envoy.Cluster_EdsClusterConfig{ EdsConfig: &envoycore.ConfigSource{ diff --git a/agent/xds/clusters_test.go b/agent/xds/clusters_test.go index 9c2d0d6a8d..69a448418a 100644 --- a/agent/xds/clusters_test.go +++ b/agent/xds/clusters_test.go @@ -6,6 +6,7 @@ import ( "sort" "testing" "text/template" + "time" envoy "github.com/envoyproxy/go-control-plane/envoy/api/v2" "github.com/hashicorp/consul/agent/proxycfg" @@ -316,6 +317,28 @@ func TestClustersFromSnapshot(t *testing.T) { } }, }, + { + name: "mesh-gateway-service-timeouts", + create: proxycfg.TestConfigSnapshotMeshGateway, + setup: func(snap *proxycfg.ConfigSnapshot) { + snap.MeshGateway.ServiceResolvers = map[structs.ServiceID]*structs.ServiceResolverConfigEntry{ + structs.NewServiceID("bar", nil): &structs.ServiceResolverConfigEntry{ + Kind: structs.ServiceResolver, + Name: "bar", + ConnectTimeout: 10 * time.Second, + Subsets: map[string]structs.ServiceResolverSubset{ + "v1": structs.ServiceResolverSubset{ + Filter: "Service.Meta.Version == 1", + }, + "v2": structs.ServiceResolverSubset{ + Filter: "Service.Meta.Version == 2", + OnlyPassing: true, + }, + }, + }, + } + }, + }, } for _, tt := range tests { diff --git a/agent/xds/testdata/clusters/mesh-gateway-service-timeouts.golden b/agent/xds/testdata/clusters/mesh-gateway-service-timeouts.golden new file mode 100644 index 0000000000..7419ce28be --- /dev/null +++ b/agent/xds/testdata/clusters/mesh-gateway-service-timeouts.golden @@ -0,0 +1,87 @@ +{ + "versionInfo": "00000001", + "resources": [ + { + "@type": "type.googleapis.com/envoy.api.v2.Cluster", + "name": "bar.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "type": "EDS", + "edsClusterConfig": { + "edsConfig": { + "ads": { + + } + } + }, + "connectTimeout": "10s", + "outlierDetection": { + + } + }, + { + "@type": "type.googleapis.com/envoy.api.v2.Cluster", + "name": "dc2.internal.11111111-2222-3333-4444-555555555555.consul", + "type": "EDS", + "edsClusterConfig": { + "edsConfig": { + "ads": { + + } + } + }, + "connectTimeout": "5s", + "outlierDetection": { + + } + }, + { + "@type": "type.googleapis.com/envoy.api.v2.Cluster", + "name": "foo.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "type": "EDS", + "edsClusterConfig": { + "edsConfig": { + "ads": { + + } + } + }, + "connectTimeout": "5s", + "outlierDetection": { + + } + }, + { + "@type": "type.googleapis.com/envoy.api.v2.Cluster", + "name": "v1.bar.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "type": "EDS", + "edsClusterConfig": { + "edsConfig": { + "ads": { + + } + } + }, + "connectTimeout": "10s", + "outlierDetection": { + + } + }, + { + "@type": "type.googleapis.com/envoy.api.v2.Cluster", + "name": "v2.bar.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "type": "EDS", + "edsClusterConfig": { + "edsConfig": { + "ads": { + + } + } + }, + "connectTimeout": "10s", + "outlierDetection": { + + } + } + ], + "typeUrl": "type.googleapis.com/envoy.api.v2.Cluster", + "nonce": "00000001" +} \ No newline at end of file diff --git a/website/source/docs/connect/proxies/envoy.md b/website/source/docs/connect/proxies/envoy.md index af502ec40c..dce51f7069 100644 --- a/website/source/docs/connect/proxies/envoy.md +++ b/website/source/docs/connect/proxies/envoy.md @@ -288,8 +288,11 @@ entry](/docs/agent/config_entries.html#proxy-defaults-proxy-defaults) to act as defaults that are inherited by all services. - `connect_timeout_ms` - The number of milliseconds to allow when making upstream - connections before timing out. Defaults to 5000 - (5 seconds). + connections before timing out. Defaults to 5000 (5 seconds). If the upstream + service has the configuration option + [`connect_timeout_ms`](/docs/agent/config-entries/service-resolver.html#connecttimeout) + set for the `service-resolver`, that timeout value will take precedence over + this mesh gateway option. - `envoy_mesh_gateway_bind_tagged_addresses` - Indicates that the mesh gateway services tagged addresses should be bound to listeners in addition to the