refactor: parrallel tests (Kuadrant#689)

* make: concurrent tests flags

* refactor: concurrent bare k8s tests

* refactor: concurrent gateway api k8s tests

* refactor: concurrent istio tests

* refactor: test packages for base concurrent integration tests

* refactor: do not wait kuadrant to be ready for integration tests

* fix: sort policy by kind, creation, condition for target status

* refactor: separate out serial tests that can affect other tests

* harden: RLP integration test updates & sort by PolicyByTargetRefKindAndAcceptedStatus tests

* refactor: rename ratelimit test package to ratelimitpolicy

* fix: wait for dnsrecords to finish deleting before deleting managed zone + timeouts

Since Kuadrant/dns-operator#156 was merged, dnsrecords with finizalers are left behind as ManagedZone was deleted causing the test namespaces to never be fully deleted.

Additionally use SpecContext and add timeouts for context so problematic tests terminate earlier

controllers/authpolicy_authconfig.go

@@ -56,7 +56,7 @@ func (r *AuthPolicyReconciler) desiredAuthConfig(ctx context.Context, ap *api.Au
 			APIVersion: authorinoapi.GroupVersion.String(),
 		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      authConfigName(client.ObjectKeyFromObject(ap)),
+			Name:      AuthConfigName(client.ObjectKeyFromObject(ap)),
 			Namespace: ap.Namespace,
 		},
 		Spec: authorinoapi.AuthConfigSpec{},
@@ -235,8 +235,8 @@ func getAffectedPolicies(t *kuadrantgatewayapi.Topology, ap *api.AuthPolicy) []k
 	return affectedPolicies
 }
 
-// authConfigName returns the name of Authorino AuthConfig CR.
-func authConfigName(apKey client.ObjectKey) string {
+// AuthConfigName returns the name of Authorino AuthConfig CR.
+func AuthConfigName(apKey client.ObjectKey) string {
 	return fmt.Sprintf("ap-%s-%s", apKey.Namespace, apKey.Name)
 }
 

controllers/authpolicy_status.go

@@ -120,7 +120,7 @@ func (r *AuthPolicyReconciler) isAuthConfigReady(ctx context.Context, policy *ap
 	apKey := client.ObjectKeyFromObject(policy)
 	authConfigKey := client.ObjectKey{
 		Namespace: policy.Namespace,
-		Name:      authConfigName(apKey),
+		Name:      AuthConfigName(apKey),
 	}
 	authConfig := &authorinoapi.AuthConfig{}
 	err := r.GetResource(ctx, authConfigKey, authConfig)

controllers/target_status_controller.go

@@ -21,6 +21,7 @@ import (
 	"errors"
 	"fmt"
 	"reflect"
+	"sort"
 
 	"github.com/go-logr/logr"
 	"github.com/google/uuid"
@@ -112,13 +113,15 @@ func (r *TargetStatusReconciler) reconcileResourcesForPolicyKind(parentCtx conte
 	}
 	policies := topology.PoliciesFromGateway(gw)
 
+	sort.Sort(kuadrantgatewayapi.PolicyByTargetRefKindAndAcceptedStatus(policies))
+
 	var errs error
 
 	// if no policies of a kind affecting the gateway → remove condition from the gateway and routes
 	gatewayPolicyExists := len(policies) > 0 && utils.Index(policies, func(p kuadrantgatewayapi.Policy) bool { return kuadrantgatewayapi.IsTargetRefGateway(p.GetTargetRef()) }) >= 0
 	if !gatewayPolicyExists {
 		// remove the condition from the gateway
-		conditionType := policyAffectedConditionType(policyKind)
+		conditionType := PolicyAffectedConditionType(policyKind)
 		if c := meta.FindStatusCondition(gw.Status.Conditions, conditionType); c == nil {
 			logger.V(1).Info("condition already absent, skipping", "condition", conditionType)
 		} else {
@@ -270,7 +273,7 @@ func (r *TargetStatusReconciler) addRouteCondition(ctx context.Context, route *g
 	logger, _ := logr.FromContext(ctx)
 	logger = logger.WithValues("route", client.ObjectKeyFromObject(route), "condition", condition.Type, "status", condition.Status)
 
-	i := utils.Index(route.Status.RouteStatus.Parents, findRouteParentStatusFunc(route, gatewayKey, controllerName))
+	i := utils.Index(route.Status.RouteStatus.Parents, FindRouteParentStatusFunc(route, gatewayKey, controllerName))
 	if i < 0 {
 		logger.V(1).Info("cannot find parent status, creating new one")
 		route.Status.RouteStatus.Parents = append(route.Status.RouteStatus.Parents, gatewayapiv1.RouteParentStatus{
@@ -282,7 +285,7 @@ func (r *TargetStatusReconciler) addRouteCondition(ctx context.Context, route *g
 			},
 			Conditions: []metav1.Condition{},
 		})
-		i = utils.Index(route.Status.RouteStatus.Parents, findRouteParentStatusFunc(route, gatewayKey, controllerName))
+		i = utils.Index(route.Status.RouteStatus.Parents, FindRouteParentStatusFunc(route, gatewayKey, controllerName))
 	}
 
 	if c := meta.FindStatusCondition(route.Status.RouteStatus.Parents[i].Conditions, condition.Type); c != nil && c.Status == condition.Status && c.Reason == condition.Reason && c.Message == condition.Message && c.ObservedGeneration == route.GetGeneration() {
@@ -301,7 +304,7 @@ func (r *TargetStatusReconciler) removeRouteCondition(ctx context.Context, route
 	logger, _ := logr.FromContext(ctx)
 	logger = logger.WithValues("route", client.ObjectKeyFromObject(route), "condition", condition.Type, "status", condition.Status)
 
-	i := utils.Index(route.Status.RouteStatus.Parents, findRouteParentStatusFunc(route, gatewayKey, controllerName))
+	i := utils.Index(route.Status.RouteStatus.Parents, FindRouteParentStatusFunc(route, gatewayKey, controllerName))
 	if i < 0 {
 		logger.V(1).Info("cannot find parent status, skipping")
 		return nil
@@ -320,7 +323,7 @@ func (r *TargetStatusReconciler) removeRouteCondition(ctx context.Context, route
 	return r.Client().Status().Update(ctx, route)
 }
 
-func findRouteParentStatusFunc(route *gatewayapiv1.HTTPRoute, gatewayKey client.ObjectKey, controllerName gatewayapiv1.GatewayController) func(gatewayapiv1.RouteParentStatus) bool {
+func FindRouteParentStatusFunc(route *gatewayapiv1.HTTPRoute, gatewayKey client.ObjectKey, controllerName gatewayapiv1.GatewayController) func(gatewayapiv1.RouteParentStatus) bool {
 	return func(p gatewayapiv1.RouteParentStatus) bool {
 		return *p.ParentRef.Kind == ("Gateway") &&
 			p.ControllerName == controllerName &&
@@ -398,7 +401,7 @@ func buildPolicyAffectedCondition(policy kuadrantgatewayapi.Policy) metav1.Condi
 	policyKind := policy.GetObjectKind().GroupVersionKind().Kind
 
 	condition := metav1.Condition{
-		Type:    policyAffectedConditionType(policyKind),
+		Type:    PolicyAffectedConditionType(policyKind),
 		Status:  metav1.ConditionTrue,
 		Reason:  string(gatewayapiv1alpha2.PolicyReasonAccepted),
 		Message: fmt.Sprintf("Object affected by %s %s", policyKind, client.ObjectKeyFromObject(policy)),
@@ -416,7 +419,7 @@ func buildPolicyAffectedCondition(policy kuadrantgatewayapi.Policy) metav1.Condi
 	return condition
 }
 
-func policyAffectedConditionType(policyKind string) string {
+func PolicyAffectedConditionType(policyKind string) string {
 	return fmt.Sprintf(PolicyAffectedConditionPattern, policyKind)
 }
 

controllers/test_common.go

@@ -23,6 +23,8 @@ limitations under the License.
 package controllers
 
 import (
+	"encoding/json"
+
 	certmanv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -31,6 +33,7 @@ import (
 	istiosecurityv1beta1 "istio.io/client-go/pkg/apis/security/v1beta1"
 	istioapis "istio.io/istio/operator/pkg/apis"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
 	istiov1alpha1 "maistra.io/istio-operator/api/v1alpha1"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -53,51 +56,6 @@ import (
 )
 
 func SetupKuadrantOperatorForTest(s *runtime.Scheme, cfg *rest.Config) {
-	err := kuadrantdnsv1alpha1.AddToScheme(s)
-	Expect(err).NotTo(HaveOccurred())
-
-	err = kuadrantv1alpha1.AddToScheme(s)
-	Expect(err).NotTo(HaveOccurred())
-
-	err = kuadrantv1beta1.AddToScheme(s)
-	Expect(err).NotTo(HaveOccurred())
-
-	err = kuadrantv1beta2.AddToScheme(s)
-	Expect(err).NotTo(HaveOccurred())
-
-	err = gatewayapiv1.Install(s)
-	Expect(err).NotTo(HaveOccurred())
-
-	err = authorinoopapi.AddToScheme(s)
-	Expect(err).NotTo(HaveOccurred())
-
-	err = authorinoapi.AddToScheme(s)
-	Expect(err).NotTo(HaveOccurred())
-
-	err = istioapis.AddToScheme(s)
-	Expect(err).NotTo(HaveOccurred())
-
-	err = istiov1alpha1.AddToScheme(s)
-	Expect(err).NotTo(HaveOccurred())
-
-	err = istiosecurityv1beta1.AddToScheme(s)
-	Expect(err).NotTo(HaveOccurred())
-
-	err = limitadorv1alpha1.AddToScheme(s)
-	Expect(err).NotTo(HaveOccurred())
-
-	err = istioclientnetworkingv1alpha3.AddToScheme(s)
-	Expect(err).NotTo(HaveOccurred())
-
-	err = istioclientgoextensionv1alpha1.AddToScheme(s)
-	Expect(err).NotTo(HaveOccurred())
-
-	err = maistraapis.AddToScheme(s)
-	Expect(err).NotTo(HaveOccurred())
-
-	err = certmanv1.AddToScheme(s)
-	Expect(err).NotTo(HaveOccurred())
-
 	mgr, err := ctrl.NewManager(cfg, ctrl.Options{
 		Scheme:                 s,
 		HealthProbeBindAddress: "0",
@@ -241,3 +199,74 @@ func SetupKuadrantOperatorForTest(s *runtime.Scheme, cfg *rest.Config) {
 		Expect(err).ToNot(HaveOccurred())
 	}()
 }
+
+// SharedConfig contains minimum cluster connection config that can be safely marshalled as rest.Config is unsafe to marshall
+type SharedConfig struct {
+	Host                  string          `json:"host"`
+	TLSClientConfig       TLSClientConfig `json:"tlsClientConfig"`
+	KuadrantNS            string          `json:"kuadrantNS"`
+	DNSProviderSecretName string          `json:"dnsProviderSecretName"`
+}
+
+type TLSClientConfig struct {
+	Insecure bool    `json:"insecure"`
+	CertData []uint8 `json:"certData,omitempty"`
+	KeyData  []uint8 `json:"keyData,omitempty"`
+	CAData   []uint8 `json:"caData,omitempty"`
+}
+
+func BootstrapScheme() *runtime.Scheme {
+	s := runtime.NewScheme()
+
+	sb := runtime.NewSchemeBuilder(
+		scheme.AddToScheme,
+		kuadrantdnsv1alpha1.AddToScheme,
+		kuadrantv1alpha1.AddToScheme,
+		kuadrantv1beta1.AddToScheme,
+		kuadrantv1beta2.AddToScheme,
+		gatewayapiv1.Install,
+		authorinoopapi.AddToScheme,
+		authorinoapi.AddToScheme,
+		istioapis.AddToScheme,
+		istiov1alpha1.AddToScheme,
+		istiosecurityv1beta1.AddToScheme,
+		limitadorv1alpha1.AddToScheme,
+		istioclientnetworkingv1alpha3.AddToScheme,
+		istioclientgoextensionv1alpha1.AddToScheme,
+		certmanv1.AddToScheme,
+		maistraapis.AddToScheme,
+	)
+
+	err := sb.AddToScheme(s)
+	Expect(err).NotTo(HaveOccurred())
+
+	return s
+}
+
+// MarshalConfig marshals the config to a shared configuration struct
+func MarshalConfig(cfg *rest.Config, opts ...func(config *SharedConfig)) []byte {
+	sharedCfg := &SharedConfig{
+		Host: cfg.Host,
+		TLSClientConfig: TLSClientConfig{
+			Insecure: cfg.TLSClientConfig.Insecure,
+			CertData: cfg.TLSClientConfig.CertData,
+			KeyData:  cfg.TLSClientConfig.KeyData,
+			CAData:   cfg.TLSClientConfig.CAData,
+		},
+	}
+
+	for _, opt := range opts {
+		opt(sharedCfg)
+	}
+
+	data, err := json.Marshal(sharedCfg)
+	Expect(err).NotTo(HaveOccurred())
+
+	return data
+}
+
+func WithKuadrantInstallNS(ns string) func(config *SharedConfig) {
+	return func(cfg *SharedConfig) {
+		cfg.KuadrantNS = ns
+	}
+}

make/integration-tests.mk

@@ -1,5 +1,7 @@
 INTEGRATION_COVER_PKGS = ./pkg/...,./controllers/...,./api/...
-INTEGRATION_TESTS_EXTRA_ARGS =
+INTEGRATION_TESTS_EXTRA_ARGS ?=
+INTEGRATION_TEST_NUM_CORES ?= 4
+INTEGRATION_TEST_NUM_PROCESSES ?= 10
 
 ##@ Integration tests
 
@@ -12,6 +14,13 @@ test-bare-k8s-integration: clean-cov generate fmt vet ginkgo ## Requires only ba
 		--output-dir $(PROJECT_PATH)/coverage/bare-k8s-integration \
 		--coverprofile cover.out \
 		-tags integration \
+		--compilers=$(INTEGRATION_TEST_NUM_CORES) \
+		--procs=$(INTEGRATION_TEST_NUM_PROCESSES) \
+		--randomize-all \
+		--randomize-suites \
+		--fail-on-pending \
+		--keep-going \
+		--trace \
 		$(INTEGRATION_TESTS_EXTRA_ARGS) ./tests/bare_k8s/...
 
 .PHONY: test-gatewayapi-env-integration
@@ -23,6 +32,13 @@ test-gatewayapi-env-integration: clean-cov generate fmt vet ginkgo ## Requires k
 		--output-dir $(PROJECT_PATH)/coverage/gatewayapi-integration \
 		--coverprofile cover.out \
 		-tags integration \
+		--compilers=$(INTEGRATION_TEST_NUM_CORES) \
+		--procs=$(INTEGRATION_TEST_NUM_PROCESSES) \
+		--randomize-all \
+		--randomize-suites \
+		--fail-on-pending \
+		--keep-going \
+		--trace \
 		$(INTEGRATION_TESTS_EXTRA_ARGS) ./tests/gatewayapi/...
 
 .PHONY: test-istio-env-integration
@@ -34,6 +50,13 @@ test-istio-env-integration: clean-cov generate fmt vet ginkgo ## Requires kubern
 		--output-dir $(PROJECT_PATH)/coverage/istio-integration \
 		--coverprofile cover.out \
 		-tags integration \
+		--compilers=$(INTEGRATION_TEST_NUM_CORES) \
+		--procs=$(INTEGRATION_TEST_NUM_PROCESSES) \
+		--randomize-all \
+		--randomize-suites \
+		--fail-on-pending \
+		--keep-going \
+		--trace \
 		$(INTEGRATION_TESTS_EXTRA_ARGS) tests/istio/...
 
 .PHONY: test-integration
@@ -45,4 +68,11 @@ test-integration: clean-cov generate fmt vet ginkgo ## Requires kubernetes clust
 		--output-dir $(PROJECT_PATH)/coverage/integration \
 		--coverprofile cover.out \
 		-tags integration \
-		$(INTEGRATION_TESTS_EXTRA_ARGS) ./controllers/...
+		--compilers=$(INTEGRATION_TEST_NUM_CORES) \
+		--procs=$(INTEGRATION_TEST_NUM_PROCESSES) \
+		--randomize-all \
+		--randomize-suites \
+		--fail-on-pending \
+		--keep-going \
+		--trace \
+		$(INTEGRATION_TESTS_EXTRA_ARGS) tests/common/...

pkg/library/gatewayapi/types.go

@@ -1,6 +1,7 @@
 package gatewayapi
 
 import (
+	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -68,3 +69,39 @@ func (a PolicyByTargetRefKindAndCreationTimeStamp) Less(i, j int) bool {
 	//  The policy appearing first in alphabetical order by "{namespace}/{name}".
 	return client.ObjectKeyFromObject(a[i]).String() < client.ObjectKeyFromObject(a[j]).String()
 }
+
+type PolicyByTargetRefKindAndAcceptedStatus []Policy
+
+func (a PolicyByTargetRefKindAndAcceptedStatus) Len() int      { return len(a) }
+func (a PolicyByTargetRefKindAndAcceptedStatus) Swap(i, j int) { a[i], a[j] = a[j], a[i] }
+func (a PolicyByTargetRefKindAndAcceptedStatus) Less(i, j int) bool {
+	targetRef1 := a[i].GetTargetRef()
+	targetRef2 := a[j].GetTargetRef()
+
+	// Compare kind first
+	if targetRef1.Kind != targetRef2.Kind {
+		if targetRef1.Kind == "Gateway" {
+			return true
+		} else if targetRef2.Kind == "HTTPRoute" {
+			return false
+		}
+		return targetRef1.Kind < targetRef2.Kind
+	}
+
+	// Compare by accepted condition
+	p1Status := meta.IsStatusConditionTrue(a[i].GetStatus().GetConditions(), string(gatewayapiv1alpha2.PolicyConditionAccepted))
+	p2Status := meta.IsStatusConditionTrue(a[j].GetStatus().GetConditions(), string(gatewayapiv1alpha2.PolicyConditionAccepted))
+	if p1Status != p2Status {
+		return p1Status
+	}
+
+	// Compare by creation timestamp
+	p1Time := ptr.To(a[i].GetCreationTimestamp())
+	p2Time := ptr.To(a[j].GetCreationTimestamp())
+	if !p1Time.Equal(p2Time) {
+		return p1Time.Before(p2Time)
+	}
+
+	//  The policy appearing first in alphabetical order by "{namespace}/{name}".
+	return client.ObjectKeyFromObject(a[i]).String() < client.ObjectKeyFromObject(a[j]).String()
+}