Skip to content
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.

Sign up for GitHub

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

Jump to bottom

cherry pick of #81262: fix azure load balancer update dns label issue #85318

Merged
merged 1 commit into from
Nov 29, 2019
Merged

cherry pick of #81262: fix azure load balancer update dns label issue #85318

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
88 changes: 57 additions & 31 deletions staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func (az *Cloud) GetLoadBalancer(ctx context.Context, clusterName string, servic
// there is a chance that we could orphan public IP resources while we delete the load blanacer (kubernetes/kubernetes#80571).
// We need to make sure the existence of the load balancer depends on the load balancer resource and public IP resource on Azure.
existsPip := func() bool {
pipName, err := az.determinePublicIPName(clusterName, service)
pipName, _, err := az.determinePublicIPName(clusterName, service)
if err != nil {
return false
}
Expand Down Expand Up @@ -423,26 +423,28 @@ func (az *Cloud) getServiceLoadBalancerStatus(service *v1.Service, lb *network.L
return nil, nil
}

func (az *Cloud) determinePublicIPName(clusterName string, service *v1.Service) (string, error) {
func (az *Cloud) determinePublicIPName(clusterName string, service *v1.Service) (string, bool, error) {
var shouldPIPExisted bool

loadBalancerIP := service.Spec.LoadBalancerIP
if len(loadBalancerIP) == 0 {
return az.getPublicIPName(clusterName, service), nil
return az.getPublicIPName(clusterName, service), shouldPIPExisted, nil
}

pipResourceGroup := az.getPublicIPAddressResourceGroup(service)

pips, err := az.ListPIP(service, pipResourceGroup)
if err != nil {
return "", err
return "", shouldPIPExisted, err
}

for _, pip := range pips {
if pip.PublicIPAddressPropertiesFormat.IPAddress != nil &&
*pip.PublicIPAddressPropertiesFormat.IPAddress == loadBalancerIP {
return *pip.Name, nil
return *pip.Name, shouldPIPExisted, nil
}
}
return "", fmt.Errorf("user supplied IP Address %s was not found in resource group %s", loadBalancerIP, pipResourceGroup)
return "", shouldPIPExisted, fmt.Errorf("user supplied IP Address %s was not found in resource group %s", loadBalancerIP, pipResourceGroup)
}

func flipServiceInternalAnnotation(service *v1.Service) *v1.Service {
Expand Down Expand Up @@ -489,42 +491,58 @@ func (az *Cloud) findServiceIPAddress(ctx context.Context, clusterName string, s
return lbStatus.Ingress[0].IP, nil
}

func (az *Cloud) ensurePublicIPExists(service *v1.Service, pipName string, domainNameLabel, clusterName string) (*network.PublicIPAddress, error) {
func (az *Cloud) ensurePublicIPExists(service *v1.Service, pipName string, domainNameLabel, clusterName string, shouldPIPExisted bool) (*network.PublicIPAddress, error) {
pipResourceGroup := az.getPublicIPAddressResourceGroup(service)
pip, existsPip, err := az.getPublicIPAddress(pipResourceGroup, pipName)
if err != nil {
return nil, err
}
if existsPip {
return &pip, nil
}

serviceName := getServiceName(service)
pip.Name = to.StringPtr(pipName)
pip.Location = to.StringPtr(az.Location)
pip.PublicIPAddressPropertiesFormat = &network.PublicIPAddressPropertiesFormat{
PublicIPAllocationMethod: network.Static,

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)
}
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)
Expand All @@ -538,6 +556,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
Expand Down Expand Up @@ -595,7 +620,7 @@ func (az *Cloud) isFrontendIPChanged(clusterName string, config network.Frontend
if loadBalancerIP == "" {
return false, nil
}
pipName, err := az.determinePublicIPName(clusterName, service)
pipName, _, err := az.determinePublicIPName(clusterName, service)
if err != nil {
return false, err
}
Expand Down Expand Up @@ -733,12 +758,12 @@ func (az *Cloud) reconcileLoadBalancer(clusterName string, service *v1.Service,

fipConfigurationProperties = &configProperties
} else {
pipName, err := az.determinePublicIPName(clusterName, service)
pipName, shouldPIPExisted, err := az.determinePublicIPName(clusterName, service)
if err != nil {
return nil, err
}
domainNameLabel := getPublicIPDomainNameLabel(service)
pip, err := az.ensurePublicIPExists(service, pipName, domainNameLabel, clusterName)
pip, err := az.ensurePublicIPExists(service, pipName, domainNameLabel, clusterName, shouldPIPExisted)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -1360,8 +1385,9 @@ func (az *Cloud) reconcilePublicIP(clusterName string, service *v1.Service, lbNa
var lb *network.LoadBalancer
var desiredPipName string
var err error
var shouldPIPExisted bool
if !isInternal && wantLb {
desiredPipName, err = az.determinePublicIPName(clusterName, service)
desiredPipName, shouldPIPExisted, err = az.determinePublicIPName(clusterName, service)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -1407,7 +1433,7 @@ func (az *Cloud) reconcilePublicIP(clusterName string, service *v1.Service, lbNa
// Confirm desired public ip resource exists
var pip *network.PublicIPAddress
domainNameLabel := getPublicIPDomainNameLabel(service)
if pip, err = az.ensurePublicIPExists(service, desiredPipName, domainNameLabel, clusterName); err != nil {
if pip, err = az.ensurePublicIPExists(service, desiredPipName, domainNameLabel, clusterName, shouldPIPExisted); err != nil {
return nil, err
}
return pip, nil
Expand Down
64 changes: 0 additions & 64 deletions staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"fmt"
"reflect"
"testing"
"time"

"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-07-01/network"
"github.com/Azure/go-autorest/autorest/to"
Expand Down Expand Up @@ -537,66 +536,3 @@ func TestGetServiceTags(t *testing.T) {
assert.Equal(t, tags, c.expected, "TestCase[%d]: %s", i, c.desc)
}
}

func TestShouldUpdateLoadBalancer(t *testing.T) {
testCases := []struct {
desc string
lbHasDeletionTimestamp bool
existsLb bool
expectedOutput bool
}{
{
desc: "should update a load balancer that does not have a deletion timestamp and exists in Azure",
lbHasDeletionTimestamp: false,
existsLb: true,
expectedOutput: true,
},
{
desc: "should not update a load balancer that is being deleted / already deleted in K8s",
lbHasDeletionTimestamp: true,
existsLb: true,
expectedOutput: false,
},
{
desc: "should not update a load balancer that does not exist in Azure",
lbHasDeletionTimestamp: false,
existsLb: false,
expectedOutput: false,
},
{
desc: "should not update a load balancer that has a deletion timestamp and does not exist in Azure",
lbHasDeletionTimestamp: true,
existsLb: false,
expectedOutput: false,
},
}

for i, test := range testCases {
az := getTestCloud()
service := getTestService("test1", v1.ProtocolTCP, 80)
if test.lbHasDeletionTimestamp {
service.ObjectMeta.DeletionTimestamp = &metav1.Time{time.Now()}
}
if test.existsLb {
lb := network.LoadBalancer{
Name: to.StringPtr("lb1"),
LoadBalancerPropertiesFormat: &network.LoadBalancerPropertiesFormat{
FrontendIPConfigurations: &[]network.FrontendIPConfiguration{
{
Name: to.StringPtr("atest1"),
FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{
PublicIPAddress: &network.PublicIPAddress{ID: to.StringPtr("id1")},
},
},
},
},
}
_, err := az.LoadBalancerClient.CreateOrUpdate(context.TODO(), "rg", *lb.Name, lb, "")
if err != nil {
t.Fatalf("TestCase[%d] meets unexpected error: %v", i, err)
}
}
shouldUpdateLoadBalancer := az.shouldUpdateLoadBalancer(testClusterName, &service)
assert.Equal(t, test.expectedOutput, shouldUpdateLoadBalancer, "TestCase[%d]: %s", i, test.desc)
}
}