Add basic aga controller e2e tests #4485

wweiwei-li · 2025-12-04T06:14:13Z

Description

Test Files:

globalaccelerator_suite_test.go - Test suite setup, checks commercial partition requirement
service_endpoint_test.go - Service endpoint tests
ingress_endpoint_test.go - Ingress endpoint tests
multi_endpoint_test.go - Multiple endpoint types
gateway_endpoint_test.go - Gateway API endpoint tests

Helper Files:

resource_stack.go - ResourceStack for deploying/cleaning up GlobalAccelerator CRs.
aga_verifier.go - Verification types & functions for AWS GlobalAccelerator configuration
test_helpers.go - Helper functions for creating test resources (deployments, services, ingresses, GlobalAccelerator CRs)
Uses ResourceStack from service/ingress/gateway packages directly for backend endpoint resources

Tests coverred:

Note: These tests require --enable-aga-tests flag to run (local testing only, not in CI). Make sure set up your local controller with https://github.com/kubernetes-sigs/aws-load-balancer-controller/blob/main/docs/guide/globalaccelerator/installation.md

Service Endpoint Tests (service_endpoint_test.go):
✅ IP target type lifecycle - Create, verify, update , verify traffic
✅ Instance target type scheme change - Deploy with internet-facing, change to internal, verify endpoint replacement
✅ Direct endpoint ID - Create GlobalAccelerator with direct load balancer ARN reference

Ingress Endpoint Tests (ingress_endpoint_test.go):
✅ Basic Ingress lifecycle - Create, verify configuration, traffic flow
✅ Auto-discovery - Automatically discover protocol and ports from Ingress
✅ IPV4 to DUAL_STACK migration - Migrate address type and verify dual-stack DNS
✅ Port overrides- Configure port overrides for listeners

Multi-Endpoint Tests (multi_endpoint_test.go):
✅ Service + Ingress endpoints - Multiple endpoint types in same namespace

Gateway Endpoint Tests (gateway_endpoint_test.go):
✅ ALB Gateway endpoint - Create GlobalAccelerator with ALB Gateway, verify configuration and traffic
✅ NLB Gateway endpoint - Create GlobalAccelerator with NLB Gateway, verify configuration and traffic

Checklist

Added tests that cover your change (if possible)
Added/modified documentation as required (such as the README.md, or the docs directory)
Manually tested
Made sure the title of the PR is a good description that can go into the release notes

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯

Backfilled missing tests for code in same general area 🎉
Refactored something and made the world a better place 🌟

k8s-ci-robot · 2025-12-04T06:14:20Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wweiwei-li

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [wweiwei-li]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

shraddhabang · 2025-12-04T08:31:47Z

test/framework/controller/installation_manager.go

 }

-func (m *defaultInstallationManager) UpgradeController(controllerImage string, enableEndPointSlices bool, enableALBTargetControlAgent bool) error {
+func (m *defaultInstallationManager) UpgradeController(controllerImage string, enableEndPointSlices bool, enableALBTargetControlAgent bool, enableGlobalAcceleratorController bool) error {


We also need to set EnableRGTAPI feature for GlobalAcceleratorController.

Since we decided not to have it run in CI, I removed this logic. Right now, these tests require the --enable-aga-tests flag to run local testing, same as gateway tests. Local testers will need to set all required feature flags. For future CI tests, we can update the controller with the required EnableRGTAPI and AGA flags

shraddhabang · 2025-12-04T08:44:46Z

test/e2e/globalaccelerator/globalaccelerator_suite_test.go

+	tf, err = framework.InitFramework()
+	Expect(err).NotTo(HaveOccurred())
+
+	if !isCommercialPartition(tf.Options.AWSRegion) {


May be we can reuse this IsGlobalAcceleratorControllerEnabled in utility function instead of just checking the partition? Or you can create this in utils and we could avoid duplication partition check done here in IsGlobalAcceleratorControllerEnabled

I moved it to util.go. We can't just reuse IsGlobalAcceleratorControllerEnabled to decide if we run tests. It only checks if AGA is enabled, not if it's supported. but this will only be needed for future CI or pipeline tests actually.

shraddhabang · 2025-12-04T08:49:10Z

test/e2e/globalaccelerator/ingress_endpoint_test.go

+		ctx = context.Background()
+		if tf.Options.ControllerImage != "" {
+			By("upgrade controller with GlobalAccelerator enabled", func() {
+				err := tf.CTRLInstallationManager.UpgradeController(tf.Options.ControllerImage, false, false, false)


Do we need to set AGA flag to true if we are using custom image for these tests? tf.CTRLInstallationManager.UpgradeController(tf.Options.ControllerImage, false, false, true) ?

Since we decided not to have it run in CI. I remove this logic. Right now, these tests require --enable-aga-tests flag to run local testing. Same as gateway tests.

shraddhabang · 2025-12-04T09:00:53Z

test/e2e/globalaccelerator/ingress_endpoint_test.go

+			})
+
+			By("verifying GlobalAccelerator status fields", func() {
+				gaARN := agaStack.GetGlobalAcceleratorARN()


We can create a function for this verification as we are repeating this in all tests.

Yes, added a helper

test/e2e/globalaccelerator/ingress_endpoint_test.go

shraddhabang · 2025-12-04T09:04:56Z

test/e2e/globalaccelerator/aga_verifier.go

+				if len(listener.PortRanges) != len(expectedListener.PortRanges) {
+					return fmt.Errorf("listener[%d] port range count mismatch: expected %d, got %d", i, len(expectedListener.PortRanges), len(listener.PortRanges))
+				}
+				for j, expectedPortRange := range expectedListener.PortRanges {


Do we need to sort these before we validate these so that we validate these irrespective of order in both ranges?

Yes, added it

shraddhabang · 2025-12-04T09:07:17Z

test/e2e/globalaccelerator/ingress_endpoint_test.go

+								{FromPort: 80, ToPort: 80},
+								{FromPort: 443, ToPort: 443},
+							},
+							ClientAffinity: string(types.ClientAffinityNone),


Missing EndpointGroupExpectation

shraddhabang · 2025-12-04T09:09:54Z

test/e2e/globalaccelerator/ingress_endpoint_test.go

+				},
+				Spec: agav1beta1.GlobalAcceleratorSpec{
+					Name:          &acceleratorName,
+					IPAddressType: agav1beta1.IPAddressTypeIPV4,


For auto-discovery case, you can also skip IPAddressType, ClientAffinity and TrafficDialPercentage. These should all be defaulted through CRD if unspsecified. We could check that as well.

Yes, skipped it

shraddhabang · 2025-12-04T09:11:15Z

test/e2e/globalaccelerator/ingress_endpoint_test.go

+							PortRanges: []PortRangeExpectation{
+								{FromPort: 80, ToPort: 80},
+							},
+							ClientAffinity: string(types.ClientAffinityNone),


Same EndpointGroupExpectation is missing

shraddhabang · 2025-12-04T09:12:00Z

test/e2e/globalaccelerator/multi_endpoint_test.go

+			},
+		}
+
+		ingStack = ingress.NewResourceStack([]*appsv1.Deployment{deployment}, []*corev1.Service{nlbSvc, nodeSvc}, []*networkingv1.Ingress{ing})


Instead of making deploying NLB through ingress stack, shall we deploy it standalone?

Yes, deploying standalone would be cleaner. Updated it

shraddhabang · 2025-12-04T09:14:15Z

test/e2e/globalaccelerator/resource_stack.go

+
+// ResourceStack orchestrates the deployment of endpoint resources with GlobalAccelerator
+type ResourceStack struct {
+	endpointStack EndpointStack                 // Endpoint resources (Service/Ingress/Gateway)


Do we need this an array for multi-endpoint stacks?

When adding gateway endpoint support, I decided not to use aga/resource_stack.go to manage all endpoint stacks (service, ingress, gateway). Instead, each endpoint type is managed by directly importing and using its own ResourceStack. I think this is clear, simpler than trying to unify different resource types under a common Interface. What do you think ?

k8s-ci-robot · 2025-12-08T08:45:02Z

@wweiwei-li: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-aws-load-balancer-controller-e2e-test	`50fc9ea`	link	true	`/test pull-aws-load-balancer-controller-e2e-test`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

k8s-ci-robot requested review from M00nF1sh and zac-nixon December 4, 2025 06:14

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Dec 4, 2025

wweiwei-li requested a review from shraddhabang December 4, 2025 06:14

shraddhabang reviewed Dec 4, 2025

View reviewed changes

test/e2e/globalaccelerator/ingress_endpoint_test.go Outdated Show resolved Hide resolved

wweiwei-li force-pushed the aga-e2e branch 2 times, most recently from ab72dbe to 3808a8d Compare December 4, 2025 22:03

wweiwei-li changed the title ~~Add basic aga controller e2e tests~~ Add basic aga controller e2e tests (WIP) Dec 5, 2025

shraddhabang reviewed Dec 5, 2025

View reviewed changes

Add basic aga controller e2e tests

50fc9ea

wweiwei-li force-pushed the aga-e2e branch from 3808a8d to 50fc9ea Compare December 8, 2025 05:13

wweiwei-li changed the title ~~Add basic aga controller e2e tests (WIP)~~ Add basic aga controller e2e tests Dec 8, 2025

Add basic aga controller e2e tests #4485

Are you sure you want to change the base?

Add basic aga controller e2e tests #4485

Conversation

wweiwei-li commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯

Uh oh!

k8s-ci-robot commented Dec 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Dec 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wweiwei-li commented Dec 4, 2025 •

edited

Loading