From 020a2b654ba78a3cf5d1a4422a2cc22f8557e40b 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 | 41 ++- .../availabilitysets/availabilitysets.go | 134 ++++----- .../availabilitysets/availabilitysets_test.go | 255 ++++++++++-------- azure/services/availabilitysets/client.go | 67 ++++- .../availabilitysets_mock.go | 105 ++++++-- .../mock_availabilitysets/client_mock.go | 79 ++++-- azure/services/availabilitysets/spec.go | 98 +++++++ azure/services/availabilitysets/spec_test.go | 92 +++++++ azure/types.go | 5 - 9 files changed, 613 insertions(+), 263 deletions(-) create mode 100644 azure/services/availabilitysets/spec.go create mode 100644 azure/services/availabilitysets/spec_test.go diff --git a/azure/scope/machine.go b/azure/scope/machine.go index f57b948e741..230e2ec5a8b 100644 --- a/azure/scope/machine.go +++ b/azure/scope/machine.go @@ -23,6 +23,7 @@ import ( "strings" "time" + "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" corev1 "k8s.io/api/core/v1" @@ -37,6 +38,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/disks" "sigs.k8s.io/cluster-api-provider-azure/azure/services/resourceskus" "sigs.k8s.io/cluster-api-provider-azure/azure/services/virtualmachines" @@ -94,9 +96,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. @@ -122,9 +125,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))) } } @@ -363,6 +373,29 @@ func (m *MachineScope) ProviderID() string { return parsed.String() } +// AvailabilitySet returns the availability set for this machine if available. +func (m *MachineScope) AvailabilitySetSpec() azure.ResourceSpecGetter { + availabilitySetName, ok := m.AvailabilitySet() + if !ok { + return nil + } + + spec := &availabilitysets.AvailabilitySetSpec{ + Name: availabilitySetName, + ResourceGroup: m.ResourceGroup(), + ClusterName: m.ClusterName(), + Location: m.Location(), + SKU: nil, + AdditionalTags: m.AdditionalTags(), + } + + if m.cache != nil { + spec.SKU = &m.cache.availabilitySetSKU + } + + return spec +} + // 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 a7952e1f588..85c86b4259c 100644 --- a/azure/services/availabilitysets/availabilitysets.go +++ b/azure/services/availabilitysets/availabilitysets.go @@ -18,95 +18,63 @@ 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/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/async" "sigs.k8s.io/cluster-api-provider-azure/azure/services/resourceskus" + "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 { azure.ClusterDescriber - AvailabilitySet() (string, bool) + azure.AsyncStatusUpdater + AvailabilitySetSpec() azure.ResourceSpecGetter } // Service provides operations on Azure resources. type Service struct { Scope AvailabilitySetScope Client + async.Reconciler resourceSKUCache *resourceskus.Cache } // New creates a new availability sets service. func New(scope AvailabilitySetScope, skuCache *resourceskus.Cache) *Service { + client := NewClient(scope) return &Service{ Scope: scope, - Client: NewClient(scope), + Client: client, resourceSKUCache: skuCache, + Reconciler: async.New(scope, client, client), } } // Reconcile creates or updates availability sets. func (s *Service) Reconcile(ctx context.Context) error { - ctx, log, done := tele.StartSpanWithLogger( - ctx, - "availabilitysets.Service.Reconcile", - ) + ctx, log, 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) - if err != nil { - return errors.Wrap(err, "failed to determine max fault domain count") + setSpec := s.Scope.AvailabilitySetSpec() + var err error + if setSpec != nil { + _, err = s.CreateResource(ctx, setSpec, serviceName) + } else { + log.V(2).Info("skip creation when no availability set spec is found") } - log.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()), - } - - _, err = s.Client.CreateOrUpdate(ctx, s.Scope.ResourceGroup(), availabilitySetName, asParams) - if err != nil { - return errors.Wrapf(err, "failed to create availability set %s", availabilitySetName) - } - - log.V(2).Info("successfully created availability set", "availability set", availabilitySetName) - - return nil + s.Scope.UpdatePutStatus(infrav1.AvailabilitySetReadyCondition, serviceName, err) + return err } // Delete deletes availability sets. @@ -114,38 +82,34 @@ func (s *Service) Delete(ctx context.Context) error { ctx, log, done := tele.StartSpanWithLogger(ctx, "availabilitysets.Service.Delete") defer done() - availabilitySetName, ok := s.Scope.AvailabilitySet() - if !ok { - return nil + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) + defer cancel() + + setSpec := s.Scope.AvailabilitySetSpec() + var resultingErr error + if setSpec == nil { + log.V(2).Info("skip deletion when no availability set spec is found") + } else { + existingSet, err := s.Client.Get(ctx, setSpec) + if err != nil { + if !azure.ResourceNotFound(err) { + resultingErr = errors.Wrapf(err, "failed to get availability set %s in resource group %s", setSpec.ResourceName(), setSpec.ResourceGroupName()) + } + } else { + availabilitySet, ok := existingSet.(compute.AvailabilitySet) + if !ok { + resultingErr = errors.Errorf("%T is not a compute.AvailabilitySet", existingSet) + } else { + // only delete when the availability set does not have any vms + if availabilitySet.AvailabilitySetProperties != nil && availabilitySet.VirtualMachines != nil && len(*availabilitySet.VirtualMachines) > 0 { + log.V(2).Info("skip deleting availability set with VMs", "availability set", setSpec.ResourceName()) + } else { + resultingErr = s.DeleteResource(ctx, setSpec, serviceName) + } + } + } } - as, err := s.Client.Get(ctx, s.Scope.ResourceGroup(), availabilitySetName) - if err != nil && azure.ResourceNotFound(err) { - // already deleted - return nil - } - - 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 - } - - log.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 - return nil - } - - if err != nil { - return errors.Wrapf(err, "failed to delete availability set %s in resource group %s", availabilitySetName, s.Scope.ResourceGroup()) - } - - log.V(2).Info("successfully delete availability set", "availability set", availabilitySetName) - - return nil + s.Scope.UpdateDeleteStatus(infrav1.AvailabilitySetReadyCondition, serviceName, resultingErr) + return resultingErr } diff --git a/azure/services/availabilitysets/availabilitysets_test.go b/azure/services/availabilitysets/availabilitysets_test.go index d9b59c6f7a6..7e87167e161 100644 --- a/azure/services/availabilitysets/availabilitysets_test.go +++ b/azure/services/availabilitysets/availabilitysets_test.go @@ -18,100 +18,101 @@ package availabilitysets import ( "context" - "errors" + "net/http" + "strconv" "testing" "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/pkg/errors" + "github.com/golang/mock/gomock" . "github.com/onsi/gomega" - "k8s.io/utils/pointer" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/async/mock_async" "sigs.k8s.io/cluster-api-provider-azure/azure/services/availabilitysets/mock_availabilitysets" "sigs.k8s.io/cluster-api-provider-azure/azure/services/resourceskus" gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock" ) +var ( + fakeFaultDomainCount = 3 + fakeSku = resourceskus.SKU{ + Capabilities: &[]compute.ResourceSkuCapabilities{ + { + Name: to.StringPtr(resourceskus.MaximumPlatformFaultDomainCount), + Value: to.StringPtr(strconv.Itoa(fakeFaultDomainCount)), + }, + }, + } + fakeSetSpec = AvailabilitySetSpec{ + Name: "test-as", + ResourceGroup: "test-rg", + ClusterName: "test-cluster", + Location: "test-location", + SKU: &fakeSku, + AdditionalTags: map[string]string{}, + } + fakeSetSpecMissing = AvailabilitySetSpec{ + Name: "test-as", + ResourceGroup: "test-rg", + ClusterName: "test-cluster", + Location: "test-location", + SKU: nil, + AdditionalTags: map[string]string{}, + } + internalError = autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error") + parameterError = errors.Errorf("some error with parameters") + notFoundError = autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not Found") + fakeSetWithVMs = compute.AvailabilitySet{ + AvailabilitySetProperties: &compute.AvailabilitySetProperties{ + VirtualMachines: &[]compute.SubResource{ + {ID: to.StringPtr("vm-id")}, + }, + }, + } +) + 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) + expect func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) }{ { name: "create or update availability set", expectedError: "", - expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder) { - 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 + expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.AvailabilitySetSpec().Return(&fakeSetSpec) + r.CreateResource(gomockinternal.AContext(), &fakeSetSpec, serviceName).Return(nil, nil) + s.UpdatePutStatus(infrav1.AvailabilitySetReadyCondition, serviceName, nil) }, }, { - name: "noop if the machine does not need to be assigned an availability set (machines without a deployment)", + name: "noop if no availability set spec is found", expectedError: "", - expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder) { - s.AvailabilitySet().Return("as-name", false) - }, - setupSKUs: func(svc *Service) { + expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.AvailabilitySetSpec().Return(nil) + s.UpdatePutStatus(infrav1.AvailabilitySetReadyCondition, serviceName, nil) }, }, { - 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.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")) + name: "missing required value in availability set spec", + expectedError: "some error with parameters", + expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.AvailabilitySetSpec().Return(&fakeSetSpecMissing) + r.CreateResource(gomockinternal.AContext(), &fakeSetSpecMissing, serviceName).Return(nil, parameterError) + s.UpdatePutStatus(infrav1.AvailabilitySetReadyCondition, serviceName, parameterError) }, - 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: "error in creating availability set", + expectedError: "#: Internal Server Error: StatusCode=500", + expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.AvailabilitySetSpec().Return(&fakeSetSpec) + r.CreateResource(gomockinternal.AContext(), &fakeSetSpec, serviceName).Return(nil, internalError) + s.UpdatePutStatus(infrav1.AvailabilitySetReadyCondition, serviceName, internalError) }, }, } @@ -124,15 +125,14 @@ func TestReconcileAvailabilitySets(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() scopeMock := mock_availabilitysets.NewMockAvailabilitySetScope(mockCtrl) - clientMock := mock_availabilitysets.NewMockClient(mockCtrl) + asyncMock := mock_async.NewMockReconciler(mockCtrl) - tc.expect(scopeMock.EXPECT(), clientMock.EXPECT()) + tc.expect(scopeMock.EXPECT(), asyncMock.EXPECT()) s := &Service{ - Scope: scopeMock, - Client: clientMock, + Scope: scopeMock, + Reconciler: asyncMock, } - tc.setupSKUs(s) err := s.Reconcile(context.TODO()) if tc.expectedError != "" { @@ -149,75 +149,94 @@ func TestDeleteAvailabilitySets(t *testing.T) { testcases := []struct { name string expectedError string - expect func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder) + expect func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) }{ { name: "deletes availability set", expectedError: "", - 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{AvailabilitySetProperties: &compute.AvailabilitySetProperties{}}, nil) - m.Delete(gomockinternal.AContext(), "my-rg", "as-name").Return(nil) + expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.AvailabilitySetSpec().Return(&fakeSetSpec) + gomock.InOrder( + m.Get(gomockinternal.AContext(), &fakeSetSpec).Return(compute.AvailabilitySet{}, nil), + r.DeleteResource(gomockinternal.AContext(), &fakeSetSpec, serviceName).Return(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) + expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.AvailabilitySetSpec().Return(nil) + s.UpdateDeleteStatus(infrav1.AvailabilitySetReadyCondition, serviceName, nil) }, }, { - name: "noop if availability set has vms", + name: "delete proceeds with missing required value in availability set spec", 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) + expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.AvailabilitySetSpec().Return(&fakeSetSpecMissing) + gomock.InOrder( + m.Get(gomockinternal.AContext(), &fakeSetSpecMissing).Return(compute.AvailabilitySet{}, nil), + r.DeleteResource(gomockinternal.AContext(), &fakeSetSpecMissing, serviceName).Return(nil), + s.UpdateDeleteStatus(infrav1.AvailabilitySetReadyCondition, serviceName, nil), + ) }, }, { - name: "noop if availability set is already deleted - get returns 404", + 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{}, - autorest.DetailedError{StatusCode: 404}) + expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.AvailabilitySetSpec().Return(&fakeSetSpec) + gomock.InOrder( + m.Get(gomockinternal.AContext(), &fakeSetSpec).Return(fakeSetWithVMs, nil), + s.UpdateDeleteStatus(infrav1.AvailabilitySetReadyCondition, serviceName, nil), + ) }, }, { - name: "noop if availability set is already deleted - delete returns 404", + name: "availability set not found", expectedError: "", - 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{}, nil) - m.Delete(gomockinternal.AContext(), "my-rg", "as-name").Return(autorest.DetailedError{StatusCode: 404}) + expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.AvailabilitySetSpec().Return(&fakeSetSpec) + gomock.InOrder( + m.Get(gomockinternal.AContext(), &fakeSetSpec).Return(nil, notFoundError), + s.UpdateDeleteStatus(infrav1.AvailabilitySetReadyCondition, serviceName, nil), + ) + }, + }, + { + name: "error in getting availability set", + expectedError: "failed to get availability set test-as in resource group test-rg: #: Internal Server Error: StatusCode=500", + expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.AvailabilitySetSpec().Return(&fakeSetSpec) + gomock.InOrder( + m.Get(gomockinternal.AContext(), &fakeSetSpec).Return(nil, internalError), + s.UpdateDeleteStatus(infrav1.AvailabilitySetReadyCondition, serviceName, gomockinternal.ErrStrEq("failed to get availability set test-as in resource group test-rg: #: Internal Server Error: StatusCode=500")), + ) }, }, { - 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")) + name: "availability set get result is not an availability set", + expectedError: "string is not a compute.AvailabilitySet", + expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.AvailabilitySetSpec().Return(&fakeSetSpec) + gomock.InOrder( + m.Get(gomockinternal.AContext(), &fakeSetSpec).Return("not an availability set", nil), + s.UpdateDeleteStatus(infrav1.AvailabilitySetReadyCondition, serviceName, gomockinternal.ErrStrEq("string is not a compute.AvailabilitySet")), + ) }, }, { - name: "returns error when delete fails", - expectedError: "failed to delete 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(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")) + name: "error in deleting availability set", + expectedError: "#: Internal Server Error: StatusCode=500", + expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.AvailabilitySetSpec().Return(&fakeSetSpec) + gomock.InOrder( + m.Get(gomockinternal.AContext(), &fakeSetSpec).Return(compute.AvailabilitySet{}, nil), + r.DeleteResource(gomockinternal.AContext(), &fakeSetSpec, serviceName).Return(internalError), + s.UpdateDeleteStatus(infrav1.AvailabilitySetReadyCondition, serviceName, internalError), + ) }, }, } @@ -231,12 +250,14 @@ func TestDeleteAvailabilitySets(t *testing.T) { defer mockCtrl.Finish() scopeMock := mock_availabilitysets.NewMockAvailabilitySetScope(mockCtrl) clientMock := mock_availabilitysets.NewMockClient(mockCtrl) + asyncMock := mock_async.NewMockReconciler(mockCtrl) - tc.expect(scopeMock.EXPECT(), clientMock.EXPECT()) + tc.expect(scopeMock.EXPECT(), clientMock.EXPECT(), asyncMock.EXPECT()) s := &Service{ - Scope: scopeMock, - Client: clientMock, + Scope: scopeMock, + Client: clientMock, + Reconciler: asyncMock, } err := s.Delete(context.TODO()) diff --git a/azure/services/availabilitysets/client.go b/azure/services/availabilitysets/client.go index 4ee39b3e2a8..389de4393e4 100644 --- a/azure/services/availabilitysets/client.go +++ b/azure/services/availabilitysets/client.go @@ -21,15 +21,20 @@ 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" ) // 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 + Get(ctx context.Context, spec azure.ResourceSpecGetter) (interface{}, error) + CreateOrUpdateAsync(context.Context, azure.ResourceSpecGetter, interface{}) (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. @@ -54,26 +59,60 @@ func newAvailabilitySetsClient(subscriptionID string, baseURI string, authorizer } // Get gets an availability set. -func (a *AzureClient) Get(ctx context.Context, resourceGroup, availabilitySetsName string) (compute.AvailabilitySet, error) { +func (ac *AzureClient) Get(ctx context.Context, spec azure.ResourceSpecGetter) (interface{}, error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "availabilitysets.AzureClient.Get") defer done() - return a.availabilitySets.Get(ctx, resourceGroup, availabilitySetsName) + return ac.availabilitySets.Get(ctx, spec.ResourceGroupName(), spec.ResourceName()) } -// 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, parameters interface{}) (interface{}, azureautorest.FutureAPI, error) { + ctx, _, done := tele.StartSpanWithLogger(ctx, "availabilitySets.AzureClient.CreateOrUpdateAsync") defer done() - return a.availabilitySets.CreateOrUpdate(ctx, resourceGroup, availabilitySetsName, params) + availabilitySet, ok := parameters.(compute.AvailabilitySet) + if !ok { + return nil, nil, errors.Errorf("%T is not a compute.AvailabilitySet", parameters) + } + + result, err := ac.availabilitySets.CreateOrUpdate(ctx, spec.ResourceGroupName(), spec.ResourceName(), availabilitySet) + return result, nil, err } -// Delete deletes an availability set. -func (a *AzureClient) Delete(ctx context.Context, resourceGroup, availabilitySetsName string) error { +// 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, _, done := tele.StartSpanWithLogger(ctx, "availabilitysets.AzureClient.Delete") defer done() - _, err := a.availabilitySets.Delete(ctx, resourceGroup, availabilitySetsName) - return err + + _, 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 baeb90044b7..58aaf983974 100644 --- a/azure/services/availabilitysets/mock_availabilitysets/availabilitysets_mock.go +++ b/azure/services/availabilitysets/mock_availabilitysets/availabilitysets_mock.go @@ -26,6 +26,8 @@ import ( autorest "github.com/Azure/go-autorest/autorest" 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. @@ -79,21 +81,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() @@ -108,6 +95,20 @@ 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 { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AvailabilitySetSpec") + ret0, _ := ret[0].(azure.ResourceSpecGetter) + return ret0 +} + +// 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() @@ -192,6 +193,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) +} + // FailureDomains mocks base method. func (m *MockAvailabilitySetScope) FailureDomains() []string { m.ctrl.T.Helper() @@ -206,6 +219,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() @@ -248,6 +275,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() @@ -275,3 +314,39 @@ func (mr *MockAvailabilitySetScopeMockRecorder) TenantID() *gomock.Call { mr.mock.ctrl.T.Helper() 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) +} diff --git a/azure/services/availabilitysets/mock_availabilitysets/client_mock.go b/azure/services/availabilitysets/mock_availabilitysets/client_mock.go index 0822b547038..8ad89c67c76 100644 --- a/azure/services/availabilitysets/mock_availabilitysets/client_mock.go +++ b/azure/services/availabilitysets/mock_availabilitysets/client_mock.go @@ -24,8 +24,9 @@ import ( context "context" 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,46 +52,78 @@ 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, arg2 interface{}) (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, arg2) + 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, arg2 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, arg2) } -// 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. -func (m *MockClient) Get(ctx context.Context, resourceGroup, availabilitySetsName string) (compute.AvailabilitySet, error) { +func (m *MockClient) Get(ctx context.Context, spec azure0.ResourceSpecGetter) (interface{}, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Get", ctx, resourceGroup, availabilitySetsName) - ret0, _ := ret[0].(compute.AvailabilitySet) + 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 *MockClientMockRecorder) Get(ctx, resourceGroup, availabilitySetsName interface{}) *gomock.Call { +func (mr *MockClientMockRecorder) Get(ctx, spec interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockClient)(nil).Get), ctx, spec) +} + +// 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, "Get", reflect.TypeOf((*MockClient)(nil).Get), ctx, resourceGroup, availabilitySetsName) + 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..1f884500897 --- /dev/null +++ b/azure/services/availabilitysets/spec.go @@ -0,0 +1,98 @@ +/* +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 ( + "strconv" + + "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" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/resourceskus" +) + +// AvailabilitySetSpec defines the specification for an availability set. +type AvailabilitySetSpec struct { + Name string + ResourceGroup string + ClusterName string + Location string + SKU *resourceskus.SKU + 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 + } + + if s.SKU == nil { + return nil, errors.New("unable to get required availability set SKU from machine cache") + } + + var faultDomainCount *int32 + faultDomainCountStr, ok := s.SKU.GetCapability(resourceskus.MaximumPlatformFaultDomainCount) + if !ok { + return nil, errors.Errorf("unable to get required availability set SKU capability %s", resourceskus.MaximumPlatformFaultDomainCount) + } + count, err := strconv.ParseInt(faultDomainCountStr, 10, 32) + if err != nil { + return nil, errors.Wrapf(err, "unable to parse availability set fault domain count") + } + faultDomainCount = to.Int32Ptr(int32(count)) + + asParams := compute.AvailabilitySet{ + Sku: &compute.Sku{ + Name: to.StringPtr(string(compute.AvailabilitySetSkuTypesAligned)), + }, + AvailabilitySetProperties: &compute.AvailabilitySetProperties{ + PlatformFaultDomainCount: 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/services/availabilitysets/spec_test.go b/azure/services/availabilitysets/spec_test.go new file mode 100644 index 00000000000..818ff532987 --- /dev/null +++ b/azure/services/availabilitysets/spec_test.go @@ -0,0 +1,92 @@ +/* +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 ( + "testing" + + "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-04-01/compute" + "github.com/Azure/go-autorest/autorest/to" + . "github.com/onsi/gomega" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/resourceskus" +) + +var ( + fakeSetSpecMissingCap = AvailabilitySetSpec{ + Name: "test-as", + ResourceGroup: "test-rg", + ClusterName: "test-cluster", + Location: "test-location", + SKU: &resourceskus.SKU{}, + AdditionalTags: map[string]string{}, + } +) + +func TestParameters(t *testing.T) { + testcases := []struct { + name string + spec *AvailabilitySetSpec + existing interface{} + expect func(g *WithT, result interface{}) + expectedError string + }{ + { + name: "error when no SKU is present", + spec: &fakeSetSpecMissing, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeNil()) + }, + expectedError: "unable to get required availability set SKU from machine cache", + }, + { + name: "error when SKU capability is missing", + spec: &fakeSetSpecMissingCap, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeNil()) + }, + expectedError: "unable to get required availability set SKU capability MaximumPlatformFaultDomainCount", + }, + { + name: "get parameters when all values are present", + spec: &fakeSetSpec, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeAssignableToTypeOf(compute.AvailabilitySet{})) + g.Expect(result.(compute.AvailabilitySet).PlatformFaultDomainCount).To(Equal(to.Int32Ptr(int32(fakeFaultDomainCount)))) + }, + expectedError: "", + }, + } + for _, tc := range testcases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + t.Parallel() + + result, err := tc.spec.Parameters(tc.existing) + if tc.expectedError != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err).To(MatchError(tc.expectedError)) + } else { + g.Expect(err).NotTo(HaveOccurred()) + } + tc.expect(g, result) + }) + } +} diff --git a/azure/types.go b/azure/types.go index 45bb67f19c5..0dde9011482 100644 --- a/azure/types.go +++ b/azure/types.go @@ -190,11 +190,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