github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-29 14:37:00 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #87246 from nilo19/qi-fix-dns-label

Browse Source 
Fix the bug PIP's DNS is deleted if no DNS label service annotation isn't set.
This commit is contained in:
Kubernetes Prow Robot 2020-01-16 14:38:32 -08:00 committed by GitHub
commit 6413f1ee2b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 70 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer.go
View File

@ -137,11 +137,11 @@ func (az *Cloud) GetLoadBalancer(ctx context.Context, clusterName string, servic
return status, true, nil return status, true, nil
} }
func getPublicIPDomainNameLabel(service *v1.Service) string { func getPublicIPDomainNameLabel(service *v1.Service) (string, bool) {
if labelName, found := service.Annotations[ServiceAnnotationDNSLabelName]; found { if labelName, found := service.Annotations[ServiceAnnotationDNSLabelName]; found {
return labelName return labelName, found
} }
return "" return "", false
} }
// EnsureLoadBalancer creates a new load balancer 'name', or updates the existing one. Returns the status of the balancer // EnsureLoadBalancer creates a new load balancer 'name', or updates the existing one. Returns the status of the balancer
@ -498,7 +498,7 @@ func (az *Cloud) findServiceIPAddress(ctx context.Context, clusterName string, s
return lbStatus.Ingress[0].IP, nil return lbStatus.Ingress[0].IP, nil
} }
func (az *Cloud) ensurePublicIPExists(service *v1.Service, pipName string, domainNameLabel, clusterName string, shouldPIPExisted bool) (*network.PublicIPAddress, error) { func (az *Cloud) ensurePublicIPExists(service *v1.Service, pipName string, domainNameLabel, clusterName string, shouldPIPExisted, foundDNSLabelAnnotation bool) (*network.PublicIPAddress, error) {
pipResourceGroup := az.getPublicIPAddressResourceGroup(service) pipResourceGroup := az.getPublicIPAddressResourceGroup(service)
pip, existsPip, err := az.getPublicIPAddress(pipResourceGroup, pipName) pip, existsPip, err := az.getPublicIPAddress(pipResourceGroup, pipName)
if err != nil { if err != nil {
@ -538,11 +538,13 @@ func (az *Cloud) ensurePublicIPExists(service *v1.Service, pipName string, domai
} }
klog.V(2).Infof("ensurePublicIPExists for service(%s): pip(%s) - creating", serviceName, *pip.Name) klog.V(2).Infof("ensurePublicIPExists for service(%s): pip(%s) - creating", serviceName, *pip.Name)
} }
if len(domainNameLabel) == 0 { if foundDNSLabelAnnotation {
pip.PublicIPAddressPropertiesFormat.DNSSettings = nil if len(domainNameLabel) == 0 {
} else { pip.PublicIPAddressPropertiesFormat.DNSSettings = nil
pip.PublicIPAddressPropertiesFormat.DNSSettings = &network.PublicIPAddressDNSSettings{ } else {
DomainNameLabel: &domainNameLabel, pip.PublicIPAddressPropertiesFormat.DNSSettings = &network.PublicIPAddressDNSSettings{
DomainNameLabel: &domainNameLabel,
}
} }
} }
@ -807,8 +809,8 @@ func (az *Cloud) reconcileLoadBalancer(clusterName string, service *v1.Service,
if err != nil { if err != nil {
return nil, err return nil, err
} }
domainNameLabel := getPublicIPDomainNameLabel(service) domainNameLabel, found := getPublicIPDomainNameLabel(service)
pip, err := az.ensurePublicIPExists(service, pipName, domainNameLabel, clusterName, shouldPIPExisted) pip, err := az.ensurePublicIPExists(service, pipName, domainNameLabel, clusterName, shouldPIPExisted, found)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -1515,8 +1517,8 @@ func (az *Cloud) reconcilePublicIP(clusterName string, service *v1.Service, lbNa
if !isInternal && wantLb { if !isInternal && wantLb {
// Confirm desired public ip resource exists // Confirm desired public ip resource exists
var pip *network.PublicIPAddress var pip *network.PublicIPAddress
domainNameLabel := getPublicIPDomainNameLabel(service) domainNameLabel, found := getPublicIPDomainNameLabel(service)
if pip, err = az.ensurePublicIPExists(service, desiredPipName, domainNameLabel, clusterName, shouldPIPExisted); err != nil { if pip, err = az.ensurePublicIPExists(service, desiredPipName, domainNameLabel, clusterName, shouldPIPExisted, found); err != nil {
return nil, err return nil, err
} }
return pip, nil return pip, nil

66
staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer_test.go
View File

@ -1983,8 +1983,8 @@ func TestReconcilePublicIP(t *testing.T) {
pip, err := az.reconcilePublicIP("testCluster", &service, "", test.wantLb) pip, err := az.reconcilePublicIP("testCluster", &service, "", test.wantLb)
if test.expectedID != "" { if test.expectedID != "" {
assert.Equal(t, test.expectedID, to.String(pip.ID), "TestCase[%d]: %s", i, test.desc) assert.Equal(t, test.expectedID, to.String(pip.ID), "TestCase[%d]: %s", i, test.desc)
} else { } else if test.expectedPIP != nil && test.expectedPIP.Name != nil {
assert.Equal(t, test.expectedPIP, pip, "TestCase[%d]: %s", i, test.desc) assert.Equal(t, *test.expectedPIP.Name, *pip.Name, "TestCase[%d]: %s", i, test.desc)
} }
assert.Equal(t, test.expectedError, err != nil, "TestCase[%d]: %s", i, test.desc) assert.Equal(t, test.expectedError, err != nil, "TestCase[%d]: %s", i, test.desc)
} }
@ -1992,12 +1992,13 @@ func TestReconcilePublicIP(t *testing.T) {
func TestEnsurePublicIPExists(t *testing.T) { func TestEnsurePublicIPExists(t *testing.T) {
testCases := []struct { testCases := []struct {
desc string desc string
existingPIPs []network.PublicIPAddress existingPIPs []network.PublicIPAddress
expectedPIP *network.PublicIPAddress inputDNSLabel string
expectedID string foundDNSLabelAnnotation bool
expectedDNS string expectedPIP *network.PublicIPAddress
expectedError bool expectedID string
expectedError bool
}{ }{
{ {
desc: "ensurePublicIPExists shall return existed PIP if there is any", desc: "ensurePublicIPExists shall return existed PIP if there is any",
@ -2014,7 +2015,9 @@ func TestEnsurePublicIPExists(t *testing.T) {
"Microsoft.Network/publicIPAddresses/pip1", "Microsoft.Network/publicIPAddresses/pip1",
}, },
{ {
desc: "ensurePublicIPExists shall update existed PIP's dns label", desc: "ensurePublicIPExists shall update existed PIP's dns label",
inputDNSLabel: "newdns",
foundDNSLabelAnnotation: true,
existingPIPs: []network.PublicIPAddress{{ existingPIPs: []network.PublicIPAddress{{
Name: to.StringPtr("pip1"), Name: to.StringPtr("pip1"),
PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{
@ -2033,7 +2036,48 @@ func TestEnsurePublicIPExists(t *testing.T) {
}, },
}, },
}, },
expectedDNS: "newdns", },
{
desc: "ensurePublicIPExists shall delete DNS from PIP if DNS label is set empty",
foundDNSLabelAnnotation: true,
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: nil,
},
},
},
{
desc: "ensurePublicIPExists shall not delete DNS from PIP if DNS label annotation is not set",
foundDNSLabelAnnotation: false,
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("previousdns"),
},
},
},
}, },
} }
@ -2046,7 +2090,7 @@ func TestEnsurePublicIPExists(t *testing.T) {
t.Fatalf("TestCase[%d] meets unexpected error: %v", i, err) t.Fatalf("TestCase[%d] meets unexpected error: %v", i, err)
} }
} }
pip, err := az.ensurePublicIPExists(&service, "pip1", test.expectedDNS, "", false) pip, err := az.ensurePublicIPExists(&service, "pip1", test.inputDNSLabel, "", false, test.foundDNSLabelAnnotation)
if test.expectedID != "" { if test.expectedID != "" {
assert.Equal(t, test.expectedID, to.String(pip.ID), "TestCase[%d]: %s", i, test.desc) assert.Equal(t, test.expectedID, to.String(pip.ID), "TestCase[%d]: %s", i, test.desc)
} else { } else {