diff --git a/azure/scope/machine.go b/azure/scope/machine.go index 944de4219043..230e2ec5a8b2 100644 --- a/azure/scope/machine.go +++ b/azure/scope/machine.go @@ -20,7 +20,6 @@ import ( "context" "encoding/base64" "encoding/json" - "strconv" "strings" "time" @@ -382,23 +381,16 @@ func (m *MachineScope) AvailabilitySetSpec() azure.ResourceSpecGetter { } spec := &availabilitysets.AvailabilitySetSpec{ - Name: availabilitySetName, - ResourceGroup: m.ResourceGroup(), - ClusterName: m.ClusterName(), - Location: m.Location(), - FaultDomainCount: nil, - AdditionalTags: m.AdditionalTags(), + Name: availabilitySetName, + ResourceGroup: m.ResourceGroup(), + ClusterName: m.ClusterName(), + Location: m.Location(), + SKU: nil, + AdditionalTags: m.AdditionalTags(), } if m.cache != nil { - asSKU := m.cache.availabilitySetSKU - faultDomainCountStr, ok := asSKU.GetCapability(resourceskus.MaximumPlatformFaultDomainCount) - if ok { - faultDomainCount, err := strconv.ParseUint(faultDomainCountStr, 10, 32) - if err == nil { - spec.FaultDomainCount = to.Int32Ptr(int32(faultDomainCount)) - } - } + spec.SKU = &m.cache.availabilitySetSKU } return spec diff --git a/azure/services/availabilitysets/availabilitysets.go b/azure/services/availabilitysets/availabilitysets.go index 73280a6d675a..12d6c762d71b 100644 --- a/azure/services/availabilitysets/availabilitysets.go +++ b/azure/services/availabilitysets/availabilitysets.go @@ -66,20 +66,20 @@ func (s *Service) Reconcile(ctx context.Context) error { defer cancel() setSpec := s.Scope.AvailabilitySetSpec() - if setSpec == nil { + 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") - s.Scope.UpdatePutStatus(infrav1.AvailabilitySetReadyCondition, serviceName, nil) - return nil } - _, err := s.CreateResource(ctx, setSpec, serviceName) s.Scope.UpdatePutStatus(infrav1.AvailabilitySetReadyCondition, serviceName, err) return err } // Delete deletes availability sets. func (s *Service) Delete(ctx context.Context) error { - ctx, log, done := tele.StartSpanWithLogger(ctx, "virtualmachines.Service.Delete") + ctx, log, done := tele.StartSpanWithLogger(ctx, "availabilitysets.Service.Delete") defer done() ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) diff --git a/azure/services/availabilitysets/availabilitysets_test.go b/azure/services/availabilitysets/availabilitysets_test.go index a2677dc4228f..70c56d56cfca 100644 --- a/azure/services/availabilitysets/availabilitysets_test.go +++ b/azure/services/availabilitysets/availabilitysets_test.go @@ -19,6 +19,7 @@ package availabilitysets import ( "context" "net/http" + "strconv" "testing" "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-04-01/compute" @@ -31,28 +32,38 @@ import ( 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", - FaultDomainCount: to.Int32Ptr(3), - AdditionalTags: map[string]string{}, + 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", - FaultDomainCount: nil, - AdditionalTags: map[string]string{}, + 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") - paramError = errors.New("some error with parameters") + parameterError = errors.Errorf("some error with parameters") notFoundError = autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not Found") fakeSetWithVMs = compute.AvailabilitySet{ AvailabilitySetProperties: &compute.AvailabilitySetProperties{ @@ -91,8 +102,8 @@ func TestReconcileAvailabilitySets(t *testing.T) { expectedError: "some error with parameters", expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_availabilitysets.MockClientMockRecorder, r *mock_async.MockReconcilerMockRecorder) { s.AvailabilitySetSpec().Return(&fakeSetSpecMissing) - r.CreateResource(gomockinternal.AContext(), &fakeSetSpecMissing, serviceName).Return(nil, paramError) - s.UpdatePutStatus(infrav1.AvailabilitySetReadyCondition, serviceName, paramError) + r.CreateResource(gomockinternal.AContext(), &fakeSetSpecMissing, serviceName).Return(nil, parameterError) + s.UpdatePutStatus(infrav1.AvailabilitySetReadyCondition, serviceName, parameterError) }, }, { diff --git a/azure/services/availabilitysets/spec.go b/azure/services/availabilitysets/spec.go index 66de91b1e6a1..c87b51da25ba 100644 --- a/azure/services/availabilitysets/spec.go +++ b/azure/services/availabilitysets/spec.go @@ -17,21 +17,24 @@ 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 - FaultDomainCount *int32 - AdditionalTags infrav1.Tags + Name string + ResourceGroup string + ClusterName string + Location string + SKU *resourceskus.SKU + AdditionalTags infrav1.Tags } // ResourceName returns the name of the availability set. @@ -59,17 +62,26 @@ func (s *AvailabilitySetSpec) Parameters(existing interface{}) (interface{}, err return nil, nil } - if s.FaultDomainCount == nil { - return nil, errors.New("unable to find required FaultDomainCount from machine cache") + if s.SKU == nil { + return nil, errors.New("unable to find required availability set SKU from machine cache") + } + + var faultDomainCount *int32 + faultDomainCountStr, ok := s.SKU.GetCapability(resourceskus.MaximumPlatformFaultDomainCount) + if ok { + 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{ - // TODO: should availability sets be given a name? Sku: &compute.Sku{ Name: to.StringPtr(string(compute.AvailabilitySetSkuTypesAligned)), }, AvailabilitySetProperties: &compute.AvailabilitySetProperties{ - PlatformFaultDomainCount: s.FaultDomainCount, + PlatformFaultDomainCount: faultDomainCount, }, Tags: converters.TagsToMap(infrav1.Build(infrav1.BuildParams{ ClusterName: s.ClusterName, diff --git a/azure/services/availabilitysets/spec_test.go b/azure/services/availabilitysets/spec_test.go index 1d53c0f45b72..20f9b77c07e8 100644 --- a/azure/services/availabilitysets/spec_test.go +++ b/azure/services/availabilitysets/spec_test.go @@ -20,6 +20,7 @@ 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" ) @@ -32,13 +33,13 @@ func TestParameters(t *testing.T) { expectedError string }{ { - name: "error when no fault domain count is present", + 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 find required FaultDomainCount from machine cache", + expectedError: "unable to find required availability set SKU from machine cache", }, { name: "get parameters when all values are present", @@ -46,7 +47,7 @@ func TestParameters(t *testing.T) { existing: nil, expect: func(g *WithT, result interface{}) { g.Expect(result).To(BeAssignableToTypeOf(compute.AvailabilitySet{})) - g.Expect(result.(compute.AvailabilitySet).PlatformFaultDomainCount).To(Equal(fakeSetSpec.FaultDomainCount)) + g.Expect(result.(compute.AvailabilitySet).PlatformFaultDomainCount).To(Equal(to.Int32Ptr(int32(fakeFaultDomainCount)))) }, expectedError: "", },