Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor response checker #40156

Merged
merged 1 commit into from
Jul 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 10 additions & 13 deletions pkg/test/framework/components/echo/check/checkers.go
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ func URL(expected string) echo.Checker {
})
}

func isDNSCaptureEnabled(t framework.TestContext) bool {
func IsDNSCaptureEnabled(t framework.TestContext) bool {
t.Helper()
mc := istio.GetOrFail(t, t).MeshConfigOrFail(t)
if mc.DefaultConfig != nil && mc.DefaultConfig.ProxyMetadata != nil {
Expand All @@ -382,7 +382,7 @@ func isDNSCaptureEnabled(t framework.TestContext) bool {
// ReachedTargetClusters is similar to ReachedClusters, except that the set of expected clusters is
// retrieved from the Target of the request.
func ReachedTargetClusters(t framework.TestContext) echo.Checker {
dnsCaptureEnabled := isDNSCaptureEnabled(t)
dnsCaptureEnabled := IsDNSCaptureEnabled(t)
return func(result echo.CallResult, err error) error {
from := result.From
to := result.Opts.To
Expand Down Expand Up @@ -431,23 +431,20 @@ func ReachedTargetClusters(t framework.TestContext) echo.Checker {
// For multi-network configurations, verifies the current (limited) Istio load balancing behavior when going through
// a gateway. Ensures that all expected networks were reached, and that all clusters on the same network as the
// client were reached.
func ReachedClusters(t framework.TestContext, allClusters cluster.Clusters, expectedClusters cluster.Clusters) echo.Checker {
func ReachedClusters(allClusters cluster.Clusters, expectedClusters cluster.Clusters) echo.Checker {
expectedByNetwork := expectedClusters.ByNetwork()
return func(result echo.CallResult, err error) error {
dnsCaptureEnabled := isDNSCaptureEnabled(t)
to := result.Opts.To
if !dnsCaptureEnabled && to.Config().IsHeadless() {
// Headless services rely on DNS resolution. If DNS capture is
// enabled, DNS will return all endpoints in the mesh, which will
// allow requests to go cross-cluster. Otherwise, k8s DNS will
// only return the endpoints within the same cluster as the source
// pod.
return checkReachedSourceClusterOnly(result, allClusters)
}
return checkReachedClusters(result, allClusters, expectedByNetwork)
}
}

// ReachedSourceCluster is similar to ReachedClusters, except it only checks the reachability of source cluster only
func ReachedSourceCluster(allClusters cluster.Clusters) echo.Checker {
return func(result echo.CallResult, err error) error {
return checkReachedSourceClusterOnly(result, allClusters)
}
}

// checkReachedSourceClusterOnly verifies that the only cluster that was reached is the cluster where
// the source workload resides.
func checkReachedSourceClusterOnly(result echo.CallResult, allClusters cluster.Clusters) error {
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/pilot/common/routing.go
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,7 @@ spec:
// Conditionally check reached clusters to work around connection load balancing issues
// See https://github.com/istio/istio/issues/32208 for details
// We want to skip this for requests from the cross-network pod
if err := check.ReachedClusters(t, t.AllClusters(), toClusters).Check(echo.CallResult{
if err := check.ReachedClusters(t.AllClusters(), toClusters).Check(echo.CallResult{
From: result.From,
Opts: result.Opts,
Responses: hostResponses,
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/pilot/ingress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ spec:
`
}

successChecker := check.And(check.OK(), check.ReachedClusters(t, t.AllClusters(), apps.B.Clusters()))
successChecker := check.And(check.OK(), check.ReachedClusters(t.AllClusters(), apps.B.Clusters()))
failureChecker := check.Status(http.StatusNotFound)
count := 2 * t.Clusters().Len()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func TestClusterLocal(t *testing.T) {
if ht == hostTypeClusterLocal {
// For calls to cluster.local, ensure that all requests stay in the same cluster
expectedClusters := cluster.Clusters{from.Config().Cluster}
checker = checkClustersReached(t, t.AllClusters(), expectedClusters)
checker = checkClustersReached(t.AllClusters(), expectedClusters)
} else {
// For calls to clusterset.local, we should fail DNS lookup. The clusterset.local host
// is only available for a service when it is exported in at least one cluster.
Expand Down Expand Up @@ -136,7 +136,7 @@ func TestMeshWide(t *testing.T) {
// Ensure that requests to clusterset.local reach all destination clusters.
expectedClusters = to.Clusters()
}
callAndValidate(t, ht, from, to, checkClustersReached(t, t.AllClusters(), expectedClusters))
callAndValidate(t, ht, from, to, checkClustersReached(t.AllClusters(), expectedClusters))
})
})
}
Expand Down Expand Up @@ -177,7 +177,7 @@ func TestServiceExportedInOneCluster(t *testing.T) {
expectedClusters = append(expectedClusters, from.Config().Cluster)
}
}
callAndValidate(t, ht, from, to, checkClustersReached(t, t.AllClusters(), expectedClusters))
callAndValidate(t, ht, from, to, checkClustersReached(t.AllClusters(), expectedClusters))
})
})
}
Expand Down Expand Up @@ -226,10 +226,10 @@ func newServiceExport(service string, serviceExportGVR schema.GroupVersionResour
}
}

func checkClustersReached(t framework.TestContext, allClusters cluster.Clusters, clusters cluster.Clusters) echo.Checker {
func checkClustersReached(allClusters cluster.Clusters, clusters cluster.Clusters) echo.Checker {
return check.And(
check.OK(),
check.ReachedClusters(t, allClusters, clusters))
check.ReachedClusters(allClusters, clusters))
}

func checkDNSLookupFailed() echo.Checker {
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/pilot/multicluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ spec:
},
Check: check.And(
check.OK(),
check.ReachedClusters(t, t.AllClusters(), cluster.Clusters{source.Config().Cluster}),
check.ReachedClusters(t.AllClusters(), cluster.Clusters{source.Config().Cluster}),
),
Retry: echo.Retry{
Options: []retry.Option{multiclusterRetryDelay, multiclusterRetryTimeout},
Expand Down
18 changes: 15 additions & 3 deletions tests/integration/security/reachability_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -453,15 +453,27 @@ func TestReachability(t *testing.T) {

// Check that the correct clusters/networks were reached.
if c.expectCrossNetwork(from, opts) {
opts.Check = check.And(opts.Check, check.ReachedTargetClusters(t))
if !check.IsDNSCaptureEnabled(t) && opts.To.Config().Headless {
opts.Check = check.And(opts.Check, check.ReachedSourceCluster(t.Clusters()))
} else {
opts.Check = check.And(opts.Check, check.ReachedTargetClusters(t))
}
} else if c.expectCrossCluster(from, opts) {
// Expect to stay in the same network as the source pod.
expectedClusters := to.Clusters().ForNetworks(from.Config().Cluster.NetworkName())
opts.Check = check.And(opts.Check, check.ReachedClusters(t, t.Clusters(), expectedClusters))
if !check.IsDNSCaptureEnabled(t) && opts.To.Config().Headless {
opts.Check = check.And(opts.Check, check.ReachedSourceCluster(t.Clusters()))
} else {
opts.Check = check.And(opts.Check, check.ReachedClusters(t.Clusters(), expectedClusters))
}
} else {
// Expect to stay in the same cluster as the source pod.
expectedClusters := cluster.Clusters{from.Config().Cluster}
opts.Check = check.And(opts.Check, check.ReachedClusters(t, t.Clusters(), expectedClusters))
if !check.IsDNSCaptureEnabled(t) && opts.To.Config().Headless {
opts.Check = check.And(opts.Check, check.ReachedSourceCluster(t.Clusters()))
} else {
opts.Check = check.And(opts.Check, check.ReachedClusters(t.Clusters(), expectedClusters))
}
}
} else {
opts.Check = check.NotOK()
Expand Down