From d95296e5a0c6111cfce0f582c57bd06eef55f475 Mon Sep 17 00:00:00 2001 From: levimm Date: Sun, 11 Aug 2019 18:26:33 +0800 Subject: [PATCH] fix azure load balancer update dns label issue --- .../azure/azure_loadbalancer.go | 64 ++++++++++++------- .../azure/azure_loadbalancer_test.go | 25 +++++++- 2 files changed, 65 insertions(+), 24 deletions(-) diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer.go index 4cc291cd9f8..3b5e2a8d3d7 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer.go @@ -492,41 +492,52 @@ func (az *Cloud) ensurePublicIPExists(service *v1.Service, pipName string, domai if err != nil { return nil, err } - if existsPip { - return &pip, nil - } serviceName := getServiceName(service) - if shouldPIPExisted { - return nil, fmt.Errorf("PublicIP from annotation azure-pip-name=%s for service %s doesn't exist", pipName, serviceName) + if existsPip { + // return if pip exist and dns label is the same + if getDomainNameLabel(&pip) == domainNameLabel { + return &pip, nil + } + klog.V(2).Infof("ensurePublicIPExists for service(%s): pip(%s) - updating", serviceName, *pip.Name) + if pip.PublicIPAddressPropertiesFormat == nil { + pip.PublicIPAddressPropertiesFormat = &network.PublicIPAddressPropertiesFormat{ + PublicIPAllocationMethod: network.Static, + } + } + } else { + if shouldPIPExisted { + return nil, fmt.Errorf("PublicIP from annotation azure-pip-name=%s for service %s doesn't exist", pipName, serviceName) + } + pip.Name = to.StringPtr(pipName) + pip.Location = to.StringPtr(az.Location) + pip.PublicIPAddressPropertiesFormat = &network.PublicIPAddressPropertiesFormat{ + PublicIPAllocationMethod: network.Static, + } + pip.Tags = map[string]*string{ + serviceTagKey: &serviceName, + clusterNameKey: &clusterName, + } + if az.useStandardLoadBalancer() { + pip.Sku = &network.PublicIPAddressSku{ + Name: network.PublicIPAddressSkuNameStandard, + } + } + klog.V(2).Infof("ensurePublicIPExists for service(%s): pip(%s) - creating", serviceName, *pip.Name) } - - pip.Name = to.StringPtr(pipName) - pip.Location = to.StringPtr(az.Location) - pip.PublicIPAddressPropertiesFormat = &network.PublicIPAddressPropertiesFormat{ - PublicIPAllocationMethod: network.Static, - } - if len(domainNameLabel) > 0 { + if len(domainNameLabel) == 0 { + pip.PublicIPAddressPropertiesFormat.DNSSettings = nil + } else { pip.PublicIPAddressPropertiesFormat.DNSSettings = &network.PublicIPAddressDNSSettings{ DomainNameLabel: &domainNameLabel, } } - pip.Tags = map[string]*string{ - serviceTagKey: &serviceName, - clusterNameKey: &clusterName, - } - if az.useStandardLoadBalancer() { - pip.Sku = &network.PublicIPAddressSku{ - Name: network.PublicIPAddressSkuNameStandard, - } - } - klog.V(2).Infof("ensurePublicIPExists for service(%s): pip(%s) - creating", serviceName, *pip.Name) klog.V(10).Infof("CreateOrUpdatePIP(%s, %q): start", pipResourceGroup, *pip.Name) err = az.CreateOrUpdatePIP(service, pipResourceGroup, pip) if err != nil { - klog.V(2).Infof("ensure(%s) abort backoff: pip(%s) - creating", serviceName, *pip.Name) + klog.V(2).Infof("ensure(%s) abort backoff: pip(%s)", serviceName, *pip.Name) return nil, err } klog.V(10).Infof("CreateOrUpdatePIP(%s, %q): end", pipResourceGroup, *pip.Name) @@ -540,6 +551,13 @@ func (az *Cloud) ensurePublicIPExists(service *v1.Service, pipName string, domai return &pip, nil } +func getDomainNameLabel(pip *network.PublicIPAddress) string { + if pip == nil || pip.PublicIPAddressPropertiesFormat == nil || pip.PublicIPAddressPropertiesFormat.DNSSettings == nil { + return "" + } + return to.String(pip.PublicIPAddressPropertiesFormat.DNSSettings.DomainNameLabel) +} + func getIdleTimeout(s *v1.Service) (*int32, error) { const ( min = 4 diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer_test.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer_test.go index 9e43362f3f0..eae2f817596 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer_test.go @@ -1833,6 +1833,7 @@ func TestEnsurePublicIPExists(t *testing.T) { existingPIPs []network.PublicIPAddress expectedPIP *network.PublicIPAddress expectedID string + expectedDNS string expectedError bool }{ { @@ -1849,6 +1850,28 @@ func TestEnsurePublicIPExists(t *testing.T) { expectedID: "/subscriptions/subscription/resourceGroups/rg/providers/" + "Microsoft.Network/publicIPAddresses/pip1", }, + { + desc: "ensurePublicIPExists shall update existed PIP's dns label", + existingPIPs: []network.PublicIPAddress{{ + Name: to.StringPtr("pip1"), + PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ + DNSSettings: &network.PublicIPAddressDNSSettings{ + DomainNameLabel: to.StringPtr("previousdns"), + }, + }, + }}, + expectedPIP: &network.PublicIPAddress{ + Name: to.StringPtr("pip1"), + ID: to.StringPtr("/subscriptions/subscription/resourceGroups/rg" + + "/providers/Microsoft.Network/publicIPAddresses/pip1"), + PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ + DNSSettings: &network.PublicIPAddressDNSSettings{ + DomainNameLabel: to.StringPtr("newdns"), + }, + }, + }, + expectedDNS: "newdns", + }, } for i, test := range testCases { @@ -1860,7 +1883,7 @@ func TestEnsurePublicIPExists(t *testing.T) { t.Fatalf("TestCase[%d] meets unexpected error: %v", i, err) } } - pip, err := az.ensurePublicIPExists(&service, "pip1", "", "", false) + pip, err := az.ensurePublicIPExists(&service, "pip1", test.expectedDNS, "", false) if test.expectedID != "" { assert.Equal(t, test.expectedID, to.String(pip.ID), "TestCase[%d]: %s", i, test.desc) } else {