diff --git a/pkg/provider/azure_zones.go b/pkg/provider/azure_zones.go index d4abbf12e4..9d69688d4d 100644 --- a/pkg/provider/azure_zones.go +++ b/pkg/provider/azure_zones.go @@ -18,6 +18,7 @@ package provider import ( "context" + "errors" "fmt" "os" "strconv" @@ -31,6 +32,7 @@ import ( azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache" "sigs.k8s.io/cloud-provider-azure/pkg/consts" + "sigs.k8s.io/cloud-provider-azure/pkg/retry" ) func (az *Cloud) refreshZones(refreshFunc func() error) { @@ -43,15 +45,14 @@ func (az *Cloud) refreshZones(refreshFunc func() error) { } func (az *Cloud) syncRegionZonesMap() error { - klog.V(2).Infof("refreshZones: starting to fetch all available zones for the subscription %s", az.SubscriptionID) + klog.V(2).Infof("syncRegionZonesMap: starting to fetch all available zones for the subscription %s", az.SubscriptionID) zones, rerr := az.ZoneClient.GetZones(context.Background(), az.SubscriptionID) if rerr != nil { - klog.Warningf("refreshZones: error when get zones: %s, will retry after %s", rerr.Error().Error(), consts.ZoneFetchingInterval.String()) + klog.Warningf("syncRegionZonesMap: error when get zones: %s, will retry after %s", rerr.Error().Error(), consts.ZoneFetchingInterval.String()) return rerr.Error() } if len(zones) == 0 { - klog.Warningf("refreshZones: empty zone list, will retry after %s", consts.ZoneFetchingInterval.String()) - return fmt.Errorf("empty zone list") + klog.Warning("syncRegionZonesMap: empty zone list") } az.updateRegionZonesMap(zones) @@ -89,25 +90,34 @@ func (az *Cloud) getRegionZonesBackoff(region string) ([]string, error) { klog.V(2).Infof("getRegionZonesMapWrapper: the region-zones map is not initialized successfully, retrying immediately") + var ( + zones map[string][]string + rerr *retry.Error + ) err := wait.ExponentialBackoff(az.RequestBackoff(), func() (done bool, err error) { - zones, rerr := az.ZoneClient.GetZones(context.Background(), az.SubscriptionID) - if len(zones) == 0 || rerr != nil { - klog.Warningf("getRegionZonesMapWrapper: failed to fetch zones information: %v", rerr.Error()) + zones, rerr = az.ZoneClient.GetZones(context.Background(), az.SubscriptionID) + if rerr != nil { + klog.Errorf("getRegionZonesMapWrapper: failed to fetch zones information: %v", rerr.Error()) return false, nil } - az.updateRegionZonesMap(zones) return true, nil }) - if err != nil { - return []string{}, fmt.Errorf("cannot get zones information of %s after %d time retry", region, az.RequestBackoff().Steps) + if errors.Is(err, wait.ErrWaitTimeout) { + return []string{}, rerr.Error() } - az.refreshZonesLock.RLock() - defer az.refreshZonesLock.RUnlock() + az.updateRegionZonesMap(zones) + + if len(az.regionZonesMap) != 0 { + az.refreshZonesLock.RLock() + defer az.refreshZonesLock.RUnlock() + + return az.regionZonesMap[region], nil + } - return az.regionZonesMap[region], nil + return []string{}, nil } // makeZone returns the zone value in format of -. diff --git a/pkg/provider/azure_zones_test.go b/pkg/provider/azure_zones_test.go index 8040bb1311..65cecf8807 100644 --- a/pkg/provider/azure_zones_test.go +++ b/pkg/provider/azure_zones_test.go @@ -23,6 +23,8 @@ import ( "net/http" "testing" + "sigs.k8s.io/cloud-provider-azure/pkg/azureclients/zoneclient/mockzoneclient" + "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2020-12-01/compute" "github.com/Azure/go-autorest/autorest/to" "github.com/golang/mock/gomock" @@ -32,6 +34,7 @@ import ( cloudprovider "k8s.io/cloud-provider" "sigs.k8s.io/cloud-provider-azure/pkg/azureclients/vmclient/mockvmclient" + "sigs.k8s.io/cloud-provider-azure/pkg/retry" ) const ( @@ -241,3 +244,77 @@ func TestAvailabilitySetGetZoneByNodeName(t *testing.T) { assert.Equal(t, fmt.Errorf("node informer is not synced when trying to GetUnmanagedNodes"), err) assert.Equal(t, cloudprovider.Zone{}, zone) } + +func TestSyncRegionZonesMap(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + for _, testCase := range []struct { + description string + zones map[string][]string + getZonesErr *retry.Error + expectedZones map[string][]string + expectedErr error + }{ + { + description: "syncRegionZonesMap should report an error if failed to get zones", + getZonesErr: retry.NewError(false, fmt.Errorf("error")), + expectedErr: fmt.Errorf("Retriable: false, RetryAfter: 0s, HTTPStatusCode: 0, RawError: error"), + }, + { + description: "syncRegionZonesMap should not report an error if the given zones map is empty", + expectedZones: make(map[string][]string), + }, + } { + t.Run(testCase.description, func(t *testing.T) { + az := &Cloud{ + ZoneClient: mockzoneclient.NewMockInterface(ctrl), + } + mockZoneClient := az.ZoneClient.(*mockzoneclient.MockInterface) + mockZoneClient.EXPECT().GetZones(gomock.Any(), gomock.Any()).Return(testCase.expectedZones, testCase.getZonesErr) + + err := az.syncRegionZonesMap() + if err != nil { + assert.EqualError(t, testCase.expectedErr, err.Error()) + } + assert.Equal(t, testCase.expectedZones, az.regionZonesMap) + }) + } +} + +func TestGetRegionZonesBackoff(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + for _, testCase := range []struct { + description string + zones map[string][]string + getZonesErr *retry.Error + expectedZones map[string][]string + expectedErr error + }{ + { + description: "syncRegionZonesMap should report an error if failed to get zones", + getZonesErr: retry.NewError(false, fmt.Errorf("error")), + expectedErr: fmt.Errorf("Retriable: false, RetryAfter: 0s, HTTPStatusCode: 0, RawError: error"), + }, + { + description: "syncRegionZonesMap should not report an error if the given zones map is empty", + expectedZones: make(map[string][]string), + }, + } { + t.Run(testCase.description, func(t *testing.T) { + az := &Cloud{ + ZoneClient: mockzoneclient.NewMockInterface(ctrl), + } + mockZoneClient := az.ZoneClient.(*mockzoneclient.MockInterface) + mockZoneClient.EXPECT().GetZones(gomock.Any(), gomock.Any()).Return(testCase.expectedZones, testCase.getZonesErr) + + _, err := az.getRegionZonesBackoff("eastus2") + if err != nil { + assert.EqualError(t, testCase.expectedErr, err.Error()) + } + assert.Equal(t, testCase.expectedZones, az.regionZonesMap) + }) + } +}