-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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: Add target-group-prefix annotation to change target group names #3404
base: main
Are you sure you want to change the base?
feat: Add target-group-prefix annotation to change target group names #3404
Conversation
Welcome @frankh! |
Hi @frankh. 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 Once the patch is verified, the new status will be reflected by the 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. |
95226e5
to
85722f5
Compare
85722f5
to
ac22674
Compare
docs/guide/ingress/annotations.md
Outdated
!!!example | ||
- set the target group prefix to "my-service-", the full name will look like "my-service-8f5fe20735" | ||
``` | ||
alb.ingress.kubernetes.io/target-group-prefix: my-service- |
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 it would make sense to imply the trailing dash. So instead of my-service-
it would just be my-service
and the dash would be added in the concatenation statement
/ok-to-test |
/retest |
29bf83a
to
a825fa5
Compare
/approve |
a825fa5
to
cd14986
Compare
@frankh I'm not an official maintainer or anything that was just a suggestion but thanks 😄 |
/lgtm |
/retest |
Thanks for the contribution |
@M00nF1sh can we get some 👀 please 🙏 |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AndrewCharlesHay, frankh, oliviassss 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 |
@frankh @AndrewCharlesHay e.g. this could implemented as a controller flag like
note, i used python's format string syntax here, not sure whether there is Golang alternative. However, we can have a naive implementation for pattern like this such as below:
What do you think? |
@frankh Did you want to take on those changes? I can if you don't want to |
When can we expect this feature will be released? |
/retest |
eae620b
to
5475a63
Compare
New changes are detected. LGTM label has been removed. |
Currently all target groups start with a prefix of `k8s-<namespace>-<name>-` This is not particularly useful, allow us to override per ingress and service
5475a63
to
5ad172f
Compare
Issue
#2553
Description
Currently all target groups start with a prefix of
k8s-<namespace>-<name>
This is not particularly useful, allow us to override per ingress and serviceChecklist
README.md
, or thedocs
directory)BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯