From f844e883b898cf69760330bb37089dec3754fab4 Mon Sep 17 00:00:00 2001 From: Eduard Serra Date: Sat, 31 Jul 2021 07:31:29 -0700 Subject: [PATCH] test/ads: fix test flakyness PR #3803 introduced a mapset (backed by golang map) to slice conversion, which does not guarantee same output ordering for the same input, thus was making a unit test fail from time to time. This commit forces output of the slice to be alphabetically ordered, ensuring determinism of the output slice for the same input mapset. Signed-off-by: Eduard Serra --- pkg/envoy/ads/response_test.go | 40 +++++++++++++++++----------------- pkg/envoy/ads/stream.go | 3 +++ 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/pkg/envoy/ads/response_test.go b/pkg/envoy/ads/response_test.go index d06a46ae8f..57fc407b2c 100644 --- a/pkg/envoy/ads/response_test.go +++ b/pkg/envoy/ads/response_test.go @@ -143,7 +143,7 @@ var _ = Describe("Test ADS response functions", func() { mockCertManager.EXPECT().IssueCertificate(gomock.Any(), certDuration).Return(certPEM, nil).Times(1) // Set subscribed resources for SDS - proxy.SetSubscribedResources(envoy.TypeSDS, mapset.NewSetWith("service-cert:default/bookstore", "root-cert-for-mtls-inbound:default/bookstore")) + proxy.SetSubscribedResources(envoy.TypeSDS, mapset.NewSetWith("service-cert:default/bookstore", "root-cert-for-mtls-inbound:default/bookstore", "root-cert-https:default/bookstore")) err := s.sendResponse(proxy, &server, nil, mockConfigurator, envoy.XDSResponseOrder...) Expect(err).To(BeNil()) @@ -180,25 +180,25 @@ var _ = Describe("Test ADS response functions", func() { Expect(err).To(BeNil()) Expect(proxyServiceCert.Name).To(Equal(secrets.SDSCert{ Name: proxySvcAccount.String(), - CertType: secrets.ServiceCertType, + CertType: secrets.RootCertTypeForMTLSInbound, }.String())) - serverRootCertTypeForMTLSInbound := xds_auth.Secret{} + serverRootCertTypeForHTTPS := xds_auth.Secret{} tmpResource = (*actualResponses)[4].Resources[1] - err = ptypes.UnmarshalAny(tmpResource, &serverRootCertTypeForMTLSInbound) + err = ptypes.UnmarshalAny(tmpResource, &serverRootCertTypeForHTTPS) Expect(err).To(BeNil()) - Expect(serverRootCertTypeForMTLSInbound.Name).To(Equal(secrets.SDSCert{ + Expect(serverRootCertTypeForHTTPS.Name).To(Equal(secrets.SDSCert{ Name: proxySvcAccount.String(), - CertType: secrets.RootCertTypeForMTLSInbound, + CertType: secrets.RootCertTypeForHTTPS, }.String())) - serverRootCertTypeForHTTPS := xds_auth.Secret{} + serverRootCertTypeForMTLSInbound := xds_auth.Secret{} tmpResource = (*actualResponses)[4].Resources[2] - err = ptypes.UnmarshalAny(tmpResource, &serverRootCertTypeForHTTPS) + err = ptypes.UnmarshalAny(tmpResource, &serverRootCertTypeForMTLSInbound) Expect(err).To(BeNil()) - Expect(serverRootCertTypeForHTTPS.Name).To(Equal(secrets.SDSCert{ + Expect(serverRootCertTypeForMTLSInbound.Name).To(Equal(secrets.SDSCert{ Name: proxySvcAccount.String(), - CertType: secrets.RootCertTypeForHTTPS, + CertType: secrets.ServiceCertType, }.String())) }) }) @@ -229,7 +229,7 @@ var _ = Describe("Test ADS response functions", func() { mockCertManager.EXPECT().IssueCertificate(gomock.Any(), certDuration).Return(certPEM, nil).Times(1) // Set subscribed resources - proxy.SetSubscribedResources(envoy.TypeSDS, mapset.NewSetWith("service-cert:default/bookstore", "root-cert-for-mtls-inbound:default/bookstore")) + proxy.SetSubscribedResources(envoy.TypeSDS, mapset.NewSetWith("service-cert:default/bookstore", "root-cert-for-mtls-inbound:default/bookstore", "root-cert-https:default/bookstore")) err := s.sendResponse(proxy, &server, nil, mockConfigurator, envoy.TypeSDS) Expect(err).To(BeNil()) @@ -255,25 +255,25 @@ var _ = Describe("Test ADS response functions", func() { Expect(err).To(BeNil()) Expect(proxyServiceCert.Name).To(Equal(secrets.SDSCert{ Name: proxySvcAccount.String(), - CertType: secrets.ServiceCertType, + CertType: secrets.RootCertTypeForMTLSInbound, }.String())) - serverRootCertTypeForMTLSInbound := xds_auth.Secret{} + serverRootCertTypeForHTTPS := xds_auth.Secret{} tmpResource = sdsResponse.Resources[1] - err = ptypes.UnmarshalAny(tmpResource, &serverRootCertTypeForMTLSInbound) + err = ptypes.UnmarshalAny(tmpResource, &serverRootCertTypeForHTTPS) Expect(err).To(BeNil()) - Expect(serverRootCertTypeForMTLSInbound.Name).To(Equal(secrets.SDSCert{ + Expect(serverRootCertTypeForHTTPS.Name).To(Equal(secrets.SDSCert{ Name: proxySvcAccount.String(), - CertType: secrets.RootCertTypeForMTLSInbound, + CertType: secrets.RootCertTypeForHTTPS, }.String())) - serverRootCertTypeForHTTPS := xds_auth.Secret{} + serverRootCertTypeForMTLSInbound := xds_auth.Secret{} tmpResource = sdsResponse.Resources[2] - err = ptypes.UnmarshalAny(tmpResource, &serverRootCertTypeForHTTPS) + err = ptypes.UnmarshalAny(tmpResource, &serverRootCertTypeForMTLSInbound) Expect(err).To(BeNil()) - Expect(serverRootCertTypeForHTTPS.Name).To(Equal(secrets.SDSCert{ + Expect(serverRootCertTypeForMTLSInbound.Name).To(Equal(secrets.SDSCert{ Name: proxySvcAccount.String(), - CertType: secrets.RootCertTypeForHTTPS, + CertType: secrets.ServiceCertType, }.String())) }) }) diff --git a/pkg/envoy/ads/stream.go b/pkg/envoy/ads/stream.go index 37e2eee048..a62f24e2b8 100644 --- a/pkg/envoy/ads/stream.go +++ b/pkg/envoy/ads/stream.go @@ -2,6 +2,7 @@ package ads import ( "context" + "sort" "strconv" "strings" @@ -285,6 +286,7 @@ func getRequestedResourceNamesSet(discoveryRequest *xds_discovery.DiscoveryReque } // getResourceSliceFromMapset is a helper to convert a mapset of resource names to a string slice +// return slice is alphabetically ordered to ensure output determinism for a given input func getResourceSliceFromMapset(resourceMap mapset.Set) []string { resourceSlice := []string{} it := resourceMap.Iterator() @@ -297,6 +299,7 @@ func getResourceSliceFromMapset(resourceMap mapset.Set) []string { } resourceSlice = append(resourceSlice, resString) } + sort.Strings(resourceSlice) return resourceSlice }