From 6dd9cc6dbfe7a92a26597e0b3ba736591f6a7eff Mon Sep 17 00:00:00 2001 From: Pengfei Ni Date: Fri, 29 Dec 2017 10:50:57 +0800 Subject: [PATCH 1/2] Fix vmss listing for Azure cloud provider --- .../providers/azure/azure_util_vmss.go | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/pkg/cloudprovider/providers/azure/azure_util_vmss.go b/pkg/cloudprovider/providers/azure/azure_util_vmss.go index 01a4d92876..1e03f70187 100644 --- a/pkg/cloudprovider/providers/azure/azure_util_vmss.go +++ b/pkg/cloudprovider/providers/azure/azure_util_vmss.go @@ -164,20 +164,20 @@ func (ss *scaleSet) getCachedVirtualMachine(nodeName string) (scaleSetVMInfo, er ss.cacheMutex.Lock() defer ss.cacheMutex.Unlock() - getVMFromCache := func(nodeName string) (scaleSetVMInfo, error) { + getVMFromCache := func(nodeName string) (scaleSetVMInfo, bool) { for scaleSetName := range ss.cache { for _, vm := range ss.cache[scaleSetName] { if vm.NodeName == nodeName { - return vm, nil + return vm, true } } } - return scaleSetVMInfo{}, cloudprovider.InstanceNotFound + return scaleSetVMInfo{}, false } - vm, err := getVMFromCache(nodeName) - if err == nil { + vm, found := getVMFromCache(nodeName) + if found { return vm, nil } @@ -187,11 +187,11 @@ func (ss *scaleSet) getCachedVirtualMachine(nodeName string) (scaleSetVMInfo, er } // Update cache and try again. - if err = ss.updateCache(); err != nil { + if err := ss.updateCache(); err != nil { return scaleSetVMInfo{}, err } - vm, err = getVMFromCache(nodeName) - if err == nil { + vm, found = getVMFromCache(nodeName) + if found { return vm, nil } @@ -204,35 +204,35 @@ func (ss *scaleSet) getCachedVirtualMachineByInstanceID(scaleSetName, instanceID ss.cacheMutex.Lock() defer ss.cacheMutex.Unlock() - getVMByID := func(scaleSetName, instanceID string) (scaleSetVMInfo, error) { + getVMByID := func(scaleSetName, instanceID string) (scaleSetVMInfo, bool) { vms, ok := ss.cache[scaleSetName] if !ok { glog.V(4).Infof("scale set (%s) not found", scaleSetName) - return scaleSetVMInfo{}, cloudprovider.InstanceNotFound + return scaleSetVMInfo{}, false } for _, vm := range vms { if vm.InstanceID == instanceID { glog.V(4).Infof("getCachedVirtualMachineByInstanceID gets vm (%s) by instanceID (%s) within scale set (%s)", vm.NodeName, instanceID, scaleSetName) - return vm, nil + return vm, true } } glog.V(4).Infof("instanceID (%s) not found in scale set (%s)", instanceID, scaleSetName) - return scaleSetVMInfo{}, cloudprovider.InstanceNotFound + return scaleSetVMInfo{}, false } - vm, err := getVMByID(scaleSetName, instanceID) - if err == nil { + vm, found := getVMByID(scaleSetName, instanceID) + if found { return vm, nil } // Update cache and try again. - if err = ss.updateCache(); err != nil { + if err := ss.updateCache(); err != nil { return scaleSetVMInfo{}, err } - vm, err = getVMByID(scaleSetName, instanceID) - if err == nil { + vm, found = getVMByID(scaleSetName, instanceID) + if found { return vm, nil } @@ -407,7 +407,7 @@ func (ss *scaleSet) listScaleSetsWithRetry() ([]string, error) { return nil, backoffError } - appendResults := (result.Value != nil && len(*result.Value) > 1) + appendResults := (result.Value != nil && len(*result.Value) > 0) for appendResults { for _, scaleSet := range *result.Value { allScaleSets = append(allScaleSets, *scaleSet.Name) @@ -431,7 +431,7 @@ func (ss *scaleSet) listScaleSetsWithRetry() ([]string, error) { return nil, backoffError } - appendResults = (result.Value != nil && len(*result.Value) > 1) + appendResults = (result.Value != nil && len(*result.Value) > 0) } } @@ -461,7 +461,7 @@ func (ss *scaleSet) listScaleSetVMsWithRetry(scaleSetName string) ([]compute.Vir return nil, backoffError } - appendResults := (result.Value != nil && len(*result.Value) > 1) + appendResults := (result.Value != nil && len(*result.Value) > 0) for appendResults { allVMs = append(allVMs, *result.Value...) appendResults = false @@ -483,7 +483,7 @@ func (ss *scaleSet) listScaleSetVMsWithRetry(scaleSetName string) ([]compute.Vir return nil, backoffError } - appendResults = (result.Value != nil && len(*result.Value) > 1) + appendResults = (result.Value != nil && len(*result.Value) > 0) } } From 5805a7fefbea8f3a4042af7cfdc1405baa02be90 Mon Sep 17 00:00:00 2001 From: Pengfei Ni Date: Fri, 29 Dec 2017 10:59:54 +0800 Subject: [PATCH 2/2] Add more verbose logs --- pkg/cloudprovider/providers/azure/azure_util_vmss.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/cloudprovider/providers/azure/azure_util_vmss.go b/pkg/cloudprovider/providers/azure/azure_util_vmss.go index 1e03f70187..2c8630c2c8 100644 --- a/pkg/cloudprovider/providers/azure/azure_util_vmss.go +++ b/pkg/cloudprovider/providers/azure/azure_util_vmss.go @@ -160,11 +160,13 @@ func (ss *scaleSet) updateCache() error { } // getCachedVirtualMachine gets virtualMachine by nodeName from cache. +// It returns cloudprovider.InstanceNotFound if node does not belong to any scale sets. func (ss *scaleSet) getCachedVirtualMachine(nodeName string) (scaleSetVMInfo, error) { ss.cacheMutex.Lock() defer ss.cacheMutex.Unlock() getVMFromCache := func(nodeName string) (scaleSetVMInfo, bool) { + glog.V(8).Infof("Getting scaleSetVMInfo for %q from cache %v", nodeName, ss.cache) for scaleSetName := range ss.cache { for _, vm := range ss.cache[scaleSetName] { if vm.NodeName == nodeName { @@ -183,11 +185,13 @@ func (ss *scaleSet) getCachedVirtualMachine(nodeName string) (scaleSetVMInfo, er // Known node not managed by scale sets. if ss.availabilitySetNodesCache.Has(nodeName) { + glog.V(10).Infof("Found node %q in availabilitySetNodesCache", nodeName) return scaleSetVMInfo{}, cloudprovider.InstanceNotFound } // Update cache and try again. if err := ss.updateCache(); err != nil { + glog.Errorf("updateCache failed with error: %v", err) return scaleSetVMInfo{}, err } vm, found = getVMFromCache(nodeName) @@ -196,15 +200,19 @@ func (ss *scaleSet) getCachedVirtualMachine(nodeName string) (scaleSetVMInfo, er } // Node still not found, assuming it is not managed by scale sets. + glog.V(8).Infof("Node %q doesn't belong to any scale sets, adding it to availabilitySetNodesCache", nodeName) ss.availabilitySetNodesCache.Insert(nodeName) return scaleSetVMInfo{}, cloudprovider.InstanceNotFound } +// getCachedVirtualMachineByInstanceID gets scaleSetVMInfo from cache. +// The node must belong to one of scale sets. func (ss *scaleSet) getCachedVirtualMachineByInstanceID(scaleSetName, instanceID string) (scaleSetVMInfo, error) { ss.cacheMutex.Lock() defer ss.cacheMutex.Unlock() getVMByID := func(scaleSetName, instanceID string) (scaleSetVMInfo, bool) { + glog.V(8).Infof("Getting scaleSetVMInfo with scaleSetName: %q and instanceID %q from cache %v", scaleSetName, instanceID, ss.cache) vms, ok := ss.cache[scaleSetName] if !ok { glog.V(4).Infof("scale set (%s) not found", scaleSetName) @@ -229,6 +237,7 @@ func (ss *scaleSet) getCachedVirtualMachineByInstanceID(scaleSetName, instanceID // Update cache and try again. if err := ss.updateCache(); err != nil { + glog.Errorf("updateCache failed with error: %v", err) return scaleSetVMInfo{}, err } vm, found = getVMByID(scaleSetName, instanceID)