Skip to content

Commit b9bde60

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#77490 from feiskyer/azure-lb-route-race
Fix race conditions for Azure loadbalancer and route updates
2 parents e9af72c + 7ca1c83 commit b9bde60

File tree

5 files changed

+191
-37
lines changed

5 files changed

+191
-37
lines changed

‎staging/src/k8s.io/legacy-cloud-providers/azure/azure_backoff.go‎

Lines changed: 49 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ func (az *Cloud) CreateOrUpdateLB(service *v1.Service, lb network.LoadBalancer)
197197
ctx, cancel := getContextWithCancel()
198198
defer cancel()
199199

200-
resp, err := az.LoadBalancerClient.CreateOrUpdate(ctx, az.ResourceGroup, *lb.Name, lb)
200+
resp, err := az.LoadBalancerClient.CreateOrUpdate(ctx, az.ResourceGroup, *lb.Name, lb, to.String(lb.Etag))
201201
klog.V(10).Infof("LoadBalancerClient.CreateOrUpdate(%s): end", *lb.Name)
202202
if err == nil {
203203
if isSuccessHTTPResponse(resp) {
@@ -207,6 +207,11 @@ func (az *Cloud) CreateOrUpdateLB(service *v1.Service, lb network.LoadBalancer)
207207
return fmt.Errorf("HTTP response %q", resp.Status)
208208
}
209209
}
210+
211+
// Invalidate the cache because ETAG precondition mismatch.
212+
if resp != nil && resp.StatusCode == http.StatusPreconditionFailed {
213+
az.lbCache.Delete(*lb.Name)
214+
}
210215
return err
211216
}
212217

@@ -219,14 +224,20 @@ func (az *Cloud) createOrUpdateLBWithRetry(service *v1.Service, lb network.LoadB
219224
ctx, cancel := getContextWithCancel()
220225
defer cancel()
221226

222-
resp, err := az.LoadBalancerClient.CreateOrUpdate(ctx, az.ResourceGroup, *lb.Name, lb)
227+
resp, err := az.LoadBalancerClient.CreateOrUpdate(ctx, az.ResourceGroup, *lb.Name, lb, to.String(lb.Etag))
223228
klog.V(10).Infof("LoadBalancerClient.CreateOrUpdate(%s): end", *lb.Name)
224-
done, err := az.processHTTPRetryResponse(service, "CreateOrUpdateLoadBalancer", resp, err)
229+
done, retryError := az.processHTTPRetryResponse(service, "CreateOrUpdateLoadBalancer", resp, err)
225230
if done && err == nil {
226231
// Invalidate the cache right after updating
227232
az.lbCache.Delete(*lb.Name)
228233
}
229-
return done, err
234+
235+
// Invalidate the cache and abort backoff because ETAG precondition mismatch.
236+
if resp != nil && resp.StatusCode == http.StatusPreconditionFailed {
237+
az.nsgCache.Delete(*lb.Name)
238+
return true, err
239+
}
240+
return done, retryError
230241
})
231242
}
232243

@@ -441,7 +452,10 @@ func (az *Cloud) CreateOrUpdateRouteTable(routeTable network.RouteTable) error {
441452
ctx, cancel := getContextWithCancel()
442453
defer cancel()
443454

444-
resp, err := az.RouteTablesClient.CreateOrUpdate(ctx, az.RouteTableResourceGroup, az.RouteTableName, routeTable)
455+
resp, err := az.RouteTablesClient.CreateOrUpdate(ctx, az.RouteTableResourceGroup, az.RouteTableName, routeTable, to.String(routeTable.Etag))
456+
if resp != nil && resp.StatusCode == http.StatusPreconditionFailed {
457+
az.rtCache.Delete(*routeTable.Name)
458+
}
445459
return az.processHTTPResponse(nil, "", resp, err)
446460
}
447461

@@ -454,8 +468,19 @@ func (az *Cloud) createOrUpdateRouteTableWithRetry(routeTable network.RouteTable
454468
ctx, cancel := getContextWithCancel()
455469
defer cancel()
456470

457-
resp, err := az.RouteTablesClient.CreateOrUpdate(ctx, az.RouteTableResourceGroup, az.RouteTableName, routeTable)
458-
return az.processHTTPRetryResponse(nil, "", resp, err)
471+
resp, err := az.RouteTablesClient.CreateOrUpdate(ctx, az.RouteTableResourceGroup, az.RouteTableName, routeTable, to.String(routeTable.Etag))
472+
done, retryError := az.processHTTPRetryResponse(nil, "", resp, err)
473+
if done && err == nil {
474+
az.rtCache.Delete(*routeTable.Name)
475+
return done, nil
476+
}
477+
478+
// Invalidate the cache and abort backoff because ETAG precondition mismatch.
479+
if resp != nil && resp.StatusCode == http.StatusPreconditionFailed {
480+
az.rtCache.Delete(*routeTable.Name)
481+
return true, err
482+
}
483+
return done, retryError
459484
})
460485
}
461486

@@ -465,8 +490,11 @@ func (az *Cloud) CreateOrUpdateRoute(route network.Route) error {
465490
ctx, cancel := getContextWithCancel()
466491
defer cancel()
467492

468-
resp, err := az.RoutesClient.CreateOrUpdate(ctx, az.RouteTableResourceGroup, az.RouteTableName, *route.Name, route)
493+
resp, err := az.RoutesClient.CreateOrUpdate(ctx, az.RouteTableResourceGroup, az.RouteTableName, *route.Name, route, to.String(route.Etag))
469494
klog.V(10).Infof("RoutesClient.CreateOrUpdate(%s): end", *route.Name)
495+
if resp != nil && resp.StatusCode == http.StatusPreconditionFailed {
496+
az.rtCache.Delete(az.RouteTableName)
497+
}
470498
return az.processHTTPResponse(nil, "", resp, err)
471499
}
472500

@@ -479,9 +507,20 @@ func (az *Cloud) createOrUpdateRouteWithRetry(route network.Route) error {
479507
ctx, cancel := getContextWithCancel()
480508
defer cancel()
481509

482-
resp, err := az.RoutesClient.CreateOrUpdate(ctx, az.RouteTableResourceGroup, az.RouteTableName, *route.Name, route)
510+
resp, err := az.RoutesClient.CreateOrUpdate(ctx, az.RouteTableResourceGroup, az.RouteTableName, *route.Name, route, to.String(route.Etag))
483511
klog.V(10).Infof("RoutesClient.CreateOrUpdate(%s): end", *route.Name)
484-
return az.processHTTPRetryResponse(nil, "", resp, err)
512+
done, retryError := az.processHTTPRetryResponse(nil, "", resp, err)
513+
if done && err == nil {
514+
az.rtCache.Delete(az.RouteTableName)
515+
return done, nil
516+
}
517+
518+
// Invalidate the cache and abort backoff because ETAG precondition mismatch.
519+
if resp != nil && resp.StatusCode == http.StatusPreconditionFailed {
520+
az.rtCache.Delete(az.RouteTableName)
521+
return true, err
522+
}
523+
return done, retryError
485524
})
486525
}
487526

‎staging/src/k8s.io/legacy-cloud-providers/azure/azure_client.go‎

Lines changed: 118 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ import (
3232
"k8s.io/client-go/util/flowcontrol"
3333
)
3434

35+
const (
36+
// The version number is taken from "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2017-09-01/network".
37+
azureNetworkAPIVersion = "2017-09-01"
38+
)
39+
3540
// Helpers for rate limiting error/error channel creation
3641
func createRateLimitErr(isWrite bool, opName string) error {
3742
opType := "read"
@@ -57,7 +62,7 @@ type InterfacesClient interface {
5762

5863
// LoadBalancersClient defines needed functions for azure network.LoadBalancersClient
5964
type LoadBalancersClient interface {
60-
CreateOrUpdate(ctx context.Context, resourceGroupName string, loadBalancerName string, parameters network.LoadBalancer) (resp *http.Response, err error)
65+
CreateOrUpdate(ctx context.Context, resourceGroupName string, loadBalancerName string, parameters network.LoadBalancer, etag string) (resp *http.Response, err error)
6166
Delete(ctx context.Context, resourceGroupName string, loadBalancerName string) (resp *http.Response, err error)
6267
Get(ctx context.Context, resourceGroupName string, loadBalancerName string, expand string) (result network.LoadBalancer, err error)
6368
List(ctx context.Context, resourceGroupName string) (result []network.LoadBalancer, err error)
@@ -103,13 +108,13 @@ type VirtualMachineScaleSetVMsClient interface {
103108

104109
// RoutesClient defines needed functions for azure network.RoutesClient
105110
type RoutesClient interface {
106-
CreateOrUpdate(ctx context.Context, resourceGroupName string, routeTableName string, routeName string, routeParameters network.Route) (resp *http.Response, err error)
111+
CreateOrUpdate(ctx context.Context, resourceGroupName string, routeTableName string, routeName string, routeParameters network.Route, etag string) (resp *http.Response, err error)
107112
Delete(ctx context.Context, resourceGroupName string, routeTableName string, routeName string) (resp *http.Response, err error)
108113
}
109114

110115
// RouteTablesClient defines needed functions for azure network.RouteTablesClient
111116
type RouteTablesClient interface {
112-
CreateOrUpdate(ctx context.Context, resourceGroupName string, routeTableName string, parameters network.RouteTable) (resp *http.Response, err error)
117+
CreateOrUpdate(ctx context.Context, resourceGroupName string, routeTableName string, parameters network.RouteTable, etag string) (resp *http.Response, err error)
113118
Get(ctx context.Context, resourceGroupName string, routeTableName string, expand string) (result network.RouteTable, err error)
114119
}
115120

@@ -356,7 +361,7 @@ func newAzLoadBalancersClient(config *azClientConfig) *azLoadBalancersClient {
356361
}
357362
}
358363

359-
func (az *azLoadBalancersClient) CreateOrUpdate(ctx context.Context, resourceGroupName string, loadBalancerName string, parameters network.LoadBalancer) (resp *http.Response, err error) {
364+
func (az *azLoadBalancersClient) CreateOrUpdate(ctx context.Context, resourceGroupName string, loadBalancerName string, parameters network.LoadBalancer, etag string) (resp *http.Response, err error) {
360365
/* Write rate limiting */
361366
if !az.rateLimiterWriter.TryAccept() {
362367
err = createRateLimitErr(true, "LBCreateOrUpdate")
@@ -369,9 +374,15 @@ func (az *azLoadBalancersClient) CreateOrUpdate(ctx context.Context, resourceGro
369374
}()
370375

371376
mc := newMetricContext("load_balancers", "create_or_update", resourceGroupName, az.client.SubscriptionID)
372-
future, err := az.client.CreateOrUpdate(ctx, resourceGroupName, loadBalancerName, parameters)
373-
mc.Observe(err)
377+
req, err := az.createOrUpdatePreparer(ctx, resourceGroupName, loadBalancerName, parameters, etag)
374378
if err != nil {
379+
mc.Observe(err)
380+
return nil, err
381+
}
382+
383+
future, err := az.client.CreateOrUpdateSender(req)
384+
if err != nil {
385+
mc.Observe(err)
375386
return future.Response(), err
376387
}
377388

@@ -380,6 +391,33 @@ func (az *azLoadBalancersClient) CreateOrUpdate(ctx context.Context, resourceGro
380391
return future.Response(), err
381392
}
382393

394+
// createOrUpdatePreparer prepares the CreateOrUpdate request.
395+
func (az *azLoadBalancersClient) createOrUpdatePreparer(ctx context.Context, resourceGroupName string, loadBalancerName string, parameters network.LoadBalancer, etag string) (*http.Request, error) {
396+
pathParameters := map[string]interface{}{
397+
"loadBalancerName": autorest.Encode("path", loadBalancerName),
398+
"resourceGroupName": autorest.Encode("path", resourceGroupName),
399+
"subscriptionId": autorest.Encode("path", az.client.SubscriptionID),
400+
}
401+
402+
queryParameters := map[string]interface{}{
403+
"api-version": azureNetworkAPIVersion,
404+
}
405+
406+
preparerDecorators := []autorest.PrepareDecorator{
407+
autorest.AsContentType("application/json; charset=utf-8"),
408+
autorest.AsPut(),
409+
autorest.WithBaseURL(az.client.BaseURI),
410+
autorest.WithPathParameters("/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Network/loadBalancers/{loadBalancerName}", pathParameters),
411+
autorest.WithJSON(parameters),
412+
autorest.WithQueryParameters(queryParameters),
413+
}
414+
if etag != "" {
415+
preparerDecorators = append(preparerDecorators, autorest.WithHeader("If-Match", autorest.String(etag)))
416+
}
417+
preparer := autorest.CreatePreparer(preparerDecorators...)
418+
return preparer.Prepare((&http.Request{}).WithContext(ctx))
419+
}
420+
383421
func (az *azLoadBalancersClient) Delete(ctx context.Context, resourceGroupName string, loadBalancerName string) (resp *http.Response, err error) {
384422
/* Write rate limiting */
385423
if !az.rateLimiterWriter.TryAccept() {
@@ -752,9 +790,8 @@ func (az *azSecurityGroupsClient) createOrUpdatePreparer(ctx context.Context, re
752790
"subscriptionId": autorest.Encode("path", az.client.SubscriptionID),
753791
}
754792

755-
const APIVersion = "2017-09-01"
756793
queryParameters := map[string]interface{}{
757-
"api-version": APIVersion,
794+
"api-version": azureNetworkAPIVersion,
758795
}
759796

760797
preparerDecorators := []autorest.PrepareDecorator{
@@ -1051,7 +1088,7 @@ func newAzRoutesClient(config *azClientConfig) *azRoutesClient {
10511088
}
10521089
}
10531090

1054-
func (az *azRoutesClient) CreateOrUpdate(ctx context.Context, resourceGroupName string, routeTableName string, routeName string, routeParameters network.Route) (resp *http.Response, err error) {
1091+
func (az *azRoutesClient) CreateOrUpdate(ctx context.Context, resourceGroupName string, routeTableName string, routeName string, routeParameters network.Route, etag string) (resp *http.Response, err error) {
10551092
/* Write rate limiting */
10561093
if !az.rateLimiterWriter.TryAccept() {
10571094
err = createRateLimitErr(true, "RouteCreateOrUpdate")
@@ -1064,7 +1101,13 @@ func (az *azRoutesClient) CreateOrUpdate(ctx context.Context, resourceGroupName
10641101
}()
10651102

10661103
mc := newMetricContext("routes", "create_or_update", resourceGroupName, az.client.SubscriptionID)
1067-
future, err := az.client.CreateOrUpdate(ctx, resourceGroupName, routeTableName, routeName, routeParameters)
1104+
req, err := az.createOrUpdatePreparer(ctx, resourceGroupName, routeTableName, routeName, routeParameters, etag)
1105+
if err != nil {
1106+
mc.Observe(err)
1107+
return nil, err
1108+
}
1109+
1110+
future, err := az.client.CreateOrUpdateSender(req)
10681111
if err != nil {
10691112
mc.Observe(err)
10701113
return future.Response(), err
@@ -1075,6 +1118,35 @@ func (az *azRoutesClient) CreateOrUpdate(ctx context.Context, resourceGroupName
10751118
return future.Response(), err
10761119
}
10771120

1121+
// createOrUpdatePreparer prepares the CreateOrUpdate request.
1122+
func (az *azRoutesClient) createOrUpdatePreparer(ctx context.Context, resourceGroupName string, routeTableName string, routeName string, routeParameters network.Route, etag string) (*http.Request, error) {
1123+
pathParameters := map[string]interface{}{
1124+
"resourceGroupName": autorest.Encode("path", resourceGroupName),
1125+
"routeName": autorest.Encode("path", routeName),
1126+
"routeTableName": autorest.Encode("path", routeTableName),
1127+
"subscriptionId": autorest.Encode("path", az.client.SubscriptionID),
1128+
}
1129+
1130+
queryParameters := map[string]interface{}{
1131+
"api-version": azureNetworkAPIVersion,
1132+
}
1133+
1134+
preparerDecorators := []autorest.PrepareDecorator{
1135+
autorest.AsContentType("application/json; charset=utf-8"),
1136+
autorest.AsPut(),
1137+
autorest.WithBaseURL(az.client.BaseURI),
1138+
autorest.WithPathParameters("/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Network/routeTables/{routeTableName}/routes/{routeName}", pathParameters),
1139+
autorest.WithJSON(routeParameters),
1140+
autorest.WithQueryParameters(queryParameters),
1141+
}
1142+
if etag != "" {
1143+
preparerDecorators = append(preparerDecorators, autorest.WithHeader("If-Match", autorest.String(etag)))
1144+
}
1145+
preparer := autorest.CreatePreparer(preparerDecorators...)
1146+
1147+
return preparer.Prepare((&http.Request{}).WithContext(ctx))
1148+
}
1149+
10781150
func (az *azRoutesClient) Delete(ctx context.Context, resourceGroupName string, routeTableName string, routeName string) (resp *http.Response, err error) {
10791151
/* Write rate limiting */
10801152
if !az.rateLimiterWriter.TryAccept() {
@@ -1124,7 +1196,7 @@ func newAzRouteTablesClient(config *azClientConfig) *azRouteTablesClient {
11241196
}
11251197
}
11261198

1127-
func (az *azRouteTablesClient) CreateOrUpdate(ctx context.Context, resourceGroupName string, routeTableName string, parameters network.RouteTable) (resp *http.Response, err error) {
1199+
func (az *azRouteTablesClient) CreateOrUpdate(ctx context.Context, resourceGroupName string, routeTableName string, parameters network.RouteTable, etag string) (resp *http.Response, err error) {
11281200
/* Write rate limiting */
11291201
if !az.rateLimiterWriter.TryAccept() {
11301202
err = createRateLimitErr(true, "RouteTableCreateOrUpdate")
@@ -1137,9 +1209,15 @@ func (az *azRouteTablesClient) CreateOrUpdate(ctx context.Context, resourceGroup
11371209
}()
11381210

11391211
mc := newMetricContext("route_tables", "create_or_update", resourceGroupName, az.client.SubscriptionID)
1140-
future, err := az.client.CreateOrUpdate(ctx, resourceGroupName, routeTableName, parameters)
1141-
mc.Observe(err)
1212+
req, err := az.createOrUpdatePreparer(ctx, resourceGroupName, routeTableName, parameters, etag)
11421213
if err != nil {
1214+
mc.Observe(err)
1215+
return nil, err
1216+
}
1217+
1218+
future, err := az.client.CreateOrUpdateSender(req)
1219+
if err != nil {
1220+
mc.Observe(err)
11431221
return future.Response(), err
11441222
}
11451223

@@ -1148,6 +1226,33 @@ func (az *azRouteTablesClient) CreateOrUpdate(ctx context.Context, resourceGroup
11481226
return future.Response(), err
11491227
}
11501228

1229+
// createOrUpdatePreparer prepares the CreateOrUpdate request.
1230+
func (az *azRouteTablesClient) createOrUpdatePreparer(ctx context.Context, resourceGroupName string, routeTableName string, parameters network.RouteTable, etag string) (*http.Request, error) {
1231+
pathParameters := map[string]interface{}{
1232+
"resourceGroupName": autorest.Encode("path", resourceGroupName),
1233+
"routeTableName": autorest.Encode("path", routeTableName),
1234+
"subscriptionId": autorest.Encode("path", az.client.SubscriptionID),
1235+
}
1236+
1237+
queryParameters := map[string]interface{}{
1238+
"api-version": azureNetworkAPIVersion,
1239+
}
1240+
preparerDecorators := []autorest.PrepareDecorator{
1241+
autorest.AsContentType("application/json; charset=utf-8"),
1242+
autorest.AsPut(),
1243+
autorest.WithBaseURL(az.client.BaseURI),
1244+
autorest.WithPathParameters("/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Network/routeTables/{routeTableName}", pathParameters),
1245+
autorest.WithJSON(parameters),
1246+
autorest.WithQueryParameters(queryParameters),
1247+
}
1248+
if etag != "" {
1249+
preparerDecorators = append(preparerDecorators, autorest.WithHeader("If-Match", autorest.String(etag)))
1250+
}
1251+
preparer := autorest.CreatePreparer(preparerDecorators...)
1252+
1253+
return preparer.Prepare((&http.Request{}).WithContext(ctx))
1254+
}
1255+
11511256
func (az *azRouteTablesClient) Get(ctx context.Context, resourceGroupName string, routeTableName string, expand string) (result network.RouteTable, err error) {
11521257
if !az.rateLimiterReader.TryAccept() {
11531258
err = createRateLimitErr(false, "GetRouteTable")

‎staging/src/k8s.io/legacy-cloud-providers/azure/azure_fakes.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func newFakeAzureLBClient() *fakeAzureLBClient {
5252
return fLBC
5353
}
5454

55-
func (fLBC *fakeAzureLBClient) CreateOrUpdate(ctx context.Context, resourceGroupName string, loadBalancerName string, parameters network.LoadBalancer) (resp *http.Response, err error) {
55+
func (fLBC *fakeAzureLBClient) CreateOrUpdate(ctx context.Context, resourceGroupName string, loadBalancerName string, parameters network.LoadBalancer, etag string) (resp *http.Response, err error) {
5656
fLBC.mutex.Lock()
5757
defer fLBC.mutex.Unlock()
5858

@@ -642,7 +642,7 @@ func newFakeRoutesClient() *fakeRoutesClient {
642642
return fRC
643643
}
644644

645-
func (fRC *fakeRoutesClient) CreateOrUpdate(ctx context.Context, resourceGroupName string, routeTableName string, routeName string, routeParameters network.Route) (resp *http.Response, err error) {
645+
func (fRC *fakeRoutesClient) CreateOrUpdate(ctx context.Context, resourceGroupName string, routeTableName string, routeName string, routeParameters network.Route, etag string) (resp *http.Response, err error) {
646646
fRC.mutex.Lock()
647647
defer fRC.mutex.Unlock()
648648

@@ -683,7 +683,7 @@ func newFakeRouteTablesClient() *fakeRouteTablesClient {
683683
return fRTC
684684
}
685685

686-
func (fRTC *fakeRouteTablesClient) CreateOrUpdate(ctx context.Context, resourceGroupName string, routeTableName string, parameters network.RouteTable) (resp *http.Response, err error) {
686+
func (fRTC *fakeRouteTablesClient) CreateOrUpdate(ctx context.Context, resourceGroupName string, routeTableName string, parameters network.RouteTable, etag string) (resp *http.Response, err error) {
687687
fRTC.mutex.Lock()
688688
defer fRTC.mutex.Unlock()
689689

0 commit comments

Comments
 (0)