-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[feat aga] Add AGA accelerator deployer #4434
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] Add AGA accelerator deployer #4434
Conversation
ae22611 to
8dfe522
Compare
cb1348c to
1a1363d
Compare
1a1363d to
6b4d4fd
Compare
| return nil | ||
| } | ||
|
|
||
| acceleratorARN := *ga.Status.AcceleratorARN |
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 think we need a fallback mechanism. Suppose that we provision an accelerator but are unable to persist the ARN. If before the next reconcile run happens and the customer deletes the resource from the cluster, we will orphan the accelerator because of this line.
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 tried implementing this after your suggestion but realized that since we will do look up based on tags fro finding the accelerator, we may end up getting multiple resources if the customer configures these resource tags on any other accelerator manually (very rare but they could) and we might delete the unwanted resource due to this tags error and disrupt the traffic. Since this is edge case, I don't think its worth to delete the orphaned resource instead of cleaning up a resource which might be serving traffic. What do you think?
6b4d4fd to
2518efe
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 |
2518efe to
f20dd09
Compare
f20dd09 to
c588936
Compare
|
/lgtm |
Description
This PR introduces support for AGA deployer allowing Kubernetes users to manage Global Accelerator resources through Kubernetes custom resources. I have divided this PR into smaller commits to make it easier to review.
Here is the brief description on each of them
Setup AGA SDK Client (a39933f)
Adds AWS Global Accelerator SDK integration with the controller. Implements client interfaces, mock implementations, and cloud provider extensions to interact with the Global Accelerator service. Updates dependencies to include required AWS SDK packages for Global Accelerator.
Add AGA CRD Status Updates Utils (c34f2eb)
Implements status updating utilities for Global Accelerator CRDs. Provides mechanisms to reflect the state of AWS Global Accelerator resources back to Kubernetes custom resources, enabling users to track resource status, lifecycle events, and error conditions directly in Kubernetes.
Add AGA Tags Reconciler (9484b80)
Implements tagging functionality for Global Accelerator resources to maintain proper resource ownership and tracking. Provides interfaces and implementations for applying, updating, and reconciling tags on AWS Global Accelerator resources with comprehensive test coverage. Please note I have moved tracking tags to deployer instead of builders. Also I have added one more region specific tags so that multiple controller in same account in different region do not override each other
Add AGA Controller Config Flags (9695137)
I have disabled AGAController for now until its ready for GA. We will make this enabled by default when we are ready to release this for prod. We have also added exponential back-off duration flags for AGA controller which customer can configure. I wanna also know if we should make requeue time configurable as well.
Add AGA Deployer (47a7bec)
Implements the core deployment logic for Global Accelerator resources. This includes:
update controller-gen version(8dfe522)
Updating the controller-gen version to use v0.19.0
To fix the failing CRD verifications in CI/CD
TODO: I will add e2e tests once we build all the resources separately as this was already getting very big PR.
TODO: I have not added IAM policies. I wanna add when we are done with the upcoming release.
Checklist
README.md, or thedocsdirectory)BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯