-
Notifications
You must be signed in to change notification settings - Fork 431
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
Allow to set custom backend pool names for load balancers #2714
Conversation
Thanks for picking this up from #2432 |
@CecileRobertMichon Hi! I updated the PR as you asked. Could you take a look again? |
api/v1beta1/types.go
Outdated
// Name specifies the name of backend pool for the load balancer. If not specified, the default name will | ||
// be set, depending on the load balancer role. | ||
// +optional | ||
Name *string `json:"name,omitempty"` |
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.
is there value in making this a string pointer given that ""
will never be a valid name? If it was just a string
it would avoid having to dereference it in a bunch of places (and it would be more consistent with other Name
fields in the API)
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, I agree. I copied it from the original PR where Name is a pointer. I updated it as you asked.
/cc @nawazkh @willie-yao for additional 👀 |
27d3268
to
c1c3f79
Compare
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.
Thanks for putting this together!
/lgtm
@Fedosin please squash |
/lgtm |
Currently, there is no way to specify the backend pool name. This PR makes LB's backend pool name customizable. The changes introduced in this PR are backward compatible and should not break existing deployments.
@CecileRobertMichon now it's squashed |
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.
/lgtm
/approve
Thank you @nawazkh @willie-yao for reviewing 🙌
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: CecileRobertMichon 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 |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Currently, there is no way to specify the backend pool name. This PR makes LB's backend pool name customizable. The changes introduced in this PR are backward compatible and should not break existing deployments.
Special notes for your reviewer:
Adding a struct
Backend
to API allows for future expansion of the configuration. If we later wanted to add some additional config, it would be preferable to have them all in a struct rather than BackendPoolName, BackendPoolPort, etcTODOs:
Release note: