Skip to content

Commit

Permalink
fix test race (istio#16095)
Browse files Browse the repository at this point in the history
  • Loading branch information
@hzxuzhonghu @istio-testing
hzxuzhonghu authored and istio-testing committed Aug 7, 2019
1 parent 362ea88 commit 64dd528
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 62 deletions.
29 changes: 17 additions & 12 deletions pilot/pkg/proxy/envoy/v2/eds_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,11 @@ func TestEds(t *testing.T) {
})
t.Run("CDSSave", func(t *testing.T) {
// Moved from cds_test, using new client
if len(adscConn.Clusters) == 0 {
clusters := adscConn.GetClusters()
if len(clusters) == 0 {
t.Error("No clusters in ADS response")
}
strResponse, _ := json.MarshalIndent(adscConn.Clusters, " ", " ")
strResponse, _ := json.MarshalIndent(clusters, " ", " ")
_ = ioutil.WriteFile(env.IstioOut+"/cdsv2_sidecar.json", strResponse, 0644)

})
Expand All @@ -117,7 +118,8 @@ func TestEds(t *testing.T) {

adscConn := adsConnectAndWait(t, 0x0a0a0a0a)
defer adscConn.Close()
lbe, f := adscConn.EDS["outbound|80||weighted.static.svc.cluster.local"]
endpoints := adscConn.GetEndpoints()
lbe, f := endpoints["outbound|80||weighted.static.svc.cluster.local"]
if !f || len(lbe.Endpoints) == 0 {
t.Fatalf("No lb endpoints for %v, %v", "outbound|80||weighted.static.svc.cluster.local", adscConn.EndpointsJSON())
}
Expand Down Expand Up @@ -167,7 +169,7 @@ func adsConnectAndWait(t *testing.T, ip int) *adsc.ADSC {
t.Fatal("Error getting initial config ", err)
}

if len(adscConn.EDS) == 0 {
if len(adscConn.GetEndpoints()) == 0 {
t.Fatal("No endpoints")
}
return adscConn
Expand Down Expand Up @@ -222,7 +224,7 @@ func testTCPEndpoints(expected string, adsc *adsc.ADSC, t *testing.T) {
// address.
func testEndpoints(expected string, cluster string, adsc *adsc.ADSC, t *testing.T) {
t.Helper()
lbe, f := adsc.EDS[cluster]
lbe, f := adsc.GetEndpoints()[cluster]
if !f || len(lbe.Endpoints) == 0 {
t.Fatalf("No lb endpoints for %v, %v", cluster, adsc.EndpointsJSON())
}
Expand All @@ -243,12 +245,15 @@ func testEndpoints(expected string, cluster string, adsc *adsc.ADSC, t *testing.
}

func testLocalityPrioritizedEndpoints(adsc *adsc.ADSC, adsc2 *adsc.ADSC, t *testing.T) {
verifyLocalityPriorities(asdcLocality, adsc.EDS["outbound|80||locality.cluster.local"].GetEndpoints(), t)
verifyLocalityPriorities(asdc2Locality, adsc2.EDS["outbound|80||locality.cluster.local"].GetEndpoints(), t)
endpoints1 := adsc.GetEndpoints()
endpoints2 := adsc2.GetEndpoints()

verifyLocalityPriorities(asdcLocality, endpoints1["outbound|80||locality.cluster.local"].GetEndpoints(), t)
verifyLocalityPriorities(asdc2Locality, endpoints2["outbound|80||locality.cluster.local"].GetEndpoints(), t)

// No outlier detection specified for this cluster, so we shouldn't apply priority.
verifyNoLocalityPriorities(adsc.EDS["outbound|80||locality-no-outlier-detection.cluster.local"].GetEndpoints(), t)
verifyNoLocalityPriorities(adsc2.EDS["outbound|80||locality-no-outlier-detection.cluster.local"].GetEndpoints(), t)
verifyNoLocalityPriorities(endpoints1["outbound|80||locality-no-outlier-detection.cluster.local"].GetEndpoints(), t)
verifyNoLocalityPriorities(endpoints2["outbound|80||locality-no-outlier-detection.cluster.local"].GetEndpoints(), t)
}

// Tests that Services with multiple ports sharing the same port number are properly sent endpoints.
Expand Down Expand Up @@ -303,9 +308,9 @@ func verifyLocalityPriorities(proxyLocality string, eps []*endpoint.LocalityLbEn
// Verify server sends UDS endpoints
func testUdsEndpoints(_ *bootstrap.Server, adsc *adsc.ADSC, t *testing.T) {
// Check the UDS endpoint ( used to be separate test - but using old unused GRPC method)
// The new test also verifies CDS is pusing the UDS cluster, since adsc.EDS is
// The new test also verifies CDS is pusing the UDS cluster, since adsc.eds is
// populated using CDS response
lbe, f := adsc.EDS["outbound|0||localuds.cluster.local"]
lbe, f := adsc.GetEndpoints()["outbound|0||localuds.cluster.local"]
if !f || len(lbe.Endpoints) == 0 {
t.Error("No UDS lb endpoints")
} else {
Expand Down Expand Up @@ -478,7 +483,7 @@ func multipleRequest(server *bootstrap.Server, inc bool, nclients,
return
}

if len(adscConn.EDS) == 0 {
if len(adscConn.GetEndpoints()) == 0 {
errChan <- errors.New("no endpoints")
wgConnect.Done()
return
Expand Down
46 changes: 23 additions & 23 deletions pilot/pkg/proxy/envoy/v2/lds_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,18 +78,18 @@ func TestLDSIsolated(t *testing.T) {
// 7071 (inbound), 2001 (service - also as http proxy), 15002 (http-proxy)
// We dont get mixer on 9091 or 15004 because there are no services defined in istio-system namespace
// in the none.yaml setup
if len(ldsr.HTTPListeners) != 3 {
if len(ldsr.GetHTTPListeners()) != 3 {
// TODO: we are still debating if for HTTP services we have any use case to create a 127.0.0.1:port outbound
// for the service (the http proxy is already covering this)
t.Error("HTTP listeners, expecting 5 got ", len(ldsr.HTTPListeners), ldsr.HTTPListeners)
t.Error("HTTP listeners, expecting 5 got ", len(ldsr.GetHTTPListeners()), ldsr.GetHTTPListeners())
}

// s1tcp:2000 outbound, bind=true (to reach other instances of the service)
// s1:5005 outbound, bind=true
// :443 - https external, bind=false
// 10.11.0.1_7070, bind=true -> inbound|2000|s1 - on port 7070, fwd to 37070
// virtual
if len(ldsr.TCPListeners) == 0 {
if len(ldsr.GetTCPListeners()) == 0 {
t.Fatal("No response")
}

Expand Down Expand Up @@ -241,22 +241,22 @@ func TestLDSWithDefaultSidecar(t *testing.T) {
}

// Expect 7 listeners : 2 orig_dst, 1 http inbound + 4 outbound (http, tcp1, istio-policy and istio-telemetry)
if (len(adsResponse.HTTPListeners) + len(adsResponse.TCPListeners)) != 7 {
t.Fatalf("Expected 7 listeners, got %d\n", len(adsResponse.HTTPListeners)+len(adsResponse.TCPListeners))
if (len(adsResponse.GetHTTPListeners()) + len(adsResponse.GetTCPListeners())) != 7 {
t.Fatalf("Expected 7 listeners, got %d\n", len(adsResponse.GetHTTPListeners())+len(adsResponse.GetTCPListeners()))
}

// Expect 12 CDS clusters:
// 3 inbound(http, inbound passthroughipv4 and inbound passthroughipv6)
// 9 outbound (2 http services, 1 tcp service, 2 istio-system services,
// and 2 subsets of http1, 1 blackhole, 1 passthrough)
if (len(adsResponse.Clusters) + len(adsResponse.EDSClusters)) != 12 {
t.Fatalf("Expected 12 Clusters in CDS output. Got %d", len(adsResponse.Clusters)+len(adsResponse.EDSClusters))
if (len(adsResponse.GetClusters()) + len(adsResponse.GetEdsClusters())) != 12 {
t.Fatalf("Expected 12 clusters in CDS output. Got %d", len(adsResponse.GetClusters())+len(adsResponse.GetEdsClusters()))
}

// Expect two vhost blocks in RDS output for 8080 (one for http1, another for http2)
// plus one extra due to mem registry
if len(adsResponse.Routes["8080"].VirtualHosts) != 3 {
t.Fatalf("Expected 3 VirtualHosts in RDS output. Got %d", len(adsResponse.Routes["8080"].VirtualHosts))
if len(adsResponse.GetRoutes()["8080"].VirtualHosts) != 3 {
t.Fatalf("Expected 3 VirtualHosts in RDS output. Got %d", len(adsResponse.GetRoutes()["8080"].VirtualHosts))
}
}

Expand Down Expand Up @@ -306,13 +306,13 @@ func TestLDSWithIngressGateway(t *testing.T) {

// Expect 2 listeners : 1 for 80, 1 for 443
// where 443 listener has 3 filter chains
if (len(adsResponse.HTTPListeners) + len(adsResponse.TCPListeners)) != 2 {
t.Fatalf("Expected 2 listeners, got %d\n", len(adsResponse.HTTPListeners)+len(adsResponse.TCPListeners))
if (len(adsResponse.GetHTTPListeners()) + len(adsResponse.GetTCPListeners())) != 2 {
t.Fatalf("Expected 2 listeners, got %d\n", len(adsResponse.GetHTTPListeners())+len(adsResponse.GetTCPListeners()))
}

// TODO: This is flimsy. The ADSC code treats any listener with http connection manager as a HTTP listener
// instead of looking at it as a listener with multiple filter chains
l := adsResponse.HTTPListeners["0.0.0.0_443"]
l := adsResponse.GetHTTPListeners()["0.0.0.0_443"]

if l != nil {
if len(l.FilterChains) != 3 {
Expand Down Expand Up @@ -431,13 +431,13 @@ func TestLDSWithSidecarForWorkloadWithoutService(t *testing.T) {
}

// Expect 1 HTTP listeners for 8081
if len(adsResponse.HTTPListeners) != 1 {
t.Fatalf("Expected 1 http listeners, got %d", len(adsResponse.HTTPListeners))
if len(adsResponse.GetHTTPListeners()) != 1 {
t.Fatalf("Expected 1 http listeners, got %d", len(adsResponse.GetHTTPListeners()))
}

// TODO: This is flimsy. The ADSC code treats any listener with http connection manager as a HTTP listener
// instead of looking at it as a listener with multiple filter chains
if l := adsResponse.HTTPListeners["0.0.0.0_8081"]; l != nil {
if l := adsResponse.GetHTTPListeners()["0.0.0.0_8081"]; l != nil {
expected := 1
if features.RestrictPodIPTrafficLoops.Get() {
expected = 2
Expand All @@ -449,12 +449,12 @@ func TestLDSWithSidecarForWorkloadWithoutService(t *testing.T) {
t.Fatal("Expected listener for 0.0.0.0_8081")
}

// Expect only one EDS cluster for http1.ns1.svc.cluster.local
if len(adsResponse.EDSClusters) != 1 {
t.Fatalf("Expected 1 eds cluster, got %d", len(adsResponse.EDSClusters))
// Expect only one eds cluster for http1.ns1.svc.cluster.local
if len(adsResponse.GetEdsClusters()) != 1 {
t.Fatalf("Expected 1 eds cluster, got %d", len(adsResponse.GetEdsClusters()))
}
if cluster, ok := adsResponse.EDSClusters["outbound|8081||http1.ns1.svc.cluster.local"]; !ok {
t.Fatalf("Expected EDS cluster outbound|8081||http1.ns1.svc.cluster.local, got %v", cluster.Name)
if cluster, ok := adsResponse.GetEdsClusters()["outbound|8081||http1.ns1.svc.cluster.local"]; !ok {
t.Fatalf("Expected eds cluster outbound|8081||http1.ns1.svc.cluster.local, got %v", cluster.Name)
}
}

Expand Down Expand Up @@ -532,12 +532,12 @@ func TestLDSEnvoyFilterWithWorkloadSelector(t *testing.T) {
}

// Expect 1 HTTP listeners for 8081
if len(adsResponse.HTTPListeners) != 1 {
t.Fatalf("Expected 1 http listeners, got %d", len(adsResponse.HTTPListeners))
if len(adsResponse.GetHTTPListeners()) != 1 {
t.Fatalf("Expected 1 http listeners, got %d", len(adsResponse.GetHTTPListeners()))
}
// TODO: This is flimsy. The ADSC code treats any listener with http connection manager as a HTTP listener
// instead of looking at it as a listener with multiple filter chains
l := adsResponse.HTTPListeners["0.0.0.0_8081"]
l := adsResponse.GetHTTPListeners()["0.0.0.0_8081"]

expectLuaFilter(t, l, test.expectLuaFilter)
})
Expand Down
Loading

0 comments on commit 64dd528

Please sign in to comment.