From f05ddc92260245421d59ec514a2a9f2576a6dbf6 Mon Sep 17 00:00:00 2001 From: Pengfei Ni Date: Mon, 29 Jul 2019 14:33:56 +0800 Subject: [PATCH] Fix the public IP getting issues for VMSS nodes --- .../azure/azure_client.go | 18 ++++++ .../azure/azure_fakes.go | 16 ++++- .../azure/azure_vmss.go | 62 +++++++++++++------ 3 files changed, 75 insertions(+), 21 deletions(-) diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_client.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_client.go index edadd4358ff..b93341c7759 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_client.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_client.go @@ -73,6 +73,7 @@ type PublicIPAddressesClient interface { CreateOrUpdate(ctx context.Context, resourceGroupName string, publicIPAddressName string, parameters network.PublicIPAddress) (resp *http.Response, err error) Delete(ctx context.Context, resourceGroupName string, publicIPAddressName string) (resp *http.Response, err error) Get(ctx context.Context, resourceGroupName string, publicIPAddressName string, expand string) (result network.PublicIPAddress, err error) + GetVirtualMachineScaleSetPublicIPAddress(ctx context.Context, resourceGroupName string, virtualMachineScaleSetName string, virtualmachineIndex string, networkInterfaceName string, IPConfigurationName string, publicIPAddressName string, expand string) (result network.PublicIPAddress, err error) List(ctx context.Context, resourceGroupName string) (result []network.PublicIPAddress, err error) } @@ -568,6 +569,23 @@ func (az *azPublicIPAddressesClient) Get(ctx context.Context, resourceGroupName return } +func (az *azPublicIPAddressesClient) GetVirtualMachineScaleSetPublicIPAddress(ctx context.Context, resourceGroupName string, virtualMachineScaleSetName string, virtualmachineIndex string, networkInterfaceName string, IPConfigurationName string, publicIPAddressName string, expand string) (result network.PublicIPAddress, err error) { + if !az.rateLimiterReader.TryAccept() { + err = createRateLimitErr(false, "VMSSPublicIPGet") + return + } + + klog.V(10).Infof("azPublicIPAddressesClient.GetVirtualMachineScaleSetPublicIPAddress(%q,%q): start", resourceGroupName, publicIPAddressName) + defer func() { + klog.V(10).Infof("azPublicIPAddressesClient.GetVirtualMachineScaleSetPublicIPAddress(%q,%q): end", resourceGroupName, publicIPAddressName) + }() + + mc := newMetricContext("vmss_public_ip_addresses", "get", resourceGroupName, az.client.SubscriptionID, "") + result, err = az.client.GetVirtualMachineScaleSetPublicIPAddress(ctx, resourceGroupName, virtualMachineScaleSetName, virtualmachineIndex, networkInterfaceName, IPConfigurationName, publicIPAddressName, expand) + mc.Observe(err) + return +} + func (az *azPublicIPAddressesClient) List(ctx context.Context, resourceGroupName string) ([]network.PublicIPAddress, error) { if !az.rateLimiterReader.TryAccept() { return nil, createRateLimitErr(false, "PublicIPList") diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_fakes.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_fakes.go index 9fd5d589f16..959ca1809e1 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_fakes.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_fakes.go @@ -192,7 +192,21 @@ func (fAPC *fakeAzurePIPClient) Get(ctx context.Context, resourceGroupName strin } return result, autorest.DetailedError{ StatusCode: http.StatusNotFound, - Message: "Not such PIP", + Message: "No such PIP", + } +} + +func (fAPC *fakeAzurePIPClient) GetVirtualMachineScaleSetPublicIPAddress(ctx context.Context, resourceGroupName string, virtualMachineScaleSetName string, virtualmachineIndex string, networkInterfaceName string, IPConfigurationName string, publicIPAddressName string, expand string) (result network.PublicIPAddress, err error) { + fAPC.mutex.Lock() + defer fAPC.mutex.Unlock() + if _, ok := fAPC.FakeStore[resourceGroupName]; ok { + if entity, ok := fAPC.FakeStore[resourceGroupName][publicIPAddressName]; ok { + return entity, nil + } + } + return result, autorest.DetailedError{ + StatusCode: http.StatusNotFound, + Message: "No such PIP", } } diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_vmss.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_vmss.go index f2cee92ebd8..5bdd24c6bf5 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_vmss.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_vmss.go @@ -39,10 +39,11 @@ var ( // ErrorNotVmssInstance indicates an instance is not belongint to any vmss. ErrorNotVmssInstance = errors.New("not a vmss instance") - scaleSetNameRE = regexp.MustCompile(`.*/subscriptions/(?:.*)/Microsoft.Compute/virtualMachineScaleSets/(.+)/virtualMachines(?:.*)`) - resourceGroupRE = regexp.MustCompile(`.*/subscriptions/(?:.*)/resourceGroups/(.+)/providers/Microsoft.Compute/virtualMachineScaleSets/(?:.*)/virtualMachines(?:.*)`) - vmssMachineIDTemplate = "/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/virtualMachineScaleSets/%s/virtualMachines/%s" - vmssIPConfigurationRE = regexp.MustCompile(`.*/subscriptions/(?:.*)/resourceGroups/(.+)/providers/Microsoft.Compute/virtualMachineScaleSets/(.+)/virtualMachines/(.+)/networkInterfaces(?:.*)`) + scaleSetNameRE = regexp.MustCompile(`.*/subscriptions/(?:.*)/Microsoft.Compute/virtualMachineScaleSets/(.+)/virtualMachines(?:.*)`) + resourceGroupRE = regexp.MustCompile(`.*/subscriptions/(?:.*)/resourceGroups/(.+)/providers/Microsoft.Compute/virtualMachineScaleSets/(?:.*)/virtualMachines(?:.*)`) + vmssMachineIDTemplate = "/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/virtualMachineScaleSets/%s/virtualMachines/%s" + vmssIPConfigurationRE = regexp.MustCompile(`.*/subscriptions/(?:.*)/resourceGroups/(.+)/providers/Microsoft.Compute/virtualMachineScaleSets/(.+)/virtualMachines/(.+)/networkInterfaces(?:.*)`) + vmssPIPConfigurationRE = regexp.MustCompile(`.*/subscriptions/(?:.*)/resourceGroups/(.+)/providers/Microsoft.Compute/virtualMachineScaleSets/(.+)/virtualMachines/(.+)/networkInterfaces/(.+)/ipConfigurations/(.+)/publicIPAddresses/(.+)`) ) // scaleSet implements VMSet interface for Azure scale set. @@ -315,28 +316,49 @@ func (ss *scaleSet) GetIPByNodeName(nodeName string) (string, string, error) { publicIP := "" if ipConfig.PublicIPAddress != nil && ipConfig.PublicIPAddress.ID != nil { pipID := *ipConfig.PublicIPAddress.ID - pipName, err := getLastSegment(pipID) - if err != nil { - return "", "", fmt.Errorf("failed to get publicIP name for node %q with pipID %q", nodeName, pipID) - } - - resourceGroup, err := ss.GetNodeResourceGroup(nodeName) - if err != nil { - return "", "", err - } - - pip, existsPip, err := ss.getPublicIPAddress(resourceGroup, pipName) - if err != nil { - return "", "", err - } - if existsPip { - publicIP = *pip.IPAddress + matches := vmssPIPConfigurationRE.FindStringSubmatch(pipID) + if len(matches) == 7 { + resourceGroupName := matches[1] + virtualMachineScaleSetName := matches[2] + virtualmachineIndex := matches[3] + networkInterfaceName := matches[4] + IPConfigurationName := matches[5] + publicIPAddressName := matches[6] + pip, existsPip, err := ss.getVMSSPublicIPAddress(resourceGroupName, virtualMachineScaleSetName, virtualmachineIndex, networkInterfaceName, IPConfigurationName, publicIPAddressName) + if err != nil { + klog.Errorf("ss.getVMSSPublicIPAddress() failed with error: %v", err) + return "", "", err + } + if existsPip && pip.IPAddress != nil { + publicIP = *pip.IPAddress + } + } else { + klog.Warningf("Failed to get VMSS Public IP with ID %s", pipID) } } return internalIP, publicIP, nil } +func (ss *scaleSet) getVMSSPublicIPAddress(resourceGroupName string, virtualMachineScaleSetName string, virtualmachineIndex string, networkInterfaceName string, IPConfigurationName string, publicIPAddressName string) (pip network.PublicIPAddress, exists bool, err error) { + var realErr error + var message string + ctx, cancel := getContextWithCancel() + defer cancel() + pip, err = ss.PublicIPAddressesClient.GetVirtualMachineScaleSetPublicIPAddress(ctx, resourceGroupName, virtualMachineScaleSetName, virtualmachineIndex, networkInterfaceName, IPConfigurationName, publicIPAddressName, "") + exists, message, realErr = checkResourceExistsFromError(err) + if realErr != nil { + return pip, false, realErr + } + + if !exists { + klog.V(2).Infof("Public IP %q not found with message: %q", publicIPAddressName, message) + return pip, false, nil + } + + return pip, exists, err +} + // returns a list of private ips assigned to node // TODO (khenidak): This should read all nics, not just the primary // allowing users to split ipv4/v6 on multiple nics