aws-load-balancer-controller
aws-load-balancer-controller copied to clipboard
Target groups can now also be specified by Name
Issue
Partially solves https://github.com/kubernetes-sigs/aws-load-balancer-controller/issues/2373
Description
On TargetGroupBinding
one can now choose a target group by it's name, using the targetGroupName
field.
Example
---
apiVersion: elbv2.k8s.aws/v1beta1
kind: TargetGroupBinding
metadata:
name: mdiezalbtest2
namespace: default
spec:
ipAddressType: ipv4
serviceRef:
name: mdiezalbtest
port: 8080
targetGroupName: mdiezalbtest-tg
Implementation detail:
All I did was to make the field targetGroupName
valid, make targetGroupARN
non mandatory and intercept MutateCreate
and checkRequiredFields
, so that if targetGroupARN
is empty and targetGroupName
is provided, AWS is queried for the ARN.
As a future PR, I could from time to time check if a new target group with targetGroupName
was created and later bind it.
One can currently test this using the following container: marcosdiez/aws-alb-ingress-controller:20220524-1006
Checklist
- [X] Added tests that cover your change (if possible)
- [X] Added/modified documentation as required (such as the
README.md
, or thedocs
directory) - [X] Manually tested
- [X] 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?! :exploding_head:
- [ ] Backfilled missing tests for code in same general area :tada:
- [ ] Refactored something and made the world a better place :star2:
Hi @marcosdiez. Thanks for your PR.
I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test
on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test
label.
I understand the commands that are listed here.
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/test-infra repository.
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: marcosdiez
To complete the pull request process, please assign kishorj after the PR has been reviewed.
You can assign the PR to them by writing /assign @kishorj
in a comment when ready.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve
in a comment
Approvers can cancel approval by writing /approve cancel
in a comment
Codecov Report
Base: 54.07% // Head: 54.07% // Decreases project coverage by -0.00%
:warning:
Coverage data is based on head (
0efe885
) compared to base (a92e689
). Patch coverage: 55.55% of modified lines in pull request are covered.
Additional details and impacted files
@@ Coverage Diff @@
## main #2655 +/- ##
==========================================
- Coverage 54.07% 54.07% -0.01%
==========================================
Files 144 144
Lines 8301 8335 +34
==========================================
+ Hits 4489 4507 +18
- Misses 3484 3492 +8
- Partials 328 336 +8
Impacted Files | Coverage Δ | |
---|---|---|
webhooks/elbv2/targetgroupbinding_mutator.go | 66.23% <44.44%> (-6.65%) |
:arrow_down: |
webhooks/elbv2/targetgroupbinding_validator.go | 73.00% <66.66%> (-2.00%) |
:arrow_down: |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
Hello @M00nF1sh ! Could you please take a look at this PR ?
@M00nF1sh don't forget me!
@kishorj could you please take a look at this PR ?
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.
This bot triages PRs according to the following rules:
- After 90d of inactivity,
lifecycle/stale
is applied - After 30d of inactivity since
lifecycle/stale
was applied,lifecycle/rotten
is applied - After 30d of inactivity since
lifecycle/rotten
was applied, the PR is closed
You can:
- Mark this PR as fresh with
/remove-lifecycle stale
- Close this PR with
/close
- Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
/remove-lifecycle stale
Would it be possible to review this MR?
@kishorj when you have a capacity, would you be able to review this MR and share your feedback please?
Hi, @marcosdiez do you have the capacity to add suggested changes and update MR?
Hi, What is missing on this end?
We would very much like not having to depend on the full Target Group ARN because it is not deterministic so it requires a certain amount of handover from our Terraform-based IaC to the in-cluster reconciliation.
If this was implemented, we can make assumptions on how to identify the Target Group, helping us to decouple the two reconciliation engines.
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: marcosdiez Once this PR has been reviewed and has the lgtm label, please assign johngmyers for approval. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve
in a comment
Approvers can cancel approval by writing /approve cancel
in a comment
Any chance to release it as experimental feature. Buy that time the community could provide feedback. And for sure we could fix the bugs if any. This mr is already 2 years old.
PR needs rebase.
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/test-infra repository.