-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Add basic aga controller e2e tests #4485
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.
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
base: main
Are you sure you want to change the base?
Conversation
ab72dbe to
3808a8d
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: shraddhabang, 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:
Approvers can indicate their approval by writing |
| dnsName := stack.GetGlobalAcceleratorDualStackDNSName() | ||
| listenerPort := 80 | ||
| if len(port) > 0 { | ||
| listenerPort = port[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering, why are you using an array when you only use the first value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You reminded me ... actually I was planning to have a test for multiple listeners and check traffic for multiple listening ports. Updated the helper and also added a multiple listener test. Should create GlobalAccelerator with two listeners on ports 80 and 443
| aga = createAGAWithIngressEndpoint(gaName, namespace, acceleratorName, ingName, agav1beta1.IPAddressTypeIPV4, | ||
| &[]agav1beta1.GlobalAcceleratorListener{{ | ||
| Protocol: &protocol, | ||
| PortRanges: &[]agav1beta1.PortRange{{FromPort: 443, ToPort: 443}}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see a test where you validate that we can support a port range e.g. ports 100 to 200.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I added a test for Should create GlobalAccelerator with port range 80-443
|
|
||
| // IsCommercialPartition returns true if the region is in the commercial AWS partition | ||
| func IsCommercialPartition(region string) bool { | ||
| unsupportedPrefixes := []string{"cn-", "us-gov-", "us-iso", "eu-isoe-"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Do you have an ARN you can parse instead? If so, it makes this check more robust.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, there's no ARN available in BeforeSuite.
| gwv1 "sigs.k8s.io/gateway-api/apis/v1" | ||
| ) | ||
|
|
||
| var _ = Describe("GlobalAccelerator with Gateway endpoint", func() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does accelerator controller read the route data? If so, it would be interesting to validate that cross namespace routes work. If the accelerator controller doesn't read route data, then that test case doesn't seem necessarily.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aga controller does not read route data. Also, for initial release, aga controller does not support cross namespace. So the test actually only verify that the gateway itself can be referenced and it's load balancer can be resolved.
Description
Test Files:
Helper Files:
Tests coverred:
Note: These tests require
--enable-aga-testsflag 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.mdService 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
README.md, or thedocsdirectory)BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯