Add basic aga controller e2e tests (WIP) #4485
Conversation
|
[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:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
cncf-cla: yes
| } | ||
|
|
||
| func (m *defaultInstallationManager) UpgradeController(controllerImage string, enableEndPointSlices bool, enableALBTargetControlAgent bool) error { | ||
| func (m *defaultInstallationManager) UpgradeController(controllerImage string, enableEndPointSlices bool, enableALBTargetControlAgent bool, enableGlobalAcceleratorController bool) error { |
There was a problem hiding this comment.
We also need to set EnableRGTAPI feature for GlobalAcceleratorController.
| tf, err = framework.InitFramework() | ||
| Expect(err).NotTo(HaveOccurred()) | ||
|
|
||
| if !isCommercialPartition(tf.Options.AWSRegion) { |
There was a problem hiding this comment.
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
| ctx = context.Background() | ||
| if tf.Options.ControllerImage != "" { | ||
| By("upgrade controller with GlobalAccelerator enabled", func() { | ||
| err := tf.CTRLInstallationManager.UpgradeController(tf.Options.ControllerImage, false, false, false) |
There was a problem hiding this comment.
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) ?
| }) | ||
|
|
||
| By("verifying GlobalAccelerator status fields", func() { | ||
| gaARN := agaStack.GetGlobalAcceleratorARN() |
There was a problem hiding this comment.
We can create a function for this verification as we are repeating this in all tests.
|
@wweiwei-li: The following test failed, say
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. |
wweiwei-li
changed the title
| 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 { |
There was a problem hiding this comment.
Do we need to sort these before we validate these so that we validate these irrespective of order in both ranges?
| {FromPort: 80, ToPort: 80}, | ||
| {FromPort: 443, ToPort: 443}, | ||
| }, | ||
| ClientAffinity: string(types.ClientAffinityNone), |
There was a problem hiding this comment.
Missing EndpointGroupExpectation
| }, | ||
| Spec: agav1beta1.GlobalAcceleratorSpec{ | ||
| Name: &acceleratorName, | ||
| IPAddressType: agav1beta1.IPAddressTypeIPV4, |
There was a problem hiding this comment.
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.
| PortRanges: []PortRangeExpectation{ | ||
| {FromPort: 80, ToPort: 80}, | ||
| }, | ||
| ClientAffinity: string(types.ClientAffinityNone), |
There was a problem hiding this comment.
Same EndpointGroupExpectation is missing
| }, | ||
| } | ||
|
|
||
| ingStack = ingress.NewResourceStack([]*appsv1.Deployment{deployment}, []*corev1.Service{nlbSvc, nodeSvc}, []*networkingv1.Ingress{ing}) |
There was a problem hiding this comment.
Instead of making deploying NLB through ingress stack, shall we deploy it standalone?
|
|
||
| // ResourceStack orchestrates the deployment of endpoint resources with GlobalAccelerator | ||
| type ResourceStack struct { | ||
| endpointStack EndpointStack // Endpoint resources (Service/Ingress/Gateway) |
There was a problem hiding this comment.
Do we need this an array for multi-endpoint stacks?
3 participants
Description
Test Files:
Helper Files:
Tests coverred:
Service Endpoint Tests (service_endpoint_test.go):
Ingress Endpoint Tests (ingress_endpoint_test.go):
Multi-Endpoint Tests (multi_endpoint_test.go):
Checklist
README.md, or thedocsdirectory)BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯