From 25f580bcaa05348f5e4f762531e331a449e9eb02 Mon Sep 17 00:00:00 2001 From: Matt Keeler Date: Tue, 2 Jul 2019 15:53:06 -0400 Subject: [PATCH] Fix a bunch of xds flaky tests The clusters/endpoints test were still relying on deterministic ordering of clusters/endpoints which cannot be relied upon due to golang purposefully not providing any guarantee about consistent interation ordering of maps. Also fixed a small bug in the connect proxy cluster generation that was causing the clusters slice to be double the size it needed to with the first half being all nil pointers. --- agent/xds/clusters.go | 2 +- agent/xds/clusters_test.go | 5 + agent/xds/endpoints_test.go | 4 + .../testdata/clusters/custom-local-app.golden | 26 ++-- .../testdata/clusters/custom-timeouts.golden | 50 +++---- agent/xds/testdata/clusters/defaults.golden | 50 +++---- .../mesh-gateway-service-subsets.golden | 32 ++--- .../xds/testdata/clusters/mesh-gateway.golden | 32 ++--- .../mesh-gateway-service-subsets.golden | 136 +++++++++--------- .../testdata/endpoints/mesh-gateway.golden | 92 ++++++------ 10 files changed, 219 insertions(+), 210 deletions(-) diff --git a/agent/xds/clusters.go b/agent/xds/clusters.go index 8012f81a3b..a58e42f2d0 100644 --- a/agent/xds/clusters.go +++ b/agent/xds/clusters.go @@ -40,7 +40,7 @@ func (s *Server) clustersFromSnapshot(cfgSnap *proxycfg.ConfigSnapshot, token st // (upstreams) in the snapshot. func (s *Server) clustersFromSnapshotConnectProxy(cfgSnap *proxycfg.ConfigSnapshot, token string) ([]proto.Message, error) { // TODO(rb): this sizing is a low bound. - clusters := make([]proto.Message, len(cfgSnap.Proxy.Upstreams)+1) + clusters := make([]proto.Message, 0, len(cfgSnap.Proxy.Upstreams)+1) // Include the "app" cluster for the public listener appCluster, err := s.makeAppCluster(cfgSnap) diff --git a/agent/xds/clusters_test.go b/agent/xds/clusters_test.go index 8ca9ec9dc3..fba7399b17 100644 --- a/agent/xds/clusters_test.go +++ b/agent/xds/clusters_test.go @@ -5,9 +5,11 @@ import ( "log" "os" "path" + "sort" "testing" "text/template" + envoy "github.com/envoyproxy/go-control-plane/envoy/api/v2" "github.com/hashicorp/consul/agent/proxycfg" "github.com/hashicorp/consul/agent/structs" testinf "github.com/mitchellh/go-testing-interface" @@ -153,6 +155,9 @@ func TestClustersFromSnapshot(t *testing.T) { clusters, err := s.clustersFromSnapshot(snap, "my-token") require.NoError(err) + sort.Slice(clusters, func(i, j int) bool { + return clusters[i].(*envoy.Cluster).Name < clusters[j].(*envoy.Cluster).Name + }) r, err := createResponse(ClusterType, "00000001", "00000001", clusters) require.NoError(err) diff --git a/agent/xds/endpoints_test.go b/agent/xds/endpoints_test.go index c1de4c925f..964ffa56ab 100644 --- a/agent/xds/endpoints_test.go +++ b/agent/xds/endpoints_test.go @@ -4,6 +4,7 @@ import ( "log" "os" "path" + "sort" "testing" "github.com/mitchellh/copystructure" @@ -295,6 +296,9 @@ func Test_endpointsFromSnapshot(t *testing.T) { s := Server{Logger: log.New(os.Stderr, "", log.LstdFlags)} endpoints, err := s.endpointsFromSnapshot(snap, "my-token") + sort.Slice(endpoints, func(i, j int) bool { + return endpoints[i].(*envoy.ClusterLoadAssignment).ClusterName < endpoints[j].(*envoy.ClusterLoadAssignment).ClusterName + }) require.NoError(err) r, err := createResponse(EndpointType, "00000001", "00000001", endpoints) require.NoError(err) diff --git a/agent/xds/testdata/clusters/custom-local-app.golden b/agent/xds/testdata/clusters/custom-local-app.golden index 5266b1fc13..a17d285f8c 100644 --- a/agent/xds/testdata/clusters/custom-local-app.golden +++ b/agent/xds/testdata/clusters/custom-local-app.golden @@ -1,19 +1,6 @@ { "versionInfo": "00000001", "resources": [ - { - "@type": "type.googleapis.com/envoy.api.v2.Cluster", - "name": "mylocal", - "connectTimeout": "5s", - "hosts": [ - { - "socketAddress": { - "address": "127.0.0.1", - "portValue": 8080 - } - } - ] - }, { "@type": "type.googleapis.com/envoy.api.v2.Cluster", "name": "db", @@ -53,6 +40,19 @@ } }, + { + "@type": "type.googleapis.com/envoy.api.v2.Cluster", + "name": "mylocal", + "connectTimeout": "5s", + "hosts": [ + { + "socketAddress": { + "address": "127.0.0.1", + "portValue": 8080 + } + } + ] + }, { "@type": "type.googleapis.com/envoy.api.v2.Cluster", "name": "prepared_query:geo-cache", diff --git a/agent/xds/testdata/clusters/custom-timeouts.golden b/agent/xds/testdata/clusters/custom-timeouts.golden index f1846f8eb9..c8c776fed0 100644 --- a/agent/xds/testdata/clusters/custom-timeouts.golden +++ b/agent/xds/testdata/clusters/custom-timeouts.golden @@ -1,31 +1,6 @@ { "versionInfo": "00000001", "resources": [ - { - "@type": "type.googleapis.com/envoy.api.v2.Cluster", - "name": "local_app", - "type": "STATIC", - "connectTimeout": "1.234s", - "loadAssignment": { - "clusterName": "local_app", - "endpoints": [ - { - "lbEndpoints": [ - { - "endpoint": { - "address": { - "socketAddress": { - "address": "127.0.0.1", - "portValue": 8080 - } - } - } - } - ] - } - ] - } - }, { "@type": "type.googleapis.com/envoy.api.v2.Cluster", "name": "db", @@ -65,6 +40,31 @@ } }, + { + "@type": "type.googleapis.com/envoy.api.v2.Cluster", + "name": "local_app", + "type": "STATIC", + "connectTimeout": "1.234s", + "loadAssignment": { + "clusterName": "local_app", + "endpoints": [ + { + "lbEndpoints": [ + { + "endpoint": { + "address": { + "socketAddress": { + "address": "127.0.0.1", + "portValue": 8080 + } + } + } + } + ] + } + ] + } + }, { "@type": "type.googleapis.com/envoy.api.v2.Cluster", "name": "prepared_query:geo-cache", diff --git a/agent/xds/testdata/clusters/defaults.golden b/agent/xds/testdata/clusters/defaults.golden index f1f404456f..11074a4355 100644 --- a/agent/xds/testdata/clusters/defaults.golden +++ b/agent/xds/testdata/clusters/defaults.golden @@ -1,31 +1,6 @@ { "versionInfo": "00000001", "resources": [ - { - "@type": "type.googleapis.com/envoy.api.v2.Cluster", - "name": "local_app", - "type": "STATIC", - "connectTimeout": "5s", - "loadAssignment": { - "clusterName": "local_app", - "endpoints": [ - { - "lbEndpoints": [ - { - "endpoint": { - "address": { - "socketAddress": { - "address": "127.0.0.1", - "portValue": 8080 - } - } - } - } - ] - } - ] - } - }, { "@type": "type.googleapis.com/envoy.api.v2.Cluster", "name": "db", @@ -65,6 +40,31 @@ } }, + { + "@type": "type.googleapis.com/envoy.api.v2.Cluster", + "name": "local_app", + "type": "STATIC", + "connectTimeout": "5s", + "loadAssignment": { + "clusterName": "local_app", + "endpoints": [ + { + "lbEndpoints": [ + { + "endpoint": { + "address": { + "socketAddress": { + "address": "127.0.0.1", + "portValue": 8080 + } + } + } + } + ] + } + ] + } + }, { "@type": "type.googleapis.com/envoy.api.v2.Cluster", "name": "prepared_query:geo-cache", diff --git a/agent/xds/testdata/clusters/mesh-gateway-service-subsets.golden b/agent/xds/testdata/clusters/mesh-gateway-service-subsets.golden index 2a01dc2509..aa8d426119 100644 --- a/agent/xds/testdata/clusters/mesh-gateway-service-subsets.golden +++ b/agent/xds/testdata/clusters/mesh-gateway-service-subsets.golden @@ -1,6 +1,22 @@ { "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": "5s", + "outlierDetection": { + + } + }, { "@type": "type.googleapis.com/envoy.api.v2.Cluster", "name": "dc2.internal.11111111-2222-3333-4444-555555555555.consul", @@ -33,22 +49,6 @@ } }, - { - "@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": "5s", - "outlierDetection": { - - } - }, { "@type": "type.googleapis.com/envoy.api.v2.Cluster", "name": "v1.bar.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", diff --git a/agent/xds/testdata/clusters/mesh-gateway.golden b/agent/xds/testdata/clusters/mesh-gateway.golden index 7c8ef1f42b..f6d5f1f087 100644 --- a/agent/xds/testdata/clusters/mesh-gateway.golden +++ b/agent/xds/testdata/clusters/mesh-gateway.golden @@ -1,6 +1,22 @@ { "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": "5s", + "outlierDetection": { + + } + }, { "@type": "type.googleapis.com/envoy.api.v2.Cluster", "name": "dc2.internal.11111111-2222-3333-4444-555555555555.consul", @@ -31,22 +47,6 @@ "connectTimeout": "5s", "outlierDetection": { - } - }, - { - "@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": "5s", - "outlierDetection": { - } } ], diff --git a/agent/xds/testdata/endpoints/mesh-gateway-service-subsets.golden b/agent/xds/testdata/endpoints/mesh-gateway-service-subsets.golden index 87ac0b806a..91e4079143 100644 --- a/agent/xds/testdata/endpoints/mesh-gateway-service-subsets.golden +++ b/agent/xds/testdata/endpoints/mesh-gateway-service-subsets.golden @@ -1,6 +1,52 @@ { "versionInfo": "00000001", "resources": [ + { + "@type": "type.googleapis.com/envoy.api.v2.ClusterLoadAssignment", + "clusterName": "bar.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "endpoints": [ + { + "lbEndpoints": [ + { + "endpoint": { + "address": { + "socketAddress": { + "address": "172.16.1.6", + "portValue": 2222 + } + } + }, + "healthStatus": "HEALTHY", + "loadBalancingWeight": 1 + }, + { + "endpoint": { + "address": { + "socketAddress": { + "address": "172.16.1.7", + "portValue": 2222 + } + } + }, + "healthStatus": "HEALTHY", + "loadBalancingWeight": 1 + }, + { + "endpoint": { + "address": { + "socketAddress": { + "address": "172.16.1.8", + "portValue": 2222 + } + } + }, + "healthStatus": "HEALTHY", + "loadBalancingWeight": 1 + } + ] + } + ] + }, { "@type": "type.googleapis.com/envoy.api.v2.ClusterLoadAssignment", "clusterName": "dc2.internal.11111111-2222-3333-4444-555555555555.consul", @@ -93,52 +139,6 @@ } ] }, - { - "@type": "type.googleapis.com/envoy.api.v2.ClusterLoadAssignment", - "clusterName": "bar.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", - "endpoints": [ - { - "lbEndpoints": [ - { - "endpoint": { - "address": { - "socketAddress": { - "address": "172.16.1.6", - "portValue": 2222 - } - } - }, - "healthStatus": "HEALTHY", - "loadBalancingWeight": 1 - }, - { - "endpoint": { - "address": { - "socketAddress": { - "address": "172.16.1.7", - "portValue": 2222 - } - } - }, - "healthStatus": "HEALTHY", - "loadBalancingWeight": 1 - }, - { - "endpoint": { - "address": { - "socketAddress": { - "address": "172.16.1.8", - "portValue": 2222 - } - } - }, - "healthStatus": "HEALTHY", - "loadBalancingWeight": 1 - } - ] - } - ] - }, { "@type": "type.googleapis.com/envoy.api.v2.ClusterLoadAssignment", "clusterName": "v1.bar.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", @@ -173,28 +173,6 @@ } ] }, - { - "@type": "type.googleapis.com/envoy.api.v2.ClusterLoadAssignment", - "clusterName": "v2.bar.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", - "endpoints": [ - { - "lbEndpoints": [ - { - "endpoint": { - "address": { - "socketAddress": { - "address": "172.16.1.8", - "portValue": 2222 - } - } - }, - "healthStatus": "HEALTHY", - "loadBalancingWeight": 1 - } - ] - } - ] - }, { "@type": "type.googleapis.com/envoy.api.v2.ClusterLoadAssignment", "clusterName": "v1.foo.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", @@ -229,6 +207,28 @@ } ] }, + { + "@type": "type.googleapis.com/envoy.api.v2.ClusterLoadAssignment", + "clusterName": "v2.bar.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "endpoints": [ + { + "lbEndpoints": [ + { + "endpoint": { + "address": { + "socketAddress": { + "address": "172.16.1.8", + "portValue": 2222 + } + } + }, + "healthStatus": "HEALTHY", + "loadBalancingWeight": 1 + } + ] + } + ] + }, { "@type": "type.googleapis.com/envoy.api.v2.ClusterLoadAssignment", "clusterName": "v2.foo.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", diff --git a/agent/xds/testdata/endpoints/mesh-gateway.golden b/agent/xds/testdata/endpoints/mesh-gateway.golden index ad013f1df0..edabb3223a 100644 --- a/agent/xds/testdata/endpoints/mesh-gateway.golden +++ b/agent/xds/testdata/endpoints/mesh-gateway.golden @@ -1,6 +1,52 @@ { "versionInfo": "00000001", "resources": [ + { + "@type": "type.googleapis.com/envoy.api.v2.ClusterLoadAssignment", + "clusterName": "bar.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", + "endpoints": [ + { + "lbEndpoints": [ + { + "endpoint": { + "address": { + "socketAddress": { + "address": "172.16.1.6", + "portValue": 2222 + } + } + }, + "healthStatus": "HEALTHY", + "loadBalancingWeight": 1 + }, + { + "endpoint": { + "address": { + "socketAddress": { + "address": "172.16.1.7", + "portValue": 2222 + } + } + }, + "healthStatus": "HEALTHY", + "loadBalancingWeight": 1 + }, + { + "endpoint": { + "address": { + "socketAddress": { + "address": "172.16.1.8", + "portValue": 2222 + } + } + }, + "healthStatus": "HEALTHY", + "loadBalancingWeight": 1 + } + ] + } + ] + }, { "@type": "type.googleapis.com/envoy.api.v2.ClusterLoadAssignment", "clusterName": "dc2.internal.11111111-2222-3333-4444-555555555555.consul", @@ -92,52 +138,6 @@ ] } ] - }, - { - "@type": "type.googleapis.com/envoy.api.v2.ClusterLoadAssignment", - "clusterName": "bar.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", - "endpoints": [ - { - "lbEndpoints": [ - { - "endpoint": { - "address": { - "socketAddress": { - "address": "172.16.1.6", - "portValue": 2222 - } - } - }, - "healthStatus": "HEALTHY", - "loadBalancingWeight": 1 - }, - { - "endpoint": { - "address": { - "socketAddress": { - "address": "172.16.1.7", - "portValue": 2222 - } - } - }, - "healthStatus": "HEALTHY", - "loadBalancingWeight": 1 - }, - { - "endpoint": { - "address": { - "socketAddress": { - "address": "172.16.1.8", - "portValue": 2222 - } - } - }, - "healthStatus": "HEALTHY", - "loadBalancingWeight": 1 - } - ] - } - ] } ], "typeUrl": "type.googleapis.com/envoy.api.v2.ClusterLoadAssignment",