Test framework improvement, doing proper clean up in BeforeSuite(), A…

…fterEach(), AfterSuite() (#225)

* Add proper expectTobeClean:

BeforeEach(func() { framework.ExpectToBeClean(ctx) })
AfterEach(func() { framework.ExpectToBeClean(ctx) })

* Add more `FOCUS` environment variable description, do `ExpectToBeClean` in AfterEach

* remove CleanServiceNetworkVpcAssociationForCurrentK8sClusterVpc

* Change back DiscoveryLabel to a non-random name

* Change developer.md

---------

Co-authored-by: Zijun Wang <[email protected]>

Makefile

@@ -47,6 +47,7 @@ help: ## Display this help.
 run: ## Run in development mode
 	go run main.go
 
+
 .PHONY: presubmit
 presubmit: vet test ## Run all commands before submitting code
 

docs/developer.md

@@ -28,6 +28,9 @@ GATEWAY_API_CONTROLLER_LOGLEVEL=debug make run
 LATTICE_ENDPOINT=https://vpc-lattice.us-west-2.amazonaws.com/ make run
 ```
 
+To easier load environment variables, if you hope to run the controller by GoLand IDE locally, you could run the `scripts/load_env_variables.sh`
+And use "EnvFile" GoLand plugin to read the env variables from the generated `.env` file.
+
 ### End-to-End Testing
 
 Run the following command to run the end-to-end tests against the Kubernetes cluster pointed to by `kubectl config current-context`:
@@ -36,6 +39,27 @@ You should set up the correct `REGION` env variable
 export REGION=us-west-2
 make e2etest
 ```
+Pass `FOCUS` environment variable to run some specific test cases based on filter condition.
+You could assign the string in the Describe("xxxxxx") or It("xxxxxx") to the FOCUS environment variable to run the specific test cases.
+```go
+var _ = Describe("HTTPRoute path matches", func() {
+	It("HTTPRoute should support multiple path matches", func() {
+        // test case body
+    })
+```
+
+```
+export FOCUS="HTTPRoute should support multiple path matches"
+export REGION=us-west-2
+make e2etest
+```
+
+For example, to run the test case "HTTPRoute should support multiple path matches", you could run the following command:
+
+For more detail on filter condition for ginkgo
+https://onsi.github.io/ginkgo/#focused-specs
+https://onsi.github.io/ginkgo/#description-based-filtering
+
 Notice: the prerequisites for running the end-to-end tests success are:
 - Current eks cluster don't have any k8s resource
 - The vpc used by current eks cluster don't have any vpc service network association

scripts/load_env_variables.sh

@@ -0,0 +1,46 @@
+#!/bin/sh
+
+# When Running the k8s Controller by GoLand IDE locally, it is required to set the environment variables, this script will generate a file called `envFile`
+# that contains all needed environment variables, this `envFile` file can be use by GoLand "EnvFile" plugin
+# When config the "green arrow"(how we `run` or `debug` this project) in  GoLand, we could set a "before launch" task to run this script `load_env_variables.sh`, and set the "EnvFile" plugin to load the `envFile` file
+# "EnvFile" is a plugin, you can install it from GoLand plugin marketplace
+
+echo "Setting environment variables"
+
+> envFile
+
+# Set KUBEBUILDER_ASSETS if not set
+if [ -z "$KUBEBUILDER_ASSETS" ]; then
+  KUBEBUILDER_ASSETS=${HOME}/.kubebuilder/bin
+fi
+echo "KUBEBUILDER_ASSETS=$KUBEBUILDER_ASSETS" >> envFile
+
+
+# Set CLUSTER_NAME if not set
+if [ -z "$CLUSTER_NAME" ]; then
+  CLUSTER_NAME=$(kubectl config view --minify -o jsonpath='{.clusters[].name}' | rev | cut -d"/" -f1 | rev | cut -d"." -f1)
+fi
+echo "CLUSTER_NAME=$CLUSTER_NAME" >> envFile
+
+
+# Set CLUSTER_VPC_ID if not set
+if [ -z "$CLUSTER_VPC_ID" ]; then
+  CLUSTER_VPC_ID=$(aws eks describe-cluster --name ${CLUSTER_NAME} | jq -r ".cluster.resourcesVpcConfig.vpcId")
+fi
+echo "CLUSTER_VPC_ID=$CLUSTER_VPC_ID" >> envFile
+
+# Set AWS_ACCOUNT_ID if not set
+if [ -z "$AWS_ACCOUNT_ID" ]; then
+  AWS_ACCOUNT_ID=$(aws sts get-caller-identity --query Account --output text)
+fi
+echo "AWS_ACCOUNT_ID=$AWS_ACCOUNT_ID" >> envFile
+
+if [ -z "$REGION" ]; then
+  REGION=us-west-2
+fi
+echo "REGION=$REGION" >> envFile
+
+
+GATEWAY_API_CONTROLLER_LOGLEVEL=debug
+echo "GATEWAY_API_CONTROLLER_LOGLEVEL=$GATEWAY_API_CONTROLLER_LOGLEVEL" >> envFile
+

test/pkg/test/framework.go

@@ -62,6 +62,7 @@ type Framework struct {
 	TestCasesCreatedServiceNetworkNames map[string]bool //key: ServiceNetworkName; value: not in use, meaningless
 	TestCasesCreatedServiceNames        map[string]bool //key: ServiceName; value not in use, meaningless
 	TestCasesCreatedTargetGroupNames    map[string]bool //key: TargetGroupName; value: not in use, meaningless
+	TestCasesCreatedK8sResource         []client.Object
 }
 
 func NewFramework(ctx context.Context) *Framework {
@@ -82,10 +83,8 @@ func NewFramework(ctx context.Context) *Framework {
 
 	SetDefaultEventuallyTimeout(180 * time.Second)
 	SetDefaultEventuallyPollingInterval(10 * time.Second)
+	BeforeEach(func() { framework.ExpectToBeClean(ctx) })
 	AfterEach(func() { framework.ExpectToBeClean(ctx) })
-	AfterSuite(func() { framework.CleanTestEnvironment(ctx) })
-	// TODO: in BeforeSuite(), need to make sure current cluster's VPC don't have any Vpc ServiceNetwork association,
-	//  and current k8s cluster is clean (no existing service, deployment(pods), gateway, httproute)
 	return framework
 }
 
@@ -98,6 +97,11 @@ func (env *Framework) ExpectToBeClean(ctx context.Context) {
 	})
 
 	currentClusterVpcId := os.Getenv("CLUSTER_VPC_ID")
+	retrievedServiceNetworkVpcAssociations, _ := env.LatticeClient.ListServiceNetworkVpcAssociationsAsList(ctx, &vpclattice.ListServiceNetworkVpcAssociationsInput{
+		VpcIdentifier: aws.String(currentClusterVpcId),
+	})
+	Logger(ctx).Infof("Expect VPC used by current cluster don't have any ServiceNetworkVPCAssociation, if it has you should manually delete it")
+	Expect(len(retrievedServiceNetworkVpcAssociations)).To(Equal(0))
 	Eventually(func(g Gomega) {
 		retrievedServiceNetworks, _ := env.LatticeClient.ListServiceNetworksAsList(ctx, &vpclattice.ListServiceNetworksInput{})
 		for _, sn := range retrievedServiceNetworks {
@@ -150,35 +154,19 @@ func (env *Framework) CleanTestEnvironment(ctx context.Context) {
 	// Kubernetes API Objects
 	namespaces := &v1.NamespaceList{}
 	Expect(env.List(ctx, namespaces)).WithOffset(1).To(Succeed())
-	Eventually(func(g Gomega) {
-		for _, namespace := range namespaces.Items {
-			if namespace.Name == "kube-system" {
-				continue
-			}
-			parallel.ForEach(TestObjects, func(testObject TestObject, _ int) {
-				log.Println("ExpectDeleteAllToSucceed for testObject.Type", testObject.Type, "in namespace", namespace.Name)
-				env.ExpectDeleteAllToSucceed(ctx, testObject.Type, namespace.Name)
-			})
-		}
-	}).Should(Succeed())
+	for _, object := range env.TestCasesCreatedK8sResource {
+		Logger(ctx).Infof("Deleting k8s resource %s %s/%s", reflect.TypeOf(object), object.GetNamespace(), object.GetName())
+		env.Delete(ctx, object)
+		//Ignore resource-not-found error here, as the test case logic itself could already clear the resources
+	}
 
 	//Theoretically, Deleting all k8s resource by `env.ExpectDeleteAllToSucceed()`, will make controller delete all related VPC Lattice resource,
 	//but the controller is still developing in the progress and may leaking some vPCLattice resource, need to invoke vpcLattice api to double confirm and delete leaking resource.
 	env.DeleteAllFrameworkTracedServiceNetworks(ctx)
 	env.DeleteAllFrameworkTracedVpcLatticeServices(ctx)
 	env.DeleteAllFrameworkTracedTargetGroups(ctx)
-	Eventually(func(g Gomega) {
-		for _, namespace := range namespaces.Items {
-			if namespace.Name == "kube-system" {
-				continue
-			}
-			parallel.ForEach(TestObjects, func(testObject TestObject, _ int) {
-				log.Println("EventuallyExpectNoneFound for testObject.Type", testObject.Type, "in namespace", namespace.Name)
-				defer GinkgoRecover()
-				env.EventuallyExpectNoneFound(ctx, testObject.ListType)
-			})
-		}
-	}).Should(Succeed())
+	env.EventuallyExpectNotFound(ctx, env.TestCasesCreatedK8sResource...)
+	env.TestCasesCreatedK8sResource = nil
 
 }
 
@@ -210,9 +198,10 @@ func (env *Framework) ExpectDeleteAllToSucceed(ctx context.Context, object clien
 func (env *Framework) EventuallyExpectNotFound(ctx context.Context, objects ...client.Object) {
 	Eventually(func(g Gomega) {
 		for _, object := range objects {
+			Logger(ctx).Infof("Checking whether %s %s %s is not found", reflect.TypeOf(object), object.GetNamespace(), object.GetName())
 			g.Expect(errors.IsNotFound(env.Get(ctx, client.ObjectKeyFromObject(object), object))).To(BeTrue())
 		}
-	}).Should(Succeed())
+	}).WithOffset(1).Should(Succeed())
 }
 
 func (env *Framework) EventuallyExpectNoneFound(ctx context.Context, objectList client.ObjectList) {
@@ -277,8 +266,15 @@ func (env *Framework) GetTargets(ctx context.Context, targetGroup *vpclattice.Ta
 	var found []*vpclattice.TargetSummary
 	Eventually(func(g Gomega) {
 		log.Println("Trying to retrieve registered targets for targetGroup", targetGroup)
+		log.Println("deployment.Spec.Selector.MatchLabels:", deployment.Spec.Selector.MatchLabels)
 		podList := &v1.PodList{}
-		g.Expect(env.List(ctx, podList, client.MatchingLabels(deployment.Spec.Selector.MatchLabels))).To(Succeed())
+		expectedMatchingLabels := make(map[string]string, len(deployment.Spec.Selector.MatchLabels))
+		for k, v := range deployment.Spec.Selector.MatchLabels {
+			expectedMatchingLabels[k] = v
+		}
+		expectedMatchingLabels[DiscoveryLabel] = "true"
+		log.Println("Expected matching labels:", expectedMatchingLabels)
+		g.Expect(env.List(ctx, podList, client.MatchingLabels(expectedMatchingLabels))).To(Succeed())
 		g.Expect(podList.Items).To(HaveLen(int(*deployment.Spec.Replicas)))
 		retrievedTargets, err := env.LatticeClient.ListTargetsAsList(ctx, &vpclattice.ListTargetsInput{TargetGroupIdentifier: targetGroup.Id})
 		g.Expect(err).To(BeNil())
@@ -324,7 +320,25 @@ func (env *Framework) DeleteAllFrameworkTracedServiceNetworks(ctx aws.Context) {
 		}
 	}
 
-	//TODO: check and delete serviceNetwork VPC associations
+	var allServiceNetworkVpcAssociationIdsToBeDeleted []*string
+	for _, snIdWithRemainingAssociations := range serviceNetworkIdsWithRemainingAssociations {
+		associations, err := env.LatticeClient.ListServiceNetworkVpcAssociationsAsList(ctx, &vpclattice.ListServiceNetworkVpcAssociationsInput{
+			ServiceNetworkIdentifier: snIdWithRemainingAssociations,
+		})
+		Expect(err).ToNot(HaveOccurred())
+
+		snvaIds := lo.Map(associations, func(association *vpclattice.ServiceNetworkVpcAssociationSummary, _ int) *string {
+			return association.Id
+		})
+		allServiceNetworkVpcAssociationIdsToBeDeleted = append(allServiceNetworkVpcAssociationIdsToBeDeleted, snvaIds...)
+	}
+
+	for _, snvaId := range allServiceNetworkVpcAssociationIdsToBeDeleted {
+		_, err := env.LatticeClient.DeleteServiceNetworkVpcAssociationWithContext(ctx, &vpclattice.DeleteServiceNetworkVpcAssociationInput{
+			ServiceNetworkVpcAssociationIdentifier: snvaId,
+		})
+		Expect(err).ToNot(HaveOccurred())
+	}
 
 	var allServiceNetworkServiceAssociationIdsToBeDeleted []*string
 
@@ -348,12 +362,20 @@ func (env *Framework) DeleteAllFrameworkTracedServiceNetworks(ctx aws.Context) {
 	}
 
 	Eventually(func(g Gomega) {
+		for _, snvaId := range allServiceNetworkVpcAssociationIdsToBeDeleted {
+			_, err := env.LatticeClient.GetServiceNetworkVpcAssociationWithContext(ctx, &vpclattice.GetServiceNetworkVpcAssociationInput{
+				ServiceNetworkVpcAssociationIdentifier: snvaId,
+			})
+			if err != nil {
+				g.Expect(err.(awserr.Error).Code()).To(Equal(vpclattice.ErrCodeResourceNotFoundException))
+			}
+		}
 		for _, snsaId := range allServiceNetworkServiceAssociationIdsToBeDeleted {
 			_, err := env.LatticeClient.GetServiceNetworkServiceAssociationWithContext(ctx, &vpclattice.GetServiceNetworkServiceAssociationInput{
 				ServiceNetworkServiceAssociationIdentifier: snsaId,
 			})
 			if err != nil {
-				Expect(err.(awserr.Error).Code()).To(Equal(vpclattice.ErrCodeResourceNotFoundException))
+				g.Expect(err.(awserr.Error).Code()).To(Equal(vpclattice.ErrCodeResourceNotFoundException))
 			}
 		}
 	}).Should(Succeed())
@@ -492,6 +514,7 @@ func (env *Framework) DeleteAllFrameworkTracedTargetGroups(ctx aws.Context) {
 }
 
 func (env *Framework) GetVpcLatticeServiceDns(httpRouteName string, httpRouteNamespace string) string {
+	log.Println("GetVpcLatticeServiceDns: ", httpRouteName, httpRouteNamespace)
 	httproute := v1beta1.HTTPRoute{}
 	env.Get(env.ctx, types.NamespacedName{Name: httpRouteName, Namespace: httpRouteNamespace}, &httproute)
 	vpcLatticeServiceDns := httproute.Annotations[controllers.LatticeAssignedDomainName]

test/pkg/test/gateway.go

@@ -15,15 +15,23 @@ func (env *Framework) NewGateway() *v1beta1.Gateway {
 			},
 			Spec: v1beta1.GatewaySpec{
 				GatewayClassName: "amazon-vpc-lattice",
-				Listeners: []v1beta1.Listener{{
-					Name:     "http",
-					Protocol: v1beta1.HTTPProtocolType,
-					Port:     80,
-				}},
+				Listeners: []v1beta1.Listener{
+					{
+						Name:     "http",
+						Protocol: v1beta1.HTTPProtocolType,
+						Port:     80,
+					},
+					{
+						Name:     "https",
+						Protocol: v1beta1.HTTPSProtocolType,
+						Port:     443,
+					},
+				},
 			},
 			Status: v1beta1.GatewayStatus{},
 		},
 	)
 	env.TestCasesCreatedServiceNetworkNames[gateway.Name] = true
+	env.TestCasesCreatedK8sResource = append(env.TestCasesCreatedK8sResource, gateway)
 	return gateway
 }

test/pkg/test/header_match_httproute.go

@@ -51,5 +51,6 @@ func (env *Framework) NewHeaderMatchHttpRoute(parentRefsGateway *v1beta1.Gateway
 	})
 
 	env.TestCasesCreatedServiceNames[latticestore.AWSServiceName(httpRoute.Name, httpRoute.Namespace)] = true
+	env.TestCasesCreatedK8sResource = append(env.TestCasesCreatedK8sResource, httpRoute)
 	return httpRoute
 }

test/pkg/test/httpapp.go

@@ -35,7 +35,8 @@ func (env *Framework) NewHttpApp(options HTTPAppOptions) (*appsv1.Deployment, *v
 			Template: v1.PodTemplateSpec{
 				ObjectMeta: metav1.ObjectMeta{
 					Labels: map[string]string{
-						"app": options.Name,
+						"app":          options.Name,
+						DiscoveryLabel: "true",
 					},
 				},
 				Spec: v1.PodSpec{
@@ -53,6 +54,9 @@ func (env *Framework) NewHttpApp(options HTTPAppOptions) (*appsv1.Deployment, *v
 	}, options.MergeFromDeployment...)
 
 	service := New(&v1.Service{
+		TypeMeta: metav1.TypeMeta{
+			Kind: "Service",
+		},
 		Spec: v1.ServiceSpec{
 			Selector: map[string]string{
 				"app": options.Name,
@@ -65,7 +69,7 @@ func (env *Framework) NewHttpApp(options HTTPAppOptions) (*appsv1.Deployment, *v
 		},
 	}, options.MergeFromService...)
 	env.TestCasesCreatedTargetGroupNames[latticestore.TargetGroupName(service.Name, service.Namespace)] = true
-
+	env.TestCasesCreatedK8sResource = append(env.TestCasesCreatedK8sResource, service, deployment)
 	return deployment, service
 
 }

test/pkg/test/metadata.go

@@ -16,12 +16,16 @@ var (
 	sequentialNumber     = 0
 	randomizer           = rand.New(rand.NewSource(time.Now().UnixNano())) //nolint
 	sequentialNumberLock = new(sync.Mutex)
-	DiscoveryLabel       = "testing.kubernetes.io-" + randomdata.Alphanumeric(8) // Each time test suite run have a different label for k8s resource
+	DiscoveryLabel       = "testing.kubernetes.io"
 )
 
 func New[T client.Object](t T, mergeFrom ...T) T {
-	t.SetName(RandomName())
-	t.SetNamespace("default")
+	if t.GetName() == "" {
+		t.SetName(RandomName())
+	}
+	if t.GetNamespace() == "" {
+		t.SetNamespace("default")
+	}
 	t.SetLabels(map[string]string{DiscoveryLabel: "true"})
 	return MustMerge(t, mergeFrom...)
 }
@@ -39,5 +43,5 @@ func RandomName() string {
 	sequentialNumberLock.Lock()
 	defer sequentialNumberLock.Unlock()
 	sequentialNumber++
-	return strings.ToLower(fmt.Sprintf("%s-%d-%s", randomdata.SillyName(), sequentialNumber, randomdata.Alphanumeric(10)))
+	return strings.ToLower(fmt.Sprintf("%s-%d-%s", randomdata.SillyName()[:5], sequentialNumber, randomdata.Alphanumeric(10)))
 }

test/pkg/test/path_match_httproute.go

@@ -3,20 +3,21 @@ package test
 import (
 	"github.com/aws/aws-application-networking-k8s/pkg/latticestore"
 	"github.com/samber/lo"
-	v1 "k8s.io/api/core/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/gateway-api/apis/v1beta1"
 	"strconv"
 )
 
-func (env *Framework) NewPathMatchHttpRoute(parentRefsGateway *v1beta1.Gateway, services []*v1.Service) *v1beta1.HTTPRoute {
+func (env *Framework) NewPathMatchHttpRoute(parentRefsGateway *v1beta1.Gateway, backendRefObjects []client.Object,
+	gwListenerSectionName string) *v1beta1.HTTPRoute {
 	var rules []v1beta1.HTTPRouteRule
-	for i, service := range services {
+	for i, object := range backendRefObjects {
 		rule := v1beta1.HTTPRouteRule{
 			BackendRefs: []v1beta1.HTTPBackendRef{{
 				BackendRef: v1beta1.BackendRef{
 					BackendObjectReference: v1beta1.BackendObjectReference{
-						Name: v1beta1.ObjectName(service.Name),
-						Kind: lo.ToPtr(v1beta1.Kind("Service")),
+						Name: v1beta1.ObjectName(object.GetName()),
+						Kind: lo.ToPtr(v1beta1.Kind(object.GetObjectKind().GroupVersionKind().Kind)),
 					},
 				},
 			}},
@@ -36,13 +37,13 @@ func (env *Framework) NewPathMatchHttpRoute(parentRefsGateway *v1beta1.Gateway,
 			CommonRouteSpec: v1beta1.CommonRouteSpec{
 				ParentRefs: []v1beta1.ParentReference{{
 					Name:        v1beta1.ObjectName(parentRefsGateway.Name),
-					SectionName: lo.ToPtr(v1beta1.SectionName("http")),
+					SectionName: lo.ToPtr(v1beta1.SectionName(gwListenerSectionName)),
 				}},
 			},
 			Rules: rules,
 		},
 	})
-
 	env.TestCasesCreatedServiceNames[latticestore.AWSServiceName(httpRoute.Name, httpRoute.Namespace)] = true
+	env.TestCasesCreatedK8sResource = append(env.TestCasesCreatedK8sResource, httpRoute)
 	return httpRoute
 }

test/suites/integration/httproute_header_match_test.go

@@ -28,7 +28,7 @@ var _ = Describe("HTTPRoute header matches", func() {
 			service3,
 			deployment3)
 
-		time.Sleep(2 * time.Minute)
+		time.Sleep(3 * time.Minute)
 		vpcLatticeService := testFramework.GetVpcLatticeService(ctx, headerMatchHttpRoute)
 		Expect(*vpcLatticeService.DnsEntry).To(ContainSubstring(latticestore.AWSServiceName(headerMatchHttpRoute.Name, headerMatchHttpRoute.Namespace)))
 		Eventually(func(g Gomega) {