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.

Sign up for GitHub

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

Jump to bottom

Acceptance tests: increase api-gateway retries #2716

Merged
merged 6 commits into from
Aug 3, 2023
Merged

Acceptance tests: increase api-gateway retries #2716

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4d65805
wait for pods to start before doing things
curtbushko Aug 2, 2023
788f30e
try and remove flakes
curtbushko Aug 2, 2023
d6efd47
double retries
curtbushko Aug 2, 2023
a200e39
Update acceptance/tests/partitions/partitions_gateway_test.go
curtbushko Aug 3, 2023
3bc6123
remove comment based on review
curtbushko Aug 3, 2023
05f20c8
fix lint
curtbushko Aug 3, 2023
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
2 changes: 1 addition & 1 deletion acceptance/tests/api-gateway/api_gateway_external_servers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func TestAPIGateway_ExternalServers(t *testing.T) {
// leader election so we may need to wait a long time for
// the reconcile loop to run (hence a ~1m timeout here).
var gatewayAddress string
retryCheck(t, 30, func(r *retry.R) {
retryCheck(t, 60, func(r *retry.R) {
var gateway gwv1beta1.Gateway
err := k8sClient.Get(context.Background(), types.NamespacedName{Name: "gateway", Namespace: "default"}, &gateway)
require.NoError(r, err)
Expand Down
14 changes: 7 additions & 7 deletions acceptance/tests/api-gateway/api_gateway_lifecycle_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ func TestAPIGateway_Lifecycle(t *testing.T) {

// check that the route is unbound and all Consul objects and Kubernetes statuses are cleaned up
logger.Log(t, "checking that http route one is not bound to gateway two")
retryCheck(t, 30, func(r *retry.R) {
retryCheck(t, 60, func(r *retry.R) {
var route gwv1beta1.HTTPRoute
err := k8sClient.Get(context.Background(), types.NamespacedName{Name: routeOneName, Namespace: defaultNamespace}, &route)
require.NoError(r, err)
Expand Down Expand Up @@ -246,7 +246,7 @@ func TestAPIGateway_Lifecycle(t *testing.T) {

// check that the Kubernetes gateway is cleaned up
logger.Log(t, "checking that gateway one is cleaned up in Kubernetes")
retryCheck(t, 30, func(r *retry.R) {
retryCheck(t, 60, func(r *retry.R) {
var route gwv1beta1.Gateway
err := k8sClient.Get(context.Background(), types.NamespacedName{Name: controlledGatewayOneName, Namespace: defaultNamespace}, &route)
require.NoError(r, err)
Expand Down Expand Up @@ -299,7 +299,7 @@ func checkConsulNotExists(t *testing.T, client *api.Client, kind, name string, n
opts.Namespace = namespace[0]
}

retryCheck(t, 30, func(r *retry.R) {
retryCheck(t, 60, func(r *retry.R) {
_, _, err := client.ConfigEntries().Get(kind, name, opts)
require.Error(r, err)
require.EqualError(r, err, fmt.Sprintf("Unexpected response code: 404 (Config entry not found for %q / %q)", kind, name))
Expand All @@ -309,7 +309,7 @@ func checkConsulNotExists(t *testing.T, client *api.Client, kind, name string, n
func checkConsulExists(t *testing.T, client *api.Client, kind, name string) {
t.Helper()

retryCheck(t, 30, func(r *retry.R) {
retryCheck(t, 60, func(r *retry.R) {
_, _, err := client.ConfigEntries().Get(kind, name, nil)
require.NoError(r, err)
})
Expand All @@ -318,7 +318,7 @@ func checkConsulExists(t *testing.T, client *api.Client, kind, name string) {
func checkConsulRouteParent(t *testing.T, client *api.Client, name, parent string) {
t.Helper()

retryCheck(t, 30, func(r *retry.R) {
retryCheck(t, 60, func(r *retry.R) {
entry, _, err := client.ConfigEntries().Get(api.HTTPRoute, name, nil)
require.NoError(r, err)
route := entry.(*api.HTTPRouteConfigEntry)
Expand All @@ -331,7 +331,7 @@ func checkConsulRouteParent(t *testing.T, client *api.Client, name, parent strin
func checkEmptyRoute(t *testing.T, client client.Client, name, namespace string) {
t.Helper()

retryCheck(t, 30, func(r *retry.R) {
retryCheck(t, 60, func(r *retry.R) {
var route gwv1beta1.HTTPRoute
err := client.Get(context.Background(), types.NamespacedName{Name: name, Namespace: namespace}, &route)
require.NoError(r, err)
Expand All @@ -344,7 +344,7 @@ func checkEmptyRoute(t *testing.T, client client.Client, name, namespace string)
func checkRouteBound(t *testing.T, client client.Client, name, namespace, parent string) {
t.Helper()

retryCheck(t, 30, func(r *retry.R) {
retryCheck(t, 60, func(r *retry.R) {
var route gwv1beta1.HTTPRoute
err := client.Get(context.Background(), types.NamespacedName{Name: name, Namespace: namespace}, &route)
require.NoError(r, err)
Expand Down
6 changes: 3 additions & 3 deletions acceptance/tests/api-gateway/api_gateway_tenancy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func TestAPIGateway_Tenancy(t *testing.T) {
k8sClient := ctx.ControllerRuntimeClient(t)
consulClient, _ := consulCluster.SetupConsulClient(t, c.secure)

retryCheck(t, 60, func(r *retry.R) {
retryCheck(t, 120, func(r *retry.R) {
var gateway gwv1beta1.Gateway
err := k8sClient.Get(context.Background(), types.NamespacedName{Name: "gateway", Namespace: gatewayNamespace}, &gateway)
require.NoError(r, err)
Expand All @@ -153,7 +153,7 @@ func TestAPIGateway_Tenancy(t *testing.T) {
checkConsulNotExists(t, consulClient, api.APIGateway, "gateway", namespaceForConsul(c.namespaceMirroring, gatewayNamespace))

// route failure
retryCheck(t, 30, func(r *retry.R) {
retryCheck(t, 60, func(r *retry.R) {
var httproute gwv1beta1.HTTPRoute
err := k8sClient.Get(context.Background(), types.NamespacedName{Name: "route", Namespace: routeNamespace}, &httproute)
require.NoError(r, err)
Expand All @@ -175,7 +175,7 @@ func TestAPIGateway_Tenancy(t *testing.T) {
createReferenceGrant(t, k8sClient, "route-service", routeNamespace, serviceNamespace)

// gateway updated with references allowed
retryCheck(t, 30, func(r *retry.R) {
retryCheck(t, 60, func(r *retry.R) {
var gateway gwv1beta1.Gateway
err := k8sClient.Get(context.Background(), types.NamespacedName{Name: "gateway", Namespace: gatewayNamespace}, &gateway)
require.NoError(r, err)
Expand Down
35 changes: 20 additions & 15 deletions acceptance/tests/api-gateway/api_gateway_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@ func TestAPIGateway_Basic(t *testing.T) {
logger.Log(t, "creating target http server")
k8s.DeployKustomize(t, ctx.KubectlOptions(t), cfg.NoCleanupOnFailure, cfg.NoCleanup, cfg.DebugDirectory, "../fixtures/cases/static-server-inject")

// We use the static-client pod so that we can make calls to the api gateway
// via kubectl exec without needing a route into the cluster from the test machine.
logger.Log(t, "creating static-client pod")
k8s.DeployKustomize(t, ctx.KubectlOptions(t), cfg.NoCleanupOnFailure, cfg.NoCleanup, cfg.DebugDirectory, "../fixtures/bases/static-client")

logger.Log(t, "patching route to target http server")
k8s.RunKubectl(t, ctx.KubectlOptions(t), "patch", "httproute", "http-route", "-p", `{"spec":{"rules":[{"backendRefs":[{"name":"static-server","port":80}]}]}}`, "--type=merge")

Expand All @@ -115,11 +120,6 @@ func TestAPIGateway_Basic(t *testing.T) {
k8s.RunKubectlAndGetOutputE(t, ctx.KubectlOptions(t), "delete", "-f", "../fixtures/cases/api-gateways/tcproute/route.yaml")
})

// We use the static-client pod so that we can make calls to the api gateway
// via kubectl exec without needing a route into the cluster from the test machine.
logger.Log(t, "creating static-client pod")
k8s.DeployKustomize(t, ctx.KubectlOptions(t), cfg.NoCleanupOnFailure, cfg.NoCleanup, cfg.DebugDirectory, "../fixtures/bases/static-client")

Comment on lines -118 to -122
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

~ Why the relocation above for this block?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was shuffling things around to add retries & checks to the deployments but it didn't work out.

Copy link
Member

@zalimeni zalimeni Aug 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, like we don't need to move this after all? (I'm good either way, was just curious what motivated it)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really. It'll give it a few ms to deploy and get ready but it shouldn't make a difference.

// Grab a kubernetes client so that we can verify binding
// behavior prior to issuing requests through the gateway.
k8sClient := ctx.ControllerRuntimeClient(t)
Expand All @@ -128,7 +128,7 @@ func TestAPIGateway_Basic(t *testing.T) {
// leader election so we may need to wait a long time for
// the reconcile loop to run (hence the 1m timeout here).
var gatewayAddress string
counter := &retry.Counter{Count: 60, Wait: 2 * time.Second}
counter := &retry.Counter{Count: 120, Wait: 2 * time.Second}
retry.RunWith(counter, t, func(r *retry.R) {
var gateway gwv1beta1.Gateway
err := k8sClient.Get(context.Background(), types.NamespacedName{Name: "gateway", Namespace: "default"}, &gateway)
Expand Down Expand Up @@ -209,17 +209,22 @@ func TestAPIGateway_Basic(t *testing.T) {
checkStatusCondition(t, tcpRoute.Status.Parents[0].Conditions, trueCondition("ConsulAccepted", "Accepted"))

// check that the Consul entries were created
entry, _, err := consulClient.ConfigEntries().Get(api.APIGateway, "gateway", nil)
require.NoError(t, err)
gateway := entry.(*api.APIGatewayConfigEntry)
var gateway *api.APIGatewayConfigEntry
var httpRoute *api.HTTPRouteConfigEntry
var route *api.TCPRouteConfigEntry
retry.RunWith(counter, t, func(r *retry.R) {
entry, _, err := consulClient.ConfigEntries().Get(api.APIGateway, "gateway", nil)
require.NoError(r, err)
gateway = entry.(*api.APIGatewayConfigEntry)

entry, _, err = consulClient.ConfigEntries().Get(api.HTTPRoute, "http-route", nil)
require.NoError(t, err)
httpRoute := entry.(*api.HTTPRouteConfigEntry)
entry, _, err = consulClient.ConfigEntries().Get(api.HTTPRoute, "http-route", nil)
require.NoError(r, err)
httpRoute = entry.(*api.HTTPRouteConfigEntry)

entry, _, err = consulClient.ConfigEntries().Get(api.TCPRoute, "tcp-route", nil)
require.NoError(t, err)
route := entry.(*api.TCPRouteConfigEntry)
entry, _, err = consulClient.ConfigEntries().Get(api.TCPRoute, "tcp-route", nil)
require.NoError(r, err)
route = entry.(*api.TCPRouteConfigEntry)
})

// now check the gateway status conditions
checkConsulStatusCondition(t, gateway.Status.Conditions, trueConsulCondition("Accepted", "Accepted"))
Expand Down
21 changes: 21 additions & 0 deletions acceptance/tests/partitions/partitions_gateway_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/sdk/testutil/retry"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"

gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
Expand Down Expand Up @@ -255,6 +256,16 @@ func TestPartitions_Gateway(t *testing.T) {
logger.Log(t, "creating target server in secondary partition cluster")
k8s.DeployKustomize(t, secondaryPartitionClusterStaticServerOpts, cfg.NoCleanupOnFailure, cfg.NoCleanup, cfg.DebugDirectory, "../fixtures/cases/static-server-inject")

// Check that static-server injected 2 containers.
for _, labelSelector := range []string{"app=static-server"} {
podList, err := secondaryPartitionClusterContext.KubernetesClient(t).CoreV1().Pods(metav1.NamespaceAll).List(context.Background(), metav1.ListOptions{
LabelSelector: labelSelector,
})
require.NoError(t, err)
require.Len(t, podList.Items, 1)
require.Len(t, podList.Items[0].Spec.Containers, 2)
}

logger.Log(t, "patching route to target server")
k8s.RunKubectl(t, secondaryPartitionClusterStaticServerOpts, "patch", "httproute", "http-route", "-p", `{"spec":{"rules":[{"backendRefs":[{"group":"consul.hashicorp.com","kind":"MeshService","name":"mesh-service","port":80}]}]}}`, "--type=merge")

Expand Down Expand Up @@ -293,6 +304,16 @@ func TestPartitions_Gateway(t *testing.T) {
logger.Log(t, "creating target server in default partition cluster")
k8s.DeployKustomize(t, defaultPartitionClusterStaticServerOpts, cfg.NoCleanupOnFailure, cfg.NoCleanup, cfg.DebugDirectory, "../fixtures/cases/static-server-inject")

// Check that static-server injected 2 containers.
for _, labelSelector := range []string{"app=static-server"} {
podList, err := defaultPartitionClusterContext.KubernetesClient(t).CoreV1().Pods(metav1.NamespaceAll).List(context.Background(), metav1.ListOptions{
LabelSelector: labelSelector,
})
require.NoError(t, err)
require.Len(t, podList.Items, 1)
require.Len(t, podList.Items[0].Spec.Containers, 2)
}

logger.Log(t, "creating exported services")
k8s.KubectlApplyK(t, defaultPartitionClusterContext.KubectlOptions(t), "../fixtures/cases/crd-partitions/default-partition-ns1")
helpers.Cleanup(t, cfg.NoCleanupOnFailure, cfg.NoCleanup, func() {
Expand Down