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.

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

fix: do not crash if the region does not support zones #854

Merged
merged 1 commit into from Oct 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
36 changes: 23 additions & 13 deletions pkg/provider/azure_zones.go
Expand Up @@ -18,6 +18,7 @@ package provider

import (
"context"
"errors"
"fmt"
"os"
"strconv"
Expand All @@ -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) {
Expand All @@ -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)
Expand Down Expand Up @@ -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 <region>-<zone-id>.
Expand Down
77 changes: 77 additions & 0 deletions pkg/provider/azure_zones_test.go
Expand Up @@ -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"
Expand All @@ -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 (
Expand Down Expand Up @@ -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)
})
}
}