From fae99326cd166c19b714959aa15d5d023c578330 Mon Sep 17 00:00:00 2001 From: Jonathan Tong Date: Fri, 12 Nov 2021 18:15:42 -0500 Subject: [PATCH] Make availability set reconcile/delete async --- azure/scope/machine.go | 64 +++- .../availabilitysets/availabilitysets.go | 120 ++----- .../availabilitysets/availabilitysets_test.go | 333 +++++++++--------- azure/services/availabilitysets/client.go | 93 ++++- .../availabilitysets_mock.go | 106 +++++- .../mock_availabilitysets/client_mock.go | 68 +++- azure/services/availabilitysets/spec.go | 80 +++++ azure/types.go | 5 - controllers/azuremachine_reconciler.go | 2 +- 9 files changed, 567 insertions(+), 304 deletions(-) create mode 100644 azure/services/availabilitysets/spec.go diff --git a/azure/scope/machine.go b/azure/scope/machine.go index 27ee6646703..05481767187 100644 --- a/azure/scope/machine.go +++ b/azure/scope/machine.go @@ -20,9 +20,11 @@ import ( "context" "encoding/base64" "encoding/json" + "strconv" "strings" "time" + "github.com/Azure/azure-sdk-for-go/profiles/latest/compute/mgmt/compute" "github.com/Azure/go-autorest/autorest/to" "github.com/go-logr/logr" "github.com/pkg/errors" @@ -39,6 +41,7 @@ import ( 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/availabilitysets" "sigs.k8s.io/cluster-api-provider-azure/azure/services/resourceskus" "sigs.k8s.io/cluster-api-provider-azure/azure/services/virtualmachines" "sigs.k8s.io/cluster-api-provider-azure/util/futures" @@ -101,9 +104,10 @@ type MachineScope struct { // MachineCache stores common machine information so we don't have to hit the API multiple times within the same reconcile loop. type MachineCache struct { - BootstrapData string - VMImage *infrav1.Image - VMSKU resourceskus.SKU + BootstrapData string + VMImage *infrav1.Image + VMSKU resourceskus.SKU + availabilitySetSKU resourceskus.SKU } // InitMachineCache sets cached information about the machine to be used in the scope. @@ -129,9 +133,16 @@ func (m *MachineScope) InitMachineCache(ctx context.Context) error { if err != nil { return err } + m.cache.VMSKU, err = skuCache.Get(ctx, m.AzureMachine.Spec.VMSize, resourceskus.VirtualMachines) if err != nil { - return azure.WithTerminalError(errors.Wrapf(err, "failed to get SKU %s in compute api", m.AzureMachine.Spec.VMSize)) + return azure.WithTerminalError(errors.Wrapf(err, "failed to get VM SKU %s in compute api", m.AzureMachine.Spec.VMSize)) + } + + m.cache.availabilitySetSKU, err = skuCache.Get(ctx, string(compute.AvailabilitySetSkuTypesAligned), resourceskus.AvailabilitySets) + if err != nil { + // TODO: verify error message + return azure.WithTerminalError(errors.Wrapf(err, "failed to get availability set SKU %s in compute api", string(compute.AvailabilitySetSkuTypesAligned))) } } @@ -366,6 +377,51 @@ func (m *MachineScope) ProviderID() string { return parsed.String() } +// AvailabilitySet returns the availability set for this machine if available. +func (m *MachineScope) AvailabilitySetSpec() (azure.ResourceSpecGetter, error) { + if !m.AvailabilitySetEnabled() { + // Availability set not enabled, not an error + return nil, nil + } + spec := &availabilitysets.AvailabilitySetSpec{ + ResourceGroup: m.ResourceGroup(), + ClusterName: m.ClusterName(), + Location: m.Location(), + AdditionalTags: m.AdditionalTags(), + } + + if m.IsControlPlane() { + spec.Name = azure.GenerateAvailabilitySetName(m.ClusterName(), azure.ControlPlaneNodeGroup) + } else if mdName, ok := m.Machine.Labels[clusterv1.MachineDeploymentLabelName]; ok { + // get machine deployment name from labels for machines that maybe part of a machine deployment. + spec.Name = azure.GenerateAvailabilitySetName(m.ClusterName(), mdName) + } else if msName, ok := m.Machine.Labels[clusterv1.MachineSetLabelName]; ok { + // if machine deployment name label is not available, use machine set name. + spec.Name = azure.GenerateAvailabilitySetName(m.ClusterName(), msName) + } else { + // Name not available, not an error + return nil, nil + } + + if m.cache != nil { + asSKU := m.cache.availabilitySetSKU + faultDomainCountStr, ok := asSKU.GetCapability(resourceskus.MaximumPlatformFaultDomainCount) + if !ok { + return nil, errors.Errorf("cannot find capability %s sku %s", resourceskus.MaximumPlatformFaultDomainCount, asSKU.Name) + } + + faultDomainCount, err := strconv.ParseUint(faultDomainCountStr, 10, 32) + if err != nil { + return nil, errors.Wrap(err, "failed to determine max fault domain count") + } + + spec.FaultDomainCount = faultDomainCount + return spec, nil + } else { + return nil, errors.Errorf("cache not initialized for availability set") + } +} + // AvailabilitySet returns the availability set for this machine if available. func (m *MachineScope) AvailabilitySet() (string, bool) { if !m.AvailabilitySetEnabled() { diff --git a/azure/services/availabilitysets/availabilitysets.go b/azure/services/availabilitysets/availabilitysets.go index a6cfa35f434..eabd6bb50e4 100644 --- a/azure/services/availabilitysets/availabilitysets.go +++ b/azure/services/availabilitysets/availabilitysets.go @@ -18,137 +18,83 @@ package availabilitysets import ( "context" - "strconv" - "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-04-01/compute" - "github.com/Azure/go-autorest/autorest/to" "github.com/go-logr/logr" - "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/azure/services/resourceskus" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/async" + "sigs.k8s.io/cluster-api-provider-azure/util/reconciler" "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) +const serviceName = "availabilitysets" + // AvailabilitySetScope defines the scope interface for a availability sets service. type AvailabilitySetScope interface { logr.Logger azure.ClusterDescriber - AvailabilitySet() (string, bool) + azure.AsyncStatusUpdater + AvailabilitySetSpec() (azure.ResourceSpecGetter, error) } // Service provides operations on Azure resources. type Service struct { Scope AvailabilitySetScope Client - resourceSKUCache *resourceskus.Cache } // New creates a new availability sets service. -func New(scope AvailabilitySetScope, skuCache *resourceskus.Cache) *Service { +func New(scope AvailabilitySetScope) *Service { return &Service{ - Scope: scope, - Client: NewClient(scope), - resourceSKUCache: skuCache, + Scope: scope, + Client: NewClient(scope), } } // Reconcile creates or updates availability sets. func (s *Service) Reconcile(ctx context.Context) error { - ctx, _, done := tele.StartSpanWithLogger( - ctx, - "availabilitysets.Service.Reconcile", - ) + ctx, _, done := tele.StartSpanWithLogger(ctx, "availabilitysets.Service.Reconcile") defer done() - availabilitySetName, ok := s.Scope.AvailabilitySet() - if !ok { - return nil - } - - asSku, err := s.resourceSKUCache.Get(ctx, string(compute.AvailabilitySetSkuTypesAligned), resourceskus.AvailabilitySets) - if err != nil { - return errors.Wrap(err, "failed to get availability sets sku") - } - - faultDomainCountStr, ok := asSku.GetCapability(resourceskus.MaximumPlatformFaultDomainCount) - if !ok { - return errors.Errorf("cannot find capability %s sku %s", resourceskus.MaximumPlatformFaultDomainCount, *asSku.Name) - } + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) + defer cancel() - faultDomainCount, err := strconv.ParseUint(faultDomainCountStr, 10, 32) + setSpec, err := s.Scope.AvailabilitySetSpec() if err != nil { - return errors.Wrap(err, "failed to determine max fault domain count") - } - - s.Scope.V(2).Info("creating availability set", "availability set", availabilitySetName) - - asParams := compute.AvailabilitySet{ - Sku: &compute.Sku{ - Name: to.StringPtr(string(compute.AvailabilitySetSkuTypesAligned)), - }, - AvailabilitySetProperties: &compute.AvailabilitySetProperties{ - PlatformFaultDomainCount: to.Int32Ptr(int32(faultDomainCount)), - }, - Tags: converters.TagsToMap(infrav1.Build(infrav1.BuildParams{ - ClusterName: s.Scope.ClusterName(), - Lifecycle: infrav1.ResourceLifecycleOwned, - Name: to.StringPtr(availabilitySetName), - Role: to.StringPtr(infrav1.CommonRole), - Additional: s.Scope.AdditionalTags(), - })), - Location: to.StringPtr(s.Scope.Location()), + s.Scope.UpdatePutStatus(infrav1.AvailabilitySetReadyCondition, serviceName, err) + return err } - - _, err = s.Client.CreateOrUpdate(ctx, s.Scope.ResourceGroup(), availabilitySetName, asParams) - if err != nil { - return errors.Wrapf(err, "failed to create availability set %s", availabilitySetName) + if setSpec == nil && err == nil { // Availability set not available + s.Scope.UpdatePutStatus(infrav1.AvailabilitySetReadyCondition, serviceName, nil) + return nil } - s.Scope.V(2).Info("successfully created availability set", "availability set", availabilitySetName) - - return nil + _, err = async.CreateResource(ctx, s.Scope, s.Client, setSpec, serviceName) + s.Scope.UpdatePutStatus(infrav1.AvailabilitySetReadyCondition, serviceName, err) + return err } // Delete deletes availability sets. func (s *Service) Delete(ctx context.Context) error { - ctx, _, done := tele.StartSpanWithLogger(ctx, "availabilitysets.Service.Delete") + + ctx, _, done := tele.StartSpanWithLogger(ctx, "virtualmachines.Service.Delete") defer done() - availabilitySetName, ok := s.Scope.AvailabilitySet() - if !ok { - return nil - } - - as, err := s.Client.Get(ctx, s.Scope.ResourceGroup(), availabilitySetName) - if err != nil && azure.ResourceNotFound(err) { - // already deleted - return nil - } + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) + defer cancel() + setSpec, err := s.Scope.AvailabilitySetSpec() if err != nil { - return errors.Wrapf(err, "failed to get availability set %s in resource group %s", availabilitySetName, s.Scope.ResourceGroup()) - } - - // only delete when the availability set does not have any vms - if as.AvailabilitySetProperties != nil && as.VirtualMachines != nil && len(*as.VirtualMachines) > 0 { - return nil + s.Scope.UpdateDeleteStatus(infrav1.AvailabilitySetReadyCondition, serviceName, err) + return err } - - s.Scope.V(2).Info("deleting availability set", "availability set", availabilitySetName) - err = s.Client.Delete(ctx, s.Scope.ResourceGroup(), availabilitySetName) - if err != nil && azure.ResourceNotFound(err) { - // already deleted + if setSpec == nil && err == nil { // Availability set not available + s.Scope.UpdateDeleteStatus(infrav1.AvailabilitySetReadyCondition, serviceName, nil) return nil } - if err != nil { - return errors.Wrapf(err, "failed to delete availability set %s in resource group %s", availabilitySetName, s.Scope.ResourceGroup()) - } - - s.Scope.V(2).Info("successfully delete availability set", "availability set", availabilitySetName) - - return nil + err = async.DeleteResource(ctx, s.Scope, s.Client, setSpec, serviceName) + s.Scope.UpdateDeleteStatus(infrav1.AvailabilitySetReadyCondition, serviceName, err) + return err } diff --git a/azure/services/availabilitysets/availabilitysets_test.go b/azure/services/availabilitysets/availabilitysets_test.go index 7f08dd58dc2..9e56f35ce07 100644 --- a/azure/services/availabilitysets/availabilitysets_test.go +++ b/azure/services/availabilitysets/availabilitysets_test.go @@ -19,135 +19,153 @@ package availabilitysets import ( "context" "errors" + "fmt" + "net/http" "testing" - "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-04-01/compute" + // "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-04-01/compute" "github.com/Azure/go-autorest/autorest" - "github.com/Azure/go-autorest/autorest/to" + // "github.com/Azure/go-autorest/autorest/to" "github.com/golang/mock/gomock" . "github.com/onsi/gomega" - "k8s.io/utils/pointer" - "sigs.k8s.io/cluster-api-provider-azure/azure/services/resourceskus" + + // "k8s.io/utils/pointer" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" + // "sigs.k8s.io/cluster-api-provider-azure/azure/services/resourceskus" gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock" "k8s.io/klog/v2/klogr" "sigs.k8s.io/cluster-api-provider-azure/azure/services/availabilitysets/mock_availabilitysets" ) -func TestReconcileAvailabilitySets(t *testing.T) { - testcases := []struct { - name string - expectedError string - expect func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder) - setupSKUs func(svc *Service) - }{ - { - name: "create or update availability set", - expectedError: "", - expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder) { - s.V(gomock.AssignableToTypeOf(2)).MinTimes(2).Return(klogr.New()) - s.AvailabilitySet().Return("as-name", true) - s.ResourceGroup().Return("my-rg") - s.ClusterName().Return("cl-name") - s.AdditionalTags().Return(map[string]string{}) - s.Location().Return("test-location") - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "as-name", - compute.AvailabilitySet{ - Sku: &compute.Sku{Name: to.StringPtr("Aligned")}, - AvailabilitySetProperties: &compute.AvailabilitySetProperties{ - PlatformFaultDomainCount: pointer.Int32Ptr(3), - }, - Tags: map[string]*string{"sigs.k8s.io_cluster-api-provider-azure_cluster_cl-name": to.StringPtr("owned"), - "sigs.k8s.io_cluster-api-provider-azure_role": to.StringPtr("common"), "Name": to.StringPtr("as-name")}, - Location: to.StringPtr("test-location"), - }).Return(compute.AvailabilitySet{}, nil) - }, - setupSKUs: func(svc *Service) { - skus := []compute.ResourceSku{ - { - Name: to.StringPtr("Aligned"), - Kind: to.StringPtr(string(resourceskus.AvailabilitySets)), - Capabilities: &[]compute.ResourceSkuCapabilities{ - { - Name: to.StringPtr(resourceskus.MaximumPlatformFaultDomainCount), - Value: to.StringPtr("3"), - }, - }, - }, - } - resourceSkusCache := resourceskus.NewStaticCache(skus, "") - svc.resourceSKUCache = resourceSkusCache - }, - }, - { - name: "noop if the machine does not need to be assigned an availability set (machines without a deployment)", - expectedError: "", - expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder) { - s.AvailabilitySet().Return("as-name", false) - }, - setupSKUs: func(svc *Service) { - }, - }, - { - name: "return error", - expectedError: "failed to create availability set as-name: something went wrong", - expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder) { - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.AvailabilitySet().Return("as-name", true) - s.ResourceGroup().Return("my-rg") - s.ClusterName().Return("cl-name") - s.AdditionalTags().Return(map[string]string{}) - s.Location().Return("test-location") - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "as-name", - gomock.AssignableToTypeOf(compute.AvailabilitySet{})).Return(compute.AvailabilitySet{}, errors.New("something went wrong")) - }, - setupSKUs: func(svc *Service) { - skus := []compute.ResourceSku{ - { - Name: to.StringPtr("Aligned"), - Kind: to.StringPtr(string(resourceskus.AvailabilitySets)), - Capabilities: &[]compute.ResourceSkuCapabilities{ - { - Name: to.StringPtr(resourceskus.MaximumPlatformFaultDomainCount), - Value: to.StringPtr("3"), - }, - }, - }, - } - resourceSkusCache := resourceskus.NewStaticCache(skus, "") - svc.resourceSKUCache = resourceSkusCache - }, - }, +var ( + fakeSetSpec = AvailabilitySetSpec{ + Name: "test-as", + ResourceGroup: "test-rg", + ClusterName: "test-cluster", + Location: "test-location", + FaultDomainCount: 3, + AdditionalTags: map[string]string{}, } - for _, tc := range testcases { - tc := tc - t.Run(tc.name, func(t *testing.T) { - g := NewWithT(t) + internalError = autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error") + notFoundError = autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not Found") + fakeSpecError = errors.New("Error getting availability set spec") +) - t.Parallel() - mockCtrl := gomock.NewController(t) - defer mockCtrl.Finish() - scopeMock := mock_availabilitysets.NewMockAvailabilitySetScope(mockCtrl) - clientMock := mock_availabilitysets.NewMockClient(mockCtrl) +// func TestReconcileAvailabilitySets(t *testing.T) { +// testcases := []struct { +// name string +// expectedError string +// expect func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder) +// setupSKUs func(svc *Service) +// }{ +// { +// name: "create or update availability set", +// expectedError: "", +// expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder) { +// s.V(gomock.AssignableToTypeOf(2)).MinTimes(2).Return(klogr.New()) +// s.AvailabilitySet().Return("as-name", true) +// s.ResourceGroup().Return("my-rg") +// s.ClusterName().Return("cl-name") +// s.AdditionalTags().Return(map[string]string{}) +// s.Location().Return("test-location") +// m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "as-name", +// compute.AvailabilitySet{ +// Sku: &compute.Sku{Name: to.StringPtr("Aligned")}, +// AvailabilitySetProperties: &compute.AvailabilitySetProperties{ +// PlatformFaultDomainCount: pointer.Int32Ptr(3), +// }, +// Tags: map[string]*string{"sigs.k8s.io_cluster-api-provider-azure_cluster_cl-name": to.StringPtr("owned"), +// "sigs.k8s.io_cluster-api-provider-azure_role": to.StringPtr("common"), "Name": to.StringPtr("as-name")}, +// Location: to.StringPtr("test-location"), +// }).Return(compute.AvailabilitySet{}, nil) +// }, +// setupSKUs: func(svc *Service) { +// skus := []compute.ResourceSku{ +// { +// Name: to.StringPtr("Aligned"), +// Kind: to.StringPtr(string(resourceskus.AvailabilitySets)), +// Capabilities: &[]compute.ResourceSkuCapabilities{ +// { +// Name: to.StringPtr(resourceskus.MaximumPlatformFaultDomainCount), +// Value: to.StringPtr("3"), +// }, +// }, +// }, +// } +// resourceSkusCache := resourceskus.NewStaticCache(skus, "") +// svc.resourceSKUCache = resourceSkusCache +// }, +// }, +// { +// name: "noop if the machine does not need to be assigned an availability set (machines without a deployment)", +// expectedError: "", +// expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder) { +// s.AvailabilitySet().Return("as-name", false) +// }, +// setupSKUs: func(svc *Service) { +// }, +// }, +// { +// name: "return error", +// expectedError: "failed to create availability set as-name: something went wrong", +// expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder) { +// s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) +// s.AvailabilitySet().Return("as-name", true) +// s.ResourceGroup().Return("my-rg") +// s.ClusterName().Return("cl-name") +// s.AdditionalTags().Return(map[string]string{}) +// s.Location().Return("test-location") +// m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "as-name", +// gomock.AssignableToTypeOf(compute.AvailabilitySet{})).Return(compute.AvailabilitySet{}, errors.New("something went wrong")) +// }, +// setupSKUs: func(svc *Service) { +// skus := []compute.ResourceSku{ +// { +// Name: to.StringPtr("Aligned"), +// Kind: to.StringPtr(string(resourceskus.AvailabilitySets)), +// Capabilities: &[]compute.ResourceSkuCapabilities{ +// { +// Name: to.StringPtr(resourceskus.MaximumPlatformFaultDomainCount), +// Value: to.StringPtr("3"), +// }, +// }, +// }, +// } +// resourceSkusCache := resourceskus.NewStaticCache(skus, "") +// svc.resourceSKUCache = resourceSkusCache +// }, +// }, +// } +// for _, tc := range testcases { +// tc := tc +// t.Run(tc.name, func(t *testing.T) { +// g := NewWithT(t) - tc.expect(scopeMock.EXPECT(), clientMock.EXPECT()) +// t.Parallel() +// mockCtrl := gomock.NewController(t) +// defer mockCtrl.Finish() +// scopeMock := mock_availabilitysets.NewMockAvailabilitySetScope(mockCtrl) +// clientMock := mock_availabilitysets.NewMockClient(mockCtrl) - s := &Service{ - Scope: scopeMock, - Client: clientMock, - } - tc.setupSKUs(s) +// tc.expect(scopeMock.EXPECT(), clientMock.EXPECT()) - err := s.Reconcile(context.TODO()) - if tc.expectedError != "" { - g.Expect(err).To(HaveOccurred()) - g.Expect(err).To(MatchError(tc.expectedError)) - } else { - g.Expect(err).NotTo(HaveOccurred()) - } - }) - } -} +// s := &Service{ +// Scope: scopeMock, +// Client: clientMock, +// } +// tc.setupSKUs(s) + +// err := s.Reconcile(context.TODO()) +// if tc.expectedError != "" { +// g.Expect(err).To(HaveOccurred()) +// g.Expect(err).To(MatchError(tc.expectedError)) +// } else { +// g.Expect(err).NotTo(HaveOccurred()) +// } +// }) +// } +// } func TestDeleteAvailabilitySets(t *testing.T) { testcases := []struct { @@ -160,72 +178,63 @@ func TestDeleteAvailabilitySets(t *testing.T) { expectedError: "", expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.AvailabilitySet().Return("as-name", true) - s.ResourceGroup().Return("my-rg").Times(2) - m.Get(gomockinternal.AContext(), "my-rg", "as-name"). - Return(compute.AvailabilitySet{AvailabilitySetProperties: &compute.AvailabilitySetProperties{}}, nil) - m.Delete(gomockinternal.AContext(), "my-rg", "as-name").Return(nil) + s.AvailabilitySetSpec().Return(&fakeSetSpec, nil) + gomock.InOrder( + s.GetLongRunningOperationState("test-as", serviceName), + m.DeleteAsync(gomockinternal.AContext(), &fakeSetSpec).Return(nil, nil), + s.UpdateDeleteStatus(infrav1.AvailabilitySetReadyCondition, serviceName, nil), + ) }, }, { - name: "noop if AvailabilitySet returns false", + name: "noop if AvailabilitySetSpec returns nil", expectedError: "", expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder) { - s.AvailabilitySet().Return("as-name", false) - }, - }, - { - name: "noop if availability set has vms", - expectedError: "", - expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder) { - s.AvailabilitySet().Return("as-name", true) - s.ResourceGroup().Return("my-rg") - m.Get(gomockinternal.AContext(), "my-rg", "as-name").Return(compute.AvailabilitySet{ - AvailabilitySetProperties: &compute.AvailabilitySetProperties{VirtualMachines: &[]compute.SubResource{ - {ID: to.StringPtr("vm-id")}}}}, nil) - }, - }, - { - name: "noop if availability set is already deleted - get returns 404", - expectedError: "", - expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder) { - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.AvailabilitySet().Return("as-name", true) - s.ResourceGroup().Return("my-rg") - m.Get(gomockinternal.AContext(), "my-rg", "as-name").Return(compute.AvailabilitySet{}, - autorest.DetailedError{StatusCode: 404}) + s.AvailabilitySetSpec().Return(nil, nil) + s.UpdateDeleteStatus(infrav1.AvailabilitySetReadyCondition, serviceName, nil) }, }, + // { + // name: "noop if availability set has vms", + // expectedError: "", + // expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder) { + // s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + // s.AvailabilitySetSpec().Return(&fakeSetSpec, nil) + // gomock.InOrder( + // s.GetLongRunningOperationState("test-as", serviceName), + // m.DeleteAsync(gomockinternal.AContext(), &fakeSetSpec).Return(nil, nil), + // s.UpdateDeleteStatus(infrav1.AvailabilitySetReadyCondition, serviceName, nil), + // ) + + // // m.Get(gomockinternal.AContext(), "my-rg", "as-name").Return(compute.AvailabilitySet{ + // // AvailabilitySetProperties: &compute.AvailabilitySetProperties{VirtualMachines: &[]compute.SubResource{ + // // {ID: to.StringPtr("vm-id")}}}}, nil) + // }, + // }, { name: "noop if availability set is already deleted - delete returns 404", expectedError: "", expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.AvailabilitySet().Return("as-name", true) - s.ResourceGroup().Return("my-rg").Times(2) - m.Get(gomockinternal.AContext(), "my-rg", "as-name").Return(compute.AvailabilitySet{}, nil) - m.Delete(gomockinternal.AContext(), "my-rg", "as-name").Return(autorest.DetailedError{StatusCode: 404}) - }, - }, - { - name: "returns error when availability set get fails", - expectedError: "failed to get availability set as-name in resource group my-rg: something went wrong", - expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder) { - s.AvailabilitySet().Return("as-name", true) - s.ResourceGroup().Return("my-rg").Times(2) - m.Get(gomockinternal.AContext(), "my-rg", "as-name").Return(compute.AvailabilitySet{}, - errors.New("something went wrong")) + s.AvailabilitySetSpec().Return(&fakeSetSpec, nil) + gomock.InOrder( + s.GetLongRunningOperationState("test-as", serviceName), + m.DeleteAsync(gomockinternal.AContext(), &fakeSetSpec).Return(nil, notFoundError), + s.UpdateDeleteStatus(infrav1.AvailabilitySetReadyCondition, serviceName, nil), + ) }, }, { - name: "returns error when delete fails", - expectedError: "failed to delete availability set as-name in resource group my-rg: something went wrong", + name: "returns error when availability set delete fails", + expectedError: "failed to delete resource test-rg/test-as (service: availabilitysets): #: Internal Server Error: StatusCode=500", expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.AvailabilitySet().Return("as-name", true) - s.ResourceGroup().Return("my-rg").Times(3) - m.Get(gomockinternal.AContext(), "my-rg", "as-name").Return(compute.AvailabilitySet{}, nil) - m.Delete(gomockinternal.AContext(), "my-rg", "as-name").Return(errors.New("something went wrong")) + s.AvailabilitySetSpec().Return(&fakeSetSpec, nil) + gomock.InOrder( + s.GetLongRunningOperationState("test-as", serviceName), + m.DeleteAsync(gomockinternal.AContext(), &fakeSetSpec).Return(nil, internalError), + s.UpdateDeleteStatus(infrav1.AvailabilitySetReadyCondition, serviceName, gomockinternal.ErrStrEq(fmt.Sprintf("failed to delete resource test-rg/test-as (service: availabilitysets): %s", internalError.Error()))), + ) }, }, } diff --git a/azure/services/availabilitysets/client.go b/azure/services/availabilitysets/client.go index 4ee39b3e2a8..f238e87aae8 100644 --- a/azure/services/availabilitysets/client.go +++ b/azure/services/availabilitysets/client.go @@ -21,6 +21,9 @@ import ( "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-04-01/compute" "github.com/Azure/go-autorest/autorest" + azureautorest "github.com/Azure/go-autorest/autorest/azure" + "github.com/pkg/errors" + "sigs.k8s.io/cluster-api-provider-azure/azure" "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) @@ -28,8 +31,10 @@ import ( // Client wraps go-sdk. type Client interface { Get(ctx context.Context, resourceGroup, availabilitySetsName string) (compute.AvailabilitySet, error) - CreateOrUpdate(ctx context.Context, resourceGroup, availabilitySetsName string, params compute.AvailabilitySet) (compute.AvailabilitySet, error) - Delete(ctx context.Context, resourceGroup, availabilitySetsName string) error + CreateOrUpdateAsync(context.Context, azure.ResourceSpecGetter) (interface{}, azureautorest.FutureAPI, error) + DeleteAsync(context.Context, azure.ResourceSpecGetter) (azureautorest.FutureAPI, error) + IsDone(context.Context, azureautorest.FutureAPI) (bool, error) + Result(context.Context, azureautorest.FutureAPI, string) (interface{}, error) } // AzureClient contains the Azure go-sdk Client. @@ -61,19 +66,81 @@ func (a *AzureClient) Get(ctx context.Context, resourceGroup, availabilitySetsNa return a.availabilitySets.Get(ctx, resourceGroup, availabilitySetsName) } -// CreateOrUpdate creates or updates an availability set. -func (a *AzureClient) CreateOrUpdate(ctx context.Context, resourceGroup string, availabilitySetsName string, - params compute.AvailabilitySet) (compute.AvailabilitySet, error) { - ctx, _, done := tele.StartSpanWithLogger(ctx, "availabilitysets.AzureClient.CreateOrUpdate") +// CreateOrUpdateAsync creates or updates a availability set 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 +// progress of the operation. +func (ac *AzureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter) (interface{}, azureautorest.FutureAPI, error) { + ctx, _, done := tele.StartSpanWithLogger(ctx, "availabilitySets.AzureClient.CreateOrUpdate") defer done() - return a.availabilitySets.CreateOrUpdate(ctx, resourceGroup, availabilitySetsName, params) + var existingSet interface{} + + if existing, err := ac.Get(ctx, spec.ResourceGroupName(), spec.ResourceName()); err != nil && !azure.ResourceNotFound(err) { + return nil, nil, errors.Wrapf(err, "failed to get availability set %s in %s", spec.ResourceName(), spec.ResourceGroupName()) + } else if err == nil { + existingSet = existing + } + + params, err := spec.Parameters(existingSet) + if err != nil { + return nil, nil, errors.Wrapf(err, "failed to get desired parameters for availability set %s", spec.ResourceName()) + } + + set, ok := params.(compute.AvailabilitySet) + if !ok { + if params == nil { + // nothing to do here. + return existingSet, nil, nil + } + return nil, nil, errors.Errorf("%T is not a compute.AvailabilitySet", params) + } + + result, err := ac.availabilitySets.CreateOrUpdate(ctx, spec.ResourceGroupName(), spec.ResourceName(), set) + return result, nil, err } -// Delete deletes an availability set. -func (a *AzureClient) Delete(ctx context.Context, resourceGroup, availabilitySetsName string) error { - ctx, _, done := tele.StartSpanWithLogger(ctx, "availabilitysets.AzureClient.Delete") - defer done() - _, err := a.availabilitySets.Delete(ctx, resourceGroup, availabilitySetsName) - return err +// DeleteAsync deletes a availability set 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 +// progress of the operation. +func (ac *AzureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (azureautorest.FutureAPI, error) { + ctx, span := tele.Tracer().Start(ctx, "availabilitysets.AzureClient.Delete") + defer span.End() + + existingSet, err := ac.Get(ctx, spec.ResourceGroupName(), spec.ResourceName()) + if err != nil { + return nil, err + } + + // only delete when the availability set does not have any vms + if existingSet.AvailabilitySetProperties != nil && existingSet.VirtualMachines != nil && len(*existingSet.VirtualMachines) > 0 { + // TODO: should this return an error? + return nil, nil + } + + _, err = ac.availabilitySets.Delete(ctx, spec.ResourceGroupName(), spec.ResourceName()) + + if err != nil { + return nil, err + } + + return nil, nil +} + +// Result fetches the result of a long-running operation future. +func (ac *AzureClient) Result(ctx context.Context, futureData azureautorest.FutureAPI, futureType string) (interface{}, error) { + // Result is a no-op for resource groups as only Delete operations return a future. + return nil, nil +} + +// IsDone returns true if the long-running operation has completed. +func (ac *AzureClient) IsDone(ctx context.Context, future azureautorest.FutureAPI) (bool, error) { + ctx, span := tele.Tracer().Start(ctx, "availabilitysets.AzureClient.IsDone") + defer span.End() + + done, err := future.DoneWithContext(ctx, ac.availabilitySets) + if err != nil { + return false, errors.Wrap(err, "failed checking if the operation was complete") + } + + return done, nil } diff --git a/azure/services/availabilitysets/mock_availabilitysets/availabilitysets_mock.go b/azure/services/availabilitysets/mock_availabilitysets/availabilitysets_mock.go index dc3ab411970..8515b7dcdd1 100644 --- a/azure/services/availabilitysets/mock_availabilitysets/availabilitysets_mock.go +++ b/azure/services/availabilitysets/mock_availabilitysets/availabilitysets_mock.go @@ -27,6 +27,8 @@ import ( logr "github.com/go-logr/logr" gomock "github.com/golang/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" ) // MockAvailabilitySetScope is a mock of AvailabilitySetScope interface. @@ -80,21 +82,6 @@ func (mr *MockAvailabilitySetScopeMockRecorder) Authorizer() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Authorizer", reflect.TypeOf((*MockAvailabilitySetScope)(nil).Authorizer)) } -// AvailabilitySet mocks base method. -func (m *MockAvailabilitySetScope) AvailabilitySet() (string, bool) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "AvailabilitySet") - ret0, _ := ret[0].(string) - ret1, _ := ret[1].(bool) - return ret0, ret1 -} - -// AvailabilitySet indicates an expected call of AvailabilitySet. -func (mr *MockAvailabilitySetScopeMockRecorder) AvailabilitySet() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AvailabilitySet", reflect.TypeOf((*MockAvailabilitySetScope)(nil).AvailabilitySet)) -} - // AvailabilitySetEnabled mocks base method. func (m *MockAvailabilitySetScope) AvailabilitySetEnabled() bool { m.ctrl.T.Helper() @@ -109,6 +96,21 @@ func (mr *MockAvailabilitySetScopeMockRecorder) AvailabilitySetEnabled() *gomock return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AvailabilitySetEnabled", reflect.TypeOf((*MockAvailabilitySetScope)(nil).AvailabilitySetEnabled)) } +// AvailabilitySetSpec mocks base method. +func (m *MockAvailabilitySetScope) AvailabilitySetSpec() (azure.ResourceSpecGetter, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AvailabilitySetSpec") + ret0, _ := ret[0].(azure.ResourceSpecGetter) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// AvailabilitySetSpec indicates an expected call of AvailabilitySetSpec. +func (mr *MockAvailabilitySetScopeMockRecorder) AvailabilitySetSpec() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AvailabilitySetSpec", reflect.TypeOf((*MockAvailabilitySetScope)(nil).AvailabilitySetSpec)) +} + // BaseURI mocks base method. func (m *MockAvailabilitySetScope) BaseURI() string { m.ctrl.T.Helper() @@ -193,6 +195,18 @@ func (mr *MockAvailabilitySetScopeMockRecorder) ClusterName() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ClusterName", reflect.TypeOf((*MockAvailabilitySetScope)(nil).ClusterName)) } +// DeleteLongRunningOperationState mocks base method. +func (m *MockAvailabilitySetScope) DeleteLongRunningOperationState(arg0, arg1 string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "DeleteLongRunningOperationState", arg0, arg1) +} + +// DeleteLongRunningOperationState indicates an expected call of DeleteLongRunningOperationState. +func (mr *MockAvailabilitySetScopeMockRecorder) DeleteLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteLongRunningOperationState", reflect.TypeOf((*MockAvailabilitySetScope)(nil).DeleteLongRunningOperationState), arg0, arg1) +} + // Enabled mocks base method. func (m *MockAvailabilitySetScope) Enabled() bool { m.ctrl.T.Helper() @@ -238,6 +252,20 @@ func (mr *MockAvailabilitySetScopeMockRecorder) FailureDomains() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FailureDomains", reflect.TypeOf((*MockAvailabilitySetScope)(nil).FailureDomains)) } +// GetLongRunningOperationState mocks base method. +func (m *MockAvailabilitySetScope) GetLongRunningOperationState(arg0, arg1 string) *v1beta1.Future { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetLongRunningOperationState", arg0, arg1) + ret0, _ := ret[0].(*v1beta1.Future) + return ret0 +} + +// GetLongRunningOperationState indicates an expected call of GetLongRunningOperationState. +func (mr *MockAvailabilitySetScopeMockRecorder) GetLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLongRunningOperationState", reflect.TypeOf((*MockAvailabilitySetScope)(nil).GetLongRunningOperationState), arg0, arg1) +} + // HashKey mocks base method. func (m *MockAvailabilitySetScope) HashKey() string { m.ctrl.T.Helper() @@ -297,6 +325,18 @@ func (mr *MockAvailabilitySetScopeMockRecorder) ResourceGroup() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResourceGroup", reflect.TypeOf((*MockAvailabilitySetScope)(nil).ResourceGroup)) } +// SetLongRunningOperationState mocks base method. +func (m *MockAvailabilitySetScope) SetLongRunningOperationState(arg0 *v1beta1.Future) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetLongRunningOperationState", arg0) +} + +// SetLongRunningOperationState indicates an expected call of SetLongRunningOperationState. +func (mr *MockAvailabilitySetScopeMockRecorder) SetLongRunningOperationState(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetLongRunningOperationState", reflect.TypeOf((*MockAvailabilitySetScope)(nil).SetLongRunningOperationState), arg0) +} + // SubscriptionID mocks base method. func (m *MockAvailabilitySetScope) SubscriptionID() string { m.ctrl.T.Helper() @@ -325,6 +365,42 @@ func (mr *MockAvailabilitySetScopeMockRecorder) TenantID() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TenantID", reflect.TypeOf((*MockAvailabilitySetScope)(nil).TenantID)) } +// UpdateDeleteStatus mocks base method. +func (m *MockAvailabilitySetScope) 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 *MockAvailabilitySetScopeMockRecorder) UpdateDeleteStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateDeleteStatus", reflect.TypeOf((*MockAvailabilitySetScope)(nil).UpdateDeleteStatus), arg0, arg1, arg2) +} + +// UpdatePatchStatus mocks base method. +func (m *MockAvailabilitySetScope) 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 *MockAvailabilitySetScopeMockRecorder) UpdatePatchStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePatchStatus", reflect.TypeOf((*MockAvailabilitySetScope)(nil).UpdatePatchStatus), arg0, arg1, arg2) +} + +// UpdatePutStatus mocks base method. +func (m *MockAvailabilitySetScope) 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 *MockAvailabilitySetScopeMockRecorder) UpdatePutStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePutStatus", reflect.TypeOf((*MockAvailabilitySetScope)(nil).UpdatePutStatus), arg0, arg1, arg2) +} + // V mocks base method. func (m *MockAvailabilitySetScope) V(level int) logr.Logger { m.ctrl.T.Helper() diff --git a/azure/services/availabilitysets/mock_availabilitysets/client_mock.go b/azure/services/availabilitysets/mock_availabilitysets/client_mock.go index 0822b547038..ecf67b14ef5 100644 --- a/azure/services/availabilitysets/mock_availabilitysets/client_mock.go +++ b/azure/services/availabilitysets/mock_availabilitysets/client_mock.go @@ -25,7 +25,9 @@ import ( reflect "reflect" compute "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-04-01/compute" + azure "github.com/Azure/go-autorest/autorest/azure" gomock "github.com/golang/mock/gomock" + azure0 "sigs.k8s.io/cluster-api-provider-azure/azure" ) // MockClient is a mock of Client interface. @@ -51,33 +53,35 @@ func (m *MockClient) EXPECT() *MockClientMockRecorder { return m.recorder } -// CreateOrUpdate mocks base method. -func (m *MockClient) CreateOrUpdate(ctx context.Context, resourceGroup, availabilitySetsName string, params compute.AvailabilitySet) (compute.AvailabilitySet, error) { +// CreateOrUpdateAsync mocks base method. +func (m *MockClient) CreateOrUpdateAsync(arg0 context.Context, arg1 azure0.ResourceSpecGetter) (interface{}, azure.FutureAPI, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateOrUpdate", ctx, resourceGroup, availabilitySetsName, params) - ret0, _ := ret[0].(compute.AvailabilitySet) - ret1, _ := ret[1].(error) - return ret0, ret1 + ret := m.ctrl.Call(m, "CreateOrUpdateAsync", arg0, arg1) + ret0, _ := ret[0].(interface{}) + ret1, _ := ret[1].(azure.FutureAPI) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 } -// CreateOrUpdate indicates an expected call of CreateOrUpdate. -func (mr *MockClientMockRecorder) CreateOrUpdate(ctx, resourceGroup, availabilitySetsName, params interface{}) *gomock.Call { +// CreateOrUpdateAsync indicates an expected call of CreateOrUpdateAsync. +func (mr *MockClientMockRecorder) CreateOrUpdateAsync(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdate", reflect.TypeOf((*MockClient)(nil).CreateOrUpdate), ctx, resourceGroup, availabilitySetsName, params) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdateAsync", reflect.TypeOf((*MockClient)(nil).CreateOrUpdateAsync), arg0, arg1) } -// Delete mocks base method. -func (m *MockClient) Delete(ctx context.Context, resourceGroup, availabilitySetsName string) error { +// DeleteAsync mocks base method. +func (m *MockClient) DeleteAsync(arg0 context.Context, arg1 azure0.ResourceSpecGetter) (azure.FutureAPI, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Delete", ctx, resourceGroup, availabilitySetsName) - ret0, _ := ret[0].(error) - return ret0 + ret := m.ctrl.Call(m, "DeleteAsync", arg0, arg1) + ret0, _ := ret[0].(azure.FutureAPI) + ret1, _ := ret[1].(error) + return ret0, ret1 } -// Delete indicates an expected call of Delete. -func (mr *MockClientMockRecorder) Delete(ctx, resourceGroup, availabilitySetsName interface{}) *gomock.Call { +// DeleteAsync indicates an expected call of DeleteAsync. +func (mr *MockClientMockRecorder) DeleteAsync(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Delete", reflect.TypeOf((*MockClient)(nil).Delete), ctx, resourceGroup, availabilitySetsName) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAsync", reflect.TypeOf((*MockClient)(nil).DeleteAsync), arg0, arg1) } // Get mocks base method. @@ -94,3 +98,33 @@ func (mr *MockClientMockRecorder) Get(ctx, resourceGroup, availabilitySetsName i mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockClient)(nil).Get), ctx, resourceGroup, availabilitySetsName) } + +// IsDone mocks base method. +func (m *MockClient) IsDone(arg0 context.Context, arg1 azure.FutureAPI) (bool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsDone", arg0, arg1) + ret0, _ := ret[0].(bool) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// IsDone indicates an expected call of IsDone. +func (mr *MockClientMockRecorder) IsDone(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsDone", reflect.TypeOf((*MockClient)(nil).IsDone), arg0, arg1) +} + +// Result mocks base method. +func (m *MockClient) Result(arg0 context.Context, arg1 azure.FutureAPI, arg2 string) (interface{}, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Result", arg0, arg1, arg2) + ret0, _ := ret[0].(interface{}) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Result indicates an expected call of Result. +func (mr *MockClientMockRecorder) Result(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Result", reflect.TypeOf((*MockClient)(nil).Result), arg0, arg1, arg2) +} diff --git a/azure/services/availabilitysets/spec.go b/azure/services/availabilitysets/spec.go new file mode 100644 index 00000000000..3066dc9f734 --- /dev/null +++ b/azure/services/availabilitysets/spec.go @@ -0,0 +1,80 @@ +/* +Copyright 2021 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 availabilitysets + +import ( + "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-04-01/compute" + "github.com/Azure/go-autorest/autorest/to" + "github.com/pkg/errors" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" + "sigs.k8s.io/cluster-api-provider-azure/azure/converters" +) + +// AvailabilitySetSpec defines the specification for an availability set. +type AvailabilitySetSpec struct { + Name string + ResourceGroup string + ClusterName string + Location string + FaultDomainCount uint64 + AdditionalTags infrav1.Tags +} + +// ResourceName returns the name of the availability set. +func (s *AvailabilitySetSpec) ResourceName() string { + return s.Name +} + +// ResourceGroupName returns the name of the resource group. +func (s *AvailabilitySetSpec) ResourceGroupName() string { + return s.ResourceGroup +} + +// OwnerResourceName is a no-op for availability sets. +func (s *AvailabilitySetSpec) OwnerResourceName() string { + return "" +} + +// Parameters returns the parameters for the availability set. +func (s *AvailabilitySetSpec) Parameters(existing interface{}) (interface{}, error) { + if existing != nil { + if _, ok := existing.(compute.AvailabilitySet); !ok { + return nil, errors.Errorf("%T is not a compute.AvailabilitySet", existing) + } + // availability set already exists + return nil, nil + } + + asParams := compute.AvailabilitySet{ + Sku: &compute.Sku{ + Name: to.StringPtr(string(compute.AvailabilitySetSkuTypesAligned)), + }, + AvailabilitySetProperties: &compute.AvailabilitySetProperties{ + PlatformFaultDomainCount: to.Int32Ptr(int32(s.FaultDomainCount)), + }, + Tags: converters.TagsToMap(infrav1.Build(infrav1.BuildParams{ + ClusterName: s.ClusterName, + Lifecycle: infrav1.ResourceLifecycleOwned, + Name: to.StringPtr(s.Name), + Role: to.StringPtr(infrav1.CommonRole), + Additional: s.AdditionalTags, + })), + Location: to.StringPtr(s.Location), + } + + return asParams, nil +} diff --git a/azure/types.go b/azure/types.go index 0cf5e361969..592f09c8418 100644 --- a/azure/types.go +++ b/azure/types.go @@ -195,11 +195,6 @@ type PrivateDNSLinkSpec struct { LinkName string } -// AvailabilitySetSpec defines the specification for an availability set. -type AvailabilitySetSpec struct { - Name string -} - // ExtensionSpec defines the specification for a VM or VMScaleSet extension. type ExtensionSpec struct { Name string diff --git a/controllers/azuremachine_reconciler.go b/controllers/azuremachine_reconciler.go index b8a356dcfe1..ef8efe7f2c5 100644 --- a/controllers/azuremachine_reconciler.go +++ b/controllers/azuremachine_reconciler.go @@ -70,7 +70,7 @@ func newAzureMachineService(machineScope *scope.MachineScope) (*azureMachineServ publicIPsSvc: publicips.New(machineScope), tagsSvc: tags.New(machineScope), vmExtensionsSvc: vmextensions.New(machineScope), - availabilitySetsSvc: availabilitysets.New(machineScope, cache), + availabilitySetsSvc: availabilitysets.New(machineScope), skuCache: cache, }, nil }