Skip to content

Commit

Permalink
chore: improve the stability of coverage test in CI (envoyproxy#2692)
Browse files Browse the repository at this point in the history 
* improve test in kubernetes_test.go

Signed-off-by: shawnh2 <shawnhxh@outlook.com>

* fix test by changing return to assert action

Signed-off-by: shawnh2 <shawnhxh@outlook.com>

* fix race test in routes_test.go

Signed-off-by: shawnh2 <shawnhxh@outlook.com>

* fix conflicted constants in routes_test.go

Signed-off-by: shawnh2 <shawnhxh@outlook.com>

* dedup the code and decrease the wait time

Signed-off-by: shawnh2 <shawnhxh@outlook.com>

---------

Signed-off-by: shawnh2 <shawnhxh@outlook.com>
Co-authored-by: zirain <zirain2009@gmail.com>
  • Loading branch information
@shawnh2 @zirain
shawnh2 and zirain authored Feb 27, 2024
1 parent 698e6f1 commit 82d9b6f
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 48 deletions.
111 changes: 67 additions & 44 deletions internal/provider/kubernetes/kubernetes_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,14 +139,14 @@ func testGatewayClassAcceptedStatus(ctx context.Context, t *testing.T, provider
return false
}, defaultWait, defaultTick)

require.Eventually(t, func() bool {
return resources.GatewayAPIResources.Len() != 0
}, defaultWait, defaultTick)

// Even though no gateways exist, the controller loads the empty resource map
// to support gateway deletions.
require.Eventually(t, func() bool {
gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName)
if !ok {
return false
}
_, ok = (*gatewayClassResources)[gc.Name]
_, ok := waitUntilGatewayClassResourcesAreReady(resources, gc.Name)
return ok
}, defaultWait, defaultTick)
}
Expand Down Expand Up @@ -201,11 +201,11 @@ func testGatewayClassWithParamRef(ctx context.Context, t *testing.T, provider *P
}, defaultWait, defaultTick)

require.Eventually(t, func() bool {
gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName)
if !ok {
return false
}
res, ok := (*gatewayClassResources)[gc.Name]
return resources.GatewayAPIResources.Len() != 0
}, defaultWait, defaultTick)

require.Eventually(t, func() bool {
res, ok := waitUntilGatewayClassResourcesAreReady(resources, gc.Name)
if !ok {
return false
}
Expand Down Expand Up @@ -341,10 +341,16 @@ func testGatewayScheduledStatus(ctx context.Context, t *testing.T, provider *Pro
require.NoError(t, cli.Delete(ctx, gw))
}()

require.Eventually(t, func() bool {
return resources.GatewayAPIResources.Len() != 0
}, defaultWait, defaultTick)

// Ensure the number of Gateways in the Gateway resource table is as expected.
require.Eventually(t, func() bool {
gatewayClassResources, _ := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName)
res := (*gatewayClassResources)[gc.Name]
res, ok := waitUntilGatewayClassResourcesAreReady(resources, gc.Name)
if !ok {
return false
}
return res != nil && len(res.Gateways) == 1
}, defaultWait, defaultTick)

Expand All @@ -361,6 +367,8 @@ func testGatewayScheduledStatus(ctx context.Context, t *testing.T, provider *Pro
}, defaultWait, defaultTick)

gatewayClassResources, _ := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName)
assert.NotNil(t, gatewayClassResources)

res := (*gatewayClassResources)[gc.Name]
// Only check if the spec is equal
// The watchable map will not store a resource
Expand Down Expand Up @@ -888,18 +896,22 @@ func testHTTPRoute(ctx context.Context, t *testing.T, provider *Provider, resour
}, defaultWait, defaultTick)

require.Eventually(t, func() bool {
gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName)
res := (*gatewayClassResources)[gc.Name]
res, ok := waitUntilGatewayClassResourcesAreReady(resources, gc.Name)
if !ok {
return false
}
return ok && len(res.HTTPRoutes) != 0
}, defaultWait, defaultTick)

gatewayClassResources, _ := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName)
assert.NotNil(t, gatewayClassResources)

res := (*gatewayClassResources)[gc.Name]
assert.Equal(t, &testCase.route, res.HTTPRoutes[0])

// Ensure the HTTPRoute Namespace is in the Namespace resource map.
require.Eventually(t, func() bool {
gatewayClassResources, _ := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName)
res, ok := (*gatewayClassResources)[testCase.route.Namespace]
res, ok := waitUntilGatewayClassResourcesAreReady(resources, gc.Name)
if !ok {
return false
}
Expand All @@ -918,8 +930,7 @@ func testHTTPRoute(ctx context.Context, t *testing.T, provider *Provider, resour
return true
}

gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName)
res, ok := (*gatewayClassResources)[gc.Name]
res, ok := waitUntilGatewayClassResourcesAreReady(resources, gc.Name)
if !ok {
return false
}
Expand Down Expand Up @@ -1036,21 +1047,22 @@ func testTLSRoute(ctx context.Context, t *testing.T, provider *Provider, resourc
}, defaultWait, defaultTick)

require.Eventually(t, func() bool {
gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName)
res, ok := (*gatewayClassResources)[gc.Name]
res, ok := waitUntilGatewayClassResourcesAreReady(resources, gc.Name)
if !ok {
return false
}
return ok && len(res.TLSRoutes) != 0
}, defaultWait, defaultTick)

gatewayClassResources, _ := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName)
assert.NotNil(t, gatewayClassResources)

res, _ := (*gatewayClassResources)[gc.Name]
assert.Equal(t, &testCase.route, res.TLSRoutes[0])

// Ensure the HTTPRoute Namespace is in the Namespace resource map.
require.Eventually(t, func() bool {
gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName)
res, ok := (*gatewayClassResources)[gc.Name]
res, ok := waitUntilGatewayClassResourcesAreReady(resources, gc.Name)
if !ok {
return false
}
Expand All @@ -1064,8 +1076,7 @@ func testTLSRoute(ctx context.Context, t *testing.T, provider *Provider, resourc

// Ensure the Service is in the resource map.
require.Eventually(t, func() bool {
gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName)
res, ok := (*gatewayClassResources)[gc.Name]
res, ok := waitUntilGatewayClassResourcesAreReady(resources, gc.Name)
if !ok {
return false
}
Expand Down Expand Up @@ -1192,10 +1203,13 @@ func testServiceCleanupForMultipleRoutes(ctx context.Context, t *testing.T, prov
require.NoError(t, cli.Create(ctx, &tlsRoute))
require.NoError(t, cli.Create(ctx, &httpRoute))

require.Eventually(t, func() bool {
return resources.GatewayAPIResources.Len() != 0
}, defaultWait, defaultTick)

// Check that the Service is present in the resource map
require.Eventually(t, func() bool {
gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName)
res, ok := (*gatewayClassResources)[gc.Name]
res, ok := waitUntilGatewayClassResourcesAreReady(resources, gc.Name)
if !ok {
return false
}
Expand All @@ -1210,8 +1224,7 @@ func testServiceCleanupForMultipleRoutes(ctx context.Context, t *testing.T, prov
// Delete the TLSRoute, and check if the Service is still present
require.NoError(t, cli.Delete(ctx, &tlsRoute))
require.Eventually(t, func() bool {
gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName)
res, ok := (*gatewayClassResources)[gc.Name]
res, ok := waitUntilGatewayClassResourcesAreReady(resources, gc.Name)
if !ok {
return false
}
Expand All @@ -1226,8 +1239,7 @@ func testServiceCleanupForMultipleRoutes(ctx context.Context, t *testing.T, prov
// Delete the HTTPRoute, and check if the Service is also removed
require.NoError(t, cli.Delete(ctx, &httpRoute))
require.Eventually(t, func() bool {
gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName)
res, ok := (*gatewayClassResources)[gc.Name]
res, ok := waitUntilGatewayClassResourcesAreReady(resources, gc.Name)
if !ok {
return false
}
Expand Down Expand Up @@ -1374,8 +1386,11 @@ func TestNamespaceSelectorProvider(t *testing.T) {
}()

require.Eventually(t, func() bool {
gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName)
res, ok := (*gatewayClassResources)[gc.Name]
return resources.GatewayAPIResources.Len() != 0
}, defaultWait, defaultTick)

require.Eventually(t, func() bool {
res, ok := waitUntilGatewayClassResourcesAreReady(resources, gc.Name)
if !ok {
return false
}
Expand Down Expand Up @@ -1527,8 +1542,7 @@ func TestNamespaceSelectorProvider(t *testing.T) {
}()

require.Eventually(t, func() bool {
gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName)
res, ok := (*gatewayClassResources)[gc.Name]
res, ok := waitUntilGatewayClassResourcesAreReady(resources, gc.Name)
if !ok {
return false
}
Expand All @@ -1537,48 +1551,57 @@ func TestNamespaceSelectorProvider(t *testing.T) {
}, defaultWait, defaultTick)

require.Eventually(t, func() bool {
gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName)
res, ok := (*gatewayClassResources)[gc.Name]
res, ok := waitUntilGatewayClassResourcesAreReady(resources, gc.Name)
if !ok {
return false
}
return res != nil && len(res.HTTPRoutes) == 1
}, defaultWait, defaultTick)

require.Eventually(t, func() bool {
gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName)
res, ok := (*gatewayClassResources)[gc.Name]
res, ok := waitUntilGatewayClassResourcesAreReady(resources, gc.Name)
if !ok {
return false
}
return res != nil && len(res.TCPRoutes) == 1
}, defaultWait, defaultTick)

require.Eventually(t, func() bool {
gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName)
res, ok := (*gatewayClassResources)[gc.Name]
res, ok := waitUntilGatewayClassResourcesAreReady(resources, gc.Name)
if !ok {
return false
}
return res != nil && len(res.TLSRoutes) == 1
}, defaultWait, defaultTick)

require.Eventually(t, func() bool {
gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName)
res, ok := (*gatewayClassResources)[gc.Name]
res, ok := waitUntilGatewayClassResourcesAreReady(resources, gc.Name)
if !ok {
return false
}
return res != nil && len(res.UDPRoutes) == 1
}, defaultWait, defaultTick)

require.Eventually(t, func() bool {
gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName)
res, ok := (*gatewayClassResources)[gc.Name]
res, ok := waitUntilGatewayClassResourcesAreReady(resources, gc.Name)
if !ok {
return false
}
return res != nil && len(res.GRPCRoutes) == 1
}, defaultWait, defaultTick)

}

func waitUntilGatewayClassResourcesAreReady(resources *message.ProviderResources, gatewayClassName string) (*gatewayapi.Resources, bool) {
gatewayClassResources, ok := resources.GatewayAPIResources.Load(egv1a1.GatewayControllerName)
if !ok {
return nil, false
}

res, ok := (*gatewayClassResources)[gatewayClassName]
if !ok {
return nil, false
}

return res, true
}
25 changes: 21 additions & 4 deletions internal/provider/kubernetes/routes_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package kubernetes
import (
"context"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand All @@ -30,6 +31,11 @@ import (
)

func TestProcessHTTPRoutes(t *testing.T) {
const (
defaultWait = time.Second * 10
defaultTick = time.Millisecond * 20
)

// The gatewayclass configured for the reconciler and referenced by test cases.
gcCtrlName := gwapiv1.GatewayController(egv1a1.GatewayControllerName)
gc := &gwapiv1.GatewayClass{
Expand Down Expand Up @@ -62,6 +68,8 @@ func TestProcessHTTPRoutes(t *testing.T) {

invalidDuration := gwapiv1.Duration("invalid duration")

httpRouteNS := "test"

testCases := []struct {
name string
routes []*gwapiv1.HTTPRoute
Expand All @@ -74,7 +82,7 @@ func TestProcessHTTPRoutes(t *testing.T) {
routes: []*gwapiv1.HTTPRoute{
{
ObjectMeta: metav1.ObjectMeta{
Namespace: "test",
Namespace: httpRouteNS,
Name: "test",
},
Spec: gwapiv1.HTTPRouteSpec{
Expand Down Expand Up @@ -118,7 +126,7 @@ func TestProcessHTTPRoutes(t *testing.T) {
routes: []*gwapiv1.HTTPRoute{
{
ObjectMeta: metav1.ObjectMeta{
Namespace: "test",
Namespace: httpRouteNS,
Name: "test",
},
Spec: gwapiv1.HTTPRouteSpec{
Expand Down Expand Up @@ -172,7 +180,7 @@ func TestProcessHTTPRoutes(t *testing.T) {
"kind": "Foo",
"metadata": map[string]interface{}{
"name": "test",
"namespace": "test",
"namespace": httpRouteNS,
},
},
},
Expand All @@ -191,7 +199,7 @@ func TestProcessHTTPRoutes(t *testing.T) {
routes: []*gwapiv1.HTTPRoute{
{
ObjectMeta: metav1.ObjectMeta{
Namespace: "test",
Namespace: httpRouteNS,
Name: "test",
},
Spec: gwapiv1.HTTPRouteSpec{
Expand Down Expand Up @@ -269,6 +277,15 @@ func TestProcessHTTPRoutes(t *testing.T) {
WithIndex(&gwapiv1.HTTPRoute{}, gatewayHTTPRouteIndex, gatewayHTTPRouteIndexFunc).
Build()

// Wait until all the httproutes have been initialized.
require.Eventually(t, func() bool {
httpRoutes := gwapiv1.HTTPRouteList{}
if err := r.client.List(ctx, &httpRoutes, client.InNamespace(httpRouteNS)); err != nil {
return false
}
return len(httpRoutes.Items) > 0
}, defaultWait, defaultTick)

// Process the test case httproutes.
resourceTree := gatewayapi.NewResources()
resourceMap := newResourceMapping()
Expand Down

0 comments on commit 82d9b6f

Please sign in to comment.