Add support for bring-your-own ip address for Services on Azure #42034
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-github-robot
added
the
approved
k8s-github-robot
added
size/M
|
This change is |
|
cc: @raveeram |
|
@k8s-bot kubemark e2e test this |
| return fmt.Sprintf("%s-%s", clusterName, cloudprovider.GetLoadBalancerName(service)), nil | ||
| } | ||
|
|
||
| list, err := az.PublicIPAddressesClient.List(az.ResourceGroup) |
There was a problem hiding this comment.
Are we sure we should we limit this to the existing RG?
There was a problem hiding this comment.
lifted to all IP addresses in the subscription.
There was a problem hiding this comment.
I forgot how much this is oriented around "the" resource group. This returns the public ip name, and all of the code that looks at the public ip looks for it in the current resource group. I think you have to revert the last change or make a larger change to refactor getPublicIPName to getPublicIP.
|
/lgtm maybe consider looking through all public ips instead of just the current RG. I would use this functionality when available (to avoid having to update DNS as I destroy/recreate clusters)... but only if I can stash the PublicIP in a "permanent resource group" and then have it available as I destroy/create new RGs/clusters. |
|
@colemickens: you can't LGTM a PR unless you are an assignee. In response to this comment:
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. I understand the commands that are listed here. |
brendandburns
added
release-note
| @@ -61,10 +66,37 @@ func (az *Cloud) GetLoadBalancer(clusterName string, service *v1.Service) (statu | |||
| }, true, nil | |||
| } | |||
|
|
|||
| func (az *Cloud) getPublicIPName(clusterName string, service *v1.Service) (string, error) { | |||
| loadBalancerIP := service.Spec.LoadBalancerIP | |||
There was a problem hiding this comment.
Would this change mean the user always has to bring his own IP? If so, it would probably be a breaking change.
|
comment addressed ptal. |
|
I shouldn't be an OWNER in any of the cloud provider / cluster lifecycle stuff anymore. I guess the file was never updated. @kubernetes/sig-cluster-lifecycle-pr-reviews please find an assignee for this. |
brendandburns
added
the
lgtm
|
self-lgtm on the previous LGTM from @colemickens (and I'd like this to make the freeze) |
|
@brendandburns I had made another inline comment. Repasting here so you see:
|
brendandburns
removed
the
lgtm
|
@colemickens I reverted the ListAll part, ptal. Thanks |
|
Looks good to me but I'm pretty sure I can't /lgtm since I'm not the assignee (but I'll try anyway). /lgtm |
|
@colemickens: you can't LGTM a PR unless you are an assignee. In response to this comment:
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. I understand the commands that are listed here. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: brendandburns, colemickens
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
@k8s-bot kubemark e2e test this |
brendandburns
added
the
lgtm
|
self-lgtm on lgtm from @colemickens and a conviction that @lavalamp doesn't care that much about the Azure cloud provider :P |
|
Automatic merge from submit-queue (batch tested with PRs 41139, 41186, 38882, 37698, 42034) |
…dbalancer Automatic merge from submit-queue Cherry pick azure internal loadbalancer Backports fixes for Azure load balancer: - [Add support for Azure internal load balancer](#43510) - [Add support for bring-your-own ip address for Services on Azure](#42034) **Release note**: ```release-note Add support for Azure internal load balancer. ```
@colemickens @codablock