From f427d279feee2776d62b066415269646363a56b9 Mon Sep 17 00:00:00 2001 From: Pengfei Ni Date: Tue, 8 May 2018 22:42:42 +0800 Subject: [PATCH] Do not check vmSetName when getting node IP --- .../providers/azure/azure_fakes.go | 2 +- .../providers/azure/azure_standard.go | 26 +++++++++++++------ .../providers/azure/azure_vmsets.go | 6 ++--- .../providers/azure/azure_vmss.go | 18 +++++-------- 4 files changed, 28 insertions(+), 24 deletions(-) diff --git a/pkg/cloudprovider/providers/azure/azure_fakes.go b/pkg/cloudprovider/providers/azure/azure_fakes.go index 154b7b40ff9..a37e07b1423 100644 --- a/pkg/cloudprovider/providers/azure/azure_fakes.go +++ b/pkg/cloudprovider/providers/azure/azure_fakes.go @@ -850,7 +850,7 @@ func (f *fakeVMSet) GetIPByNodeName(name, vmSetName string) (string, string, err return ip, "", nil } -func (f *fakeVMSet) GetPrimaryInterface(nodeName, vmSetName string) (network.Interface, error) { +func (f *fakeVMSet) GetPrimaryInterface(nodeName string) (network.Interface, error) { return network.Interface{}, fmt.Errorf("unimplemented") } diff --git a/pkg/cloudprovider/providers/azure/azure_standard.go b/pkg/cloudprovider/providers/azure/azure_standard.go index 2dab8d443f5..e30579febc8 100644 --- a/pkg/cloudprovider/providers/azure/azure_standard.go +++ b/pkg/cloudprovider/providers/azure/azure_standard.go @@ -289,7 +289,7 @@ outer: } func (az *Cloud) getIPForMachine(nodeName types.NodeName) (string, string, error) { - return az.vmSet.GetIPByNodeName(string(nodeName), "") + return az.vmSet.GetIPByNodeName(string(nodeName)) } var polyTable = crc32.MakeTable(crc32.Koopman) @@ -429,8 +429,8 @@ func (as *availabilitySet) GetPrimaryVMSetName() string { } // GetIPByNodeName gets machine private IP and public IP by node name. -func (as *availabilitySet) GetIPByNodeName(name, vmSetName string) (string, string, error) { - nic, err := as.GetPrimaryInterface(name, vmSetName) +func (as *availabilitySet) GetIPByNodeName(name string) (string, string, error) { + nic, err := as.GetPrimaryInterface(name) if err != nil { return "", "", err } @@ -554,8 +554,13 @@ func (as *availabilitySet) GetVMSetNames(service *v1.Service, nodes []*v1.Node) return availabilitySetNames, nil } -// GetPrimaryInterface gets machine primary network interface by node name and vmSet. -func (as *availabilitySet) GetPrimaryInterface(nodeName, vmSetName string) (network.Interface, error) { +// GetPrimaryInterface gets machine primary network interface by node name. +func (as *availabilitySet) GetPrimaryInterface(nodeName string) (network.Interface, error) { + return as.getPrimaryInterfaceWithVMSet(nodeName, "") +} + +// getPrimaryInterfaceWithVMSet gets machine primary network interface by node name and vmSet. +func (as *availabilitySet) getPrimaryInterfaceWithVMSet(nodeName, vmSetName string) (network.Interface, error) { var machine compute.VirtualMachine machine, err := as.GetVirtualMachineWithRetry(types.NodeName(nodeName)) @@ -573,8 +578,13 @@ func (as *availabilitySet) GetPrimaryInterface(nodeName, vmSetName string) (netw return network.Interface{}, err } - // Check availability set. - // Backends of Standard load balancer could belong to multiple VMAS, so we don't check vmSet for it. + // Check availability set name. Note that vmSetName is empty string when getting + // the Node's IP address. While vmSetName is not empty, it should be checked with + // Node's real availability set name: + // - For basic SKU load balancer, errNotInVMSet should be returned if the node's + // availability set is mismatched with vmSetName. + // - For standard SKU load balancer, backend could belong to multiple VMAS, so we + // don't check vmSet for it. if vmSetName != "" && !as.useStandardLoadBalancer() { expectedAvailabilitySetName := as.getAvailabilitySetID(vmSetName) if machine.AvailabilitySet == nil || !strings.EqualFold(*machine.AvailabilitySet.ID, expectedAvailabilitySetName) { @@ -598,7 +608,7 @@ func (as *availabilitySet) GetPrimaryInterface(nodeName, vmSetName string) (netw // participating in the specified LoadBalancer Backend Pool. func (as *availabilitySet) ensureHostInPool(serviceName string, nodeName types.NodeName, backendPoolID string, vmSetName string, isInternal bool) error { vmName := mapNodeNameToVMName(nodeName) - nic, err := as.GetPrimaryInterface(vmName, vmSetName) + nic, err := as.getPrimaryInterfaceWithVMSet(vmName, vmSetName) if err != nil { if err == errNotInVMSet { glog.V(3).Infof("ensureHostInPool skips node %s because it is not in the vmSet %s", nodeName, vmSetName) diff --git a/pkg/cloudprovider/providers/azure/azure_vmsets.go b/pkg/cloudprovider/providers/azure/azure_vmsets.go index 47ea1c89154..34b69e8595c 100644 --- a/pkg/cloudprovider/providers/azure/azure_vmsets.go +++ b/pkg/cloudprovider/providers/azure/azure_vmsets.go @@ -35,9 +35,9 @@ type VMSet interface { // GetInstanceTypeByNodeName gets the instance type by node name. GetInstanceTypeByNodeName(name string) (string, error) // GetIPByNodeName gets machine private IP and public IP by node name. - GetIPByNodeName(name, vmSetName string) (string, string, error) - // GetPrimaryInterface gets machine primary network interface by node name and vmSet. - GetPrimaryInterface(nodeName, vmSetName string) (network.Interface, error) + GetIPByNodeName(name string) (string, string, error) + // GetPrimaryInterface gets machine primary network interface by node name. + GetPrimaryInterface(nodeName string) (network.Interface, error) // GetNodeNameByProviderID gets the node name by provider ID. GetNodeNameByProviderID(providerID string) (types.NodeName, error) diff --git a/pkg/cloudprovider/providers/azure/azure_vmss.go b/pkg/cloudprovider/providers/azure/azure_vmss.go index 19ba8946987..c7fa190b164 100644 --- a/pkg/cloudprovider/providers/azure/azure_vmss.go +++ b/pkg/cloudprovider/providers/azure/azure_vmss.go @@ -247,10 +247,10 @@ func (ss *scaleSet) GetPrimaryVMSetName() string { // GetIPByNodeName gets machine private IP and public IP by node name. // TODO(feiskyer): Azure vmss doesn't support associating a public IP to single virtual machine yet, // fix this after it is supported. -func (ss *scaleSet) GetIPByNodeName(nodeName, vmSetName string) (string, string, error) { - nic, err := ss.GetPrimaryInterface(nodeName, vmSetName) +func (ss *scaleSet) GetIPByNodeName(nodeName string) (string, string, error) { + nic, err := ss.GetPrimaryInterface(nodeName) if err != nil { - glog.Errorf("error: ss.GetIPByNodeName(%s), GetPrimaryInterface(%q, %q), err=%v", nodeName, nodeName, vmSetName, err) + glog.Errorf("error: ss.GetIPByNodeName(%s), GetPrimaryInterface(%q), err=%v", nodeName, nodeName, err) return "", "", err } @@ -418,7 +418,7 @@ func (ss *scaleSet) GetVMSetNames(service *v1.Service, nodes []*v1.Node) (vmSetN } // GetPrimaryInterface gets machine primary network interface by node name and vmSet. -func (ss *scaleSet) GetPrimaryInterface(nodeName, vmSetName string) (network.Interface, error) { +func (ss *scaleSet) GetPrimaryInterface(nodeName string) (network.Interface, error) { managedByAS, err := ss.isNodeManagedByAvailabilitySet(nodeName) if err != nil { glog.Errorf("Failed to check isNodeManagedByAvailabilitySet: %v", err) @@ -426,7 +426,7 @@ func (ss *scaleSet) GetPrimaryInterface(nodeName, vmSetName string) (network.Int } if managedByAS { // vm is managed by availability set. - return ss.availabilitySet.GetPrimaryInterface(nodeName, "") + return ss.availabilitySet.GetPrimaryInterface(nodeName) } ssName, instanceID, vm, err := ss.getVmssVM(nodeName) @@ -435,12 +435,6 @@ func (ss *scaleSet) GetPrimaryInterface(nodeName, vmSetName string) (network.Int return network.Interface{}, err } - // Check scale set name. - // Backends of Standard load balancer could belong to multiple VMSS, so we don't check vmSet for it. - if !strings.EqualFold(ssName, vmSetName) && !ss.useStandardLoadBalancer() { - return network.Interface{}, errNotInVMSet - } - primaryInterfaceID, err := ss.getPrimaryInterfaceID(vm) if err != nil { glog.Errorf("error: ss.GetPrimaryInterface(%s), ss.getPrimaryInterfaceID(), err=%v", nodeName, err) @@ -699,7 +693,7 @@ func (ss *scaleSet) EnsureHostsInPool(serviceName string, nodes []*v1.Node, back } for ssName, instanceIDs := range scalesets { - // Only add nodes belonging to specified vmSet to basic LB backends. + // Only add nodes belonging to specified vmSet for basic SKU LB. if !ss.useStandardLoadBalancer() && !strings.EqualFold(ssName, vmSetName) { continue }