Merge pull request #2238 from CecileRobertMichon/result-not-found-error

Delete long running operation state when resource is not found
kubernetes-sigs · Apr 20, 2022 · d1c2f87 · d1c2f87
2 parents 90cba6b + df7adf0
commit d1c2f87
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 2 deletions.
diff --git a/azure/services/async/async.go b/azure/services/async/async.go
@@ -79,7 +79,10 @@ func processOngoingOperation(ctx context.Context, scope FutureScope, client Futu
 	// Resource has been created/deleted/updated.
 	log.V(2).Info("long running operation has completed", "service", serviceName, "resource", resourceName)
 	result, err = client.Result(ctx, sdkFuture, future.Type)
-	if err == nil {
+	if err == nil || azure.ResourceNotFound(err) {
+		// Once we have the result, we can delete the long running operation state.
+		// If the resource is not found, we also reset the long-running operation state so we can attempt to create it again.
+		// This can happen if the resource was deleted by another process before we could get the result.
 		scope.DeleteLongRunningOperationState(resourceName, serviceName)
 	}
 	return result, err

diff --git a/azure/services/async/async_test.go b/azure/services/async/async_test.go
@@ -120,8 +120,33 @@ func TestProcessOngoingOperation(t *testing.T) {
 			expect: func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockFutureHandlerMockRecorder) {
 				s.GetLongRunningOperationState("test-resource", "test-service").Return(&validDeleteFuture)
 				c.IsDone(gomockinternal.AContext(), gomock.AssignableToTypeOf(&azureautorest.Future{})).Return(true, nil)
-				s.DeleteLongRunningOperationState("test-resource", "test-service")
 				c.Result(gomockinternal.AContext(), gomock.AssignableToTypeOf(&azureautorest.Future{}), infrav1.DeleteFuture).Return(&fakeExistingResource, nil)
+				s.DeleteLongRunningOperationState("test-resource", "test-service")
+			},
+		},
+		{
+			name:           "resource was deleted by an external process",
+			expectedError:  fakeNotFoundError.Error(),
+			expectedResult: nil,
+			resourceName:   "test-resource",
+			serviceName:    "test-service",
+			expect: func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockFutureHandlerMockRecorder) {
+				s.GetLongRunningOperationState("test-resource", "test-service").Return(&validDeleteFuture)
+				c.IsDone(gomockinternal.AContext(), gomock.AssignableToTypeOf(&azureautorest.Future{})).Return(true, nil)
+				c.Result(gomockinternal.AContext(), gomock.AssignableToTypeOf(&azureautorest.Future{}), infrav1.DeleteFuture).Return(nil, fakeNotFoundError)
+				s.DeleteLongRunningOperationState("test-resource", "test-service")
+			},
+		},
+		{
+			name:           "failed to get resulting resource",
+			expectedError:  fakeInternalError.Error(),
+			expectedResult: nil,
+			resourceName:   "test-resource",
+			serviceName:    "test-service",
+			expect: func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockFutureHandlerMockRecorder) {
+				s.GetLongRunningOperationState("test-resource", "test-service").Return(&validDeleteFuture)
+				c.IsDone(gomockinternal.AContext(), gomock.AssignableToTypeOf(&azureautorest.Future{})).Return(true, nil)
+				c.Result(gomockinternal.AContext(), gomock.AssignableToTypeOf(&azureautorest.Future{}), infrav1.DeleteFuture).Return(nil, fakeInternalError)
 			},
 		},
 	}