-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[feat aga] Implement endpoint group management with port override conflict resolution #4463
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
[feat aga] Implement endpoint group management with port override conflict resolution #4463
Conversation
3f69379 to
46a1fc3
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: shraddhabang 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 |
46a1fc3 to
7be9eee
Compare
7be9eee to
a5bfc10
Compare
|
/lgtm |
| r.logger.Info("Reconciling GlobalAccelerator resources", "globalAccelerator", k8s.NamespacedName(ga)) | ||
|
|
||
| // Get all endpoints from GA | ||
| endpoints := aga.GetAllEndpointsFromGA(ga) |
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'd consider calling this GetDesiredEndpointsFromGA; so that it's obvious these are coming from the spec and not from the current state of the accelerator.
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.
Yes makes sense
| } | ||
|
|
||
| // UpdateReferencesForGA updates the tracking information for a GlobalAccelerator | ||
| func (t *ReferenceTracker) UpdateReferencesForGA(ga *agaapi.GlobalAccelerator, endpoints []EndpointReference) { |
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.
Assuming endpoints is existing / current / actual Endpoints, let's specify that here.
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.
Makes sens. These are desired ones coming from spec. I will rename these.
This PR introduces comprehensive endpoint group management capabilities for AGA in the AWS Load Balancer Controller. The implementation includes endpoint group reconciliation logic, conflict detection and resolution, and full lifecycle management (creation, updates, and deletion) within the AGA controller.
This PR is divided in multiple commits for easier review. (The earlier PR is not yet merged so please ignore the duplicate commits from the older PR only focus on the following commits for this PR)
Description
[feat aga] Implement model builder for endpoint groups
[feat aga] few bugfixes and code refactoring
[feat aga] Implement endpoint group deployer with port override conf
This commit includes two parts.
Endpoint Group Management
synthesizeEndpointGroupsOnListenerPort Override Conflict Resolution
Implemented
detectConflictsWithSDKEndpointGroupsto identify region-specific port conflictsCreated two-phase conflict resolution algorithm in
ProcessEndpointGroupPortOverrides:Added validation rules ensuring endpoint ports don't overlap with listener port ranges
Ensured listener ports in port overrides remain within listener port range specifications
Checklist
README.md, or thedocsdirectory)BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯