From 7078375e6dbb295e5b9d8d3ec30d52d1ba78870e Mon Sep 17 00:00:00 2001 From: Matt Boersma Date: Wed, 23 Aug 2023 13:22:39 -0600 Subject: [PATCH 1/2] Add asyncpoller framework for azure-sdk-for-go v2 --- azure/services/asyncpoller/asyncpoller.go | 194 +++++++++++ .../services/asyncpoller/asyncpoller_test.go | 309 +++++++++++++++++ azure/services/asyncpoller/interfaces.go | 51 +++ .../mock_asyncpoller/asyncpoller_mock.go | 311 ++++++++++++++++++ .../asyncpoller/mock_asyncpoller/doc.go | 21 ++ 5 files changed, 886 insertions(+) create mode 100644 azure/services/asyncpoller/asyncpoller.go create mode 100644 azure/services/asyncpoller/asyncpoller_test.go create mode 100644 azure/services/asyncpoller/interfaces.go create mode 100644 azure/services/asyncpoller/mock_asyncpoller/asyncpoller_mock.go create mode 100644 azure/services/asyncpoller/mock_asyncpoller/doc.go diff --git a/azure/services/asyncpoller/asyncpoller.go b/azure/services/asyncpoller/asyncpoller.go new file mode 100644 index 00000000000..390b3e29704 --- /dev/null +++ b/azure/services/asyncpoller/asyncpoller.go @@ -0,0 +1,194 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package asyncpoller + +import ( + "context" + "fmt" + "net/http" + "strconv" + "time" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/pkg/errors" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" + "sigs.k8s.io/cluster-api-provider-azure/azure" + "sigs.k8s.io/cluster-api-provider-azure/azure/converters" + "sigs.k8s.io/cluster-api-provider-azure/util/reconciler" + "sigs.k8s.io/cluster-api-provider-azure/util/tele" +) + +const ( + // DefaultPollerFrequency is how often a poller should check for completion, in seconds. + DefaultPollerFrequency = 1 * time.Second +) + +// Service handles asynchronous creation and deletion of resources. It implements the Reconciler interface. +type Service[C, D any] struct { + Scope FutureScope + Creator[C] + Deleter[D] +} + +// New creates an async Service. +func New[C, D any](scope FutureScope, createClient Creator[C], deleteClient Deleter[D]) *Service[C, D] { + return &Service[C, D]{ + Scope: scope, + Creator: createClient, + Deleter: deleteClient, + } +} + +// CreateOrUpdateResource creates a new resource or updates an existing one asynchronously. +func (s *Service[C, D]) CreateOrUpdateResource(ctx context.Context, spec azure.ResourceSpecGetter, serviceName string) (result interface{}, err error) { + ctx, log, done := tele.StartSpanWithLogger(ctx, "asyncpoller.Service.CreateOrUpdateResource") + defer done() + + resourceName := spec.ResourceName() + rgName := spec.ResourceGroupName() + futureType := infrav1.PutFuture + + // Check if there is an ongoing long-running operation. + resumeToken := "" + if future := s.Scope.GetLongRunningOperationState(resourceName, serviceName, futureType); future != nil { + t, err := converters.FutureToResumeToken(*future) + if err != nil { + s.Scope.DeleteLongRunningOperationState(resourceName, serviceName, futureType) + return "", errors.Wrap(err, "could not decode future data, resetting long-running operation state") + } + resumeToken = t + } + + // Get the resource if it already exists, and use it to construct the desired resource parameters. + var existingResource interface{} + if existing, err := s.Creator.Get(ctx, spec); err != nil && !azure.ResourceNotFound(err) { + errWrapped := errors.Wrapf(err, "failed to get existing resource %s/%s (service: %s)", rgName, resourceName, serviceName) + return nil, azure.WithTransientError(errWrapped, getRetryAfterFromError(err)) + } else if err == nil { + existingResource = existing + log.V(2).Info("successfully got existing resource", "service", serviceName, "resource", resourceName, "resourceGroup", rgName) + } + + // Construct parameters using the resource spec and information from the existing resource, if there is one. + parameters, err := spec.Parameters(ctx, existingResource) + if err != nil { + return nil, errors.Wrapf(err, "failed to get desired parameters for resource %s/%s (service: %s)", rgName, resourceName, serviceName) + } else if parameters == nil { + // Nothing to do, don't create or update the resource and return the existing resource. + log.V(2).Info("resource up to date", "service", serviceName, "resource", resourceName, "resourceGroup", rgName) + return existingResource, nil + } + + // Create or update the resource with the desired parameters. + logMessageVerbPrefix := "creat" + if existingResource != nil { + logMessageVerbPrefix = "updat" + } + log.V(2).Info(fmt.Sprintf("%sing resource", logMessageVerbPrefix), "service", serviceName, "resource", resourceName, "resourceGroup", rgName) + result, poller, err := s.Creator.CreateOrUpdateAsync(ctx, spec, resumeToken, parameters) + errWrapped := errors.Wrapf(err, fmt.Sprintf("failed to %se resource %s/%s (service: %s)", logMessageVerbPrefix, rgName, resourceName, serviceName)) + if poller != nil { + future, err := converters.PollerToFuture(poller, infrav1.PutFuture, serviceName, resourceName, rgName) + if err != nil { + return nil, errWrapped + } + s.Scope.SetLongRunningOperationState(future) + return nil, azure.WithTransientError(azure.NewOperationNotDoneError(future), requeueTime()) + } else if err != nil { + return nil, errWrapped + } + + // Once the operation is done, delete the long-running operation state. + s.Scope.DeleteLongRunningOperationState(resourceName, serviceName, futureType) + + log.V(2).Info(fmt.Sprintf("successfully %sed resource", logMessageVerbPrefix), "service", serviceName, "resource", resourceName, "resourceGroup", rgName) + return result, nil +} + +// DeleteResource deletes a resource asynchronously. +func (s *Service[C, D]) DeleteResource(ctx context.Context, spec azure.ResourceSpecGetter, serviceName string) (err error) { + ctx, log, done := tele.StartSpanWithLogger(ctx, "asyncpoller.Service.DeleteResource") + defer done() + + resourceName := spec.ResourceName() + rgName := spec.ResourceGroupName() + futureType := infrav1.DeleteFuture + + // Check for an ongoing long-running operation. + resumeToken := "" + if future := s.Scope.GetLongRunningOperationState(resourceName, serviceName, futureType); future != nil { + t, err := converters.FutureToResumeToken(*future) + if err != nil { + s.Scope.DeleteLongRunningOperationState(resourceName, serviceName, futureType) + return errors.Wrap(err, "could not decode future data, resetting long-running operation state") + } + resumeToken = t + } + + // Delete the resource. + log.V(2).Info("deleting resource", "service", serviceName, "resource", resourceName, "resourceGroup", rgName) + poller, err := s.Deleter.DeleteAsync(ctx, spec, resumeToken) + if poller != nil { + future, err := converters.PollerToFuture(poller, infrav1.DeleteFuture, serviceName, resourceName, rgName) + if err != nil { + return errors.Wrap(err, "failed to convert poller to future") + } + s.Scope.SetLongRunningOperationState(future) + return azure.WithTransientError(azure.NewOperationNotDoneError(future), requeueTime()) + } else if err != nil && !azure.ResourceNotFound(err) { + return errors.Wrapf(err, "failed to delete resource %s/%s (service: %s)", rgName, resourceName, serviceName) + } + + // Once the operation is done, delete the long-running operation state. + s.Scope.DeleteLongRunningOperationState(resourceName, serviceName, futureType) + + log.V(2).Info("successfully deleted resource", "service", serviceName, "resource", resourceName, "resourceGroup", rgName) + return nil +} + +// requeueTime returns the time to wait before requeuing a reconciliation. +// It would be ideal to use the "retry-after" header from the API response, but +// that is not readily accessible in the SDK v2 Poller framework. +func requeueTime() time.Duration { + return reconciler.DefaultReconcilerRequeue +} + +// getRetryAfterFromError returns the time.Duration from the http.Response in the azcore.ResponseError. +// If there is no Response object, or if there is no meaningful Retry-After header data, it returns a default. +func getRetryAfterFromError(err error) time.Duration { + // In case we aren't able to introspect Retry-After from the error type, we'll return this default + ret := reconciler.DefaultReconcilerRequeue + var responseError *azcore.ResponseError + // if we have a strongly typed azcore.ResponseError then we can introspect the HTTP response data + if errors.As(err, &responseError) && responseError.RawResponse != nil { + // If we have Retry-After HTTP header data for any reason, prefer it + if retryAfter := responseError.RawResponse.Header.Get("Retry-After"); retryAfter != "" { + // This handles the case where Retry-After data is in the form of units of seconds + if rai, err := strconv.Atoi(retryAfter); err == nil { + ret = time.Duration(rai) * time.Second + // This handles the case where Retry-After data is in the form of absolute time + } else if t, err := time.Parse(time.RFC1123, retryAfter); err == nil { + ret = time.Until(t) + } + // If we didn't find Retry-After HTTP header data but the response type is 429, + // we'll have to come up with our sane default. + } else if responseError.RawResponse.StatusCode == http.StatusTooManyRequests { + ret = reconciler.DefaultHTTP429RetryAfter + } + } + return ret +} diff --git a/azure/services/asyncpoller/asyncpoller_test.go b/azure/services/asyncpoller/asyncpoller_test.go new file mode 100644 index 00000000000..39fb015a78f --- /dev/null +++ b/azure/services/asyncpoller/asyncpoller_test.go @@ -0,0 +1,309 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package asyncpoller + +import ( + "context" + "encoding/base64" + "errors" + "io" + "net/http" + "net/url" + "reflect" + "strings" + "testing" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources" + . "github.com/onsi/gomega" + "go.uber.org/mock/gomock" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" + "sigs.k8s.io/cluster-api-provider-azure/azure" + "sigs.k8s.io/cluster-api-provider-azure/azure/mock_azure" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/asyncpoller/mock_asyncpoller" + gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock" +) + +func TestServiceCreateOrUpdateResource(t *testing.T) { + testcases := []struct { + name string + serviceName string + expectedError string + expectedResult interface{} + expect func(g *WithT, s *mock_asyncpoller.MockFutureScopeMockRecorder, c *mock_asyncpoller.MockCreatorMockRecorder[MockCreator], r *mock_azure.MockResourceSpecGetterMockRecorder) + }{ + { + name: "invalid future", + serviceName: serviceName, + expectedError: "could not decode future data, resetting long-running operation state", + expect: func(g *WithT, s *mock_asyncpoller.MockFutureScopeMockRecorder, c *mock_asyncpoller.MockCreatorMockRecorder[MockCreator], r *mock_azure.MockResourceSpecGetterMockRecorder) { + gomock.InOrder( + r.ResourceName().Return(resourceName), + r.ResourceGroupName().Return(resourceGroupName), + s.GetLongRunningOperationState(resourceName, serviceName, infrav1.PutFuture).Return(invalidPutFuture), + s.DeleteLongRunningOperationState(resourceName, serviceName, infrav1.PutFuture), + ) + }, + }, + { + name: "operation completed", + serviceName: serviceName, + expectedError: "", + expectedResult: fakeResource, + expect: func(g *WithT, s *mock_asyncpoller.MockFutureScopeMockRecorder, c *mock_asyncpoller.MockCreatorMockRecorder[MockCreator], r *mock_azure.MockResourceSpecGetterMockRecorder) { + gomock.InOrder( + r.ResourceName().Return(resourceName), + r.ResourceGroupName().Return(resourceGroupName), + s.GetLongRunningOperationState(resourceName, serviceName, infrav1.PutFuture).Return(validPutFuture), + c.Get(gomockinternal.AContext(), gomock.AssignableToTypeOf(azureResourceGetterType)).Return(fakeResource, nil), + r.Parameters(gomockinternal.AContext(), fakeResource).Return(fakeParameters, nil), + c.CreateOrUpdateAsync(gomockinternal.AContext(), gomock.AssignableToTypeOf(azureResourceGetterType), resumeToken, gomock.Any()).Return(fakeResource, nil, nil), + s.DeleteLongRunningOperationState(resourceName, serviceName, infrav1.PutFuture), + ) + }, + }, + { + name: "operation in progress", + serviceName: serviceName, + expectedError: "operation type PUT on Azure resource mock-resourcegroup/mock-resource is not done. Object will be requeued after 15s", + expect: func(g *WithT, s *mock_asyncpoller.MockFutureScopeMockRecorder, c *mock_asyncpoller.MockCreatorMockRecorder[MockCreator], r *mock_azure.MockResourceSpecGetterMockRecorder) { + gomock.InOrder( + r.ResourceName().Return(resourceName), + r.ResourceGroupName().Return(resourceGroupName), + s.GetLongRunningOperationState(resourceName, serviceName, infrav1.PutFuture).Return(validPutFuture), + c.Get(gomockinternal.AContext(), gomock.AssignableToTypeOf(azureResourceGetterType)).Return(fakeResource, nil), + r.Parameters(gomockinternal.AContext(), fakeResource).Return(fakeParameters, nil), + c.CreateOrUpdateAsync(gomockinternal.AContext(), gomock.AssignableToTypeOf(azureResourceGetterType), resumeToken, gomock.Any()).Return(nil, fakePoller[MockCreator](g, http.StatusAccepted), nil), + s.SetLongRunningOperationState(gomock.AssignableToTypeOf(&infrav1.Future{})), + ) + }, + }, + { + name: "get returns resource not found error", + serviceName: serviceName, + expectedError: "operation type PUT on Azure resource mock-resourcegroup/mock-resource is not done. Object will be requeued after 15s", + expect: func(g *WithT, s *mock_asyncpoller.MockFutureScopeMockRecorder, c *mock_asyncpoller.MockCreatorMockRecorder[MockCreator], r *mock_azure.MockResourceSpecGetterMockRecorder) { + gomock.InOrder( + r.ResourceName().Return(resourceName), + r.ResourceGroupName().Return(resourceGroupName), + s.GetLongRunningOperationState(resourceName, serviceName, infrav1.PutFuture).Return(validPutFuture), + c.Get(gomockinternal.AContext(), gomock.AssignableToTypeOf(azureResourceGetterType)).Return(nil, &azcore.ResponseError{StatusCode: http.StatusNotFound}), + r.Parameters(gomockinternal.AContext(), nil).Return(fakeParameters, nil), + c.CreateOrUpdateAsync(gomockinternal.AContext(), gomock.AssignableToTypeOf(azureResourceGetterType), resumeToken, gomock.Any()).Return(nil, fakePoller[MockCreator](g, http.StatusAccepted), nil), + s.SetLongRunningOperationState(gomock.AssignableToTypeOf(&infrav1.Future{})), + ) + }, + }, + { + name: "get returns unexpected error", + serviceName: serviceName, + expectedError: "failed to get existing resource mock-resourcegroup/mock-resource (service: mock-service): foo. Object will be requeued after 15s", + expect: func(g *WithT, s *mock_asyncpoller.MockFutureScopeMockRecorder, c *mock_asyncpoller.MockCreatorMockRecorder[MockCreator], r *mock_azure.MockResourceSpecGetterMockRecorder) { + gomock.InOrder( + r.ResourceName().Return(resourceName), + r.ResourceGroupName().Return(resourceGroupName), + s.GetLongRunningOperationState(resourceName, serviceName, infrav1.PutFuture).Return(validPutFuture), + c.Get(gomockinternal.AContext(), gomock.AssignableToTypeOf(azureResourceGetterType)).Return(nil, errors.New("foo")), + ) + }, + }, + { + name: "parameters are nil: up to date", + serviceName: serviceName, + expectedError: "", + expectedResult: fakeResource, + expect: func(g *WithT, s *mock_asyncpoller.MockFutureScopeMockRecorder, c *mock_asyncpoller.MockCreatorMockRecorder[MockCreator], r *mock_azure.MockResourceSpecGetterMockRecorder) { + gomock.InOrder( + r.ResourceName().Return(resourceName), + r.ResourceGroupName().Return(resourceGroupName), + s.GetLongRunningOperationState(resourceName, serviceName, infrav1.PutFuture).Return(validPutFuture), + c.Get(gomockinternal.AContext(), gomock.AssignableToTypeOf(azureResourceGetterType)).Return(fakeResource, nil), + r.Parameters(gomockinternal.AContext(), fakeResource).Return(nil, nil), + ) + }, + }, + { + name: "parameters returns error", + serviceName: serviceName, + expectedError: "failed to get desired parameters for resource mock-resourcegroup/mock-resource (service: mock-service): foo", + expectedResult: nil, + expect: func(g *WithT, s *mock_asyncpoller.MockFutureScopeMockRecorder, c *mock_asyncpoller.MockCreatorMockRecorder[MockCreator], r *mock_azure.MockResourceSpecGetterMockRecorder) { + gomock.InOrder( + r.ResourceName().Return(resourceName), + r.ResourceGroupName().Return(resourceGroupName), + s.GetLongRunningOperationState(resourceName, serviceName, infrav1.PutFuture).Return(validPutFuture), + c.Get(gomockinternal.AContext(), gomock.AssignableToTypeOf(azureResourceGetterType)).Return(fakeResource, nil), + r.Parameters(gomockinternal.AContext(), fakeResource).Return(nil, errors.New("foo")), + ) + }, + }, + } + + for _, tc := range testcases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + t.Parallel() + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + scopeMock := mock_asyncpoller.NewMockFutureScope(mockCtrl) + creatorMock := mock_asyncpoller.NewMockCreator[MockCreator](mockCtrl) + svc := New[MockCreator, MockDeleter](scopeMock, creatorMock, nil) + specMock := mock_azure.NewMockResourceSpecGetter(mockCtrl) + + tc.expect(g, scopeMock.EXPECT(), creatorMock.EXPECT(), specMock.EXPECT()) + + result, err := svc.CreateOrUpdateResource(context.TODO(), specMock, serviceName) + if tc.expectedError != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tc.expectedError)) + } else { + g.Expect(err).NotTo(HaveOccurred()) + if tc.expectedResult != nil { + g.Expect(result).To(Equal(tc.expectedResult)) + } else { + g.Expect(result).To(BeNil()) + } + } + }) + } +} + +func TestServiceDeleteResource(t *testing.T) { + testcases := []struct { + name string + serviceName string + expectedError string + expectedResult interface{} + expect func(s *mock_asyncpoller.MockFutureScopeMockRecorder, d *mock_asyncpoller.MockDeleterMockRecorder[MockDeleter], r *mock_azure.MockResourceSpecGetterMockRecorder) + }{ + { + name: "invalid future", + serviceName: serviceName, + expectedError: "could not decode future data", + expect: func(s *mock_asyncpoller.MockFutureScopeMockRecorder, d *mock_asyncpoller.MockDeleterMockRecorder[MockDeleter], r *mock_azure.MockResourceSpecGetterMockRecorder) { + gomock.InOrder( + r.ResourceName().Return(resourceName), + r.ResourceGroupName().Return(resourceGroupName), + s.GetLongRunningOperationState(resourceName, serviceName, infrav1.DeleteFuture).Return(invalidDeleteFuture), + s.DeleteLongRunningOperationState(resourceName, serviceName, infrav1.DeleteFuture), + ) + }, + }, + { + name: "valid future", + serviceName: serviceName, + expectedError: "", + expect: func(s *mock_asyncpoller.MockFutureScopeMockRecorder, d *mock_asyncpoller.MockDeleterMockRecorder[MockDeleter], r *mock_azure.MockResourceSpecGetterMockRecorder) { + gomock.InOrder( + r.ResourceName().Return(resourceName), + r.ResourceGroupName().Return(resourceGroupName), + s.GetLongRunningOperationState(resourceName, serviceName, infrav1.DeleteFuture).Return(validDeleteFuture), + d.DeleteAsync(gomockinternal.AContext(), gomock.AssignableToTypeOf(azureResourceGetterType), gomock.Any()), + s.DeleteLongRunningOperationState(resourceName, serviceName, infrav1.DeleteFuture), + ) + }, + }, + } + + for _, tc := range testcases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + t.Parallel() + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + scopeMock := mock_asyncpoller.NewMockFutureScope(mockCtrl) + deleterMock := mock_asyncpoller.NewMockDeleter[MockDeleter](mockCtrl) + svc := New[MockCreator, MockDeleter](scopeMock, nil, deleterMock) + specMock := mock_azure.NewMockResourceSpecGetter(mockCtrl) + + tc.expect(scopeMock.EXPECT(), deleterMock.EXPECT(), specMock.EXPECT()) + + err := svc.DeleteResource(context.TODO(), specMock, tc.serviceName) + if tc.expectedError != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tc.expectedError)) + } else { + g.Expect(err).NotTo(HaveOccurred()) + } + }) + } +} + +const ( + resourceGroupName = "mock-resourcegroup" + resourceName = "mock-resource" + serviceName = "mock-service" + resumeToken = "mock-resume-token" + invalidResumeToken = "!invalid-resume-token" +) + +var ( + validPutFuture = &infrav1.Future{ + Type: infrav1.PutFuture, + ServiceName: serviceName, + Name: resourceName, + ResourceGroup: resourceGroupName, + Data: base64.URLEncoding.EncodeToString([]byte(resumeToken)), + } + invalidPutFuture = &infrav1.Future{ + Type: infrav1.PutFuture, + ServiceName: serviceName, + Name: resourceName, + ResourceGroup: resourceGroupName, + Data: invalidResumeToken, + } + validDeleteFuture = &infrav1.Future{ + Type: infrav1.DeleteFuture, + ServiceName: serviceName, + Name: resourceName, + ResourceGroup: resourceGroupName, + Data: base64.URLEncoding.EncodeToString([]byte(resumeToken)), + } + invalidDeleteFuture = &infrav1.Future{ + Type: infrav1.DeleteFuture, + ServiceName: serviceName, + Name: resourceName, + ResourceGroup: resourceGroupName, + Data: invalidResumeToken, + } + fakeResource = armresources.GenericResource{} + fakeParameters = armresources.GenericResource{} + azureResourceGetterType = reflect.TypeOf((*azure.ResourceSpecGetter)(nil)).Elem() +) + +func fakePoller[T any](g *GomegaWithT, statusCode int) *runtime.Poller[T] { + response := &http.Response{ + Body: io.NopCloser(strings.NewReader("")), + Request: &http.Request{ + Method: http.MethodPut, + URL: &url.URL{Path: "/"}, + }, + StatusCode: statusCode, + } + pipeline := runtime.NewPipeline("testmodule", "v0.1.0", runtime.PipelineOptions{}, nil) + poller, err := runtime.NewPoller[T](response, pipeline, nil) + g.Expect(err).NotTo(HaveOccurred()) + return poller +} + +type MockCreator struct{} +type MockDeleter struct{} diff --git a/azure/services/asyncpoller/interfaces.go b/azure/services/asyncpoller/interfaces.go new file mode 100644 index 00000000000..7a3073c81e3 --- /dev/null +++ b/azure/services/asyncpoller/interfaces.go @@ -0,0 +1,51 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package asyncpoller + +import ( + "context" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" + "sigs.k8s.io/cluster-api-provider-azure/azure" +) + +// FutureScope stores and retrieves Futures and Conditions. +type FutureScope interface { + azure.AsyncStatusUpdater +} + +// Getter gets a resource. +type Getter interface { + Get(ctx context.Context, spec azure.ResourceSpecGetter) (result interface{}, err error) +} + +// Creator creates or updates a resource asynchronously. +type Creator[T any] interface { + Getter + CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, resumeToken string, parameters interface{}) (result interface{}, poller *runtime.Poller[T], err error) +} + +// Deleter deletes a resource asynchronously. +type Deleter[T any] interface { + DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter, resumeToken string) (poller *runtime.Poller[T], err error) +} + +// Reconciler reconciles a resource. +type Reconciler interface { + CreateOrUpdateResource(ctx context.Context, spec azure.ResourceSpecGetter, serviceName string) (result interface{}, err error) + DeleteResource(ctx context.Context, spec azure.ResourceSpecGetter, serviceName string) (err error) +} diff --git a/azure/services/asyncpoller/mock_asyncpoller/asyncpoller_mock.go b/azure/services/asyncpoller/mock_asyncpoller/asyncpoller_mock.go new file mode 100644 index 00000000000..6a3b79268bb --- /dev/null +++ b/azure/services/asyncpoller/mock_asyncpoller/asyncpoller_mock.go @@ -0,0 +1,311 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by MockGen. DO NOT EDIT. +// Source: ../interfaces.go + +// Package mock_asyncpoller is a generated GoMock package. +package mock_asyncpoller + +import ( + context "context" + reflect "reflect" + + runtime "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" + gomock "go.uber.org/mock/gomock" + v1beta1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" + azure "sigs.k8s.io/cluster-api-provider-azure/azure" + v1beta10 "sigs.k8s.io/cluster-api/api/v1beta1" +) + +// MockFutureScope is a mock of FutureScope interface. +type MockFutureScope struct { + ctrl *gomock.Controller + recorder *MockFutureScopeMockRecorder +} + +// MockFutureScopeMockRecorder is the mock recorder for MockFutureScope. +type MockFutureScopeMockRecorder struct { + mock *MockFutureScope +} + +// NewMockFutureScope creates a new mock instance. +func NewMockFutureScope(ctrl *gomock.Controller) *MockFutureScope { + mock := &MockFutureScope{ctrl: ctrl} + mock.recorder = &MockFutureScopeMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockFutureScope) EXPECT() *MockFutureScopeMockRecorder { + return m.recorder +} + +// DeleteLongRunningOperationState mocks base method. +func (m *MockFutureScope) DeleteLongRunningOperationState(arg0, arg1, arg2 string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "DeleteLongRunningOperationState", arg0, arg1, arg2) +} + +// DeleteLongRunningOperationState indicates an expected call of DeleteLongRunningOperationState. +func (mr *MockFutureScopeMockRecorder) DeleteLongRunningOperationState(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteLongRunningOperationState", reflect.TypeOf((*MockFutureScope)(nil).DeleteLongRunningOperationState), arg0, arg1, arg2) +} + +// GetLongRunningOperationState mocks base method. +func (m *MockFutureScope) GetLongRunningOperationState(arg0, arg1, arg2 string) *v1beta1.Future { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetLongRunningOperationState", arg0, arg1, arg2) + ret0, _ := ret[0].(*v1beta1.Future) + return ret0 +} + +// GetLongRunningOperationState indicates an expected call of GetLongRunningOperationState. +func (mr *MockFutureScopeMockRecorder) GetLongRunningOperationState(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLongRunningOperationState", reflect.TypeOf((*MockFutureScope)(nil).GetLongRunningOperationState), arg0, arg1, arg2) +} + +// SetLongRunningOperationState mocks base method. +func (m *MockFutureScope) SetLongRunningOperationState(arg0 *v1beta1.Future) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetLongRunningOperationState", arg0) +} + +// SetLongRunningOperationState indicates an expected call of SetLongRunningOperationState. +func (mr *MockFutureScopeMockRecorder) SetLongRunningOperationState(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetLongRunningOperationState", reflect.TypeOf((*MockFutureScope)(nil).SetLongRunningOperationState), arg0) +} + +// UpdateDeleteStatus mocks base method. +func (m *MockFutureScope) UpdateDeleteStatus(arg0 v1beta10.ConditionType, arg1 string, arg2 error) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "UpdateDeleteStatus", arg0, arg1, arg2) +} + +// UpdateDeleteStatus indicates an expected call of UpdateDeleteStatus. +func (mr *MockFutureScopeMockRecorder) UpdateDeleteStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateDeleteStatus", reflect.TypeOf((*MockFutureScope)(nil).UpdateDeleteStatus), arg0, arg1, arg2) +} + +// UpdatePatchStatus mocks base method. +func (m *MockFutureScope) UpdatePatchStatus(arg0 v1beta10.ConditionType, arg1 string, arg2 error) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "UpdatePatchStatus", arg0, arg1, arg2) +} + +// UpdatePatchStatus indicates an expected call of UpdatePatchStatus. +func (mr *MockFutureScopeMockRecorder) UpdatePatchStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePatchStatus", reflect.TypeOf((*MockFutureScope)(nil).UpdatePatchStatus), arg0, arg1, arg2) +} + +// UpdatePutStatus mocks base method. +func (m *MockFutureScope) UpdatePutStatus(arg0 v1beta10.ConditionType, arg1 string, arg2 error) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "UpdatePutStatus", arg0, arg1, arg2) +} + +// UpdatePutStatus indicates an expected call of UpdatePutStatus. +func (mr *MockFutureScopeMockRecorder) UpdatePutStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePutStatus", reflect.TypeOf((*MockFutureScope)(nil).UpdatePutStatus), arg0, arg1, arg2) +} + +// MockGetter is a mock of Getter interface. +type MockGetter struct { + ctrl *gomock.Controller + recorder *MockGetterMockRecorder +} + +// MockGetterMockRecorder is the mock recorder for MockGetter. +type MockGetterMockRecorder struct { + mock *MockGetter +} + +// NewMockGetter creates a new mock instance. +func NewMockGetter(ctrl *gomock.Controller) *MockGetter { + mock := &MockGetter{ctrl: ctrl} + mock.recorder = &MockGetterMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockGetter) EXPECT() *MockGetterMockRecorder { + return m.recorder +} + +// Get mocks base method. +func (m *MockGetter) Get(ctx context.Context, spec azure.ResourceSpecGetter) (interface{}, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Get", ctx, spec) + ret0, _ := ret[0].(interface{}) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Get indicates an expected call of Get. +func (mr *MockGetterMockRecorder) Get(ctx, spec interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockGetter)(nil).Get), ctx, spec) +} + +// MockCreator is a mock of Creator interface. +type MockCreator[T any] struct { + ctrl *gomock.Controller + recorder *MockCreatorMockRecorder[T] +} + +// MockCreatorMockRecorder is the mock recorder for MockCreator. +type MockCreatorMockRecorder[T any] struct { + mock *MockCreator[T] +} + +// NewMockCreator creates a new mock instance. +func NewMockCreator[T any](ctrl *gomock.Controller) *MockCreator[T] { + mock := &MockCreator[T]{ctrl: ctrl} + mock.recorder = &MockCreatorMockRecorder[T]{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockCreator[T]) EXPECT() *MockCreatorMockRecorder[T] { + return m.recorder +} + +// CreateOrUpdateAsync mocks base method. +func (m *MockCreator[T]) CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, resumeToken string, parameters interface{}) (interface{}, *runtime.Poller[T], error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CreateOrUpdateAsync", ctx, spec, resumeToken, parameters) + ret0, _ := ret[0].(interface{}) + ret1, _ := ret[1].(*runtime.Poller[T]) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// CreateOrUpdateAsync indicates an expected call of CreateOrUpdateAsync. +func (mr *MockCreatorMockRecorder[T]) CreateOrUpdateAsync(ctx, spec, resumeToken, parameters interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdateAsync", reflect.TypeOf((*MockCreator[T])(nil).CreateOrUpdateAsync), ctx, spec, resumeToken, parameters) +} + +// Get mocks base method. +func (m *MockCreator[T]) Get(ctx context.Context, spec azure.ResourceSpecGetter) (interface{}, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Get", ctx, spec) + ret0, _ := ret[0].(interface{}) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Get indicates an expected call of Get. +func (mr *MockCreatorMockRecorder[T]) Get(ctx, spec interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockCreator[T])(nil).Get), ctx, spec) +} + +// MockDeleter is a mock of Deleter interface. +type MockDeleter[T any] struct { + ctrl *gomock.Controller + recorder *MockDeleterMockRecorder[T] +} + +// MockDeleterMockRecorder is the mock recorder for MockDeleter. +type MockDeleterMockRecorder[T any] struct { + mock *MockDeleter[T] +} + +// NewMockDeleter creates a new mock instance. +func NewMockDeleter[T any](ctrl *gomock.Controller) *MockDeleter[T] { + mock := &MockDeleter[T]{ctrl: ctrl} + mock.recorder = &MockDeleterMockRecorder[T]{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockDeleter[T]) EXPECT() *MockDeleterMockRecorder[T] { + return m.recorder +} + +// DeleteAsync mocks base method. +func (m *MockDeleter[T]) DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter, resumeToken string) (*runtime.Poller[T], error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteAsync", ctx, spec, resumeToken) + ret0, _ := ret[0].(*runtime.Poller[T]) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// DeleteAsync indicates an expected call of DeleteAsync. +func (mr *MockDeleterMockRecorder[T]) DeleteAsync(ctx, spec, resumeToken interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAsync", reflect.TypeOf((*MockDeleter[T])(nil).DeleteAsync), ctx, spec, resumeToken) +} + +// MockReconciler is a mock of Reconciler interface. +type MockReconciler struct { + ctrl *gomock.Controller + recorder *MockReconcilerMockRecorder +} + +// MockReconcilerMockRecorder is the mock recorder for MockReconciler. +type MockReconcilerMockRecorder struct { + mock *MockReconciler +} + +// NewMockReconciler creates a new mock instance. +func NewMockReconciler(ctrl *gomock.Controller) *MockReconciler { + mock := &MockReconciler{ctrl: ctrl} + mock.recorder = &MockReconcilerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockReconciler) EXPECT() *MockReconcilerMockRecorder { + return m.recorder +} + +// CreateOrUpdateResource mocks base method. +func (m *MockReconciler) CreateOrUpdateResource(ctx context.Context, spec azure.ResourceSpecGetter, serviceName string) (interface{}, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CreateOrUpdateResource", ctx, spec, serviceName) + ret0, _ := ret[0].(interface{}) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CreateOrUpdateResource indicates an expected call of CreateOrUpdateResource. +func (mr *MockReconcilerMockRecorder) CreateOrUpdateResource(ctx, spec, serviceName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdateResource", reflect.TypeOf((*MockReconciler)(nil).CreateOrUpdateResource), ctx, spec, serviceName) +} + +// DeleteResource mocks base method. +func (m *MockReconciler) DeleteResource(ctx context.Context, spec azure.ResourceSpecGetter, serviceName string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteResource", ctx, spec, serviceName) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteResource indicates an expected call of DeleteResource. +func (mr *MockReconcilerMockRecorder) DeleteResource(ctx, spec, serviceName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteResource", reflect.TypeOf((*MockReconciler)(nil).DeleteResource), ctx, spec, serviceName) +} diff --git a/azure/services/asyncpoller/mock_asyncpoller/doc.go b/azure/services/asyncpoller/mock_asyncpoller/doc.go new file mode 100644 index 00000000000..c6750cf8fb6 --- /dev/null +++ b/azure/services/asyncpoller/mock_asyncpoller/doc.go @@ -0,0 +1,21 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Run go generate to regenerate this mock. +// +//go:generate ../../../../hack/tools/bin/mockgen -destination asyncpoller_mock.go -package mock_asyncpoller -source ../interfaces.go FutureHandler +//go:generate /usr/bin/env bash -c "cat ../../../../hack/boilerplate/boilerplate.generatego.txt asyncpoller_mock.go > _asyncpoller_mock.go && mv _asyncpoller_mock.go asyncpoller_mock.go" +package mock_asyncpoller From 517cfe2773ea7a0aef2ea9b875e832f96ceaf866 Mon Sep 17 00:00:00 2001 From: Matt Boersma Date: Wed, 23 Aug 2023 13:23:55 -0600 Subject: [PATCH 2/2] Convert natgateways service to SDKv2 --- azure/services/natgateways/client.go | 125 +++++++----------- azure/services/natgateways/natgateways.go | 24 ++-- .../services/natgateways/natgateways_test.go | 8 +- azure/services/natgateways/spec.go | 37 +++--- controllers/azurecluster_reconciler.go | 6 +- go.mod | 2 + go.sum | 4 + 7 files changed, 96 insertions(+), 110 deletions(-) diff --git a/azure/services/natgateways/client.go b/azure/services/natgateways/client.go index c951c0d9fde..b1b7621a8d9 100644 --- a/azure/services/natgateways/client.go +++ b/azure/services/natgateways/client.go @@ -18,34 +18,41 @@ package natgateways import ( "context" - "encoding/json" - "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-08-01/network" - "github.com/Azure/go-autorest/autorest" - azureautorest "github.com/Azure/go-autorest/autorest/azure" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v4" "github.com/pkg/errors" - infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" "sigs.k8s.io/cluster-api-provider-azure/azure" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/asyncpoller" "sigs.k8s.io/cluster-api-provider-azure/util/reconciler" "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) // azureClient contains the Azure go-sdk Client. type azureClient struct { - natgateways network.NatGatewaysClient + natgateways *armnetwork.NatGatewaysClient } // newClient creates a new VM client from subscription ID. -func newClient(auth azure.Authorizer) *azureClient { - c := netNatGatewaysClient(auth.SubscriptionID(), auth.BaseURI(), auth.Authorizer()) - return &azureClient{c} +func newClient(auth azure.Authorizer) (*azureClient, error) { + c, err := netNatGatewaysClient(auth) + if err != nil { + return nil, errors.Wrap(err, "failed to create natgateways client") + } + return &azureClient{c}, nil } // netNatGatewaysClient creates a new nat gateways client from subscription ID. -func netNatGatewaysClient(subscriptionID string, baseURI string, authorizer autorest.Authorizer) network.NatGatewaysClient { - natGatewaysClient := network.NewNatGatewaysClientWithBaseURI(baseURI, subscriptionID) - azure.SetAutoRestClientDefaults(&natGatewaysClient.Client, authorizer) - return natGatewaysClient +func netNatGatewaysClient(auth azure.Authorizer) (*armnetwork.NatGatewaysClient, error) { + opts, err := azure.ARMClientOptions(auth.CloudEnvironment()) + if err != nil { + return &armnetwork.NatGatewaysClient{}, errors.Wrap(err, "failed to create natgateways client options") + } + factory, err := armnetwork.NewClientFactory(auth.SubscriptionID(), auth.Token(), opts) + if err != nil { + return &armnetwork.NatGatewaysClient{}, errors.Wrap(err, "failed to create armnetwork client factory") + } + return factory.NewNatGatewaysClient(), nil } // Get gets the specified nat gateway. @@ -53,22 +60,28 @@ func (ac *azureClient) Get(ctx context.Context, spec azure.ResourceSpecGetter) ( ctx, _, done := tele.StartSpanWithLogger(ctx, "natgateways.azureClient.Get") defer done() - return ac.natgateways.Get(ctx, spec.ResourceGroupName(), spec.ResourceName(), "") + resp, err := ac.natgateways.Get(ctx, spec.ResourceGroupName(), spec.ResourceName(), nil) + if err != nil { + return nil, err + } + return resp.NatGateway, nil } // CreateOrUpdateAsync creates or updates a Nat Gateway asynchronously. -// It sends a PUT request to Azure and if accepted without error, the func will return a Future which can be used to track the ongoing +// It sends a PUT request to Azure and if accepted without error, the func will return a Poller which can be used to track the ongoing // progress of the operation. -func (ac *azureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, parameters interface{}) (result interface{}, future azureautorest.FutureAPI, err error) { - ctx, _, done := tele.StartSpanWithLogger(ctx, "natgateways.azureClient.CreateOrUpdateAsync") +func (ac *azureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, resumeToken string, parameters interface{}) (result interface{}, poller *runtime.Poller[armnetwork.NatGatewaysClientCreateOrUpdateResponse], err error) { + ctx, log, done := tele.StartSpanWithLogger(ctx, "natgateways.azureClient.CreateOrUpdateAsync") defer done() - natGateway, ok := parameters.(network.NatGateway) + natGateway, ok := parameters.(armnetwork.NatGateway) if !ok { - return nil, nil, errors.Errorf("%T is not a network.NatGateway", parameters) + return nil, nil, errors.Errorf("%T is not an armnetwork.NatGateway", parameters) } - createFuture, err := ac.natgateways.CreateOrUpdate(ctx, spec.ResourceGroupName(), spec.ResourceName(), natGateway) + opts := &armnetwork.NatGatewaysClientBeginCreateOrUpdateOptions{ResumeToken: resumeToken} + log.V(4).Info("sending request", "resumeToken", resumeToken) + poller, err = ac.natgateways.BeginCreateOrUpdate(ctx, spec.ResourceGroupName(), spec.ResourceName(), natGateway, opts) if err != nil { return nil, nil, err } @@ -76,26 +89,28 @@ func (ac *azureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.Resou ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) defer cancel() - err = createFuture.WaitForCompletionRef(ctx, ac.natgateways.Client) + pollOpts := &runtime.PollUntilDoneOptions{Frequency: asyncpoller.DefaultPollerFrequency} + result, err = poller.PollUntilDone(ctx, pollOpts) if err != nil { - // if an error occurs, return the future. + // if an error occurs, return the poller. // this means the long-running operation didn't finish in the specified timeout. - return nil, &createFuture, err + return nil, poller, err } - result, err = createFuture.Result(ac.natgateways) - // if the operation completed, return a nil future + // if the operation completed, return a nil poller return result, nil, err } // DeleteAsync deletes a Nat Gateway asynchronously. DeleteAsync sends a DELETE -// request to Azure and if accepted without error, the func will return a Future which can be used to track the ongoing +// request to Azure and if accepted without error, the func will return a Poller which can be used to track the ongoing // progress of the operation. -func (ac *azureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (future azureautorest.FutureAPI, err error) { - ctx, _, done := tele.StartSpanWithLogger(ctx, "natgateways.azureClient.DeleteAsync") +func (ac *azureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter, resumeToken string) (poller *runtime.Poller[armnetwork.NatGatewaysClientDeleteResponse], err error) { + ctx, log, done := tele.StartSpanWithLogger(ctx, "natgateways.azureClient.DeleteAsync") defer done() - deleteFuture, err := ac.natgateways.Delete(ctx, spec.ResourceGroupName(), spec.ResourceName()) + opts := &armnetwork.NatGatewaysClientBeginDeleteOptions{ResumeToken: resumeToken} + log.V(4).Info("sending request", "resumeToken", resumeToken) + poller, err = ac.natgateways.BeginDelete(ctx, spec.ResourceGroupName(), spec.ResourceName(), opts) if err != nil { return nil, err } @@ -103,54 +118,14 @@ func (ac *azureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecG ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) defer cancel() - err = deleteFuture.WaitForCompletionRef(ctx, ac.natgateways.Client) + pollOpts := &runtime.PollUntilDoneOptions{Frequency: asyncpoller.DefaultPollerFrequency} + _, err = poller.PollUntilDone(ctx, pollOpts) if err != nil { - // if an error occurs, return the future. + // if an error occurs, return the Poller. // this means the long-running operation didn't finish in the specified timeout. - return &deleteFuture, err + return poller, err } - _, err = deleteFuture.Result(ac.natgateways) - // if the operation completed, return a nil future. - return nil, err -} -// IsDone returns true if the long-running operation has completed. -func (ac *azureClient) IsDone(ctx context.Context, future azureautorest.FutureAPI) (isDone bool, err error) { - ctx, _, done := tele.StartSpanWithLogger(ctx, "natgateways.azureClient.IsDone") - defer done() - - return future.DoneWithContext(ctx, ac.natgateways) -} - -// Result fetches the result of a long-running operation future. -func (ac *azureClient) Result(ctx context.Context, future azureautorest.FutureAPI, futureType string) (result interface{}, err error) { - _, _, done := tele.StartSpanWithLogger(ctx, "natgateways.azureClient.Result") - defer done() - - if future == nil { - return nil, errors.Errorf("cannot get result from nil future") - } - - switch futureType { - case infrav1.PutFuture: - // Marshal and Unmarshal the future to put it into the correct future type so we can access the Result function. - // Unfortunately the FutureAPI can't be casted directly to NatGatewaysCreateOrUpdateFuture because it is a azureautorest.Future, which doesn't implement the Result function. See PR #1686 for discussion on alternatives. - // It was converted back to a generic azureautorest.Future from the CAPZ infrav1.Future type stored in Status: https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/main/azure/converters/futures.go#L49. - var createFuture *network.NatGatewaysCreateOrUpdateFuture - jsonData, err := future.MarshalJSON() - if err != nil { - return nil, errors.Wrap(err, "failed to marshal future") - } - if err := json.Unmarshal(jsonData, &createFuture); err != nil { - return nil, errors.Wrap(err, "failed to unmarshal future data") - } - return createFuture.Result(ac.natgateways) - - case infrav1.DeleteFuture: - // Delete does not return a result NAT gateway - return nil, nil - - default: - return nil, errors.Errorf("unknown future type %q", futureType) - } + // if the operation completed, return a nil poller. + return nil, err } diff --git a/azure/services/natgateways/natgateways.go b/azure/services/natgateways/natgateways.go index c15c4600755..e1b5bf4a34e 100644 --- a/azure/services/natgateways/natgateways.go +++ b/azure/services/natgateways/natgateways.go @@ -19,11 +19,11 @@ package natgateways import ( "context" - "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-08-01/network" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v4" "github.com/pkg/errors" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" "sigs.k8s.io/cluster-api-provider-azure/azure" - "sigs.k8s.io/cluster-api-provider-azure/azure/services/async" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/asyncpoller" "sigs.k8s.io/cluster-api-provider-azure/util/reconciler" "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) @@ -41,16 +41,20 @@ type NatGatewayScope interface { // Service provides operations on azure resources. type Service struct { Scope NatGatewayScope - async.Reconciler + asyncpoller.Reconciler } // New creates a new service. -func New(scope NatGatewayScope) *Service { - client := newClient(scope) - return &Service{ - Scope: scope, - Reconciler: async.New(scope, client, client), +func New(scope NatGatewayScope) (*Service, error) { + client, err := newClient(scope) + if err != nil { + return nil, err } + return &Service{ + Scope: scope, + Reconciler: asyncpoller.New[armnetwork.NatGatewaysClientCreateOrUpdateResponse, + armnetwork.NatGatewaysClientDeleteResponse](scope, client, client), + }, nil } // Name returns the service name. @@ -91,10 +95,10 @@ func (s *Service) Reconcile(ctx context.Context) error { } } if err == nil { - natGateway, ok := result.(network.NatGateway) + natGateway, ok := result.(armnetwork.NatGateway) if !ok { // Return out of loop since this would be an unexpected fatal error - resultingErr = errors.Errorf("created resource %T is not a network.NatGateway", result) + resultingErr = errors.Errorf("created resource %T is not an armnetwork.NatGateway", result) break } diff --git a/azure/services/natgateways/natgateways_test.go b/azure/services/natgateways/natgateways_test.go index 759d97d1bd2..3ad663b422a 100644 --- a/azure/services/natgateways/natgateways_test.go +++ b/azure/services/natgateways/natgateways_test.go @@ -21,7 +21,7 @@ import ( "net/http" "testing" - "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-08-01/network" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v4" "github.com/Azure/go-autorest/autorest" . "github.com/onsi/gomega" "go.uber.org/mock/gomock" @@ -48,7 +48,7 @@ var ( ClusterName: "my-cluster", NatGatewayIP: infrav1.PublicIPSpec{Name: "pip-node-subnet"}, } - natGateway1 = network.NatGateway{ + natGateway1 = armnetwork.NatGateway{ ID: ptr.To("/subscriptions/my-sub/resourceGroups/my-rg/providers/Microsoft.Network/natGateways/my-node-natgateway-1"), } customVNetTags = infrav1.Tags{ @@ -114,12 +114,12 @@ func TestReconcileNatGateways(t *testing.T) { { name: "result is not a NAT gateway", tags: ownedVNetTags, - expectedError: "created resource string is not a network.NatGateway", + expectedError: "created resource string is not an armnetwork.NatGateway", expect: func(s *mock_natgateways.MockNatGatewayScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { s.IsVnetManaged().Return(true) s.NatGatewaySpecs().Return([]azure.ResourceSpecGetter{&natGatewaySpec1}) r.CreateOrUpdateResource(gomockinternal.AContext(), &natGatewaySpec1, serviceName).Return("not a nat gateway", nil) - s.UpdatePutStatus(infrav1.NATGatewaysReadyCondition, serviceName, gomockinternal.ErrStrEq("created resource string is not a network.NatGateway")) + s.UpdatePutStatus(infrav1.NATGatewaysReadyCondition, serviceName, gomockinternal.ErrStrEq("created resource string is not an armnetwork.NatGateway")) }, }, } diff --git a/azure/services/natgateways/spec.go b/azure/services/natgateways/spec.go index e8ce0ec528e..6f0323126ac 100644 --- a/azure/services/natgateways/spec.go +++ b/azure/services/natgateways/spec.go @@ -19,7 +19,7 @@ package natgateways import ( "context" - "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-08-01/network" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v4" "github.com/pkg/errors" "k8s.io/utils/ptr" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" @@ -57,9 +57,9 @@ func (s *NatGatewaySpec) OwnerResourceName() string { // Parameters returns the parameters for the NAT gateway. func (s *NatGatewaySpec) Parameters(ctx context.Context, existing interface{}) (params interface{}, err error) { if existing != nil { - existingNatGateway, ok := existing.(network.NatGateway) + existingNatGateway, ok := existing.(armnetwork.NatGateway) if !ok { - return nil, errors.Errorf("%T is not a network.NatGateway", existing) + return nil, errors.Errorf("%T is not an armnetwork.NatGateway", existing) } if hasPublicIP(existingNatGateway, s.NatGatewayIP.Name) { @@ -68,12 +68,12 @@ func (s *NatGatewaySpec) Parameters(ctx context.Context, existing interface{}) ( } } - natGatewayToCreate := network.NatGateway{ + natGatewayToCreate := armnetwork.NatGateway{ Name: ptr.To(s.Name), Location: ptr.To(s.Location), - Sku: &network.NatGatewaySku{Name: network.NatGatewaySkuNameStandard}, - NatGatewayPropertiesFormat: &network.NatGatewayPropertiesFormat{ - PublicIPAddresses: &[]network.SubResource{ + SKU: &armnetwork.NatGatewaySKU{Name: ptr.To(armnetwork.NatGatewaySKUNameStandard)}, + Properties: &armnetwork.NatGatewayPropertiesFormat{ + PublicIPAddresses: []*armnetwork.SubResource{ { ID: ptr.To(azure.PublicIPID(s.SubscriptionID, s.ResourceGroupName(), s.NatGatewayIP.Name)), }, @@ -90,19 +90,16 @@ func (s *NatGatewaySpec) Parameters(ctx context.Context, existing interface{}) ( return natGatewayToCreate, nil } -func hasPublicIP(natGateway network.NatGateway, publicIPName string) bool { - // We must have a non-nil, non-"empty" PublicIPAddresses - if !(natGateway.PublicIPAddresses != nil && len(*natGateway.PublicIPAddresses) > 0) { - return false - } - - for _, publicIP := range *natGateway.PublicIPAddresses { - resource, err := azureutil.ParseResourceID(*publicIP.ID) - if err != nil { - continue - } - if resource.Name == publicIPName { - return true +func hasPublicIP(natGateway armnetwork.NatGateway, publicIPName string) bool { + for _, publicIP := range natGateway.Properties.PublicIPAddresses { + if publicIP != nil && publicIP.ID != nil { + resource, err := azureutil.ParseResourceID(*publicIP.ID) + if err != nil { + continue + } + if resource.Name == publicIPName { + return true + } } } return false diff --git a/controllers/azurecluster_reconciler.go b/controllers/azurecluster_reconciler.go index 9e1bb070081..a55b5ffad23 100644 --- a/controllers/azurecluster_reconciler.go +++ b/controllers/azurecluster_reconciler.go @@ -55,6 +55,10 @@ func newAzureClusterService(scope *scope.ClusterScope) (*azureClusterService, er if err != nil { return nil, errors.Wrap(err, "failed creating a NewCache") } + natGatewaysSvc, err := natgateways.New(scope) + if err != nil { + return nil, err + } return &azureClusterService{ scope: scope, services: []azure.ServiceReconciler{ @@ -63,7 +67,7 @@ func newAzureClusterService(scope *scope.ClusterScope) (*azureClusterService, er securitygroups.New(scope), routetables.New(scope), publicips.New(scope), - natgateways.New(scope), + natGatewaysSvc, subnets.New(scope), vnetpeerings.New(scope), loadbalancers.New(scope), diff --git a/go.mod b/go.mod index cce6158e7c2..0bdf960da94 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,8 @@ require ( github.com/Azure/azure-sdk-for-go/sdk/azcore v1.7.1 github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.3.1 github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5 v5.1.0 + github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v4 v4.0.0 + github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources v1.1.1 github.com/Azure/azure-service-operator/v2 v2.2.0 github.com/Azure/go-autorest/autorest v0.11.29 github.com/Azure/go-autorest/autorest/azure/auth v0.5.12 diff --git a/go.sum b/go.sum index 2d1562ec601..11a7aa44914 100644 --- a/go.sum +++ b/go.sum @@ -55,8 +55,12 @@ github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/cosmos/armcosmos v1.0.0 h1 github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/internal v1.1.2 h1:mLY+pNLjCUeKhgnAJWAKhEUQM+RJQo2H1fuGSw1Ky1E= github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/iothub/armiothub v1.1.1 h1:Dh8SxVXcSyQN76LI4IseKyrnqyTUsx336Axg8zDYSMs= github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/machinelearning/armmachinelearning v1.0.0 h1:KWvCVjnOTKCZAlqED5KPNoN9AfcK2BhUeveLdiwy33Q= +github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/managementgroups/armmanagementgroups v1.0.0 h1:pPvTJ1dY0sA35JOeFq6TsY2xj6Z85Yo23Pj4wCCvu4o= +github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v4 v4.0.0 h1:pqCyNi/Paz03SbWRmGlb5WBzK14aOXVuSJuOTWzOM5M= +github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v4 v4.0.0/go.mod h1:bCUhQ1sbQHAG4nm1SqWwLlnKnRVT2e6Lu0cij7OzliM= github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/redis/armredis v1.0.0 h1:nmpTBgRg1HynngFYICRhceC7s5dmbKN9fJ/XQz/UQ2I= github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources v1.1.1 h1:7CBQ+Ei8SP2c6ydQTGCCrS35bDxgTMfoP2miAwK++OU= +github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources v1.1.1/go.mod h1:c/wcGeGx5FUPbM/JltUYHZcKmigwyVLJlDq+4HdtXaw= github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/search/armsearch v1.1.0 h1:SCO2mlFZrUMU8MmA5Y6EszSm2OGumuPBXFQXEvkESvk= github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/servicebus/armservicebus v1.1.1 h1:h+ZMdUM0/8oVqHjY9+1rupIvT0craBLapKhuzWui9lo= github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/storage/armstorage v1.0.0 h1:TMEyRFKh1zaSPmoQh3kxK+xRAYVq8guCI/7SMO0F3KY=