diff --git a/api/v1beta1/azuremachine_types.go b/api/v1beta1/azuremachine_types.go index e8b9a0a365a..6eecdc5ae9f 100644 --- a/api/v1beta1/azuremachine_types.go +++ b/api/v1beta1/azuremachine_types.go @@ -186,7 +186,9 @@ type AzureMachineStatus struct { } // +kubebuilder:object:root=true -// +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.ready",description="AzureMachine ready status" +// +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].status" +// +kubebuilder:printcolumn:name="Reason",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].reason" +// +kubebuilder:printcolumn:name="Message",type="string",priority=1,JSONPath=".status.conditions[?(@.type=='Ready')].message" // +kubebuilder:printcolumn:name="State",type="string",JSONPath=".status.vmState",description="Azure VM provisioning state" // +kubebuilder:printcolumn:name="Cluster",type="string",priority=1,JSONPath=".metadata.labels.cluster\\.x-k8s\\.io/cluster-name",description="Cluster to which this AzureMachine belongs" // +kubebuilder:printcolumn:name="Machine",type="string",priority=1,JSONPath=".metadata.ownerReferences[?(@.kind==\"Machine\")].name",description="Machine object to which this AzureMachine belongs" diff --git a/azure/converters/futures_test.go b/azure/converters/futures_test.go index e04d5a8e0aa..74fe3943329 100644 --- a/azure/converters/futures_test.go +++ b/azure/converters/futures_test.go @@ -21,7 +21,7 @@ import ( "testing" azureautorest "github.com/Azure/go-autorest/autorest/azure" - "github.com/onsi/gomega" + . "github.com/onsi/gomega" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" ) @@ -76,7 +76,7 @@ func Test_SDKToFuture(t *testing.T) { service string resourceName string rgName string - expect func(*gomega.GomegaWithT, *infrav1.Future, error) + expect func(*GomegaWithT, *infrav1.Future, error) }{ { name: "valid future", @@ -85,9 +85,9 @@ func Test_SDKToFuture(t *testing.T) { service: "test-service", resourceName: "test-resource", rgName: "test-group", - expect: func(g *gomega.GomegaWithT, f *infrav1.Future, err error) { - g.Expect(err).Should(gomega.BeNil()) - g.Expect(f).Should(gomega.BeEquivalentTo(&infrav1.Future{ + expect: func(g *GomegaWithT, f *infrav1.Future, err error) { + g.Expect(err).Should(BeNil()) + g.Expect(f).Should(BeEquivalentTo(&infrav1.Future{ Type: infrav1.DeleteFuture, ServiceName: "test-service", Name: "test-resource", @@ -102,7 +102,7 @@ func Test_SDKToFuture(t *testing.T) { c := c t.Run(c.name, func(t *testing.T) { t.Parallel() - g := gomega.NewGomegaWithT(t) + g := NewGomegaWithT(t) result, err := SDKToFuture(c.future, c.futureType, c.service, c.resourceName, c.rgName) c.expect(g, result, err) }) @@ -113,35 +113,35 @@ func Test_FutureToSDK(t *testing.T) { cases := []struct { name string future infrav1.Future - expect func(*gomega.GomegaWithT, azureautorest.FutureAPI, error) + expect func(*GomegaWithT, azureautorest.FutureAPI, error) }{ { name: "data is empty", future: emptyDataFuture, - expect: func(g *gomega.GomegaWithT, f azureautorest.FutureAPI, err error) { - g.Expect(err.Error()).Should(gomega.ContainSubstring("failed to unmarshal future data")) + expect: func(g *GomegaWithT, f azureautorest.FutureAPI, err error) { + g.Expect(err.Error()).Should(ContainSubstring("failed to unmarshal future data")) }, }, { name: "data is not base64 encoded", future: decodedDataFuture, - expect: func(g *gomega.GomegaWithT, f azureautorest.FutureAPI, err error) { - g.Expect(err.Error()).Should(gomega.ContainSubstring("failed to base64 decode future data")) + expect: func(g *GomegaWithT, f azureautorest.FutureAPI, err error) { + g.Expect(err.Error()).Should(ContainSubstring("failed to base64 decode future data")) }, }, { name: "base64 data is not a valid future", future: invalidFuture, - expect: func(g *gomega.GomegaWithT, f azureautorest.FutureAPI, err error) { - g.Expect(err.Error()).Should(gomega.ContainSubstring("failed to unmarshal future data")) + expect: func(g *GomegaWithT, f azureautorest.FutureAPI, err error) { + g.Expect(err.Error()).Should(ContainSubstring("failed to unmarshal future data")) }, }, { name: "valid future data", future: validFuture, - expect: func(g *gomega.GomegaWithT, f azureautorest.FutureAPI, err error) { - g.Expect(err).Should(gomega.BeNil()) - g.Expect(f).Should(gomega.BeAssignableToTypeOf(&azureautorest.Future{})) + expect: func(g *GomegaWithT, f azureautorest.FutureAPI, err error) { + g.Expect(err).Should(BeNil()) + g.Expect(f).Should(BeAssignableToTypeOf(&azureautorest.Future{})) }, }, } @@ -150,7 +150,7 @@ func Test_FutureToSDK(t *testing.T) { c := c t.Run(c.name, func(t *testing.T) { t.Parallel() - g := gomega.NewGomegaWithT(t) + g := NewGomegaWithT(t) result, err := FutureToSDK(c.future) c.expect(g, result, err) }) diff --git a/azure/converters/identity.go b/azure/converters/identity.go index 8ba3b7d17c7..ef447b61852 100644 --- a/azure/converters/identity.go +++ b/azure/converters/identity.go @@ -30,6 +30,29 @@ import ( // ErrUserAssignedIdentitiesNotFound is the error thrown when user assigned identities is not passed with the identity type being UserAssigned. var ErrUserAssignedIdentitiesNotFound = errors.New("the user-assigned identity provider ids must not be null or empty for 'UserAssigned' identity type") +// VMIdentityToVMSDK converts CAPZ VM identity to Azure SDK identity. +func VMIdentityToVMSDK(identity infrav1.VMIdentity, uami []infrav1.UserAssignedIdentity) (*compute.VirtualMachineIdentity, error) { + if identity == infrav1.VMIdentitySystemAssigned { + return &compute.VirtualMachineIdentity{ + Type: compute.ResourceIdentityTypeSystemAssigned, + }, nil + } + + if identity == infrav1.VMIdentityUserAssigned { + userIdentitiesMap, err := UserAssignedIdentitiesToVMSDK(uami) + if err != nil { + return nil, errors.Wrap(err, "failed to assign VM identity") + } + + return &compute.VirtualMachineIdentity{ + Type: compute.ResourceIdentityTypeUserAssigned, + UserAssignedIdentities: userIdentitiesMap, + }, nil + } + + return nil, nil +} + // UserAssignedIdentitiesToVMSDK converts CAPZ user assigned identities associated with the Virtual Machine to Azure SDK identities // The user identity dictionary key references will be ARM resource ids in the form: // '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.ManagedIdentity/userAssignedIdentities/{identityName}'. diff --git a/azure/converters/identity_test.go b/azure/converters/identity_test.go index c27b240dc46..01e273d2fed 100644 --- a/azure/converters/identity_test.go +++ b/azure/converters/identity_test.go @@ -20,7 +20,7 @@ import ( "testing" "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-04-01/compute" - "github.com/onsi/gomega" + . "github.com/onsi/gomega" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" ) @@ -49,26 +49,87 @@ var expectedVMSSSDKObject = map[string]*compute.VirtualMachineScaleSetIdentityUs "/without/prefix": {}, } +func Test_VMIdentityToVMSDK(t *testing.T) { + cases := []struct { + Name string + identityType infrav1.VMIdentity + uami []infrav1.UserAssignedIdentity + Expect func(*GomegaWithT, *compute.VirtualMachineIdentity, error) + }{ + { + Name: "Should return a system assigned identity when identity is system assigned", + identityType: infrav1.VMIdentitySystemAssigned, + Expect: func(g *GomegaWithT, m *compute.VirtualMachineIdentity, err error) { + g.Expect(err).Should(BeNil()) + g.Expect(m).Should(Equal(&compute.VirtualMachineIdentity{ + Type: compute.ResourceIdentityTypeSystemAssigned, + })) + }, + }, + { + Name: "Should return user assigned identities when identity is user assigned", + identityType: infrav1.VMIdentityUserAssigned, + uami: []infrav1.UserAssignedIdentity{{ProviderID: "my-uami-1"}, {ProviderID: "my-uami-2"}}, + Expect: func(g *GomegaWithT, m *compute.VirtualMachineIdentity, err error) { + g.Expect(err).Should(BeNil()) + g.Expect(m).Should(Equal(&compute.VirtualMachineIdentity{ + Type: compute.ResourceIdentityTypeUserAssigned, + UserAssignedIdentities: map[string]*compute.VirtualMachineIdentityUserAssignedIdentitiesValue{ + "my-uami-1": {}, + "my-uami-2": {}, + }, + })) + }, + }, + { + Name: "Should fail when no user assigned identities are specified and identity is user assigned", + identityType: infrav1.VMIdentityUserAssigned, + uami: []infrav1.UserAssignedIdentity{}, + Expect: func(g *GomegaWithT, m *compute.VirtualMachineIdentity, err error) { + g.Expect(err.Error()).Should(ContainSubstring(ErrUserAssignedIdentitiesNotFound.Error())) + }, + }, + { + Name: "Should return nil if no known identity is specified", + identityType: "", + Expect: func(g *GomegaWithT, m *compute.VirtualMachineIdentity, err error) { + g.Expect(err).Should(BeNil()) + g.Expect(m).Should(BeNil()) + }, + }, + } + + for _, c := range cases { + c := c + t.Run(c.Name, func(t *testing.T) { + t.Parallel() + g := NewGomegaWithT(t) + subject, err := VMIdentityToVMSDK(c.identityType, c.uami) + c.Expect(g, subject, err) + }) + } +} + func Test_UserAssignedIdentitiesToVMSDK(t *testing.T) { cases := []struct { Name string SubjectFactory []infrav1.UserAssignedIdentity - Expect func(*gomega.GomegaWithT, map[string]*compute.VirtualMachineIdentityUserAssignedIdentitiesValue, error) + Expect func(*GomegaWithT, map[string]*compute.VirtualMachineIdentityUserAssignedIdentitiesValue, error) }{ { Name: "ShouldPopulateWithData", SubjectFactory: sampleSubjectFactory, - Expect: func(g *gomega.GomegaWithT, m map[string]*compute.VirtualMachineIdentityUserAssignedIdentitiesValue, err error) { - g.Expect(err).Should(gomega.BeNil()) - g.Expect(m).Should(gomega.Equal(expectedVMSDKObject)) + Expect: func(g *GomegaWithT, m map[string]*compute.VirtualMachineIdentityUserAssignedIdentitiesValue, err error) { + g.Expect(err).Should(BeNil()) + g.Expect(m).Should(Equal(expectedVMSDKObject)) }, }, { Name: "ShouldFailWithError", SubjectFactory: []infrav1.UserAssignedIdentity{}, - Expect: func(g *gomega.GomegaWithT, m map[string]*compute.VirtualMachineIdentityUserAssignedIdentitiesValue, err error) { - g.Expect(err).Should(gomega.Equal(ErrUserAssignedIdentitiesNotFound)) + Expect: func(g *GomegaWithT, m map[string]*compute.VirtualMachineIdentityUserAssignedIdentitiesValue, err error) { + g.Expect(err).Should(Equal(ErrUserAssignedIdentitiesNotFound)) }, }, } @@ -77,7 +138,7 @@ func Test_UserAssignedIdentitiesToVMSDK(t *testing.T) { c := c t.Run(c.Name, func(t *testing.T) { t.Parallel() - g := gomega.NewGomegaWithT(t) + g := NewGomegaWithT(t) subject, err := UserAssignedIdentitiesToVMSDK(c.SubjectFactory) c.Expect(g, subject, err) }) @@ -88,22 +149,22 @@ func Test_UserAssignedIdentitiesToVMSSSDK(t *testing.T) { cases := []struct { Name string SubjectFactory []infrav1.UserAssignedIdentity - Expect func(*gomega.GomegaWithT, map[string]*compute.VirtualMachineScaleSetIdentityUserAssignedIdentitiesValue, error) + Expect func(*GomegaWithT, map[string]*compute.VirtualMachineScaleSetIdentityUserAssignedIdentitiesValue, error) }{ { Name: "ShouldPopulateWithData", SubjectFactory: sampleSubjectFactory, - Expect: func(g *gomega.GomegaWithT, m map[string]*compute.VirtualMachineScaleSetIdentityUserAssignedIdentitiesValue, err error) { - g.Expect(err).Should(gomega.BeNil()) - g.Expect(m).Should(gomega.Equal(expectedVMSSSDKObject)) + Expect: func(g *GomegaWithT, m map[string]*compute.VirtualMachineScaleSetIdentityUserAssignedIdentitiesValue, err error) { + g.Expect(err).Should(BeNil()) + g.Expect(m).Should(Equal(expectedVMSSSDKObject)) }, }, { Name: "ShouldFailWithError", SubjectFactory: []infrav1.UserAssignedIdentity{}, - Expect: func(g *gomega.GomegaWithT, m map[string]*compute.VirtualMachineScaleSetIdentityUserAssignedIdentitiesValue, err error) { - g.Expect(err).Should(gomega.Equal(ErrUserAssignedIdentitiesNotFound)) + Expect: func(g *GomegaWithT, m map[string]*compute.VirtualMachineScaleSetIdentityUserAssignedIdentitiesValue, err error) { + g.Expect(err).Should(Equal(ErrUserAssignedIdentitiesNotFound)) }, }, } @@ -112,7 +173,7 @@ func Test_UserAssignedIdentitiesToVMSSSDK(t *testing.T) { c := c t.Run(c.Name, func(t *testing.T) { t.Parallel() - g := gomega.NewGomegaWithT(t) + g := NewGomegaWithT(t) subject, err := UserAssignedIdentitiesToVMSSSDK(c.SubjectFactory) c.Expect(g, subject, err) }) diff --git a/azure/converters/image.go b/azure/converters/image.go index 866368e91de..cae1710d2e8 100644 --- a/azure/converters/image.go +++ b/azure/converters/image.go @@ -20,6 +20,7 @@ import ( "fmt" "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" @@ -67,3 +68,27 @@ func specificImageToSDK(image *infrav1.Image) (*compute.ImageReference, error) { ID: image.ID, }, nil } + +// ImageToPlan converts a CAPZ Image to an Azure Compute Plan. +func ImageToPlan(image *infrav1.Image) *compute.Plan { + // Plan is needed when using a Shared Gallery image with Plan details. + if image.SharedGallery != nil && image.SharedGallery.Publisher != nil && image.SharedGallery.SKU != nil && image.SharedGallery.Offer != nil { + return &compute.Plan{ + Publisher: image.SharedGallery.Publisher, + Name: image.SharedGallery.SKU, + Product: image.SharedGallery.Offer, + } + } + + // Plan is needed for third party Marketplace images. + if image.Marketplace != nil && image.Marketplace.ThirdPartyImage { + return &compute.Plan{ + Publisher: to.StringPtr(image.Marketplace.Publisher), + Name: to.StringPtr(image.Marketplace.SKU), + Product: to.StringPtr(image.Marketplace.Offer), + } + } + + // Otherwise return nil. + return nil +} diff --git a/azure/converters/image_test.go b/azure/converters/image_test.go new file mode 100644 index 00000000000..caa565ea506 --- /dev/null +++ b/azure/converters/image_test.go @@ -0,0 +1,126 @@ +/* +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 converters + +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" + + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" +) + +func Test_ImageToPlan(t *testing.T) { + cases := []struct { + name string + image *infrav1.Image + expect func(*GomegaWithT, *compute.Plan) + }{ + { + name: "Should return a plan for a SIG image with plan details", + image: &infrav1.Image{ + SharedGallery: &infrav1.AzureSharedGalleryImage{ + SubscriptionID: "fake-sub-id", + ResourceGroup: "fake-rg", + Gallery: "fake-gallery-name", + Name: "fake-image-name", + Version: "v1.0.0", + Publisher: to.StringPtr("my-publisher"), + Offer: to.StringPtr("my-offer"), + SKU: to.StringPtr("my-sku"), + }, + }, + expect: func(g *GomegaWithT, result *compute.Plan) { + g.Expect(result).To(Equal(&compute.Plan{ + Name: to.StringPtr("my-sku"), + Publisher: to.StringPtr("my-publisher"), + Product: to.StringPtr("my-offer"), + })) + }, + }, + { + name: "Should return nil for a SIG image without plan info", + image: &infrav1.Image{ + SharedGallery: &infrav1.AzureSharedGalleryImage{ + SubscriptionID: "fake-sub-id", + ResourceGroup: "fake-rg", + Gallery: "fake-gallery-name", + Name: "fake-image-name", + Version: "v1.0.0", + }, + }, + expect: func(g *GomegaWithT, result *compute.Plan) { + g.Expect(result).To(BeNil()) + }, + }, + { + name: "Should return nil for a Marketplace first party image", + image: &infrav1.Image{ + Marketplace: &infrav1.AzureMarketplaceImage{ + Publisher: "my-publisher", + Offer: "my-offer", + SKU: "my-sku", + Version: "v0.5.0", + ThirdPartyImage: false, + }, + }, + expect: func(g *GomegaWithT, result *compute.Plan) { + g.Expect(result).To(BeNil()) + }, + }, + { + name: "Should return a plan for a Marketplace third party image", + image: &infrav1.Image{ + Marketplace: &infrav1.AzureMarketplaceImage{ + Publisher: "my-publisher", + Offer: "my-offer", + SKU: "my-sku", + Version: "v0.5.0", + ThirdPartyImage: true, + }, + }, + expect: func(g *GomegaWithT, result *compute.Plan) { + g.Expect(result).To(Equal(&compute.Plan{ + Name: to.StringPtr("my-sku"), + Publisher: to.StringPtr("my-publisher"), + Product: to.StringPtr("my-offer"), + })) + }, + }, + { + name: "Should return nil for an image ID", + image: &infrav1.Image{ + ID: to.StringPtr("fake/image/id"), + }, + expect: func(g *GomegaWithT, result *compute.Plan) { + g.Expect(result).To(BeNil()) + }, + }, + } + + for _, c := range cases { + c := c + t.Run(c.name, func(t *testing.T) { + t.Parallel() + g := NewGomegaWithT(t) + result := ImageToPlan(c.image) + c.expect(g, result) + }) + } +} diff --git a/azure/errors.go b/azure/errors.go index 367fa77296d..ef50c83a161 100644 --- a/azure/errors.go +++ b/azure/errors.go @@ -89,7 +89,7 @@ func (t ReconcileError) Error() string { } switch t.errorType { case TransientErrorType: - return fmt.Sprintf("transient reconcile error occurred: %s. Object will be requeued after %s", errStr, t.requestAfter.String()) + return fmt.Sprintf("%s. Object will be requeued after %s", errStr, t.requestAfter.String()) case TerminalErrorType: return fmt.Sprintf("reconcile error that cannot be recovered occurred: %s. Object will not be requeued", errStr) default: diff --git a/azure/scope/cluster.go b/azure/scope/cluster.go index 7ea68da55f8..160ff9bb914 100644 --- a/azure/scope/cluster.go +++ b/azure/scope/cluster.go @@ -39,6 +39,7 @@ import ( "sigs.k8s.io/cluster-api-provider-azure/azure" "sigs.k8s.io/cluster-api-provider-azure/azure/services/groups" "sigs.k8s.io/cluster-api-provider-azure/util/futures" + "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) // ClusterScopeParams defines the input parameters used to create a new Scope. @@ -53,6 +54,9 @@ type ClusterScopeParams struct { // NewClusterScope creates a new Scope from the supplied parameters. // This is meant to be called for each reconcile iteration. func NewClusterScope(ctx context.Context, params ClusterScopeParams) (*ClusterScope, error) { + ctx, _, done := tele.StartSpanWithLogger(ctx, "azure.clusterScope.NewClusterScope") + defer done() + if params.Cluster == nil { return nil, errors.New("failed to generate new scope from nil Cluster") } diff --git a/azure/scope/machine.go b/azure/scope/machine.go index 8a0bd3a2e09..27ee6646703 100644 --- a/azure/scope/machine.go +++ b/azure/scope/machine.go @@ -39,7 +39,10 @@ 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/resourceskus" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/virtualmachines" "sigs.k8s.io/cluster-api-provider-azure/util/futures" + "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) // MachineScopeParams defines the input parameters used to create a new MachineScope. @@ -49,6 +52,7 @@ type MachineScopeParams struct { ClusterScope azure.ClusterScoper Machine *clusterv1.Machine AzureMachine *infrav1.AzureMachine + Cache *MachineCache } // NewMachineScope creates a new MachineScope from the supplied parameters. @@ -69,8 +73,9 @@ func NewMachineScope(params MachineScopeParams) (*MachineScope, error) { helper, err := patch.NewHelper(params.AzureMachine, params.Client) if err != nil { - return nil, errors.Errorf("failed to init patch helper: %v ", err) + return nil, errors.Wrap(err, "failed to init patch helper") } + return &MachineScope{ client: params.Client, Machine: params.Machine, @@ -78,6 +83,7 @@ func NewMachineScope(params MachineScopeParams) (*MachineScope, error) { Logger: params.Logger, patchHelper: helper, ClusterScoper: params.ClusterScope, + cache: params.Cache, }, nil } @@ -90,24 +96,76 @@ type MachineScope struct { azure.ClusterScoper Machine *clusterv1.Machine AzureMachine *infrav1.AzureMachine + cache *MachineCache +} + +// 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 +} + +// InitMachineCache sets cached information about the machine to be used in the scope. +func (m *MachineScope) InitMachineCache(ctx context.Context) error { + ctx, _, done := tele.StartSpanWithLogger(ctx, "azure.machineScope.initMachineCache") + defer done() + + if m.cache == nil { + var err error + m.cache = &MachineCache{} + + m.cache.BootstrapData, err = m.GetBootstrapData(ctx) + if err != nil { + return err + } + + m.cache.VMImage, err = m.GetVMImage() + if err != nil { + return err + } + + skuCache, err := resourceskus.GetCache(m, m.Location()) + 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 nil } // VMSpec returns the VM spec. -func (m *MachineScope) VMSpec() azure.VMSpec { - return azure.VMSpec{ +func (m *MachineScope) VMSpec() azure.ResourceSpecGetter { + spec := &virtualmachines.VMSpec{ Name: m.Name(), + Location: m.Location(), + ResourceGroup: m.ResourceGroup(), + ClusterName: m.ClusterName(), Role: m.Role(), - NICNames: m.NICNames(), + NICIDs: m.NICIDs(), SSHKeyData: m.AzureMachine.Spec.SSHPublicKey, Size: m.AzureMachine.Spec.VMSize, OSDisk: m.AzureMachine.Spec.OSDisk, DataDisks: m.AzureMachine.Spec.DataDisks, + AvailabilitySetID: m.AvailabilitySetID(), Zone: m.AvailabilityZone(), Identity: m.AzureMachine.Spec.Identity, UserAssignedIdentities: m.AzureMachine.Spec.UserAssignedIdentities, SpotVMOptions: m.AzureMachine.Spec.SpotVMOptions, SecurityProfile: m.AzureMachine.Spec.SecurityProfile, + AdditionalTags: m.AdditionalTags(), + ProviderID: m.ProviderID(), } + if m.cache != nil { + spec.SKU = m.cache.VMSKU + spec.Image = m.cache.VMImage + spec.BootstrapData = m.cache.BootstrapData + } + return spec } // TagsSpecs returns the tags for the AzureMachine. @@ -183,14 +241,14 @@ func (m *MachineScope) NICSpecs() []azure.NICSpec { return []azure.NICSpec{spec} } -// NICNames returns the NIC names. -func (m *MachineScope) NICNames() []string { +// NICIDs returns the NIC resource IDs. +func (m *MachineScope) NICIDs() []string { nicspecs := m.NICSpecs() - nicNames := make([]string, len(nicspecs)) + nicIDs := make([]string, len(nicspecs)) for i, nic := range nicspecs { - nicNames[i] = nic.Name + nicIDs[i] = azure.NetworkInterfaceID(m.SubscriptionID(), m.ResourceGroup(), nic.Name) } - return nicNames + return nicIDs } // DiskSpecs returns the disk specs. @@ -331,6 +389,15 @@ func (m *MachineScope) AvailabilitySet() (string, bool) { return "", false } +// AvailabilitySetID returns the availability set for this machine, or "" if there is no availability set. +func (m *MachineScope) AvailabilitySetID() string { + var asID string + if asName, ok := m.AvailabilitySet(); ok { + asID = azure.AvailabilitySetID(m.SubscriptionID(), m.ResourceGroup(), asName) + } + return asID +} + // SetProviderID sets the AzureMachine providerID in spec. func (m *MachineScope) SetProviderID(v string) { m.AzureMachine.Spec.ProviderID = to.StringPtr(v) @@ -389,32 +456,6 @@ func (m *MachineScope) SetBootstrapConditions(provisioningState string, extensio } } -// UpdateStatus updates the AzureMachine status. -func (m *MachineScope) UpdateStatus() { - switch m.VMState() { - case infrav1.Succeeded: - m.V(2).Info("VM is running", "id", m.GetVMID()) - conditions.MarkTrue(m.AzureMachine, infrav1.VMRunningCondition) - case infrav1.Creating: - m.V(2).Info("VM is creating", "id", m.GetVMID()) - conditions.MarkFalse(m.AzureMachine, infrav1.VMRunningCondition, infrav1.VMCreatingReason, clusterv1.ConditionSeverityInfo, "") - case infrav1.Updating: - m.V(2).Info("VM is updating", "id", m.GetVMID()) - conditions.MarkFalse(m.AzureMachine, infrav1.VMRunningCondition, infrav1.VMUpdatingReason, clusterv1.ConditionSeverityInfo, "") - case infrav1.Deleting: - m.Info("Unexpected VM deletion", "id", m.GetVMID()) - conditions.MarkFalse(m.AzureMachine, infrav1.VMRunningCondition, infrav1.VMDeletingReason, clusterv1.ConditionSeverityWarning, "") - case infrav1.Failed: - m.Error(errors.New("Failed to create or update VM"), "VM is in failed state", "id", m.GetVMID()) - m.SetFailureReason(capierrors.UpdateMachineError) - m.SetFailureMessage(errors.Errorf("Azure VM state is %s", m.VMState())) - conditions.MarkFalse(m.AzureMachine, infrav1.VMRunningCondition, infrav1.VMProvisionFailedReason, clusterv1.ConditionSeverityError, "") - default: - m.V(2).Info("VM state is undefined", "id", m.GetVMID()) - conditions.MarkUnknown(m.AzureMachine, infrav1.VMRunningCondition, "", "") - } -} - // SetAnnotation sets a key value annotation on the AzureMachine. func (m *MachineScope) SetAnnotation(key, value string) { if m.AzureMachine.Annotations == nil { diff --git a/azure/scope/machine_test.go b/azure/scope/machine_test.go index 44250bd9501..0141d6e0b12 100644 --- a/azure/scope/machine_test.go +++ b/azure/scope/machine_test.go @@ -1726,3 +1726,158 @@ func TestMachineScope_NICSpecs(t *testing.T) { }) } } + +func TestDiskSpecs(t *testing.T) { + testcases := []struct { + name string + machineScope MachineScope + want []azure.DiskSpec + }{ + { + name: "only os disk", + machineScope: MachineScope{ + ClusterScoper: &ClusterScope{ + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + }, + AzureCluster: &infrav1.AzureCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + }, + }, + AzureMachine: &infrav1.AzureMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-azure-machine", + }, + Spec: infrav1.AzureMachineSpec{ + OSDisk: infrav1.OSDisk{ + DiskSizeGB: to.Int32Ptr(30), + OSType: "Linux", + }, + }, + }, + Machine: &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "machine", + }, + }, + }, + want: []azure.DiskSpec{ + { + Name: "my-azure-machine_OSDisk", + }, + }, + }, + { + name: "os and data disks", + machineScope: MachineScope{ + ClusterScoper: &ClusterScope{ + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + }, + AzureCluster: &infrav1.AzureCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + }, + }, + AzureMachine: &infrav1.AzureMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-azure-machine", + }, + Spec: infrav1.AzureMachineSpec{ + OSDisk: infrav1.OSDisk{ + DiskSizeGB: to.Int32Ptr(30), + OSType: "Linux", + }, + DataDisks: []infrav1.DataDisk{ + { + NameSuffix: "etcddisk", + }, + }, + }, + }, + Machine: &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "machine", + }, + }, + }, + want: []azure.DiskSpec{ + { + Name: "my-azure-machine_OSDisk", + }, + { + Name: "my-azure-machine_etcddisk", + }, + }, + }, { + name: "os and multiple data disks", + machineScope: MachineScope{ + ClusterScoper: &ClusterScope{ + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + }, + AzureCluster: &infrav1.AzureCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + }, + }, + AzureMachine: &infrav1.AzureMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-azure-machine", + }, + Spec: infrav1.AzureMachineSpec{ + OSDisk: infrav1.OSDisk{ + DiskSizeGB: to.Int32Ptr(30), + OSType: "Linux", + }, + DataDisks: []infrav1.DataDisk{ + { + NameSuffix: "etcddisk", + }, + { + NameSuffix: "otherdisk", + }, + }, + }, + }, + Machine: &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "machine", + }, + }, + }, + want: []azure.DiskSpec{ + { + Name: "my-azure-machine_OSDisk", + }, + { + Name: "my-azure-machine_etcddisk", + }, + { + Name: "my-azure-machine_otherdisk", + }, + }, + }, + } + + for _, tt := range testcases { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + result := tt.machineScope.DiskSpecs() + if !reflect.DeepEqual(result, tt.want) { + t.Errorf("DiskSpecs(), DiskSpecs = %v, want %v", result, tt.want) + } + }) + } +} diff --git a/azure/scope/machinepool_test.go b/azure/scope/machinepool_test.go index 31d5e5336a5..99a8d8fabf3 100644 --- a/azure/scope/machinepool_test.go +++ b/azure/scope/machinepool_test.go @@ -119,7 +119,7 @@ func TestMachinePoolScope_SetBootstrapConditions(t *testing.T) { return string(infrav1.Creating), "bazz" }, Verify: func(g *WithT, amp *infrav1exp.AzureMachinePool, err error) { - g.Expect(err).To(MatchError("transient reconcile error occurred: extension is still in provisioning state. This likely means that bootstrapping has not yet completed on the VM. Object will be requeued after 30s")) + g.Expect(err).To(MatchError("extension is still in provisioning state. This likely means that bootstrapping has not yet completed on the VM. Object will be requeued after 30s")) g.Expect(conditions.IsFalse(amp, infrav1.BootstrapSucceededCondition)) g.Expect(conditions.GetReason(amp, infrav1.BootstrapSucceededCondition)).To(Equal(infrav1.BootstrapInProgressReason)) severity := conditions.GetSeverity(amp, infrav1.BootstrapSucceededCondition) diff --git a/azure/services/async/async.go b/azure/services/async/async.go index ab5b9f71bf0..9c211a561bc 100644 --- a/azure/services/async/async.go +++ b/azure/services/async/async.go @@ -26,15 +26,19 @@ import ( "sigs.k8s.io/cluster-api-provider-azure/azure" "sigs.k8s.io/cluster-api-provider-azure/azure/converters" "sigs.k8s.io/cluster-api-provider-azure/util/reconciler" + "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) // processOngoingOperation is a helper function that will process an ongoing operation to check if it is done. // If it is not done, it will return a transient error. -func processOngoingOperation(ctx context.Context, scope FutureScope, client FutureHandler, resourceName string, serviceName string) error { +func processOngoingOperation(ctx context.Context, scope FutureScope, client FutureHandler, resourceName string, serviceName string) (interface{}, error) { + ctx, _, done := tele.StartSpanWithLogger(ctx, "async.Service.processOngoingOperation") + defer done() + future := scope.GetLongRunningOperationState(resourceName, serviceName) if future == nil { scope.V(2).Info("no long running operation found", "service", serviceName, "resource", resourceName) - return nil + return nil, nil } sdkFuture, err := converters.FutureToSDK(*future) if err != nil { @@ -42,27 +46,33 @@ func processOngoingOperation(ctx context.Context, scope FutureScope, client Futu // In theory, this should never happen, but if for some reason the future that is already stored in Status isn't properly formatted // and we don't reset it we would be stuck in an infinite loop trying to parse it. scope.DeleteLongRunningOperationState(resourceName, serviceName) - return errors.Wrap(err, "could not decode future data, resetting long-running operation state") + return nil, errors.Wrap(err, "could not decode future data, resetting long-running operation state") } - done, err := client.IsDone(ctx, sdkFuture) + isDone, err := client.IsDone(ctx, sdkFuture) if err != nil { - return errors.Wrap(err, "failed checking if the operation was complete") + return nil, errors.Wrap(err, "failed checking if the operation was complete") } - if !done { + if !isDone { // Operation is still in progress, update conditions and requeue. scope.V(2).Info("long running operation is still ongoing", "service", serviceName, "resource", resourceName) - return azure.WithTransientError(azure.NewOperationNotDoneError(future), retryAfter(sdkFuture)) + return nil, azure.WithTransientError(azure.NewOperationNotDoneError(future), retryAfter(sdkFuture)) } // Resource has been created/deleted/updated. scope.V(2).Info("long running operation has completed", "service", serviceName, "resource", resourceName) - scope.DeleteLongRunningOperationState(resourceName, serviceName) - return nil + result, err := client.Result(ctx, sdkFuture, future.Type) + if err == nil { + scope.DeleteLongRunningOperationState(resourceName, serviceName) + } + return result, err } // CreateResource implements the logic for creating a resource Asynchronously. -func CreateResource(ctx context.Context, scope FutureScope, client Creator, spec azure.ResourceSpecGetter, serviceName string) error { +func CreateResource(ctx context.Context, scope FutureScope, client Creator, spec azure.ResourceSpecGetter, serviceName string) (interface{}, error) { + ctx, _, done := tele.StartSpanWithLogger(ctx, "async.Service.CreateResource") + defer done() + resourceName := spec.ResourceName() rgName := spec.ResourceGroupName() @@ -74,33 +84,37 @@ func CreateResource(ctx context.Context, scope FutureScope, client Creator, spec // No long running operation is active, so create the resource. scope.V(2).Info("creating resource", "service", serviceName, "resource", resourceName, "resourceGroup", rgName) - sdkFuture, err := client.CreateOrUpdateAsync(ctx, spec) + result, sdkFuture, err := client.CreateOrUpdateAsync(ctx, spec) if err != nil { if sdkFuture != nil { future, err := converters.SDKToFuture(sdkFuture, infrav1.PutFuture, serviceName, resourceName, rgName) if err != nil { - return errors.Wrapf(err, "failed to create resource %s/%s (service: %s)", rgName, resourceName, serviceName) + return nil, errors.Wrapf(err, "failed to create resource %s/%s (service: %s)", rgName, resourceName, serviceName) } scope.SetLongRunningOperationState(future) - return azure.WithTransientError(azure.NewOperationNotDoneError(future), retryAfter(sdkFuture)) + return nil, azure.WithTransientError(azure.NewOperationNotDoneError(future), retryAfter(sdkFuture)) } - return errors.Wrapf(err, "failed to create resource %s/%s (service: %s)", rgName, resourceName, serviceName) + return nil, errors.Wrapf(err, "failed to create resource %s/%s (service: %s)", rgName, resourceName, serviceName) } scope.V(2).Info("successfully created resource", "service", serviceName, "resource", resourceName, "resourceGroup", rgName) - return nil + return result, nil } // DeleteResource implements the logic for deleting a resource Asynchronously. func DeleteResource(ctx context.Context, scope FutureScope, client Deleter, spec azure.ResourceSpecGetter, serviceName string) error { + ctx, _, done := tele.StartSpanWithLogger(ctx, "async.Service.DeleteResource") + defer done() + resourceName := spec.ResourceName() rgName := spec.ResourceGroupName() // Check if there is an ongoing long running operation. future := scope.GetLongRunningOperationState(resourceName, serviceName) if future != nil { - return processOngoingOperation(ctx, scope, client, resourceName, serviceName) + _, err := processOngoingOperation(ctx, scope, client, resourceName, serviceName) + return err } // No long running operation is active, so delete the resource. diff --git a/azure/services/async/async_test.go b/azure/services/async/async_test.go index e6b76d89e64..96e264ffca0 100644 --- a/azure/services/async/async_test.go +++ b/azure/services/async/async_test.go @@ -22,8 +22,10 @@ import ( "net/http" "testing" + "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2019-05-01/resources" "github.com/Azure/go-autorest/autorest" azureautorest "github.com/Azure/go-autorest/autorest/azure" + "github.com/Azure/go-autorest/autorest/to" "github.com/golang/mock/gomock" . "github.com/onsi/gomega" "k8s.io/klog/v2/klogr" @@ -63,11 +65,12 @@ var ( // TestProcessOngoingOperation tests the processOngoingOperation function. func TestProcessOngoingOperation(t *testing.T) { testcases := []struct { - name string - resourceName string - serviceName string - expectedError string - expect func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockFutureHandlerMockRecorder) + name string + resourceName string + serviceName string + expectedError string + expectedResult interface{} + expect func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockFutureHandlerMockRecorder) }{ { name: "no future data stored in status", @@ -113,15 +116,17 @@ func TestProcessOngoingOperation(t *testing.T) { }, }, { - name: "operation is done", - expectedError: "", - resourceName: "test-resource", - serviceName: "test-service", + name: "operation is done", + expectedError: "", + expectedResult: resources.Group{Name: to.StringPtr("test-resource")}, + resourceName: "test-resource", + serviceName: "test-service", expect: func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockFutureHandlerMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) s.GetLongRunningOperationState("test-resource", "test-service").Return(&validDeleteFuture) c.IsDone(gomockinternal.AContext(), gomock.AssignableToTypeOf(&azureautorest.Future{})).Return(true, nil) s.DeleteLongRunningOperationState("test-resource", "test-service") + c.Result(gomockinternal.AContext(), gomock.AssignableToTypeOf(&azureautorest.Future{}), infrav1.DeleteFuture).Return(resources.Group{Name: to.StringPtr("test-resource")}, nil) }, }, } @@ -139,13 +144,18 @@ func TestProcessOngoingOperation(t *testing.T) { tc.expect(scopeMock.EXPECT(), clientMock.EXPECT()) - err := processOngoingOperation(context.TODO(), scopeMock, clientMock, tc.resourceName, tc.serviceName) + result, err := processOngoingOperation(context.TODO(), scopeMock, clientMock, tc.resourceName, tc.serviceName) if tc.expectedError != "" { g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(ContainSubstring(tc.expectedError)) } else { g.Expect(err).NotTo(HaveOccurred()) } + if tc.expectedResult != nil { + g.Expect(result).To(Equal(tc.expectedResult)) + } else { + g.Expect(result).To(BeNil()) + } }) } } @@ -153,14 +163,15 @@ func TestProcessOngoingOperation(t *testing.T) { // TestCreateResource tests the CreateResource function. func TestCreateResource(t *testing.T) { testcases := []struct { - name string - serviceName string - expectedError string - expect func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockCreatorMockRecorder, r *mock_azure.MockResourceSpecGetterMockRecorder) + name string + serviceName string + expectedError string + expectedResult interface{} + expect func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockCreatorMockRecorder, r *mock_azure.MockResourceSpecGetterMockRecorder) }{ { name: "create operation is already in progress", - expectedError: "transient reconcile error occurred: operation type PUT on Azure resource test-group/test-resource is not done. Object will be requeued after 15s", + expectedError: "operation type PUT on Azure resource test-group/test-resource is not done. Object will be requeued after 15s", serviceName: "test-service", expect: func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockCreatorMockRecorder, r *mock_azure.MockResourceSpecGetterMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) @@ -171,15 +182,16 @@ func TestCreateResource(t *testing.T) { }, }, { - name: "create async returns success", - expectedError: "", - serviceName: "test-service", + name: "create async returns success", + expectedError: "", + expectedResult: "test-resource", + serviceName: "test-service", expect: func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockCreatorMockRecorder, r *mock_azure.MockResourceSpecGetterMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) r.ResourceName().Return("test-resource") r.ResourceGroupName().Return("test-group") s.GetLongRunningOperationState("test-resource", "test-service").Return(nil) - c.CreateOrUpdateAsync(gomockinternal.AContext(), gomock.AssignableToTypeOf(&mock_azure.MockResourceSpecGetter{})).Return(nil, nil) + c.CreateOrUpdateAsync(gomockinternal.AContext(), gomock.AssignableToTypeOf(&mock_azure.MockResourceSpecGetter{})).Return("test-resource", nil, nil) }, }, { @@ -191,19 +203,19 @@ func TestCreateResource(t *testing.T) { r.ResourceName().Return("test-resource") r.ResourceGroupName().Return("test-group") s.GetLongRunningOperationState("test-resource", "test-service").Return(nil) - c.CreateOrUpdateAsync(gomockinternal.AContext(), gomock.AssignableToTypeOf(&mock_azure.MockResourceSpecGetter{})).Return(nil, fakeError) + c.CreateOrUpdateAsync(gomockinternal.AContext(), gomock.AssignableToTypeOf(&mock_azure.MockResourceSpecGetter{})).Return(nil, nil, fakeError) }, }, { name: "create async exits before completing", - expectedError: "transient reconcile error occurred: operation type PUT on Azure resource test-group/test-resource is not done. Object will be requeued after 15s", + expectedError: "operation type PUT on Azure resource test-group/test-resource is not done. Object will be requeued after 15s", serviceName: "test-service", expect: func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockCreatorMockRecorder, r *mock_azure.MockResourceSpecGetterMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) r.ResourceName().Return("test-resource") r.ResourceGroupName().Return("test-group") s.GetLongRunningOperationState("test-resource", "test-service").Return(nil) - c.CreateOrUpdateAsync(gomockinternal.AContext(), gomock.AssignableToTypeOf(&mock_azure.MockResourceSpecGetter{})).Return(&azureautorest.Future{}, errCtxExceeded) + c.CreateOrUpdateAsync(gomockinternal.AContext(), gomock.AssignableToTypeOf(&mock_azure.MockResourceSpecGetter{})).Return(nil, &azureautorest.Future{}, errCtxExceeded) s.SetLongRunningOperationState(gomock.AssignableToTypeOf(&infrav1.Future{})) }, }, @@ -223,12 +235,13 @@ func TestCreateResource(t *testing.T) { tc.expect(scopeMock.EXPECT(), clientMock.EXPECT(), specMock.EXPECT()) - err := CreateResource(context.TODO(), scopeMock, clientMock, specMock, tc.serviceName) + result, err := CreateResource(context.TODO(), scopeMock, clientMock, specMock, tc.serviceName) if tc.expectedError != "" { g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(ContainSubstring(tc.expectedError)) } else { g.Expect(err).NotTo(HaveOccurred()) + g.Expect(result).To(Equal(tc.expectedResult)) } }) } @@ -244,7 +257,7 @@ func TestDeleteResource(t *testing.T) { }{ { name: "delete operation is already in progress", - expectedError: "transient reconcile error occurred: operation type DELETE on Azure resource test-group/test-resource is not done. Object will be requeued after 15s", + expectedError: "operation type DELETE on Azure resource test-group/test-resource is not done. Object will be requeued after 15s", serviceName: "test-service", expect: func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockDeleterMockRecorder, r *mock_azure.MockResourceSpecGetterMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) @@ -280,7 +293,7 @@ func TestDeleteResource(t *testing.T) { }, { name: "delete async exits before completing", - expectedError: "transient reconcile error occurred: operation type DELETE on Azure resource test-group/test-resource is not done. Object will be requeued after 15s", + expectedError: "operation type DELETE on Azure resource test-group/test-resource is not done. Object will be requeued after 15s", serviceName: "test-service", expect: func(s *mock_async.MockFutureScopeMockRecorder, c *mock_async.MockDeleterMockRecorder, r *mock_azure.MockResourceSpecGetterMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) diff --git a/azure/services/async/interfaces.go b/azure/services/async/interfaces.go index 0a847aa4502..b68b7b0cea4 100644 --- a/azure/services/async/interfaces.go +++ b/azure/services/async/interfaces.go @@ -34,12 +34,14 @@ type FutureScope interface { type FutureHandler interface { // IsDone returns true if the operation is complete. IsDone(ctx context.Context, future azureautorest.FutureAPI) (bool, error) + // Result returns the result of the operation. + Result(ctx context.Context, future azureautorest.FutureAPI, futureType string) (interface{}, error) } // Creator is a client that can create or update a resource asynchronously. type Creator interface { FutureHandler - CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter) (azureautorest.FutureAPI, error) + CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter) (interface{}, azureautorest.FutureAPI, error) } // Deleter is a client that can delete a resource asynchronously. diff --git a/azure/services/async/mock_async/async_mock.go b/azure/services/async/mock_async/async_mock.go index 2de8405723d..de490f7564a 100644 --- a/azure/services/async/mock_async/async_mock.go +++ b/azure/services/async/mock_async/async_mock.go @@ -261,6 +261,21 @@ func (mr *MockFutureHandlerMockRecorder) IsDone(ctx, future interface{}) *gomock return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsDone", reflect.TypeOf((*MockFutureHandler)(nil).IsDone), ctx, future) } +// Result mocks base method. +func (m *MockFutureHandler) Result(ctx context.Context, future azure.FutureAPI, futureType string) (interface{}, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Result", ctx, future, futureType) + ret0, _ := ret[0].(interface{}) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Result indicates an expected call of Result. +func (mr *MockFutureHandlerMockRecorder) Result(ctx, future, futureType interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Result", reflect.TypeOf((*MockFutureHandler)(nil).Result), ctx, future, futureType) +} + // MockCreator is a mock of Creator interface. type MockCreator struct { ctrl *gomock.Controller @@ -285,12 +300,13 @@ func (m *MockCreator) EXPECT() *MockCreatorMockRecorder { } // CreateOrUpdateAsync mocks base method. -func (m *MockCreator) CreateOrUpdateAsync(ctx context.Context, spec azure0.ResourceSpecGetter) (azure.FutureAPI, error) { +func (m *MockCreator) CreateOrUpdateAsync(ctx context.Context, spec azure0.ResourceSpecGetter) (interface{}, azure.FutureAPI, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "CreateOrUpdateAsync", ctx, spec) - ret0, _ := ret[0].(azure.FutureAPI) - ret1, _ := ret[1].(error) - return ret0, ret1 + ret0, _ := ret[0].(interface{}) + ret1, _ := ret[1].(azure.FutureAPI) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 } // CreateOrUpdateAsync indicates an expected call of CreateOrUpdateAsync. @@ -314,6 +330,21 @@ func (mr *MockCreatorMockRecorder) IsDone(ctx, future interface{}) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsDone", reflect.TypeOf((*MockCreator)(nil).IsDone), ctx, future) } +// Result mocks base method. +func (m *MockCreator) Result(ctx context.Context, future azure.FutureAPI, futureType string) (interface{}, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Result", ctx, future, futureType) + ret0, _ := ret[0].(interface{}) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Result indicates an expected call of Result. +func (mr *MockCreatorMockRecorder) Result(ctx, future, futureType interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Result", reflect.TypeOf((*MockCreator)(nil).Result), ctx, future, futureType) +} + // MockDeleter is a mock of Deleter interface. type MockDeleter struct { ctrl *gomock.Controller @@ -366,3 +397,18 @@ func (mr *MockDeleterMockRecorder) IsDone(ctx, future interface{}) *gomock.Call mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsDone", reflect.TypeOf((*MockDeleter)(nil).IsDone), ctx, future) } + +// Result mocks base method. +func (m *MockDeleter) Result(ctx context.Context, future azure.FutureAPI, futureType string) (interface{}, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Result", ctx, future, futureType) + ret0, _ := ret[0].(interface{}) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Result indicates an expected call of Result. +func (mr *MockDeleterMockRecorder) Result(ctx, future, futureType interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Result", reflect.TypeOf((*MockDeleter)(nil).Result), ctx, future, futureType) +} diff --git a/azure/services/disks/disks_test.go b/azure/services/disks/disks_test.go index c549affbb6f..ff0427d8f66 100644 --- a/azure/services/disks/disks_test.go +++ b/azure/services/disks/disks_test.go @@ -22,18 +22,10 @@ import ( "testing" "github.com/Azure/go-autorest/autorest" - "github.com/Azure/go-autorest/autorest/to" "github.com/golang/mock/gomock" . "github.com/onsi/gomega" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/klog/v2/klogr" - clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - "sigs.k8s.io/controller-runtime/pkg/client/fake" - - 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/scope" "sigs.k8s.io/cluster-api-provider-azure/azure/services/disks/mock_disks" gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock" ) @@ -127,117 +119,3 @@ func TestDeleteDisk(t *testing.T) { }) } } - -func TestDiskSpecs(t *testing.T) { - testcases := []struct { - name string - azureMachineModifyFunc func(*infrav1.AzureMachine) - expectedDisks []azure.DiskSpec - }{ - { - name: "only os disk", - azureMachineModifyFunc: func(m *infrav1.AzureMachine) {}, - expectedDisks: []azure.DiskSpec{ - { - Name: "my-azure-machine_OSDisk", - }, - }, - }, { - name: "os and data disks", - azureMachineModifyFunc: func(m *infrav1.AzureMachine) { - m.Spec.DataDisks = []infrav1.DataDisk{{ - NameSuffix: "etcddisk", - }} - }, - expectedDisks: []azure.DiskSpec{ - { - Name: "my-azure-machine_OSDisk", - }, - { - Name: "my-azure-machine_etcddisk", - }, - }, - }, { - name: "os and multiple data disks", - azureMachineModifyFunc: func(m *infrav1.AzureMachine) { - m.Spec.DataDisks = []infrav1.DataDisk{ - { - NameSuffix: "etcddisk", - }, - { - NameSuffix: "otherdisk", - }} - }, - expectedDisks: []azure.DiskSpec{ - { - Name: "my-azure-machine_OSDisk", - }, - { - Name: "my-azure-machine_etcddisk", - }, - { - Name: "my-azure-machine_otherdisk", - }, - }, - }} - for _, tc := range testcases { - tc := tc - t.Run(tc.name, func(t *testing.T) { - g := NewWithT(t) - scheme := runtime.NewScheme() - g.Expect(infrav1.AddToScheme(scheme)).ToNot(HaveOccurred()) - g.Expect(clusterv1.AddToScheme(scheme)).ToNot(HaveOccurred()) - - t.Parallel() - cluster := &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-cluster", - }, - } - azureCluster := &infrav1.AzureCluster{ - Spec: infrav1.AzureClusterSpec{ - SubscriptionID: "1234", - }, - } - machine := &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-machine", - }, - } - - azureMachine := &infrav1.AzureMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-azure-machine", - }, - Spec: infrav1.AzureMachineSpec{ - OSDisk: infrav1.OSDisk{ - DiskSizeGB: to.Int32Ptr(30), - OSType: "Linux", - }, - }, - } - tc.azureMachineModifyFunc(azureMachine) - - client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(cluster, machine, azureCluster, azureMachine).Build() - clusterScope, err := scope.NewClusterScope(context.Background(), scope.ClusterScopeParams{ - AzureClients: scope.AzureClients{ - Authorizer: autorest.NullAuthorizer{}, - }, - Client: client, - Cluster: cluster, - AzureCluster: azureCluster, - }) - g.Expect(err).NotTo(HaveOccurred()) - machineScope, err := scope.NewMachineScope(scope.MachineScopeParams{ - Client: client, - ClusterScope: clusterScope, - Machine: machine, - AzureMachine: azureMachine, - }) - g.Expect(err).NotTo(HaveOccurred()) - - output := machineScope.DiskSpecs() - g.Expect(output).To(Equal(tc.expectedDisks)) - }) - } -} diff --git a/azure/services/groups/client.go b/azure/services/groups/client.go index 4cf07b82641..1ac851eeb8b 100644 --- a/azure/services/groups/client.go +++ b/azure/services/groups/client.go @@ -32,9 +32,10 @@ import ( // client wraps go-sdk. type client interface { Get(context.Context, string) (resources.Group, error) - CreateOrUpdateAsync(context.Context, azure.ResourceSpecGetter) (azureautorest.FutureAPI, 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. @@ -69,20 +70,20 @@ func (ac *azureClient) Get(ctx context.Context, name string) (resources.Group, e // CreateOrUpdateAsync creates or updates a resource group. // Creating a resource group is not a long running operation, so we don't ever return a future. -func (ac *azureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter) (azureautorest.FutureAPI, error) { +func (ac *azureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter) (interface{}, azureautorest.FutureAPI, error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "groups.AzureClient.CreateOrUpdate") defer done() group, err := ac.resourceGroupParams(ctx, spec) if err != nil { - return nil, errors.Wrapf(err, "failed to get desired parameters for group %s", spec.ResourceName()) + return nil, nil, errors.Wrapf(err, "failed to get desired parameters for group %s", spec.ResourceName()) } else if group == nil { // nothing to do here - return nil, nil + return nil, nil, nil } - _, err = ac.groups.CreateOrUpdate(ctx, spec.ResourceName(), *group) - return nil, err + result, err := ac.groups.CreateOrUpdate(ctx, spec.ResourceName(), *group) + return result, nil, err } // DeleteAsync deletes a resource group asynchronously. DeleteAsync sends a DELETE @@ -126,6 +127,12 @@ func (ac *azureClient) IsDone(ctx context.Context, future azureautorest.FutureAP return isDone, 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 +} + // resourceGroupParams returns the desired resource group parameters from the given spec. func (ac *azureClient) resourceGroupParams(ctx context.Context, spec azure.ResourceSpecGetter) (*resources.Group, error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "groups.AzureClient.resourceGroupParams") diff --git a/azure/services/groups/groups.go b/azure/services/groups/groups.go index 2d2ecfdf402..f86901fb6ae 100644 --- a/azure/services/groups/groups.go +++ b/azure/services/groups/groups.go @@ -65,7 +65,7 @@ func (s *Service) Reconcile(ctx context.Context) error { groupSpec := s.Scope.GroupSpec() - err := async.CreateResource(ctx, s.Scope, s.client, groupSpec, serviceName) + _, err := async.CreateResource(ctx, s.Scope, s.client, groupSpec, serviceName) s.Scope.UpdatePutStatus(infrav1.ResourceGroupReadyCondition, serviceName, err) return err } diff --git a/azure/services/groups/groups_test.go b/azure/services/groups/groups_test.go index f2c1aea36db..02e6650c2dc 100644 --- a/azure/services/groups/groups_test.go +++ b/azure/services/groups/groups_test.go @@ -81,7 +81,7 @@ func TestReconcileGroups(t *testing.T) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) s.GroupSpec().Return(&fakeGroupSpec) s.GetLongRunningOperationState("test-group", serviceName) - m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakeGroupSpec).Return(nil, nil) + m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakeGroupSpec).Return(nil, nil, nil) s.UpdatePutStatus(infrav1.ResourceGroupReadyCondition, serviceName, nil) }, }, @@ -92,7 +92,7 @@ func TestReconcileGroups(t *testing.T) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) s.GroupSpec().Return(&fakeGroupSpec) s.GetLongRunningOperationState("test-group", serviceName) - m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakeGroupSpec).Return(nil, internalError) + m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakeGroupSpec).Return(nil, nil, internalError) s.UpdatePutStatus(infrav1.ResourceGroupReadyCondition, serviceName, gomockinternal.ErrStrEq(fmt.Sprintf("failed to create resource test-group/test-group (service: group): %s", internalError.Error()))) }, }, @@ -143,13 +143,14 @@ func TestDeleteGroups(t *testing.T) { s.ClusterName().Return("test-cluster") s.GetLongRunningOperationState("test-group", serviceName).Times(2).Return(&fakeFuture) m.IsDone(gomockinternal.AContext(), gomock.AssignableToTypeOf(&azureautorest.Future{})).Return(true, nil) + m.Result(gomockinternal.AContext(), gomock.AssignableToTypeOf(&azureautorest.Future{}), infrav1.DeleteFuture).Return(nil, nil) s.DeleteLongRunningOperationState("test-group", serviceName) s.UpdateDeleteStatus(infrav1.ResourceGroupReadyCondition, serviceName, nil) }, }, { name: "long running delete operation is not done", - expectedError: "transient reconcile error occurred: operation type DELETE on Azure resource test-group/test-group is not done. Object will be requeued after 15s", + expectedError: "operation type DELETE on Azure resource test-group/test-group is not done. Object will be requeued after 15s", expect: func(s *mock_groups.MockGroupScopeMockRecorder, m *mock_groups.MockclientMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) s.GroupSpec().AnyTimes().Return(&fakeGroupSpec) @@ -157,7 +158,7 @@ func TestDeleteGroups(t *testing.T) { s.ClusterName().Return("test-cluster") s.GetLongRunningOperationState("test-group", serviceName).Times(2).Return(&fakeFuture) m.IsDone(gomockinternal.AContext(), gomock.AssignableToTypeOf(&azureautorest.Future{})).Return(false, nil) - s.UpdateDeleteStatus(infrav1.ResourceGroupReadyCondition, serviceName, gomockinternal.ErrStrEq("transient reconcile error occurred: operation type DELETE on Azure resource test-group/test-group is not done. Object will be requeued after 15s")) + s.UpdateDeleteStatus(infrav1.ResourceGroupReadyCondition, serviceName, gomockinternal.ErrStrEq("operation type DELETE on Azure resource test-group/test-group is not done. Object will be requeued after 15s")) }, }, { @@ -205,7 +206,7 @@ func TestDeleteGroups(t *testing.T) { }, { name: "context deadline exceeded while deleting resource group", - expectedError: "transient reconcile error occurred: operation type DELETE on Azure resource test-group/test-group is not done. Object will be requeued after 15s", + expectedError: "operation type DELETE on Azure resource test-group/test-group is not done. Object will be requeued after 15s", expect: func(s *mock_groups.MockGroupScopeMockRecorder, m *mock_groups.MockclientMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) s.GroupSpec().AnyTimes().Return(&fakeGroupSpec) @@ -214,7 +215,7 @@ func TestDeleteGroups(t *testing.T) { s.ClusterName().Return("test-cluster") m.DeleteAsync(gomockinternal.AContext(), &fakeGroupSpec).Return(&azureautorest.Future{}, errCtxExceeded) s.SetLongRunningOperationState(gomock.AssignableToTypeOf(&infrav1.Future{})) - s.UpdateDeleteStatus(infrav1.ResourceGroupReadyCondition, serviceName, gomockinternal.ErrStrEq("transient reconcile error occurred: operation type DELETE on Azure resource test-group/test-group is not done. Object will be requeued after 15s")) + s.UpdateDeleteStatus(infrav1.ResourceGroupReadyCondition, serviceName, gomockinternal.ErrStrEq("operation type DELETE on Azure resource test-group/test-group is not done. Object will be requeued after 15s")) }, }, { diff --git a/azure/services/groups/mock_groups/client_mock.go b/azure/services/groups/mock_groups/client_mock.go index e16d83a17df..f18a9c84c35 100644 --- a/azure/services/groups/mock_groups/client_mock.go +++ b/azure/services/groups/mock_groups/client_mock.go @@ -54,12 +54,13 @@ func (m *Mockclient) EXPECT() *MockclientMockRecorder { } // CreateOrUpdateAsync mocks base method. -func (m *Mockclient) CreateOrUpdateAsync(arg0 context.Context, arg1 azure0.ResourceSpecGetter) (azure.FutureAPI, error) { +func (m *Mockclient) CreateOrUpdateAsync(arg0 context.Context, arg1 azure0.ResourceSpecGetter) (interface{}, azure.FutureAPI, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "CreateOrUpdateAsync", arg0, arg1) - ret0, _ := ret[0].(azure.FutureAPI) - ret1, _ := ret[1].(error) - return ret0, ret1 + ret0, _ := ret[0].(interface{}) + ret1, _ := ret[1].(azure.FutureAPI) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 } // CreateOrUpdateAsync indicates an expected call of CreateOrUpdateAsync. @@ -112,3 +113,18 @@ 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/networkinterfaces/networkinterfaces.go b/azure/services/networkinterfaces/networkinterfaces.go index a305557acf3..9a3c62f82e5 100644 --- a/azure/services/networkinterfaces/networkinterfaces.go +++ b/azure/services/networkinterfaces/networkinterfaces.go @@ -167,7 +167,7 @@ func (s *Service) Delete(ctx context.Context) error { defer done() for _, nicSpec := range s.Scope.NICSpecs() { - s.Scope.V(2).Info("deleting network interface %s", "network interface", nicSpec.Name) + s.Scope.V(2).Info("deleting network interface", "network interface", nicSpec.Name) err := s.Client.Delete(ctx, s.Scope.ResourceGroup(), nicSpec.Name) if err != nil && !azure.ResourceNotFound(err) { return errors.Wrapf(err, "failed to delete network interface %s in resource group %s", nicSpec.Name, s.Scope.ResourceGroup()) diff --git a/azure/services/virtualmachines/client.go b/azure/services/virtualmachines/client.go index 903d7bdebdf..8140413f10c 100644 --- a/azure/services/virtualmachines/client.go +++ b/azure/services/virtualmachines/client.go @@ -18,26 +18,35 @@ package virtualmachines import ( "context" + "encoding/json" "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/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/util/reconciler" "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) // Client wraps go-sdk. -type Client interface { - Get(context.Context, string, string) (compute.VirtualMachine, error) - CreateOrUpdate(context.Context, string, string, compute.VirtualMachine) error - Delete(context.Context, string, string) error -} +type ( + Client interface { + Get(context.Context, string, string) (compute.VirtualMachine, 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. -type AzureClient struct { - virtualmachines compute.VirtualMachinesClient -} + // AzureClient contains the Azure go-sdk Client. + AzureClient struct { + virtualmachines compute.VirtualMachinesClient + } +) var _ Client = &AzureClient{} @@ -62,38 +71,150 @@ func (ac *AzureClient) Get(ctx context.Context, resourceGroupName, vmName string return ac.virtualmachines.Get(ctx, resourceGroupName, vmName, "") } -// CreateOrUpdate the operation to create or update a virtual machine. -func (ac *AzureClient) CreateOrUpdate(ctx context.Context, resourceGroupName, vmName string, vm compute.VirtualMachine) error { +// CreateOrUpdateAsync creates or updates a virtual machine 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, "virtualmachines.AzureClient.CreateOrUpdate") defer done() - future, err := ac.virtualmachines.CreateOrUpdate(ctx, resourceGroupName, vmName, vm) + vm, err := ac.vmParams(ctx, spec) if err != nil { - return err + return nil, nil, errors.Wrapf(err, "failed to get desired parameters for virtual machine %s", spec.ResourceName()) + } else if vm == nil { + // nothing to do here. + return nil, nil, nil } + + future, err := ac.virtualmachines.CreateOrUpdate(ctx, spec.ResourceGroupName(), spec.ResourceName(), *vm) + if err != nil { + return nil, nil, err + } + + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) + defer cancel() + err = future.WaitForCompletionRef(ctx, ac.virtualmachines.Client) if err != nil { - return err + // if an error occurs, return the future. + // this means the long-running operation didn't finish in the specified timeout. + return nil, &future, err } - _, err = future.Result(ac.virtualmachines) - return err + result, err := future.Result(ac.virtualmachines) + // if the operation completed, return a nil future + return result, nil, err } -// Delete the operation to delete a virtual machine. -func (ac *AzureClient) Delete(ctx context.Context, resourceGroupName, vmName string) error { +// DeleteAsync deletes a virtual machine 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, "virtualmachines.AzureClient.Delete") defer done() // TODO: pass variable to force the deletion or not // now we are not forcing. - future, err := ac.virtualmachines.Delete(ctx, resourceGroupName, vmName, to.BoolPtr(false)) + future, err := ac.virtualmachines.Delete(ctx, spec.ResourceGroupName(), spec.ResourceName(), to.BoolPtr(false)) if err != nil { - return err + return nil, err } + + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) + defer cancel() + err = future.WaitForCompletionRef(ctx, ac.virtualmachines.Client) if err != nil { - return err + // if an error occurs, return the future. + // this means the long-running operation didn't finish in the specified timeout. + return &future, err } _, err = future.Result(ac.virtualmachines) - return err + // if the operation completed, return a nil future. + return nil, err +} + +// IsDone returns true if the long-running operation has completed. +func (ac *AzureClient) IsDone(ctx context.Context, future azureautorest.FutureAPI) (bool, error) { + ctx, span := tele.Tracer().Start(ctx, "virtualmachines.AzureClient.IsDone") + defer span.End() + + done, err := future.DoneWithContext(ctx, ac.virtualmachines) + if err != nil { + return false, errors.Wrap(err, "failed checking if the operation was complete") + } + + return done, 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) { + if futureData == nil { + return nil, errors.Errorf("cannot get result from nil future") + } + var result func(client compute.VirtualMachinesClient) (VM compute.VirtualMachine, err error) + + switch futureType { + case infrav1.PatchFuture: + var future *compute.VirtualMachinesUpdateFuture + jsonData, err := futureData.MarshalJSON() + if err != nil { + return nil, errors.Wrap(err, "failed to marshal future") + } + if err := json.Unmarshal(jsonData, &future); err != nil { + return nil, errors.Wrap(err, "failed to unmarshal future data") + } + result = (*future).Result + + case infrav1.PutFuture: + var future *compute.VirtualMachinesCreateOrUpdateFuture + jsonData, err := futureData.MarshalJSON() + if err != nil { + return nil, errors.Wrap(err, "failed to marshal future") + } + if err := json.Unmarshal(jsonData, &future); err != nil { + return nil, errors.Wrap(err, "failed to unmarshal future data") + } + result = (*future).Result + + case infrav1.DeleteFuture: + // Delete does not return a result VM. + return nil, nil + + default: + return nil, errors.Errorf("unknown future type %q", futureType) + } + + return result(ac.virtualmachines) +} + +// vmParams creates a VirtualMachine object from the given spec. +func (ac *AzureClient) vmParams(ctx context.Context, spec azure.ResourceSpecGetter) (*compute.VirtualMachine, error) { + ctx, span := tele.Tracer().Start(ctx, "virtualmachines.AzureClient.vmParams") + defer span.End() + + var params interface{} + var existing interface{} + + if existingVM, err := ac.Get(ctx, spec.ResourceGroupName(), spec.ResourceName()); err != nil && !azure.ResourceNotFound(err) { + return nil, errors.Wrapf(err, "failed to get virtual machine %s in %s", spec.ResourceName(), spec.ResourceGroupName()) + } else if err == nil { + // virtual machine already exists + existing = existingVM + } + + params, err := spec.Parameters(existing) + if err != nil { + return nil, err + } + + vm, ok := params.(compute.VirtualMachine) + if !ok { + if params == nil { + // nothing to do here. + return nil, nil + } + return nil, errors.Errorf("%T is not a compute.VirtualMachine", params) + } + + return &vm, nil } diff --git a/azure/services/virtualmachines/mock_virtualmachines/client_mock.go b/azure/services/virtualmachines/mock_virtualmachines/client_mock.go index 82e13d7001a..2410527fb14 100644 --- a/azure/services/virtualmachines/mock_virtualmachines/client_mock.go +++ b/azure/services/virtualmachines/mock_virtualmachines/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,32 +53,35 @@ func (m *MockClient) EXPECT() *MockClientMockRecorder { return m.recorder } -// CreateOrUpdate mocks base method. -func (m *MockClient) CreateOrUpdate(arg0 context.Context, arg1, arg2 string, arg3 compute.VirtualMachine) 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", arg0, arg1, arg2, arg3) - ret0, _ := ret[0].(error) - return ret0 + 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(arg0, arg1, arg2, arg3 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), arg0, arg1, arg2, arg3) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdateAsync", reflect.TypeOf((*MockClient)(nil).CreateOrUpdateAsync), arg0, arg1) } -// Delete mocks base method. -func (m *MockClient) Delete(arg0 context.Context, arg1, arg2 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", arg0, arg1, arg2) - 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(arg0, arg1, arg2 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), arg0, arg1, arg2) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAsync", reflect.TypeOf((*MockClient)(nil).DeleteAsync), arg0, arg1) } // Get mocks base method. @@ -93,3 +98,33 @@ func (mr *MockClientMockRecorder) Get(arg0, arg1, arg2 interface{}) *gomock.Call mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockClient)(nil).Get), arg0, arg1, arg2) } + +// 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/virtualmachines/mock_virtualmachines/virtualmachines_mock.go b/azure/services/virtualmachines/mock_virtualmachines/virtualmachines_mock.go index c3377968cf1..ab9b890266b 100644 --- a/azure/services/virtualmachines/mock_virtualmachines/virtualmachines_mock.go +++ b/azure/services/virtualmachines/mock_virtualmachines/virtualmachines_mock.go @@ -21,7 +21,6 @@ limitations under the License. package mock_virtualmachines import ( - context "context" reflect "reflect" autorest "github.com/Azure/go-autorest/autorest" @@ -30,6 +29,7 @@ import ( v1 "k8s.io/api/core/v1" 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" ) // MockVMScope is a mock of VMScope interface. @@ -55,20 +55,6 @@ func (m *MockVMScope) EXPECT() *MockVMScopeMockRecorder { return m.recorder } -// AdditionalTags mocks base method. -func (m *MockVMScope) AdditionalTags() v1beta1.Tags { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "AdditionalTags") - ret0, _ := ret[0].(v1beta1.Tags) - return ret0 -} - -// AdditionalTags indicates an expected call of AdditionalTags. -func (mr *MockVMScopeMockRecorder) AdditionalTags() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AdditionalTags", reflect.TypeOf((*MockVMScope)(nil).AdditionalTags)) -} - // Authorizer mocks base method. func (m *MockVMScope) Authorizer() autorest.Authorizer { m.ctrl.T.Helper() @@ -83,35 +69,6 @@ func (mr *MockVMScopeMockRecorder) Authorizer() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Authorizer", reflect.TypeOf((*MockVMScope)(nil).Authorizer)) } -// AvailabilitySet mocks base method. -func (m *MockVMScope) 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 *MockVMScopeMockRecorder) AvailabilitySet() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AvailabilitySet", reflect.TypeOf((*MockVMScope)(nil).AvailabilitySet)) -} - -// AvailabilitySetEnabled mocks base method. -func (m *MockVMScope) AvailabilitySetEnabled() bool { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "AvailabilitySetEnabled") - ret0, _ := ret[0].(bool) - return ret0 -} - -// AvailabilitySetEnabled indicates an expected call of AvailabilitySetEnabled. -func (mr *MockVMScopeMockRecorder) AvailabilitySetEnabled() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AvailabilitySetEnabled", reflect.TypeOf((*MockVMScope)(nil).AvailabilitySetEnabled)) -} - // BaseURI mocks base method. func (m *MockVMScope) BaseURI() string { m.ctrl.T.Helper() @@ -168,32 +125,16 @@ func (mr *MockVMScopeMockRecorder) CloudEnvironment() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CloudEnvironment", reflect.TypeOf((*MockVMScope)(nil).CloudEnvironment)) } -// CloudProviderConfigOverrides mocks base method. -func (m *MockVMScope) CloudProviderConfigOverrides() *v1beta1.CloudProviderConfigOverrides { +// DeleteLongRunningOperationState mocks base method. +func (m *MockVMScope) DeleteLongRunningOperationState(arg0, arg1 string) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CloudProviderConfigOverrides") - ret0, _ := ret[0].(*v1beta1.CloudProviderConfigOverrides) - return ret0 + m.ctrl.Call(m, "DeleteLongRunningOperationState", arg0, arg1) } -// CloudProviderConfigOverrides indicates an expected call of CloudProviderConfigOverrides. -func (mr *MockVMScopeMockRecorder) CloudProviderConfigOverrides() *gomock.Call { +// DeleteLongRunningOperationState indicates an expected call of DeleteLongRunningOperationState. +func (mr *MockVMScopeMockRecorder) DeleteLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CloudProviderConfigOverrides", reflect.TypeOf((*MockVMScope)(nil).CloudProviderConfigOverrides)) -} - -// ClusterName mocks base method. -func (m *MockVMScope) ClusterName() string { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ClusterName") - ret0, _ := ret[0].(string) - return ret0 -} - -// ClusterName indicates an expected call of ClusterName. -func (mr *MockVMScopeMockRecorder) ClusterName() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ClusterName", reflect.TypeOf((*MockVMScope)(nil).ClusterName)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteLongRunningOperationState", reflect.TypeOf((*MockVMScope)(nil).DeleteLongRunningOperationState), arg0, arg1) } // Enabled mocks base method. @@ -227,48 +168,18 @@ func (mr *MockVMScopeMockRecorder) Error(err, msg interface{}, keysAndValues ... return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Error", reflect.TypeOf((*MockVMScope)(nil).Error), varargs...) } -// FailureDomains mocks base method. -func (m *MockVMScope) FailureDomains() []string { +// GetLongRunningOperationState mocks base method. +func (m *MockVMScope) GetLongRunningOperationState(arg0, arg1 string) *v1beta1.Future { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "FailureDomains") - ret0, _ := ret[0].([]string) + ret := m.ctrl.Call(m, "GetLongRunningOperationState", arg0, arg1) + ret0, _ := ret[0].(*v1beta1.Future) return ret0 } -// FailureDomains indicates an expected call of FailureDomains. -func (mr *MockVMScopeMockRecorder) FailureDomains() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FailureDomains", reflect.TypeOf((*MockVMScope)(nil).FailureDomains)) -} - -// GetBootstrapData mocks base method. -func (m *MockVMScope) GetBootstrapData(ctx context.Context) (string, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetBootstrapData", ctx) - ret0, _ := ret[0].(string) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetBootstrapData indicates an expected call of GetBootstrapData. -func (mr *MockVMScopeMockRecorder) GetBootstrapData(ctx interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetBootstrapData", reflect.TypeOf((*MockVMScope)(nil).GetBootstrapData), ctx) -} - -// GetVMImage mocks base method. -func (m *MockVMScope) GetVMImage() (*v1beta1.Image, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetVMImage") - ret0, _ := ret[0].(*v1beta1.Image) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetVMImage indicates an expected call of GetVMImage. -func (mr *MockVMScopeMockRecorder) GetVMImage() *gomock.Call { +// GetLongRunningOperationState indicates an expected call of GetLongRunningOperationState. +func (mr *MockVMScopeMockRecorder) GetLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetVMImage", reflect.TypeOf((*MockVMScope)(nil).GetVMImage)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLongRunningOperationState", reflect.TypeOf((*MockVMScope)(nil).GetLongRunningOperationState), arg0, arg1) } // HashKey mocks base method. @@ -302,48 +213,6 @@ func (mr *MockVMScopeMockRecorder) Info(msg interface{}, keysAndValues ...interf return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Info", reflect.TypeOf((*MockVMScope)(nil).Info), varargs...) } -// Location mocks base method. -func (m *MockVMScope) Location() string { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Location") - ret0, _ := ret[0].(string) - return ret0 -} - -// Location indicates an expected call of Location. -func (mr *MockVMScopeMockRecorder) Location() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Location", reflect.TypeOf((*MockVMScope)(nil).Location)) -} - -// ProviderID mocks base method. -func (m *MockVMScope) ProviderID() string { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ProviderID") - ret0, _ := ret[0].(string) - return ret0 -} - -// ProviderID indicates an expected call of ProviderID. -func (mr *MockVMScopeMockRecorder) ProviderID() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ProviderID", reflect.TypeOf((*MockVMScope)(nil).ProviderID)) -} - -// ResourceGroup mocks base method. -func (m *MockVMScope) ResourceGroup() string { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ResourceGroup") - ret0, _ := ret[0].(string) - return ret0 -} - -// ResourceGroup indicates an expected call of ResourceGroup. -func (mr *MockVMScopeMockRecorder) ResourceGroup() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResourceGroup", reflect.TypeOf((*MockVMScope)(nil).ResourceGroup)) -} - // SetAddresses mocks base method. func (m *MockVMScope) SetAddresses(arg0 []v1.NodeAddress) { m.ctrl.T.Helper() @@ -368,6 +237,18 @@ func (mr *MockVMScopeMockRecorder) SetAnnotation(arg0, arg1 interface{}) *gomock return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetAnnotation", reflect.TypeOf((*MockVMScope)(nil).SetAnnotation), arg0, arg1) } +// SetLongRunningOperationState mocks base method. +func (m *MockVMScope) SetLongRunningOperationState(arg0 *v1beta1.Future) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetLongRunningOperationState", arg0) +} + +// SetLongRunningOperationState indicates an expected call of SetLongRunningOperationState. +func (mr *MockVMScopeMockRecorder) SetLongRunningOperationState(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetLongRunningOperationState", reflect.TypeOf((*MockVMScope)(nil).SetLongRunningOperationState), arg0) +} + // SetProviderID mocks base method. func (m *MockVMScope) SetProviderID(arg0 string) { m.ctrl.T.Helper() @@ -420,16 +301,40 @@ func (mr *MockVMScopeMockRecorder) TenantID() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TenantID", reflect.TypeOf((*MockVMScope)(nil).TenantID)) } -// UpdateStatus mocks base method. -func (m *MockVMScope) UpdateStatus() { +// UpdateDeleteStatus mocks base method. +func (m *MockVMScope) 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 *MockVMScopeMockRecorder) UpdateDeleteStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateDeleteStatus", reflect.TypeOf((*MockVMScope)(nil).UpdateDeleteStatus), arg0, arg1, arg2) +} + +// UpdatePatchStatus mocks base method. +func (m *MockVMScope) 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 *MockVMScopeMockRecorder) UpdatePatchStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePatchStatus", reflect.TypeOf((*MockVMScope)(nil).UpdatePatchStatus), arg0, arg1, arg2) +} + +// UpdatePutStatus mocks base method. +func (m *MockVMScope) UpdatePutStatus(arg0 v1beta10.ConditionType, arg1 string, arg2 error) { m.ctrl.T.Helper() - m.ctrl.Call(m, "UpdateStatus") + m.ctrl.Call(m, "UpdatePutStatus", arg0, arg1, arg2) } -// UpdateStatus indicates an expected call of UpdateStatus. -func (mr *MockVMScopeMockRecorder) UpdateStatus() *gomock.Call { +// UpdatePutStatus indicates an expected call of UpdatePutStatus. +func (mr *MockVMScopeMockRecorder) UpdatePutStatus(arg0, arg1, arg2 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateStatus", reflect.TypeOf((*MockVMScope)(nil).UpdateStatus)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePutStatus", reflect.TypeOf((*MockVMScope)(nil).UpdatePutStatus), arg0, arg1, arg2) } // V mocks base method. @@ -447,10 +352,10 @@ func (mr *MockVMScopeMockRecorder) V(level interface{}) *gomock.Call { } // VMSpec mocks base method. -func (m *MockVMScope) VMSpec() azure.VMSpec { +func (m *MockVMScope) VMSpec() azure.ResourceSpecGetter { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "VMSpec") - ret0, _ := ret[0].(azure.VMSpec) + ret0, _ := ret[0].(azure.ResourceSpecGetter) return ret0 } diff --git a/azure/services/virtualmachines/spec.go b/azure/services/virtualmachines/spec.go new file mode 100644 index 00000000000..11d523245d8 --- /dev/null +++ b/azure/services/virtualmachines/spec.go @@ -0,0 +1,336 @@ +/* +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 virtualmachines + +import ( + "encoding/base64" + "fmt" + + 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" + "sigs.k8s.io/cluster-api-provider-azure/util/generators" + + "sigs.k8s.io/cluster-api-provider-azure/azure" + + "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" +) + +// VMSpec defines the specification for a Virtual Machine. +type VMSpec struct { + Name string + ResourceGroup string + Location string + ClusterName string + Role string + NICIDs []string + SSHKeyData string + Size string + AvailabilitySetID string + Zone string + Identity infrav1.VMIdentity + OSDisk infrav1.OSDisk + DataDisks []infrav1.DataDisk + UserAssignedIdentities []infrav1.UserAssignedIdentity + SpotVMOptions *infrav1.SpotVMOptions + SecurityProfile *infrav1.SecurityProfile + AdditionalTags infrav1.Tags + SKU resourceskus.SKU + Image *infrav1.Image + BootstrapData string + ProviderID string +} + +// ResourceName returns the name of the virtual machine. +func (s *VMSpec) ResourceName() string { + return s.Name +} + +// ResourceGroupName returns the name of the virtual machine. +func (s *VMSpec) ResourceGroupName() string { + return s.ResourceGroup +} + +// OwnerResourceName is a no-op for virtual machines. +func (s *VMSpec) OwnerResourceName() string { + return "" +} + +// Parameters returns the parameters for the virtual machine. +func (s *VMSpec) Parameters(existing interface{}) (interface{}, error) { + if existing != nil { + if _, ok := existing.(compute.VirtualMachine); !ok { + return nil, errors.Errorf("%T is not a compute.VirtualMachine", existing) + } + // vm already exists + return nil, nil + } + + // VM got deleted outside of capz, do not recreate it as Machines are immutable. + if s.ProviderID != "" { + return nil, azure.VMDeletedError{ProviderID: s.ProviderID} + } + + storageProfile, err := s.generateStorageProfile() + if err != nil { + return nil, err + } + + securityProfile, err := s.generateSecurityProfile() + if err != nil { + return nil, err + } + + osProfile, err := s.generateOSProfile() + if err != nil { + return nil, errors.Wrap(err, "failed to generate OS Profile") + } + + priority, evictionPolicy, billingProfile, err := converters.GetSpotVMOptions(s.SpotVMOptions) + if err != nil { + return nil, errors.Wrap(err, "failed to get Spot VM options") + } + + identity, err := converters.VMIdentityToVMSDK(s.Identity, s.UserAssignedIdentities) + if err != nil { + return nil, errors.Wrap(err, "failed to generate VM identity") + } + + return compute.VirtualMachine{ + Plan: converters.ImageToPlan(s.Image), + Location: to.StringPtr(s.Location), + Tags: converters.TagsToMap(infrav1.Build(infrav1.BuildParams{ + ClusterName: s.ClusterName, + Lifecycle: infrav1.ResourceLifecycleOwned, + Name: to.StringPtr(s.Name), + Role: to.StringPtr(s.Role), + Additional: s.AdditionalTags, + })), + VirtualMachineProperties: &compute.VirtualMachineProperties{ + AdditionalCapabilities: s.generateAdditionalCapabilities(), + AvailabilitySet: s.getAvailabilitySet(), + HardwareProfile: &compute.HardwareProfile{ + VMSize: compute.VirtualMachineSizeTypes(s.Size), + }, + StorageProfile: storageProfile, + SecurityProfile: securityProfile, + OsProfile: osProfile, + NetworkProfile: &compute.NetworkProfile{ + NetworkInterfaces: s.generateNICRefs(), + }, + Priority: priority, + EvictionPolicy: evictionPolicy, + BillingProfile: billingProfile, + DiagnosticsProfile: &compute.DiagnosticsProfile{ + BootDiagnostics: &compute.BootDiagnostics{ + Enabled: to.BoolPtr(true), + }, + }, + }, + Identity: identity, + Zones: s.getZones(), + }, nil +} + +// generateStorageProfile generates a pointer to a compute.StorageProfile which can utilized for VM creation. +func (s *VMSpec) generateStorageProfile() (*compute.StorageProfile, error) { + storageProfile := &compute.StorageProfile{ + OsDisk: &compute.OSDisk{ + Name: to.StringPtr(azure.GenerateOSDiskName(s.Name)), + OsType: compute.OperatingSystemTypes(s.OSDisk.OSType), + CreateOption: compute.DiskCreateOptionTypesFromImage, + DiskSizeGB: s.OSDisk.DiskSizeGB, + Caching: compute.CachingTypes(s.OSDisk.CachingType), + }, + } + + // Checking if the requested VM size has at least 2 vCPUS + vCPUCapability, err := s.SKU.HasCapabilityWithCapacity(resourceskus.VCPUs, resourceskus.MinimumVCPUS) + if err != nil { + return nil, azure.WithTerminalError(errors.Wrap(err, "failed to validate the vCPU capability")) + } + if !vCPUCapability { + return nil, azure.WithTerminalError(errors.New("vm size should be bigger or equal to at least 2 vCPUs")) + } + + // Checking if the requested VM size has at least 2 Gi of memory + MemoryCapability, err := s.SKU.HasCapabilityWithCapacity(resourceskus.MemoryGB, resourceskus.MinimumMemory) + if err != nil { + return nil, azure.WithTerminalError(errors.Wrap(err, "failed to validate the memory capability")) + } + + if !MemoryCapability { + return nil, azure.WithTerminalError(errors.New("vm memory should be bigger or equal to at least 2Gi")) + } + // enable ephemeral OS + if s.OSDisk.DiffDiskSettings != nil { + if !s.SKU.HasCapability(resourceskus.EphemeralOSDisk) { + return nil, azure.WithTerminalError(fmt.Errorf("vm size %s does not support ephemeral os. select a different vm size or disable ephemeral os", s.Size)) + } + + storageProfile.OsDisk.DiffDiskSettings = &compute.DiffDiskSettings{ + Option: compute.DiffDiskOptions(s.OSDisk.DiffDiskSettings.Option), + } + } + + if s.OSDisk.ManagedDisk != nil { + storageProfile.OsDisk.ManagedDisk = &compute.ManagedDiskParameters{} + if s.OSDisk.ManagedDisk.StorageAccountType != "" { + storageProfile.OsDisk.ManagedDisk.StorageAccountType = compute.StorageAccountTypes(s.OSDisk.ManagedDisk.StorageAccountType) + } + if s.OSDisk.ManagedDisk.DiskEncryptionSet != nil { + storageProfile.OsDisk.ManagedDisk.DiskEncryptionSet = &compute.DiskEncryptionSetParameters{ID: to.StringPtr(s.OSDisk.ManagedDisk.DiskEncryptionSet.ID)} + } + } + + dataDisks := make([]compute.DataDisk, len(s.DataDisks)) + for i, disk := range s.DataDisks { + dataDisks[i] = compute.DataDisk{ + CreateOption: compute.DiskCreateOptionTypesEmpty, + DiskSizeGB: to.Int32Ptr(disk.DiskSizeGB), + Lun: disk.Lun, + Name: to.StringPtr(azure.GenerateDataDiskName(s.Name, disk.NameSuffix)), + Caching: compute.CachingTypes(disk.CachingType), + } + + if disk.ManagedDisk != nil { + dataDisks[i].ManagedDisk = &compute.ManagedDiskParameters{ + StorageAccountType: compute.StorageAccountTypes(disk.ManagedDisk.StorageAccountType), + } + + if disk.ManagedDisk.DiskEncryptionSet != nil { + dataDisks[i].ManagedDisk.DiskEncryptionSet = &compute.DiskEncryptionSetParameters{ID: to.StringPtr(disk.ManagedDisk.DiskEncryptionSet.ID)} + } + + // check the support for ultra disks based on location and vm size + if disk.ManagedDisk.StorageAccountType == string(compute.StorageAccountTypesUltraSSDLRS) && !s.SKU.HasLocationCapability(resourceskus.UltraSSDAvailable, s.Location, s.Zone) { + return nil, azure.WithTerminalError(fmt.Errorf("vm size %s does not support ultra disks in location %s. select a different vm size or disable ultra disks", s.Size, s.Location)) + } + } + } + storageProfile.DataDisks = &dataDisks + + imageRef, err := converters.ImageToSDK(s.Image) + if err != nil { + return nil, err + } + + storageProfile.ImageReference = imageRef + + return storageProfile, nil +} + +func (s *VMSpec) generateOSProfile() (*compute.OSProfile, error) { + sshKey, err := base64.StdEncoding.DecodeString(s.SSHKeyData) + if err != nil { + return nil, errors.Wrap(err, "failed to decode ssh public key") + } + + osProfile := &compute.OSProfile{ + ComputerName: to.StringPtr(s.Name), + AdminUsername: to.StringPtr(azure.DefaultUserName), + CustomData: to.StringPtr(s.BootstrapData), + } + + switch s.OSDisk.OSType { + case string(compute.OperatingSystemTypesWindows): + // Cloudbase-init is used to generate a password. + // https://cloudbase-init.readthedocs.io/en/latest/plugins.html#setting-password-main + // + // We generate a random password here in case of failure + // but the password on the VM will NOT be the same as created here. + // Access is provided via SSH public key that is set during deployment + // Azure also provides a way to reset user passwords in the case of need. + osProfile.AdminPassword = to.StringPtr(generators.SudoRandomPassword(123)) + osProfile.WindowsConfiguration = &compute.WindowsConfiguration{ + EnableAutomaticUpdates: to.BoolPtr(false), + } + default: + osProfile.LinuxConfiguration = &compute.LinuxConfiguration{ + DisablePasswordAuthentication: to.BoolPtr(true), + SSH: &compute.SSHConfiguration{ + PublicKeys: &[]compute.SSHPublicKey{ + { + Path: to.StringPtr(fmt.Sprintf("/home/%s/.ssh/authorized_keys", azure.DefaultUserName)), + KeyData: to.StringPtr(string(sshKey)), + }, + }, + }, + } + } + + return osProfile, nil +} + +func (s *VMSpec) generateSecurityProfile() (*compute.SecurityProfile, error) { + if s.SecurityProfile == nil { + return nil, nil + } + + if !s.SKU.HasCapability(resourceskus.EncryptionAtHost) { + return nil, azure.WithTerminalError(errors.Errorf("encryption at host is not supported for VM type %s", s.Size)) + } + + return &compute.SecurityProfile{ + EncryptionAtHost: s.SecurityProfile.EncryptionAtHost, + }, nil +} + +func (s *VMSpec) generateNICRefs() *[]compute.NetworkInterfaceReference { + nicRefs := make([]compute.NetworkInterfaceReference, len(s.NICIDs)) + for i, id := range s.NICIDs { + primary := i == 0 + nicRefs[i] = compute.NetworkInterfaceReference{ + ID: to.StringPtr(id), + NetworkInterfaceReferenceProperties: &compute.NetworkInterfaceReferenceProperties{ + Primary: to.BoolPtr(primary), + }, + } + } + return &nicRefs +} + +func (s *VMSpec) generateAdditionalCapabilities() *compute.AdditionalCapabilities { + var capabilities *compute.AdditionalCapabilities + for _, dataDisk := range s.DataDisks { + if dataDisk.ManagedDisk != nil && dataDisk.ManagedDisk.StorageAccountType == string(compute.StorageAccountTypesUltraSSDLRS) { + capabilities = &compute.AdditionalCapabilities{ + UltraSSDEnabled: to.BoolPtr(true), + } + break + } + } + return capabilities +} + +func (s *VMSpec) getAvailabilitySet() *compute.SubResource { + var as *compute.SubResource + if s.AvailabilitySetID != "" { + as = &compute.SubResource{ID: &s.AvailabilitySetID} + } + return as +} + +func (s *VMSpec) getZones() *[]string { + var zones *[]string + if s.Zone != "" { + zones = &[]string{s.Zone} + } + return zones +} diff --git a/azure/services/virtualmachines/spec_test.go b/azure/services/virtualmachines/spec_test.go new file mode 100644 index 00000000000..8a47a7a4e80 --- /dev/null +++ b/azure/services/virtualmachines/spec_test.go @@ -0,0 +1,679 @@ +/* +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 virtualmachines + +import ( + "testing" + + "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-04-01/compute" + "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" + "github.com/Azure/go-autorest/autorest/to" + "github.com/google/go-cmp/cmp" + . "github.com/onsi/gomega" + 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/resourceskus" + gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock" +) + +var ( + validSKU = resourceskus.SKU{ + Name: to.StringPtr("Standard_D2v3"), + Kind: to.StringPtr(string(resourceskus.VirtualMachines)), + Locations: &[]string{ + "test-location", + }, + Capabilities: &[]compute.ResourceSkuCapabilities{ + { + Name: to.StringPtr(resourceskus.VCPUs), + Value: to.StringPtr("2"), + }, + { + Name: to.StringPtr(resourceskus.MemoryGB), + Value: to.StringPtr("4"), + }, + }, + } + + validSKUWithEncryptionAtHost = resourceskus.SKU{ + Name: to.StringPtr("Standard_D2v3"), + Kind: to.StringPtr(string(resourceskus.VirtualMachines)), + Locations: &[]string{ + "test-location", + }, + Capabilities: &[]compute.ResourceSkuCapabilities{ + { + Name: to.StringPtr(resourceskus.VCPUs), + Value: to.StringPtr("2"), + }, + { + Name: to.StringPtr(resourceskus.MemoryGB), + Value: to.StringPtr("4"), + }, + { + Name: to.StringPtr(resourceskus.EncryptionAtHost), + Value: to.StringPtr(string(resourceskus.CapabilitySupported)), + }, + }, + } + + validSKUWithEphemeralOS = resourceskus.SKU{ + Name: to.StringPtr("Standard_D2v3"), + Kind: to.StringPtr(string(resourceskus.VirtualMachines)), + Locations: &[]string{ + "test-location", + }, + Capabilities: &[]compute.ResourceSkuCapabilities{ + { + Name: to.StringPtr(resourceskus.VCPUs), + Value: to.StringPtr("2"), + }, + { + Name: to.StringPtr(resourceskus.MemoryGB), + Value: to.StringPtr("4"), + }, + { + Name: to.StringPtr(resourceskus.EphemeralOSDisk), + Value: to.StringPtr("True"), + }, + }, + } + + validSKUWithUltraSSD = resourceskus.SKU{ + Name: to.StringPtr("Standard_D2v3"), + Kind: to.StringPtr(string(resourceskus.VirtualMachines)), + Locations: &[]string{ + "test-location", + }, + LocationInfo: &[]compute.ResourceSkuLocationInfo{ + { + Location: to.StringPtr("test-location"), + Zones: &[]string{"1"}, + ZoneDetails: &[]compute.ResourceSkuZoneDetails{ + { + Capabilities: &[]compute.ResourceSkuCapabilities{ + { + Name: to.StringPtr("UltraSSDAvailable"), + Value: to.StringPtr("True"), + }, + }, + Name: &[]string{"1"}, + }, + }, + }, + }, + Capabilities: &[]compute.ResourceSkuCapabilities{ + { + Name: to.StringPtr(resourceskus.VCPUs), + Value: to.StringPtr("2"), + }, + { + Name: to.StringPtr(resourceskus.MemoryGB), + Value: to.StringPtr("4"), + }, + }, + } + + invalidCPUSKU = resourceskus.SKU{ + Name: to.StringPtr("Standard_D2v3"), + Kind: to.StringPtr(string(resourceskus.VirtualMachines)), + Locations: &[]string{ + "test-location", + }, + Capabilities: &[]compute.ResourceSkuCapabilities{ + { + Name: to.StringPtr(resourceskus.VCPUs), + Value: to.StringPtr("1"), + }, + { + Name: to.StringPtr(resourceskus.MemoryGB), + Value: to.StringPtr("4"), + }, + }, + } + + invalidMemSKU = resourceskus.SKU{ + Name: to.StringPtr("Standard_D2v3"), + Kind: to.StringPtr(string(resourceskus.VirtualMachines)), + Locations: &[]string{ + "test-location", + }, + Capabilities: &[]compute.ResourceSkuCapabilities{ + { + Name: to.StringPtr(resourceskus.VCPUs), + Value: to.StringPtr("2"), + }, + { + Name: to.StringPtr(resourceskus.MemoryGB), + Value: to.StringPtr("1"), + }, + }, + } +) + +func TestParameters(t *testing.T) { + testcases := []struct { + name string + spec *VMSpec + existing interface{} + expect func(g *WithT, result interface{}) + expectedError string + }{ + { + name: "fails if existing is not a VirtualMachine", + spec: &VMSpec{}, + existing: network.VirtualNetwork{}, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeNil()) + }, + expectedError: "network.VirtualNetwork is not a compute.VirtualMachine", + }, + { + name: "returns nil if vm already exists", + spec: &VMSpec{}, + existing: compute.VirtualMachine{}, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeNil()) + }, + expectedError: "", + }, + { + name: "fails if vm deleted out of band, should not recreate", + spec: &VMSpec{ + ProviderID: "fake/vm/id", + }, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeNil()) + }, + expectedError: azure.VMDeletedError{ProviderID: "fake/vm/id"}.Error(), + }, + { + name: "can create a vm with system assigned identity ", + spec: &VMSpec{ + Name: "my-vm", + Role: infrav1.Node, + NICIDs: []string{"my-nic"}, + SSHKeyData: "fakesshpublickey", + Size: "Standard_D2v3", + Zone: "1", + Image: &infrav1.Image{ID: to.StringPtr("fake-image-id")}, + Identity: infrav1.VMIdentitySystemAssigned, + SKU: validSKU, + }, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeAssignableToTypeOf(compute.VirtualMachine{})) + g.Expect(result.(compute.VirtualMachine).Identity.Type).To(Equal(compute.ResourceIdentityTypeSystemAssigned)) + g.Expect(result.(compute.VirtualMachine).Identity.UserAssignedIdentities).To(HaveLen(0)) + }, + expectedError: "", + }, + { + name: "can create a vm with user assigned identity ", + spec: &VMSpec{ + Name: "my-vm", + Role: infrav1.Node, + NICIDs: []string{"my-nic"}, + SSHKeyData: "fakesshpublickey", + Size: "Standard_D2v3", + Zone: "1", + Image: &infrav1.Image{ID: to.StringPtr("fake-image-id")}, + Identity: infrav1.VMIdentityUserAssigned, + UserAssignedIdentities: []infrav1.UserAssignedIdentity{{ProviderID: "my-user-id"}}, + SKU: validSKU, + }, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeAssignableToTypeOf(compute.VirtualMachine{})) + g.Expect(result.(compute.VirtualMachine).Identity.Type).To(Equal(compute.ResourceIdentityTypeUserAssigned)) + g.Expect(result.(compute.VirtualMachine).Identity.UserAssignedIdentities).To(Equal(map[string]*compute.VirtualMachineIdentityUserAssignedIdentitiesValue{"my-user-id": {}})) + }, + expectedError: "", + }, + { + name: "can create a spot vm", + spec: &VMSpec{ + Name: "my-vm", + Role: infrav1.Node, + NICIDs: []string{"my-nic"}, + SSHKeyData: "fakesshpublickey", + Size: "Standard_D2v3", + Zone: "1", + Image: &infrav1.Image{ID: to.StringPtr("fake-image-id")}, + SpotVMOptions: &infrav1.SpotVMOptions{}, + SKU: validSKU, + }, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeAssignableToTypeOf(compute.VirtualMachine{})) + g.Expect(result.(compute.VirtualMachine).Priority).To(Equal(compute.VirtualMachinePriorityTypesSpot)) + g.Expect(result.(compute.VirtualMachine).EvictionPolicy).To(Equal(compute.VirtualMachineEvictionPolicyTypesDeallocate)) + g.Expect(result.(compute.VirtualMachine).BillingProfile).To(BeNil()) + }, + expectedError: "", + }, + { + name: "can create a windows vm", + spec: &VMSpec{ + Name: "my-vm", + Role: infrav1.Node, + NICIDs: []string{"my-nic"}, + SSHKeyData: "fakesshpublickey", + Size: "Standard_D2v3", + Zone: "1", + Image: &infrav1.Image{ID: to.StringPtr("fake-image-id")}, + OSDisk: infrav1.OSDisk{ + OSType: "Windows", + DiskSizeGB: to.Int32Ptr(128), + ManagedDisk: &infrav1.ManagedDiskParameters{ + StorageAccountType: "Premium_LRS", + }, + }, + SKU: validSKU, + }, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeAssignableToTypeOf(compute.VirtualMachine{})) + g.Expect(result.(compute.VirtualMachine).VirtualMachineProperties.StorageProfile.OsDisk.OsType).To(Equal(compute.OperatingSystemTypesWindows)) + g.Expect(*result.(compute.VirtualMachine).VirtualMachineProperties.OsProfile.AdminPassword).Should(HaveLen(123)) + g.Expect(*result.(compute.VirtualMachine).VirtualMachineProperties.OsProfile.AdminUsername).Should(Equal("capi")) + g.Expect(*result.(compute.VirtualMachine).VirtualMachineProperties.OsProfile.WindowsConfiguration.EnableAutomaticUpdates).Should(Equal(false)) + }, + expectedError: "", + }, + { + name: "can create a vm with encryption", + spec: &VMSpec{ + Name: "my-vm", + Role: infrav1.Node, + NICIDs: []string{"my-nic"}, + SSHKeyData: "fakesshpublickey", + Size: "Standard_D2v3", + Zone: "1", + Image: &infrav1.Image{ID: to.StringPtr("fake-image-id")}, + OSDisk: infrav1.OSDisk{ + ManagedDisk: &infrav1.ManagedDiskParameters{ + StorageAccountType: "Premium_LRS", + DiskEncryptionSet: &infrav1.DiskEncryptionSetParameters{ + ID: "my-diskencryptionset-id", + }, + }, + }, + SKU: validSKU, + }, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeAssignableToTypeOf(compute.VirtualMachine{})) + g.Expect(result.(compute.VirtualMachine).VirtualMachineProperties.StorageProfile.OsDisk.ManagedDisk.DiskEncryptionSet.ID).To(Equal(to.StringPtr("my-diskencryptionset-id"))) + }, + expectedError: "", + }, + { + name: "can create a vm with encryption at host", + spec: &VMSpec{ + Name: "my-vm", + Role: infrav1.Node, + NICIDs: []string{"my-nic"}, + SSHKeyData: "fakesshpublickey", + Size: "Standard_D2v3", + Zone: "1", + Image: &infrav1.Image{ID: to.StringPtr("fake-image-id")}, + SecurityProfile: &infrav1.SecurityProfile{EncryptionAtHost: to.BoolPtr(true)}, + SKU: validSKUWithEncryptionAtHost, + }, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeAssignableToTypeOf(compute.VirtualMachine{})) + g.Expect(*result.(compute.VirtualMachine).VirtualMachineProperties.SecurityProfile.EncryptionAtHost).To(Equal(true)) + }, + expectedError: "", + }, + { + name: "can create a vm and assign it to an availability set", + spec: &VMSpec{ + Name: "my-vm", + Role: infrav1.Node, + NICIDs: []string{"my-nic"}, + SSHKeyData: "fakesshpublickey", + Size: "Standard_D2v3", + AvailabilitySetID: "fake-availability-set-id", + Zone: "", + Image: &infrav1.Image{ID: to.StringPtr("fake-image-id")}, + SKU: validSKU, + }, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeAssignableToTypeOf(compute.VirtualMachine{})) + g.Expect(result.(compute.VirtualMachine).Zones).To(BeNil()) + g.Expect(result.(compute.VirtualMachine).AvailabilitySet.ID).To(Equal(to.StringPtr("fake-availability-set-id"))) + }, + expectedError: "", + }, + { + name: "can create a vm with EphemeralOSDisk", + spec: &VMSpec{ + Name: "my-vm", + Role: infrav1.Node, + NICIDs: []string{"my-nic"}, + SSHKeyData: "fakesshpublickey", + Size: "Standard_D2v3", + OSDisk: infrav1.OSDisk{ + OSType: "Linux", + DiskSizeGB: to.Int32Ptr(128), + ManagedDisk: &infrav1.ManagedDiskParameters{ + StorageAccountType: "Premium_LRS", + }, + DiffDiskSettings: &infrav1.DiffDiskSettings{ + Option: string(compute.DiffDiskOptionsLocal), + }, + }, + Image: &infrav1.Image{ID: to.StringPtr("fake-image-id")}, + SKU: validSKUWithEphemeralOS, + }, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeAssignableToTypeOf(compute.VirtualMachine{})) + g.Expect(result.(compute.VirtualMachine).StorageProfile.OsDisk.DiffDiskSettings.Option).To(Equal(compute.DiffDiskOptionsLocal)) + }, + expectedError: "", + }, + { + name: "creating a vm with encryption at host enabled for unsupported VM type fails", + spec: &VMSpec{ + Name: "my-vm", + Role: infrav1.Node, + NICIDs: []string{"my-nic"}, + SSHKeyData: "fakesshpublickey", + Size: "Standard_D2v3", + AvailabilitySetID: "fake-availability-set-id", + Zone: "", + Image: &infrav1.Image{ID: to.StringPtr("fake-image-id")}, + SecurityProfile: &infrav1.SecurityProfile{EncryptionAtHost: to.BoolPtr(true)}, + SKU: validSKU, + }, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeNil()) + }, + expectedError: "reconcile error that cannot be recovered occurred: encryption at host is not supported for VM type Standard_D2v3. Object will not be requeued", + }, + { + name: "cannot create vm with EphemeralOSDisk if does not support ephemeral os", + spec: &VMSpec{ + Name: "my-vm", + Role: infrav1.Node, + NICIDs: []string{"my-nic"}, + SSHKeyData: "fakesshpublickey", + Size: "Standard_D2v3", + OSDisk: infrav1.OSDisk{ + OSType: "Linux", + DiskSizeGB: to.Int32Ptr(128), + ManagedDisk: &infrav1.ManagedDiskParameters{ + StorageAccountType: "Premium_LRS", + }, + DiffDiskSettings: &infrav1.DiffDiskSettings{ + Option: string(compute.DiffDiskOptionsLocal), + }, + }, + Image: &infrav1.Image{ID: to.StringPtr("fake-image-id")}, + SKU: validSKU, + }, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeNil()) + }, + expectedError: "reconcile error that cannot be recovered occurred: vm size Standard_D2v3 does not support ephemeral os. select a different vm size or disable ephemeral os. Object will not be requeued", + }, + { + name: "cannot create vm if vCPU is less than 2", + spec: &VMSpec{ + Name: "my-vm", + Role: infrav1.Node, + NICIDs: []string{"my-nic"}, + SSHKeyData: "fakesshpublickey", + Size: "Standard_D2v3", + Image: &infrav1.Image{ID: to.StringPtr("fake-image-id")}, + SKU: invalidCPUSKU, + }, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeNil()) + }, + expectedError: "reconcile error that cannot be recovered occurred: vm size should be bigger or equal to at least 2 vCPUs. Object will not be requeued", + }, + { + name: "cannot create vm if memory is less than 2Gi", + spec: &VMSpec{ + Name: "my-vm", + Role: infrav1.Node, + NICIDs: []string{"my-nic"}, + SSHKeyData: "fakesshpublickey", + Size: "Standard_D2v3", + Image: &infrav1.Image{ID: to.StringPtr("fake-image-id")}, + SKU: invalidMemSKU, + }, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeNil()) + }, + expectedError: "reconcile error that cannot be recovered occurred: vm memory should be bigger or equal to at least 2Gi. Object will not be requeued", + }, + { + name: "can create a vm with a marketplace image using a plan", + spec: &VMSpec{ + Name: "my-vm", + Role: infrav1.Node, + NICIDs: []string{"my-nic"}, + SSHKeyData: "fakesshpublickey", + Size: "Standard_D2v3", + Image: &infrav1.Image{ + Marketplace: &infrav1.AzureMarketplaceImage{ + Publisher: "fake-publisher", + Offer: "my-offer", + SKU: "sku-id", + Version: "1.0", + ThirdPartyImage: true, + }, + }, + SKU: validSKU, + }, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeAssignableToTypeOf(compute.VirtualMachine{})) + g.Expect(result.(compute.VirtualMachine).StorageProfile.ImageReference.Offer).To(Equal(to.StringPtr("my-offer"))) + g.Expect(result.(compute.VirtualMachine).StorageProfile.ImageReference.Publisher).To(Equal(to.StringPtr("fake-publisher"))) + g.Expect(result.(compute.VirtualMachine).StorageProfile.ImageReference.Sku).To(Equal(to.StringPtr("sku-id"))) + g.Expect(result.(compute.VirtualMachine).StorageProfile.ImageReference.Version).To(Equal(to.StringPtr("1.0"))) + g.Expect(result.(compute.VirtualMachine).Plan.Name).To(Equal(to.StringPtr("sku-id"))) + g.Expect(result.(compute.VirtualMachine).Plan.Publisher).To(Equal(to.StringPtr("fake-publisher"))) + g.Expect(result.(compute.VirtualMachine).Plan.Product).To(Equal(to.StringPtr("my-offer"))) + }, + expectedError: "", + }, + { + name: "can create a vm with a SIG image using a plan", + spec: &VMSpec{ + Name: "my-vm", + Role: infrav1.Node, + NICIDs: []string{"my-nic"}, + SSHKeyData: "fakesshpublickey", + Size: "Standard_D2v3", + Image: &infrav1.Image{ + SharedGallery: &infrav1.AzureSharedGalleryImage{ + SubscriptionID: "fake-sub-id", + ResourceGroup: "fake-rg", + Gallery: "fake-gallery", + Name: "fake-name", + Version: "1.0", + Publisher: to.StringPtr("fake-publisher"), + Offer: to.StringPtr("my-offer"), + SKU: to.StringPtr("sku-id"), + }, + }, + SKU: validSKU, + }, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeAssignableToTypeOf(compute.VirtualMachine{})) + g.Expect(result.(compute.VirtualMachine).StorageProfile.ImageReference.ID).To(Equal(to.StringPtr("/subscriptions/fake-sub-id/resourceGroups/fake-rg/providers/Microsoft.Compute/galleries/fake-gallery/images/fake-name/versions/1.0"))) + g.Expect(result.(compute.VirtualMachine).Plan.Name).To(Equal(to.StringPtr("sku-id"))) + g.Expect(result.(compute.VirtualMachine).Plan.Publisher).To(Equal(to.StringPtr("fake-publisher"))) + g.Expect(result.(compute.VirtualMachine).Plan.Product).To(Equal(to.StringPtr("my-offer"))) + }, + expectedError: "", + }, + { + name: "can create a vm with ultra disk enabled", + spec: &VMSpec{ + Name: "my-ultra-ssd-vm", + Role: infrav1.Node, + NICIDs: []string{"my-nic"}, + SSHKeyData: "fakesshpublickey", + Size: "Standard_D2v3", + Location: "test-location", + Zone: "1", + Image: &infrav1.Image{ID: to.StringPtr("fake-image-id")}, + DataDisks: []infrav1.DataDisk{ + { + NameSuffix: "mydisk", + DiskSizeGB: 64, + Lun: to.Int32Ptr(0), + }, + { + NameSuffix: "myDiskWithUltraDisk", + DiskSizeGB: 128, + Lun: to.Int32Ptr(1), + ManagedDisk: &infrav1.ManagedDiskParameters{ + StorageAccountType: "UltraSSD_LRS", + }, + }, + { + NameSuffix: "myDiskWithManagedDisk", + DiskSizeGB: 128, + Lun: to.Int32Ptr(2), + ManagedDisk: &infrav1.ManagedDiskParameters{ + StorageAccountType: "Premium_LRS", + }, + }, + { + NameSuffix: "managedDiskWithEncryption", + DiskSizeGB: 128, + Lun: to.Int32Ptr(3), + ManagedDisk: &infrav1.ManagedDiskParameters{ + StorageAccountType: "Premium_LRS", + DiskEncryptionSet: &infrav1.DiskEncryptionSetParameters{ + ID: "my_id", + }, + }, + }, + }, + SKU: validSKUWithUltraSSD, + }, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeAssignableToTypeOf(compute.VirtualMachine{})) + g.Expect(result.(compute.VirtualMachine).AdditionalCapabilities.UltraSSDEnabled).To(Equal(to.BoolPtr(true))) + expectedDataDisks := &[]compute.DataDisk{ + { + Lun: to.Int32Ptr(0), + Name: to.StringPtr("my-ultra-ssd-vm_mydisk"), + CreateOption: "Empty", + DiskSizeGB: to.Int32Ptr(64), + }, + { + Lun: to.Int32Ptr(1), + Name: to.StringPtr("my-ultra-ssd-vm_myDiskWithUltraDisk"), + CreateOption: "Empty", + DiskSizeGB: to.Int32Ptr(128), + ManagedDisk: &compute.ManagedDiskParameters{ + StorageAccountType: "UltraSSD_LRS", + }, + }, + { + Lun: to.Int32Ptr(2), + Name: to.StringPtr("my-ultra-ssd-vm_myDiskWithManagedDisk"), + CreateOption: "Empty", + DiskSizeGB: to.Int32Ptr(128), + ManagedDisk: &compute.ManagedDiskParameters{ + StorageAccountType: "Premium_LRS", + }, + }, + { + Lun: to.Int32Ptr(3), + Name: to.StringPtr("my-ultra-ssd-vm_managedDiskWithEncryption"), + CreateOption: "Empty", + DiskSizeGB: to.Int32Ptr(128), + ManagedDisk: &compute.ManagedDiskParameters{ + StorageAccountType: "Premium_LRS", + DiskEncryptionSet: &compute.DiskEncryptionSetParameters{ + ID: to.StringPtr("my_id"), + }, + }, + }, + } + g.Expect(gomockinternal.DiffEq(expectedDataDisks).Matches(result.(compute.VirtualMachine).StorageProfile.DataDisks)).To(BeTrue(), cmp.Diff(expectedDataDisks, result.(compute.VirtualMachine).StorageProfile.DataDisks)) + }, + expectedError: "", + }, + { + name: "creating vm with ultra disk enabled in unsupported location fails", + spec: &VMSpec{ + Name: "my-vm", + Role: infrav1.Node, + NICIDs: []string{"my-nic"}, + SSHKeyData: "fakesshpublickey", + Size: "Standard_D2v3", + Location: "test-location", + Zone: "1", + Image: &infrav1.Image{ID: to.StringPtr("fake-image-id")}, + DataDisks: []infrav1.DataDisk{ + { + NameSuffix: "myDiskWithUltraDisk", + DiskSizeGB: 128, + Lun: to.Int32Ptr(1), + ManagedDisk: &infrav1.ManagedDiskParameters{ + StorageAccountType: "UltraSSD_LRS", + }, + }, + }, + SKU: validSKU, + }, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeNil()) + }, + expectedError: "reconcile error that cannot be recovered occurred: vm size Standard_D2v3 does not support ultra disks in location test-location. select a different vm size or disable ultra disks. Object will not be requeued", + }, + } + 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/services/virtualmachines/virtualmachines.go b/azure/services/virtualmachines/virtualmachines.go index 3828ae048e8..f974f98e6da 100644 --- a/azure/services/virtualmachines/virtualmachines.go +++ b/azure/services/virtualmachines/virtualmachines.go @@ -18,8 +18,6 @@ package virtualmachines import ( "context" - "encoding/base64" - "fmt" "strings" "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-04-01/compute" @@ -31,28 +29,26 @@ 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/converters" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/async" "sigs.k8s.io/cluster-api-provider-azure/azure/services/availabilitysets" "sigs.k8s.io/cluster-api-provider-azure/azure/services/networkinterfaces" "sigs.k8s.io/cluster-api-provider-azure/azure/services/publicips" - "sigs.k8s.io/cluster-api-provider-azure/azure/services/resourceskus" - "sigs.k8s.io/cluster-api-provider-azure/util/generators" + "sigs.k8s.io/cluster-api-provider-azure/util/reconciler" "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) +const serviceName = "virtualmachine" + // VMScope defines the scope interface for a virtual machines service. type VMScope interface { logr.Logger - azure.ClusterDescriber - VMSpec() azure.VMSpec - GetBootstrapData(ctx context.Context) (string, error) - GetVMImage() (*infrav1.Image, error) + azure.Authorizer + azure.AsyncStatusUpdater + VMSpec() azure.ResourceSpecGetter SetAnnotation(string, string) - ProviderID() string - AvailabilitySet() (string, bool) SetProviderID(string) SetAddresses([]corev1.NodeAddress) SetVMState(infrav1.ProvisioningState) - UpdateStatus() } // Service provides operations on Azure resources. @@ -62,18 +58,16 @@ type Service struct { interfacesClient networkinterfaces.Client publicIPsClient publicips.Client availabilitySetsClient availabilitysets.Client - resourceSKUCache *resourceskus.Cache } // New creates a new service. -func New(scope VMScope, skuCache *resourceskus.Cache) *Service { +func New(scope VMScope) *Service { return &Service{ Scope: scope, Client: NewClient(scope), interfacesClient: networkinterfaces.NewClient(scope), publicIPsClient: publicips.NewClient(scope), availabilitySetsClient: availabilitysets.NewClient(scope), - resourceSKUCache: skuCache, } } @@ -82,133 +76,34 @@ func (s *Service) Reconcile(ctx context.Context) error { ctx, _, done := tele.StartSpanWithLogger(ctx, "virtualmachines.Service.Reconcile") defer done() - vmSpec := s.Scope.VMSpec() - existingVM, err := s.getExisting(ctx, vmSpec.Name) + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) + defer cancel() - switch { - // VM got deleted outside of capz - case err != nil && azure.ResourceNotFound(err) && s.Scope.ProviderID() != "": - s.Scope.SetVMState(infrav1.Deleted) - return azure.VMDeletedError{ProviderID: s.Scope.ProviderID()} - case err != nil && !azure.ResourceNotFound(err): - return errors.Wrapf(err, "failed to get VM %s", vmSpec.Name) - case err == nil: - // VM already exists, update the spec and skip creation. - s.Scope.SetProviderID(azure.ProviderIDPrefix + existingVM.ID) - s.Scope.SetAnnotation("cluster-api-provider-azure", "true") - s.Scope.SetAddresses(existingVM.Addresses) - s.Scope.SetVMState(existingVM.State) - s.Scope.UpdateStatus() - default: - s.Scope.V(2).Info("creating VM", "vm", vmSpec.Name) - sku, err := s.resourceSKUCache.Get(ctx, vmSpec.Size, resourceskus.VirtualMachines) - if err != nil { - return azure.WithTerminalError(errors.Wrapf(err, "failed to get SKU %s in compute api", vmSpec.Size)) - } + vmSpec := s.Scope.VMSpec() - storageProfile, err := s.generateStorageProfile(ctx, vmSpec, sku) - if err != nil { - return err + result, err := async.CreateResource(ctx, s.Scope, s.Client, vmSpec, serviceName) + s.Scope.UpdatePutStatus(infrav1.VMRunningCondition, serviceName, err) + if err == nil && result != nil { + vm, ok := result.(compute.VirtualMachine) + if !ok { + return errors.Errorf("%T is not a compute.VirtualMachine", result) } - - securityProfile, err := getSecurityProfile(vmSpec, sku) + infraVM, err := converters.SDKToVM(vm) if err != nil { return err } + s.Scope.SetProviderID(azure.ProviderIDPrefix + infraVM.ID) + s.Scope.SetAnnotation("cluster-api-provider-azure", "true") - nicRefs := make([]compute.NetworkInterfaceReference, len(vmSpec.NICNames)) - for i, nicName := range vmSpec.NICNames { - primary := i == 0 - nicRefs[i] = compute.NetworkInterfaceReference{ - ID: to.StringPtr(azure.NetworkInterfaceID(s.Scope.SubscriptionID(), s.Scope.ResourceGroup(), nicName)), - NetworkInterfaceReferenceProperties: &compute.NetworkInterfaceReferenceProperties{ - Primary: to.BoolPtr(primary), - }, - } - } - - priority, evictionPolicy, billingProfile, err := converters.GetSpotVMOptions(vmSpec.SpotVMOptions) - if err != nil { - return errors.Wrap(err, "failed to get Spot VM options") - } - - osProfile, err := s.generateOSProfile(ctx, vmSpec) + // Discover addresses for NICs associated with the VM + addresses, err := s.getAddresses(ctx, vm, vmSpec.ResourceGroupName()) if err != nil { - return errors.Wrap(err, "failed to generate OS Profile") - } - - virtualMachine := compute.VirtualMachine{ - Plan: s.generateImagePlan(), - Location: to.StringPtr(s.Scope.Location()), - Tags: converters.TagsToMap(infrav1.Build(infrav1.BuildParams{ - ClusterName: s.Scope.ClusterName(), - Lifecycle: infrav1.ResourceLifecycleOwned, - Name: to.StringPtr(vmSpec.Name), - Role: to.StringPtr(vmSpec.Role), - Additional: s.Scope.AdditionalTags(), - })), - VirtualMachineProperties: &compute.VirtualMachineProperties{ - HardwareProfile: &compute.HardwareProfile{ - VMSize: compute.VirtualMachineSizeTypes(vmSpec.Size), - }, - StorageProfile: storageProfile, - SecurityProfile: securityProfile, - OsProfile: osProfile, - NetworkProfile: &compute.NetworkProfile{ - NetworkInterfaces: &nicRefs, - }, - Priority: priority, - EvictionPolicy: evictionPolicy, - BillingProfile: billingProfile, - DiagnosticsProfile: &compute.DiagnosticsProfile{ - BootDiagnostics: &compute.BootDiagnostics{ - Enabled: to.BoolPtr(true), - }, - }, - }, - } - - // Set availability set if no failure domains are available - if asName, ok := s.Scope.AvailabilitySet(); ok { - asID := to.StringPtr(azure.AvailabilitySetID(s.Scope.SubscriptionID(), - s.Scope.ResourceGroup(), asName)) - virtualMachine.AvailabilitySet = &compute.SubResource{ID: asID} - } else if vmSpec.Zone != "" { - zones := []string{vmSpec.Zone} - virtualMachine.Zones = &zones - } - - if vmSpec.Identity == infrav1.VMIdentitySystemAssigned { - virtualMachine.Identity = &compute.VirtualMachineIdentity{ - Type: compute.ResourceIdentityTypeSystemAssigned, - } - } else if vmSpec.Identity == infrav1.VMIdentityUserAssigned { - userIdentitiesMap, err := converters.UserAssignedIdentitiesToVMSDK(vmSpec.UserAssignedIdentities) - if err != nil { - return errors.Wrapf(err, "failed to assign identity %q", vmSpec.Name) - } - virtualMachine.Identity = &compute.VirtualMachineIdentity{ - Type: compute.ResourceIdentityTypeUserAssigned, - UserAssignedIdentities: userIdentitiesMap, - } - } - - for _, dataDisk := range vmSpec.DataDisks { - if dataDisk.ManagedDisk != nil && dataDisk.ManagedDisk.StorageAccountType == string(compute.StorageAccountTypesUltraSSDLRS) { - virtualMachine.VirtualMachineProperties.AdditionalCapabilities = &compute.AdditionalCapabilities{ - UltraSSDEnabled: to.BoolPtr(true), - } - } + return errors.Wrap(err, "failed to fetch VM addresses") } - - if err := s.Client.CreateOrUpdate(ctx, s.Scope.ResourceGroup(), vmSpec.Name, virtualMachine); err != nil { - return errors.Wrapf(err, "failed to create VM %s in resource group %s", vmSpec.Name, s.Scope.ResourceGroup()) - } - - s.Scope.V(2).Info("successfully created VM", "vm", vmSpec.Name) + s.Scope.SetAddresses(addresses) + s.Scope.SetVMState(infraVM.State) } - - return nil + return err } // Delete deletes the virtual machine with the provided name. @@ -216,77 +111,22 @@ func (s *Service) Delete(ctx context.Context) error { ctx, _, done := tele.StartSpanWithLogger(ctx, "virtualmachines.Service.Delete") defer done() - vmSpec := s.Scope.VMSpec() - s.Scope.V(2).Info("deleting VM", "vm", vmSpec.Name) - err := s.Client.Delete(ctx, s.Scope.ResourceGroup(), vmSpec.Name) - if err != nil && azure.ResourceNotFound(err) { - // already deleted - return nil - } - if err != nil { - return errors.Wrapf(err, "failed to delete VM %s in resource group %s", vmSpec.Name, s.Scope.ResourceGroup()) - } - - s.Scope.V(2).Info("successfully deleted VM", "vm", vmSpec.Name) - return nil -} - -// getExisting provides information about a virtual machine. -func (s *Service) getExisting(ctx context.Context, name string) (*converters.VM, error) { - ctx, _, done := tele.StartSpanWithLogger(ctx, "virtualmachines.Service.getExisting") - defer done() - - vm, err := s.Client.Get(ctx, s.Scope.ResourceGroup(), name) - if err != nil { - return nil, err - } + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) + defer cancel() - convertedVM, err := converters.SDKToVM(vm) - if err != nil { - return convertedVM, err - } - - // Discover addresses for NICs associated with the VM - // and add them to our converted vm struct - addresses, err := s.getAddresses(ctx, vm) - if err != nil { - return convertedVM, err - } - convertedVM.Addresses = addresses - return convertedVM, nil -} + vmSpec := s.Scope.VMSpec() -func (s *Service) generateImagePlan() *compute.Plan { - image, err := s.Scope.GetVMImage() + err := async.DeleteResource(ctx, s.Scope, s.Client, vmSpec, serviceName) if err != nil { - s.Scope.Error(err, "failed to get vm image, disabling Plan") - return nil - } - - if image.SharedGallery != nil && image.SharedGallery.Publisher != nil && image.SharedGallery.SKU != nil && image.SharedGallery.Offer != nil { - return &compute.Plan{ - Publisher: image.SharedGallery.Publisher, - Name: image.SharedGallery.SKU, - Product: image.SharedGallery.Offer, - } - } - - if image.Marketplace == nil || !image.Marketplace.ThirdPartyImage { - return nil - } - - if image.Marketplace.Publisher == "" || image.Marketplace.SKU == "" || image.Marketplace.Offer == "" { - return nil - } - - return &compute.Plan{ - Publisher: to.StringPtr(image.Marketplace.Publisher), - Name: to.StringPtr(image.Marketplace.SKU), - Product: to.StringPtr(image.Marketplace.Offer), + s.Scope.SetVMState(infrav1.Deleting) + } else { + s.Scope.SetVMState(infrav1.Deleted) } + s.Scope.UpdateDeleteStatus(infrav1.VMRunningCondition, serviceName, err) + return err } -func (s *Service) getAddresses(ctx context.Context, vm compute.VirtualMachine) ([]corev1.NodeAddress, error) { +func (s *Service) getAddresses(ctx context.Context, vm compute.VirtualMachine, rgName string) ([]corev1.NodeAddress, error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "virtualmachines.Service.getAddresses") defer done() @@ -305,7 +145,7 @@ func (s *Service) getAddresses(ctx context.Context, vm compute.VirtualMachine) ( nicName := getResourceNameByID(to.String(nicRef.ID)) // Fetch nic and append its addresses - nic, err := s.interfacesClient.Get(ctx, s.Scope.ResourceGroup(), nicName) + nic, err := s.interfacesClient.Get(ctx, rgName, nicName) if err != nil { return addresses, err } @@ -329,7 +169,7 @@ func (s *Service) getAddresses(ctx context.Context, vm compute.VirtualMachine) ( // ID is the only field populated in PublicIPAddress sub-resource. // Thus, we have to go fetch the publicIP with the name. publicIPName := getResourceNameByID(to.String(ipConfig.PublicIPAddress.ID)) - publicNodeAddress, err := s.getPublicIPAddress(ctx, publicIPName) + publicNodeAddress, err := s.getPublicIPAddress(ctx, publicIPName, rgName) if err != nil { return addresses, err } @@ -341,12 +181,12 @@ func (s *Service) getAddresses(ctx context.Context, vm compute.VirtualMachine) ( } // getPublicIPAddress will fetch a public ip address resource by name and return a nodeaddresss representation. -func (s *Service) getPublicIPAddress(ctx context.Context, publicIPAddressName string) (corev1.NodeAddress, error) { +func (s *Service) getPublicIPAddress(ctx context.Context, publicIPAddressName string, rgName string) (corev1.NodeAddress, error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "virtualmachines.Service.getPublicIPAddress") defer done() retAddress := corev1.NodeAddress{} - publicIP, err := s.publicIPsClient.Get(ctx, s.Scope.ResourceGroup(), publicIPAddressName) + publicIP, err := s.publicIPsClient.Get(ctx, rgName, publicIPAddressName) if err != nil { return retAddress, err } @@ -356,103 +196,6 @@ func (s *Service) getPublicIPAddress(ctx context.Context, publicIPAddressName st return retAddress, nil } -// generateStorageProfile generates a pointer to a compute.StorageProfile which can utilized for VM creation. -func (s *Service) generateStorageProfile(ctx context.Context, vmSpec azure.VMSpec, sku resourceskus.SKU) (*compute.StorageProfile, error) { - _, _, done := tele.StartSpanWithLogger(ctx, "virtualmachines.Service.generateStorageProfile") - defer done() - - storageProfile := &compute.StorageProfile{ - OsDisk: &compute.OSDisk{ - Name: to.StringPtr(azure.GenerateOSDiskName(vmSpec.Name)), - OsType: compute.OperatingSystemTypes(vmSpec.OSDisk.OSType), - CreateOption: compute.DiskCreateOptionTypesFromImage, - DiskSizeGB: vmSpec.OSDisk.DiskSizeGB, - Caching: compute.CachingTypes(vmSpec.OSDisk.CachingType), - }, - } - - // Checking if the requested VM size has at least 2 vCPUS - vCPUCapability, err := sku.HasCapabilityWithCapacity(resourceskus.VCPUs, resourceskus.MinimumVCPUS) - if err != nil { - return nil, azure.WithTerminalError(errors.Wrap(err, "failed to validate the vCPU capability")) - } - if !vCPUCapability { - return nil, azure.WithTerminalError(errors.New("vm size should be bigger or equal to at least 2 vCPUs")) - } - - // Checking if the requested VM size has at least 2 Gi of memory - MemoryCapability, err := sku.HasCapabilityWithCapacity(resourceskus.MemoryGB, resourceskus.MinimumMemory) - if err != nil { - return nil, azure.WithTerminalError(errors.Wrap(err, "failed to validate the memory capability")) - } - - if !MemoryCapability { - return nil, azure.WithTerminalError(errors.New("vm memory should be bigger or equal to at least 2Gi")) - } - // enable ephemeral OS - if vmSpec.OSDisk.DiffDiskSettings != nil { - if !sku.HasCapability(resourceskus.EphemeralOSDisk) { - return nil, azure.WithTerminalError(fmt.Errorf("vm size %s does not support ephemeral os. select a different vm size or disable ephemeral os", vmSpec.Size)) - } - - storageProfile.OsDisk.DiffDiskSettings = &compute.DiffDiskSettings{ - Option: compute.DiffDiskOptions(vmSpec.OSDisk.DiffDiskSettings.Option), - } - } - - if vmSpec.OSDisk.ManagedDisk != nil { - storageProfile.OsDisk.ManagedDisk = &compute.ManagedDiskParameters{} - if vmSpec.OSDisk.ManagedDisk.StorageAccountType != "" { - storageProfile.OsDisk.ManagedDisk.StorageAccountType = compute.StorageAccountTypes(vmSpec.OSDisk.ManagedDisk.StorageAccountType) - } - if vmSpec.OSDisk.ManagedDisk.DiskEncryptionSet != nil { - storageProfile.OsDisk.ManagedDisk.DiskEncryptionSet = &compute.DiskEncryptionSetParameters{ID: to.StringPtr(vmSpec.OSDisk.ManagedDisk.DiskEncryptionSet.ID)} - } - } - - dataDisks := make([]compute.DataDisk, len(vmSpec.DataDisks)) - for i, disk := range vmSpec.DataDisks { - dataDisks[i] = compute.DataDisk{ - CreateOption: compute.DiskCreateOptionTypesEmpty, - DiskSizeGB: to.Int32Ptr(disk.DiskSizeGB), - Lun: disk.Lun, - Name: to.StringPtr(azure.GenerateDataDiskName(vmSpec.Name, disk.NameSuffix)), - Caching: compute.CachingTypes(disk.CachingType), - } - - if disk.ManagedDisk != nil { - dataDisks[i].ManagedDisk = &compute.ManagedDiskParameters{ - StorageAccountType: compute.StorageAccountTypes(disk.ManagedDisk.StorageAccountType), - } - - if disk.ManagedDisk.DiskEncryptionSet != nil { - dataDisks[i].ManagedDisk.DiskEncryptionSet = &compute.DiskEncryptionSetParameters{ID: to.StringPtr(disk.ManagedDisk.DiskEncryptionSet.ID)} - } - - // check the support for ultra disks based on location and vm size - location := s.Scope.Location() - if disk.ManagedDisk.StorageAccountType == string(compute.StorageAccountTypesUltraSSDLRS) && !sku.HasLocationCapability(resourceskus.UltraSSDAvailable, location, vmSpec.Zone) { - return nil, azure.WithTerminalError(fmt.Errorf("vm size %s does not support ultra disks in location %s. select a different vm size or disable ultra disks", vmSpec.Size, location)) - } - } - } - storageProfile.DataDisks = &dataDisks - - image, err := s.Scope.GetVMImage() - if err != nil { - return nil, errors.Wrap(err, "failed to get VM image") - } - - imageRef, err := converters.ImageToSDK(image) - if err != nil { - return nil, err - } - - storageProfile.ImageReference = imageRef - - return storageProfile, nil -} - // getResourceNameById takes a resource ID like // `/subscriptions/$SUB/resourceGroups/$RG/providers/Microsoft.Network/networkInterfaces/$NICNAME` // and parses out the string after the last slash. @@ -461,63 +204,3 @@ func getResourceNameByID(resourceID string) string { resourceName := explodedResourceID[len(explodedResourceID)-1] return resourceName } - -func (s *Service) generateOSProfile(ctx context.Context, vmSpec azure.VMSpec) (*compute.OSProfile, error) { - sshKey, err := base64.StdEncoding.DecodeString(vmSpec.SSHKeyData) - if err != nil { - return nil, errors.Wrap(err, "failed to decode ssh public key") - } - bootstrapData, err := s.Scope.GetBootstrapData(ctx) - if err != nil { - return nil, errors.Wrap(err, "failed to retrieve bootstrap data") - } - - osProfile := &compute.OSProfile{ - ComputerName: to.StringPtr(vmSpec.Name), - AdminUsername: to.StringPtr(azure.DefaultUserName), - CustomData: to.StringPtr(bootstrapData), - } - - switch vmSpec.OSDisk.OSType { - case string(compute.OperatingSystemTypesWindows): - // Cloudbase-init is used to generate a password. - // https://cloudbase-init.readthedocs.io/en/latest/plugins.html#setting-password-main - // - // We generate a random password here in case of failure - // but the password on the VM will NOT be the same as created here. - // Access is provided via SSH public key that is set during deployment - // Azure also provides a way to reset user passwords in the case of need. - osProfile.AdminPassword = to.StringPtr(generators.SudoRandomPassword(123)) - osProfile.WindowsConfiguration = &compute.WindowsConfiguration{ - EnableAutomaticUpdates: to.BoolPtr(false), - } - default: - osProfile.LinuxConfiguration = &compute.LinuxConfiguration{ - DisablePasswordAuthentication: to.BoolPtr(true), - SSH: &compute.SSHConfiguration{ - PublicKeys: &[]compute.SSHPublicKey{ - { - Path: to.StringPtr(fmt.Sprintf("/home/%s/.ssh/authorized_keys", azure.DefaultUserName)), - KeyData: to.StringPtr(string(sshKey)), - }, - }, - }, - } - } - - return osProfile, nil -} - -func getSecurityProfile(vmSpec azure.VMSpec, sku resourceskus.SKU) (*compute.SecurityProfile, error) { - if vmSpec.SecurityProfile == nil { - return nil, nil - } - - if !sku.HasCapability(resourceskus.EncryptionAtHost) { - return nil, azure.WithTerminalError(errors.Errorf("encryption at host is not supported for VM type %s", vmSpec.Size)) - } - - return &compute.SecurityProfile{ - EncryptionAtHost: to.BoolPtr(*vmSpec.SecurityProfile.EncryptionAtHost), - }, nil -} diff --git a/azure/services/virtualmachines/virtualmachines_test.go b/azure/services/virtualmachines/virtualmachines_test.go index 9a8210d7263..2054655986c 100644 --- a/azure/services/virtualmachines/virtualmachines_test.go +++ b/azure/services/virtualmachines/virtualmachines_test.go @@ -18,228 +18,127 @@ package virtualmachines import ( "context" + "fmt" "net/http" "testing" - "k8s.io/utils/pointer" - "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-04-01/compute" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" "github.com/Azure/go-autorest/autorest" + azureautorest "github.com/Azure/go-autorest/autorest/azure" "github.com/Azure/go-autorest/autorest/to" "github.com/golang/mock/gomock" . "github.com/onsi/gomega" + "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" "k8s.io/klog/v2/klogr" 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/availabilitysets/mock_availabilitysets" "sigs.k8s.io/cluster-api-provider-azure/azure/services/networkinterfaces/mock_networkinterfaces" "sigs.k8s.io/cluster-api-provider-azure/azure/services/publicips/mock_publicips" - "sigs.k8s.io/cluster-api-provider-azure/azure/services/resourceskus" "sigs.k8s.io/cluster-api-provider-azure/azure/services/virtualmachines/mock_virtualmachines" gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock" ) -func TestGetExistingVM(t *testing.T) { +var ( + fakeVMSpec = VMSpec{ + Name: "test-vm", + ResourceGroup: "test-group", + Location: "test-location", + ClusterName: "test-cluster", + Role: infrav1.ControlPlane, + NICIDs: []string{"nic-id-1", "nic-id-2"}, + SSHKeyData: "fake ssh key", + Size: "Standard_Fake_Size", + AvailabilitySetID: "availability-set", + Identity: infrav1.VMIdentitySystemAssigned, + AdditionalTags: map[string]string{"foo": "bar"}, + Image: &infrav1.Image{ID: to.StringPtr("fake-image-id")}, + BootstrapData: "fake data", + } + fakeFuture = infrav1.Future{ + Type: infrav1.DeleteFuture, + ServiceName: serviceName, + Name: "test-vm", + ResourceGroup: "test-group", + Data: "eyJtZXRob2QiOiJERUxFVEUiLCJwb2xsaW5nTWV0aG9kIjoiTG9jYXRpb24iLCJscm9TdGF0ZSI6IkluUHJvZ3Jlc3MifQ==", + } + internalError = autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error") + notFoundError = autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not Found") + errCtxExceeded = errors.New("ctx exceeded") +) + +func TestReconcileVM(t *testing.T) { testcases := []struct { name string - vmName string - result *converters.VM expectedError string expect func(s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) }{ { - name: "get existing vm", - vmName: "my-vm", - result: &converters.VM{ - ID: "my-id", - Name: "my-vm", - State: "Succeeded", - Identity: "", - Tags: nil, - Addresses: []corev1.NodeAddress{ - { - Type: "InternalIP", - Address: "1.2.3.4", - }, - { - Type: "ExternalIP", - Address: "4.3.2.1", - }, - }, - }, + name: "create vm succeeds", expectedError: "", expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) { - s.ResourceGroup().AnyTimes().Return("my-rg") s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - mpip.Get(gomockinternal.AContext(), "my-rg", "my-publicIP-id").Return(network.PublicIPAddress{ - PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ - PublicIPAddressVersion: network.IPVersionIPv4, - PublicIPAllocationMethod: network.IPAllocationMethodStatic, - IPAddress: to.StringPtr("4.3.2.1"), - }, - }, nil) - mnic.Get(gomockinternal.AContext(), "my-rg", gomock.Any()).Return(network.Interface{ - InterfacePropertiesFormat: &network.InterfacePropertiesFormat{ - IPConfigurations: &[]network.InterfaceIPConfiguration{ - { - Name: to.StringPtr("pipConfig"), - InterfaceIPConfigurationPropertiesFormat: &network.InterfaceIPConfigurationPropertiesFormat{ - PrivateIPAddress: to.StringPtr("1.2.3.4"), - PublicIPAddress: &network.PublicIPAddress{ - ID: to.StringPtr("my-publicIP-id"), - Name: to.StringPtr("my-publicIP"), - PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ - PublicIPAddressVersion: network.IPVersionIPv4, - PublicIPAllocationMethod: network.IPAllocationMethodStatic, - IPAddress: to.StringPtr("4.3.2.1"), - }, - }, - }, - }, - }, - }, - }, nil) - m.Get(gomockinternal.AContext(), "my-rg", "my-vm").Return(compute.VirtualMachine{ - ID: to.StringPtr("my-id"), - Name: to.StringPtr("my-vm"), + s.VMSpec().Return(&fakeVMSpec) + s.GetLongRunningOperationState("test-vm", serviceName) + m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakeVMSpec).Return(compute.VirtualMachine{ + ID: to.StringPtr("test-vm-id"), VirtualMachineProperties: &compute.VirtualMachineProperties{ ProvisioningState: to.StringPtr("Succeeded"), NetworkProfile: &compute.NetworkProfile{ NetworkInterfaces: &[]compute.NetworkInterfaceReference{ { - ID: to.StringPtr("my-nic-id"), - NetworkInterfaceReferenceProperties: &compute.NetworkInterfaceReferenceProperties{ - Primary: to.BoolPtr(true), - }, + ID: to.StringPtr("/subscriptions/123/resourceGroups/test-rg/providers/Microsoft.Network/networkInterfaces/nic-1"), }, }, }, }, - }, nil) - }, - }, - { - name: "vm not found", - vmName: "my-vm", - result: &converters.VM{}, - expectedError: "#: Not found: StatusCode=404", - expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) { - s.ResourceGroup().AnyTimes().Return("my-rg") - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - m.Get(gomockinternal.AContext(), "my-rg", "my-vm").Return(compute.VirtualMachine{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - }, - }, - { - name: "vm retrieval fails", - vmName: "my-vm", - result: &converters.VM{}, - expectedError: "#: Internal Server Error: StatusCode=500", - expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) { - s.ResourceGroup().AnyTimes().Return("my-rg") - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - m.Get(gomockinternal.AContext(), "my-rg", "my-vm").Return(compute.VirtualMachine{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) - }, - }, - { - name: "get existing vm: error getting public IP", - vmName: "my-vm", - result: &converters.VM{}, - expectedError: "#: Internal Server Error: StatusCode=500", - expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) { - s.ResourceGroup().AnyTimes().Return("my-rg") - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - mpip.Get(gomockinternal.AContext(), "my-rg", "my-publicIP-id").Return(network.PublicIPAddress{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) - mnic.Get(gomockinternal.AContext(), "my-rg", gomock.Any()).Return(network.Interface{ + }, nil, nil) + s.UpdatePutStatus(infrav1.VMRunningCondition, serviceName, nil) + s.SetProviderID("azure://test-vm-id") + s.SetAnnotation("cluster-api-provider-azure", "true") + mnic.Get(gomockinternal.AContext(), "test-group", "nic-1").Return(network.Interface{ InterfacePropertiesFormat: &network.InterfacePropertiesFormat{ IPConfigurations: &[]network.InterfaceIPConfiguration{ { - Name: to.StringPtr("pipConfig"), InterfaceIPConfigurationPropertiesFormat: &network.InterfaceIPConfigurationPropertiesFormat{ - PrivateIPAddress: to.StringPtr("1.2.3.4"), + PrivateIPAddress: to.StringPtr("10.0.0.5"), PublicIPAddress: &network.PublicIPAddress{ - ID: to.StringPtr("my-publicIP-id"), - Name: to.StringPtr("my-publicIP"), - PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ - PublicIPAddressVersion: network.IPVersionIPv4, - PublicIPAllocationMethod: network.IPAllocationMethodStatic, - IPAddress: to.StringPtr("4.3.2.1"), - }, + ID: to.StringPtr("/subscriptions/123/resourceGroups/test-rg/providers/Microsoft.Network/publicIPAddresses/pip-1"), }, }, }, }, }, }, nil) - m.Get(gomockinternal.AContext(), "my-rg", "my-vm").Return(compute.VirtualMachine{ - ID: to.StringPtr("my-id"), - Name: to.StringPtr("my-vm"), - VirtualMachineProperties: &compute.VirtualMachineProperties{ - ProvisioningState: to.StringPtr("Succeeded"), - NetworkProfile: &compute.NetworkProfile{ - NetworkInterfaces: &[]compute.NetworkInterfaceReference{ - { - ID: to.StringPtr("my-nic-id"), - NetworkInterfaceReferenceProperties: &compute.NetworkInterfaceReferenceProperties{ - Primary: to.BoolPtr(true), - }, - }, - }, - }, + mpip.Get(gomockinternal.AContext(), "test-group", "pip-1").Return(network.PublicIPAddress{ + PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ + IPAddress: to.StringPtr("10.0.0.6"), }, }, nil) + s.SetAddresses([]corev1.NodeAddress{ + { + Type: corev1.NodeInternalIP, + Address: "10.0.0.5", + }, + { + Type: corev1.NodeExternalIP, + Address: "10.0.0.6", + }, + }) + s.SetVMState(infrav1.Succeeded) }, }, { - name: "get existing vm: public IP not found", - vmName: "my-vm", - result: &converters.VM{}, - expectedError: "#: Not Found: StatusCode=404", + name: "create vm fails", + expectedError: "failed to create resource test-group/test-vm (service: virtualmachine): #: Internal Server Error: StatusCode=500", expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) { - s.ResourceGroup().AnyTimes().Return("my-rg") s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - mpip.Get(gomockinternal.AContext(), "my-rg", "my-publicIP-id").Return(network.PublicIPAddress{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not Found")) - mnic.Get(gomockinternal.AContext(), "my-rg", gomock.Any()).Return(network.Interface{ - InterfacePropertiesFormat: &network.InterfacePropertiesFormat{ - IPConfigurations: &[]network.InterfaceIPConfiguration{ - { - Name: to.StringPtr("pipConfig"), - InterfaceIPConfigurationPropertiesFormat: &network.InterfaceIPConfigurationPropertiesFormat{ - PrivateIPAddress: to.StringPtr("1.2.3.4"), - PublicIPAddress: &network.PublicIPAddress{ - ID: to.StringPtr("my-publicIP-id"), - Name: to.StringPtr("my-publicIP"), - PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{ - PublicIPAddressVersion: network.IPVersionIPv4, - PublicIPAllocationMethod: network.IPAllocationMethodStatic, - IPAddress: to.StringPtr("4.3.2.1"), - }, - }, - }, - }, - }, - }, - }, nil) - m.Get(gomockinternal.AContext(), "my-rg", "my-vm").Return(compute.VirtualMachine{ - ID: to.StringPtr("my-id"), - Name: to.StringPtr("my-vm"), - VirtualMachineProperties: &compute.VirtualMachineProperties{ - ProvisioningState: to.StringPtr("Succeeded"), - NetworkProfile: &compute.NetworkProfile{ - NetworkInterfaces: &[]compute.NetworkInterfaceReference{ - { - ID: to.StringPtr("my-nic-id"), - NetworkInterfaceReferenceProperties: &compute.NetworkInterfaceReferenceProperties{ - Primary: to.BoolPtr(true), - }, - }, - }, - }, - }, - }, nil) + s.VMSpec().Return(&fakeVMSpec) + s.GetLongRunningOperationState("test-vm", serviceName) + m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakeVMSpec).Return(nil, nil, internalError) + s.UpdatePutStatus(infrav1.VMRunningCondition, serviceName, gomockinternal.ErrStrEq(fmt.Sprintf("failed to create resource test-group/test-vm (service: virtualmachine): %s", internalError.Error()))) }, }, } @@ -256,2073 +155,108 @@ func TestGetExistingVM(t *testing.T) { clientMock := mock_virtualmachines.NewMockClient(mockCtrl) interfaceMock := mock_networkinterfaces.NewMockClient(mockCtrl) publicIPMock := mock_publicips.NewMockClient(mockCtrl) + availabilitySetsMock := mock_availabilitysets.NewMockClient(mockCtrl) tc.expect(scopeMock.EXPECT(), clientMock.EXPECT(), interfaceMock.EXPECT(), publicIPMock.EXPECT()) s := &Service{ - Scope: scopeMock, - Client: clientMock, - interfacesClient: interfaceMock, - publicIPsClient: publicIPMock, + Scope: scopeMock, + Client: clientMock, + interfacesClient: interfaceMock, + publicIPsClient: publicIPMock, + availabilitySetsClient: availabilitySetsMock, } - result, err := s.getExisting(context.TODO(), tc.vmName) + 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()) - g.Expect(result).To(BeEquivalentTo(tc.result)) } }) } } -func TestReconcileVM(t *testing.T) { +func TestDeleteVM(t *testing.T) { testcases := []struct { - Name string - Expect func(g *WithT, s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) - ExpectedError string - SetupSKUs func(svc *Service) + name string + expectedError string + expect func(s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder) }{ { - Name: "can create a vm", - Expect: func(g *WithT, s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, - mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) { - s.VMSpec().Return(azure.VMSpec{ - Name: "my-vm", - Role: infrav1.ControlPlane, - NICNames: []string{"my-nic", "second-nic"}, - SSHKeyData: "ZmFrZXNzaGtleQo=", - Size: "Standard_D2v3", - Zone: "1", - Identity: infrav1.VMIdentityNone, - OSDisk: infrav1.OSDisk{ - OSType: "Linux", - DiskSizeGB: to.Int32Ptr(128), - ManagedDisk: &infrav1.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - }, - }, - DataDisks: []infrav1.DataDisk{ - { - NameSuffix: "mydisk", - DiskSizeGB: 64, - Lun: to.Int32Ptr(0), - }, - { - NameSuffix: "myDiskWithManagedDisk", - DiskSizeGB: 128, - Lun: to.Int32Ptr(1), - ManagedDisk: &infrav1.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - }, - }, - { - NameSuffix: "managedDiskWithEncryption", - DiskSizeGB: 128, - Lun: to.Int32Ptr(2), - ManagedDisk: &infrav1.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - DiskEncryptionSet: &infrav1.DiskEncryptionSetParameters{ - ID: "my_id", - }, - }, - }, - }, - UserAssignedIdentities: nil, - SpotVMOptions: nil, - }) - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("my-rg") - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.AdditionalTags() - s.Location().Return("test-location").AnyTimes() - s.ClusterName().Return("my-cluster") - s.ProviderID().Return("") - m.Get(gomockinternal.AContext(), "my-rg", "my-vm"). - Return(compute.VirtualMachine{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - s.GetVMImage().AnyTimes().Return(&infrav1.Image{ - Marketplace: &infrav1.AzureMarketplaceImage{ - Publisher: "fake-publisher", - Offer: "my-offer", - SKU: "sku-id", - Version: "1.0", - }, - }, nil) - s.GetBootstrapData(gomockinternal.AContext()).Return("fake-bootstrap-data", nil) - s.AvailabilitySet().Return("", false) - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-vm", gomockinternal.DiffEq(compute.VirtualMachine{ - VirtualMachineProperties: &compute.VirtualMachineProperties{ - HardwareProfile: &compute.HardwareProfile{VMSize: "Standard_D2v3"}, - StorageProfile: &compute.StorageProfile{ - ImageReference: &compute.ImageReference{ - Publisher: to.StringPtr("fake-publisher"), - Offer: to.StringPtr("my-offer"), - Sku: to.StringPtr("sku-id"), - Version: to.StringPtr("1.0"), - }, - OsDisk: &compute.OSDisk{ - OsType: "Linux", - Name: to.StringPtr("my-vm_OSDisk"), - CreateOption: "FromImage", - DiskSizeGB: to.Int32Ptr(128), - ManagedDisk: &compute.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - }, - }, - DataDisks: &[]compute.DataDisk{ - { - Lun: to.Int32Ptr(0), - Name: to.StringPtr("my-vm_mydisk"), - CreateOption: "Empty", - DiskSizeGB: to.Int32Ptr(64), - }, - { - Lun: to.Int32Ptr(1), - Name: to.StringPtr("my-vm_myDiskWithManagedDisk"), - CreateOption: "Empty", - DiskSizeGB: to.Int32Ptr(128), - ManagedDisk: &compute.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - }, - }, - { - Lun: to.Int32Ptr(2), - Name: to.StringPtr("my-vm_managedDiskWithEncryption"), - CreateOption: "Empty", - DiskSizeGB: to.Int32Ptr(128), - ManagedDisk: &compute.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - DiskEncryptionSet: &compute.DiskEncryptionSetParameters{ - ID: to.StringPtr("my_id"), - }, - }, - }, - }, - }, - OsProfile: &compute.OSProfile{ - ComputerName: to.StringPtr("my-vm"), - AdminUsername: to.StringPtr("capi"), - CustomData: to.StringPtr("fake-bootstrap-data"), - LinuxConfiguration: &compute.LinuxConfiguration{ - DisablePasswordAuthentication: to.BoolPtr(true), - SSH: &compute.SSHConfiguration{ - PublicKeys: &[]compute.SSHPublicKey{ - { - Path: to.StringPtr("/home/capi/.ssh/authorized_keys"), - KeyData: to.StringPtr("fakesshkey\n"), - }, - }, - }, - }, - }, - DiagnosticsProfile: &compute.DiagnosticsProfile{ - BootDiagnostics: &compute.BootDiagnostics{ - Enabled: to.BoolPtr(true), - }, - }, - NetworkProfile: &compute.NetworkProfile{ - NetworkInterfaces: &[]compute.NetworkInterfaceReference{ - { - NetworkInterfaceReferenceProperties: &compute.NetworkInterfaceReferenceProperties{Primary: to.BoolPtr(true)}, - ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/networkInterfaces/my-nic"), - }, - { - NetworkInterfaceReferenceProperties: &compute.NetworkInterfaceReferenceProperties{Primary: to.BoolPtr(false)}, - ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/networkInterfaces/second-nic"), - }, - }, - }, - }, - Resources: nil, - Identity: nil, - Zones: &[]string{"1"}, - ID: nil, - Name: nil, - Type: nil, - Location: to.StringPtr("test-location"), - Tags: map[string]*string{ - "Name": to.StringPtr("my-vm"), - "sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": to.StringPtr("owned"), - "sigs.k8s.io_cluster-api-provider-azure_role": to.StringPtr("control-plane"), - }, - })) - }, - ExpectedError: "", - SetupSKUs: func(svc *Service) { - skus := []compute.ResourceSku{ - { - Name: to.StringPtr("Standard_D2v3"), - Kind: to.StringPtr(string(resourceskus.VirtualMachines)), - Locations: &[]string{ - "test-location", - }, - LocationInfo: &[]compute.ResourceSkuLocationInfo{ - { - Location: to.StringPtr("test-location"), - Zones: &[]string{"1"}, - }, - }, - Capabilities: &[]compute.ResourceSkuCapabilities{ - { - Name: to.StringPtr(resourceskus.VCPUs), - Value: to.StringPtr("2"), - }, - { - Name: to.StringPtr(resourceskus.MemoryGB), - Value: to.StringPtr("4"), - }, - }, - }, - } - resourceSkusCache := resourceskus.NewStaticCache(skus, "") - svc.resourceSKUCache = resourceSkusCache - }, - }, - { - Name: "can create a vm with system assigned identity", - Expect: func(g *WithT, s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) { - s.VMSpec().Return(azure.VMSpec{ - Name: "my-vm", - Role: infrav1.Node, - NICNames: []string{"my-nic"}, - SSHKeyData: "fakesshpublickey", - Size: "Standard_D2v3", - Zone: "1", - Identity: infrav1.VMIdentitySystemAssigned, - OSDisk: infrav1.OSDisk{}, - DataDisks: nil, - UserAssignedIdentities: nil, - SpotVMOptions: nil, - }) - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("my-rg") + name: "long running delete operation is done", + expectedError: "", + expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.AdditionalTags() - s.Location().Return("test-location").AnyTimes() - s.ClusterName().Return("my-cluster") - s.ProviderID().Return("") - m.Get(gomockinternal.AContext(), "my-rg", "my-vm"). - Return(compute.VirtualMachine{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - s.GetVMImage().AnyTimes().Return(&infrav1.Image{ - Marketplace: &infrav1.AzureMarketplaceImage{ - Publisher: "fake-publisher", - Offer: "my-offer", - SKU: "sku-id", - Version: "1.0", - }, - }, nil) - s.GetBootstrapData(gomockinternal.AContext()).Return("fake-bootstrap-data", nil) - s.AvailabilitySet().Return("", false) - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-vm", gomock.AssignableToTypeOf(compute.VirtualMachine{})).Do(func(_, _, _ interface{}, vm compute.VirtualMachine) { - g.Expect(vm.Identity.Type).To(Equal(compute.ResourceIdentityTypeSystemAssigned)) - g.Expect(vm.Identity.UserAssignedIdentities).To(HaveLen(0)) - }) - }, - ExpectedError: "", - SetupSKUs: func(svc *Service) { - skus := []compute.ResourceSku{ - { - Name: to.StringPtr("Standard_D2v3"), - Kind: to.StringPtr(string(resourceskus.VirtualMachines)), - Locations: &[]string{ - "test-location", - }, - LocationInfo: &[]compute.ResourceSkuLocationInfo{ - { - Location: to.StringPtr("test-location"), - Zones: &[]string{"1"}, - }, - }, - Capabilities: &[]compute.ResourceSkuCapabilities{ - { - Name: to.StringPtr(resourceskus.VCPUs), - Value: to.StringPtr("2"), - }, - { - Name: to.StringPtr(resourceskus.MemoryGB), - Value: to.StringPtr("4"), - }, - }, - }, - } - resourceSkusCache := resourceskus.NewStaticCache(skus, "") - svc.resourceSKUCache = resourceSkusCache + s.VMSpec().AnyTimes().Return(&fakeVMSpec) + s.GetLongRunningOperationState("test-vm", serviceName).Times(2).Return(&fakeFuture) + m.IsDone(gomockinternal.AContext(), gomock.AssignableToTypeOf(&azureautorest.Future{})).Return(true, nil) + m.Result(gomockinternal.AContext(), gomock.AssignableToTypeOf(&azureautorest.Future{}), infrav1.DeleteFuture).Return(nil, nil) + s.DeleteLongRunningOperationState("test-vm", serviceName) + s.SetVMState(infrav1.Deleted) + s.UpdateDeleteStatus(infrav1.VMRunningCondition, serviceName, nil) }, }, { - Name: "can create a vm with user assigned identity", - Expect: func(g *WithT, s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) { - s.VMSpec().Return(azure.VMSpec{ - Name: "my-vm", - Role: infrav1.Node, - NICNames: []string{"my-nic"}, - SSHKeyData: "fakesshpublickey", - Size: "Standard_D2v3", - Zone: "1", - Identity: infrav1.VMIdentityUserAssigned, - OSDisk: infrav1.OSDisk{}, - DataDisks: nil, - UserAssignedIdentities: []infrav1.UserAssignedIdentity{{ProviderID: "my-user-id"}}, - SpotVMOptions: nil, - }) - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("my-rg") + name: "long running delete operation is not done", + expectedError: "operation type DELETE on Azure resource test-group/test-vm is not done. Object will be requeued after 15s", + expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.AdditionalTags() - s.Location().Return("test-location").AnyTimes() - s.ClusterName().Return("my-cluster") - s.ProviderID().Return("") - m.Get(gomockinternal.AContext(), "my-rg", "my-vm"). - Return(compute.VirtualMachine{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - s.GetVMImage().AnyTimes().Return(&infrav1.Image{ - Marketplace: &infrav1.AzureMarketplaceImage{ - Publisher: "fake-publisher", - Offer: "my-offer", - SKU: "sku-id", - Version: "1.0", - }, - }, nil) - s.GetBootstrapData(gomockinternal.AContext()).Return("fake-bootstrap-data", nil) - s.AvailabilitySet().Return("", false) - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-vm", gomock.AssignableToTypeOf(compute.VirtualMachine{})).Do(func(_, _, _ interface{}, vm compute.VirtualMachine) { - g.Expect(vm.Identity.Type).To(Equal(compute.ResourceIdentityTypeUserAssigned)) - g.Expect(vm.Identity.UserAssignedIdentities).To(Equal(map[string]*compute.VirtualMachineIdentityUserAssignedIdentitiesValue{"my-user-id": {}})) - }) - }, - ExpectedError: "", - SetupSKUs: func(svc *Service) { - skus := []compute.ResourceSku{ - { - Name: to.StringPtr("Standard_D2v3"), - Kind: to.StringPtr(string(resourceskus.VirtualMachines)), - Locations: &[]string{ - "test-location", - }, - LocationInfo: &[]compute.ResourceSkuLocationInfo{ - { - Location: to.StringPtr("test-location"), - Zones: &[]string{"1"}, - }, - }, - Capabilities: &[]compute.ResourceSkuCapabilities{ - { - Name: to.StringPtr(resourceskus.VCPUs), - Value: to.StringPtr("2"), - }, - { - Name: to.StringPtr(resourceskus.MemoryGB), - Value: to.StringPtr("4"), - }, - }, - }, - } - resourceSkusCache := resourceskus.NewStaticCache(skus, "") - svc.resourceSKUCache = resourceSkusCache + s.VMSpec().AnyTimes().Return(&fakeVMSpec) + s.GetLongRunningOperationState("test-vm", serviceName).Times(2).Return(&fakeFuture) + m.IsDone(gomockinternal.AContext(), gomock.AssignableToTypeOf(&azureautorest.Future{})).Return(false, nil) + s.SetVMState(infrav1.Deleting) + s.UpdateDeleteStatus(infrav1.VMRunningCondition, serviceName, gomockinternal.ErrStrEq("operation type DELETE on Azure resource test-group/test-vm is not done. Object will be requeued after 15s")) }, }, { - Name: "can create a spot vm", - Expect: func(g *WithT, s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) { - s.VMSpec().Return(azure.VMSpec{ - Name: "my-vm", - Role: infrav1.Node, - NICNames: []string{"my-nic"}, - SSHKeyData: "fakesshpublickey", - Size: "Standard_D2v3", - Zone: "1", - Identity: "", - OSDisk: infrav1.OSDisk{}, - DataDisks: nil, - UserAssignedIdentities: nil, - SpotVMOptions: &infrav1.SpotVMOptions{}, - }) - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("my-rg") + name: "vm doesn't exist", + expectedError: "", + expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.AdditionalTags() - s.Location().Return("test-location").AnyTimes() - s.ClusterName().Return("my-cluster") - s.ProviderID().Return("") - m.Get(gomockinternal.AContext(), "my-rg", "my-vm"). - Return(compute.VirtualMachine{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - s.GetVMImage().AnyTimes().Return(&infrav1.Image{ - Marketplace: &infrav1.AzureMarketplaceImage{ - Publisher: "fake-publisher", - Offer: "my-offer", - SKU: "sku-id", - Version: "1.0", - }, - }, nil) - s.GetBootstrapData(gomockinternal.AContext()).Return("fake-bootstrap-data", nil) - s.AvailabilitySet().Return("", false) - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-vm", gomock.AssignableToTypeOf(compute.VirtualMachine{})).Do(func(_, _, _ interface{}, vm compute.VirtualMachine) { - g.Expect(vm.Priority).To(Equal(compute.VirtualMachinePriorityTypesSpot)) - g.Expect(vm.EvictionPolicy).To(Equal(compute.VirtualMachineEvictionPolicyTypesDeallocate)) - g.Expect(vm.BillingProfile).To(BeNil()) - }) - }, - ExpectedError: "", - SetupSKUs: func(svc *Service) { - skus := []compute.ResourceSku{ - { - Name: to.StringPtr("Standard_D2v3"), - Kind: to.StringPtr(string(resourceskus.VirtualMachines)), - Locations: &[]string{ - "test-location", - }, - LocationInfo: &[]compute.ResourceSkuLocationInfo{ - { - Location: to.StringPtr("test-location"), - Zones: &[]string{"1"}, - }, - }, - Capabilities: &[]compute.ResourceSkuCapabilities{ - { - Name: to.StringPtr(resourceskus.VCPUs), - Value: to.StringPtr("2"), - }, - { - Name: to.StringPtr(resourceskus.MemoryGB), - Value: to.StringPtr("4"), - }, - }, - }, - } - resourceSkusCache := resourceskus.NewStaticCache(skus, "") - svc.resourceSKUCache = resourceSkusCache + s.VMSpec().AnyTimes().Return(&fakeVMSpec) + s.GetLongRunningOperationState("test-vm", serviceName) + m.DeleteAsync(gomockinternal.AContext(), &fakeVMSpec).Return(nil, notFoundError) + s.SetVMState(infrav1.Deleted) + s.UpdateDeleteStatus(infrav1.VMRunningCondition, serviceName, nil) }, }, { - Name: "can create a windows vm", - Expect: func(g *WithT, s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) { - s.VMSpec().Return(azure.VMSpec{ - - Name: "my-vm", - Role: infrav1.ControlPlane, - NICNames: []string{"my-nic", "second-nic"}, - SSHKeyData: "ZmFrZXNzaGtleQo=", - Size: "Standard_D2v3", - Zone: "1", - Identity: infrav1.VMIdentityNone, - OSDisk: infrav1.OSDisk{ - OSType: "Windows", - DiskSizeGB: to.Int32Ptr(128), - ManagedDisk: &infrav1.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - }, - }, - DataDisks: []infrav1.DataDisk{ - { - NameSuffix: "mydisk", - DiskSizeGB: 64, - Lun: to.Int32Ptr(0), - }, - }, - UserAssignedIdentities: nil, - SpotVMOptions: nil, - }, - ) - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("my-rg") + name: "error occurs when deleting vm", + expectedError: "failed to delete resource test-group/test-vm (service: virtualmachine): #: Internal Server Error: StatusCode=500", + expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.AdditionalTags() - s.Location().Return("test-location").AnyTimes() - s.ClusterName().Return("my-cluster") - s.ProviderID().Return("") - m.Get(gomockinternal.AContext(), "my-rg", "my-vm"). - Return(compute.VirtualMachine{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - s.GetVMImage().AnyTimes().Return(&infrav1.Image{ - Marketplace: &infrav1.AzureMarketplaceImage{ - Publisher: "fake-publisher", - Offer: "my-offer", - SKU: "sku-id", - Version: "1.0", - }, - }, nil) - s.GetBootstrapData(gomockinternal.AContext()).Return("fake-bootstrap-data", nil) - s.AvailabilitySet().Return("", false) - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-vm", gomock.AssignableToTypeOf(compute.VirtualMachine{})).Do(func(_, _, _ interface{}, vm compute.VirtualMachine) { - g.Expect(vm.VirtualMachineProperties.StorageProfile.OsDisk.OsType).To(Equal(compute.OperatingSystemTypesWindows)) - g.Expect(*vm.VirtualMachineProperties.OsProfile.AdminPassword).Should(HaveLen(123)) - g.Expect(*vm.VirtualMachineProperties.OsProfile.AdminUsername).Should(Equal("capi")) - g.Expect(*vm.VirtualMachineProperties.OsProfile.WindowsConfiguration.EnableAutomaticUpdates).Should(Equal(false)) - }) - }, - ExpectedError: "", - SetupSKUs: func(svc *Service) { - skus := []compute.ResourceSku{ - { - Name: to.StringPtr("Standard_D2v3"), - Kind: to.StringPtr(string(resourceskus.VirtualMachines)), - Locations: &[]string{ - "test-location", - }, - LocationInfo: &[]compute.ResourceSkuLocationInfo{ - { - Location: to.StringPtr("test-location"), - Zones: &[]string{"1"}, - }, - }, - Capabilities: &[]compute.ResourceSkuCapabilities{ - { - Name: to.StringPtr(resourceskus.VCPUs), - Value: to.StringPtr("2"), - }, - { - Name: to.StringPtr(resourceskus.MemoryGB), - Value: to.StringPtr("4"), - }, - }, - }, - } - resourceSkusCache := resourceskus.NewStaticCache(skus, "") - svc.resourceSKUCache = resourceSkusCache + s.VMSpec().AnyTimes().Return(&fakeVMSpec) + s.GetLongRunningOperationState("test-vm", serviceName).Return(nil) + m.DeleteAsync(gomockinternal.AContext(), &fakeVMSpec).Return(nil, internalError) + s.SetVMState(infrav1.Deleting) + s.UpdateDeleteStatus(infrav1.VMRunningCondition, serviceName, gomockinternal.ErrStrEq("failed to delete resource test-group/test-vm (service: virtualmachine): #: Internal Server Error: StatusCode=500")) }, }, { - Name: "can create a vm with encryption", - Expect: func(g *WithT, s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) { - s.VMSpec().Return(azure.VMSpec{ - Name: "my-vm", - Role: infrav1.Node, - NICNames: []string{"my-nic"}, - SSHKeyData: "fakesshpublickey", - Size: "Standard_D2v3", - Zone: "1", - Identity: "", - OSDisk: infrav1.OSDisk{ - ManagedDisk: &infrav1.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - DiskEncryptionSet: &infrav1.DiskEncryptionSetParameters{ - ID: "my-diskencryptionset-id", - }, - }, - }, - DataDisks: nil, - UserAssignedIdentities: nil, - }) - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("my-rg") + name: "context deadline exceeded while deleting vm", + expectedError: "operation type DELETE on Azure resource test-group/test-vm is not done. Object will be requeued after 15s", + expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.AdditionalTags() - s.Location().Return("test-location").AnyTimes() - s.ClusterName().Return("my-cluster") - s.ProviderID().Return("") - m.Get(gomockinternal.AContext(), "my-rg", "my-vm"). - Return(compute.VirtualMachine{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - s.GetVMImage().AnyTimes().Return(&infrav1.Image{ - Marketplace: &infrav1.AzureMarketplaceImage{ - Publisher: "fake-publisher", - Offer: "my-offer", - SKU: "sku-id", - Version: "1.0", - }, - }, nil) - s.GetBootstrapData(gomockinternal.AContext()).Return("fake-bootstrap-data", nil) - s.AvailabilitySet().Return("", false) - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-vm", gomock.AssignableToTypeOf(compute.VirtualMachine{})).Do(func(_, _, _ interface{}, vm compute.VirtualMachine) { - g.Expect(vm.VirtualMachineProperties.StorageProfile.OsDisk.ManagedDisk.DiskEncryptionSet.ID).To(Equal(to.StringPtr("my-diskencryptionset-id"))) - }) - }, - ExpectedError: "", - SetupSKUs: func(svc *Service) { - skus := []compute.ResourceSku{ - { - Name: to.StringPtr("Standard_D2v3"), - Kind: to.StringPtr(string(resourceskus.VirtualMachines)), - Locations: &[]string{ - "test-location", - }, - LocationInfo: &[]compute.ResourceSkuLocationInfo{ - { - Location: to.StringPtr("test-location"), - Zones: &[]string{"1"}, - }, - }, - Capabilities: &[]compute.ResourceSkuCapabilities{ - { - Name: to.StringPtr(resourceskus.VCPUs), - Value: to.StringPtr("2"), - }, - { - Name: to.StringPtr(resourceskus.MemoryGB), - Value: to.StringPtr("4"), - }, - }, - }, - } - resourceSkusCache := resourceskus.NewStaticCache(skus, "") - svc.resourceSKUCache = resourceSkusCache + s.VMSpec().AnyTimes().Return(&fakeVMSpec) + s.GetLongRunningOperationState("test-vm", serviceName).Return(nil) + m.DeleteAsync(gomockinternal.AContext(), &fakeVMSpec).Return(&azureautorest.Future{}, errCtxExceeded) + s.SetLongRunningOperationState(gomock.AssignableToTypeOf(&infrav1.Future{})) + s.SetVMState(infrav1.Deleting) + s.UpdateDeleteStatus(infrav1.VMRunningCondition, serviceName, gomockinternal.ErrStrEq("operation type DELETE on Azure resource test-group/test-vm is not done. Object will be requeued after 15s")) }, }, { - Name: "can create a vm with encryption at host", - Expect: func(g *WithT, s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) { - s.VMSpec().Return(azure.VMSpec{ - Name: "my-vm", - Role: infrav1.Node, - NICNames: []string{"my-nic"}, - SSHKeyData: "fakesshpublickey", - Size: "Standard_D2v3", - Zone: "1", - OSDisk: infrav1.OSDisk{}, - SecurityProfile: &infrav1.SecurityProfile{EncryptionAtHost: to.BoolPtr(true)}, - }) - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("my-rg") + name: "delete the vm successfully", + expectedError: "", + expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.AdditionalTags() - s.Location().Return("test-location").AnyTimes() - s.ClusterName().Return("my-cluster") - s.ProviderID().Return("") - m.Get(gomockinternal.AContext(), "my-rg", "my-vm"). - Return(compute.VirtualMachine{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - s.GetVMImage().AnyTimes().Return(&infrav1.Image{ - Marketplace: &infrav1.AzureMarketplaceImage{ - Publisher: "fake-publisher", - Offer: "my-offer", - SKU: "sku-id", - Version: "1.0", - }, - }, nil) - s.GetBootstrapData(gomockinternal.AContext()).Return("fake-bootstrap-data", nil) - s.AvailabilitySet().Return("", false) - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-vm", gomock.AssignableToTypeOf(compute.VirtualMachine{})).Do(func(_, _, _ interface{}, vm compute.VirtualMachine) { - g.Expect(*vm.VirtualMachineProperties.SecurityProfile.EncryptionAtHost).To(Equal(true)) - }) - }, - ExpectedError: "", - SetupSKUs: func(svc *Service) { - skus := []compute.ResourceSku{ - { - Name: to.StringPtr("Standard_D2v3"), - Kind: to.StringPtr(string(resourceskus.VirtualMachines)), - Locations: &[]string{ - "test-location", - }, - LocationInfo: &[]compute.ResourceSkuLocationInfo{ - { - Location: to.StringPtr("test-location"), - Zones: &[]string{"1"}, - }, - }, - Capabilities: &[]compute.ResourceSkuCapabilities{ - { - Name: to.StringPtr(resourceskus.VCPUs), - Value: to.StringPtr("2"), - }, - { - Name: to.StringPtr(resourceskus.MemoryGB), - Value: to.StringPtr("4"), - }, - { - Name: to.StringPtr(resourceskus.EncryptionAtHost), - Value: to.StringPtr(string(resourceskus.CapabilitySupported)), - }, - }, - }, - } - - svc.resourceSKUCache = resourceskus.NewStaticCache(skus, "") - }, - }, - { - Name: "can create a vm and assign it to an availability set", - Expect: func(g *WithT, s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) { - s.VMSpec().Return(azure.VMSpec{ - Name: "my-vm", - Role: infrav1.ControlPlane, - NICNames: []string{"my-nic", "second-nic"}, - SSHKeyData: "ZmFrZXNzaGtleQo=", - Size: "Standard_D2v3", - Zone: "", - Identity: infrav1.VMIdentityNone, - OSDisk: infrav1.OSDisk{ - OSType: "Linux", - DiskSizeGB: to.Int32Ptr(128), - ManagedDisk: &infrav1.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - }, - }, - DataDisks: []infrav1.DataDisk{ - { - NameSuffix: "mydisk", - DiskSizeGB: 64, - Lun: to.Int32Ptr(0), - }, - }, - UserAssignedIdentities: nil, - SpotVMOptions: nil, - }) - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("my-rg") - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.AdditionalTags() - s.Location().Return("test-location").AnyTimes() - s.ClusterName().Return("my-cluster") - s.ProviderID().Return("") - m.Get(gomockinternal.AContext(), "my-rg", "my-vm"). - Return(compute.VirtualMachine{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - s.GetVMImage().AnyTimes().Return(&infrav1.Image{ - Marketplace: &infrav1.AzureMarketplaceImage{ - Publisher: "fake-publisher", - Offer: "my-offer", - SKU: "sku-id", - Version: "1.0", - }, - }, nil) - s.GetBootstrapData(gomockinternal.AContext()).Return("fake-bootstrap-data", nil) - s.AvailabilitySet().Return("as-name", true) - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-vm", gomockinternal.DiffEq(compute.VirtualMachine{ - VirtualMachineProperties: &compute.VirtualMachineProperties{ - HardwareProfile: &compute.HardwareProfile{VMSize: "Standard_D2v3"}, - StorageProfile: &compute.StorageProfile{ - ImageReference: &compute.ImageReference{ - Publisher: to.StringPtr("fake-publisher"), - Offer: to.StringPtr("my-offer"), - Sku: to.StringPtr("sku-id"), - Version: to.StringPtr("1.0"), - }, - OsDisk: &compute.OSDisk{ - OsType: "Linux", - Name: to.StringPtr("my-vm_OSDisk"), - CreateOption: "FromImage", - DiskSizeGB: to.Int32Ptr(128), - ManagedDisk: &compute.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - }, - }, - DataDisks: &[]compute.DataDisk{ - { - Lun: to.Int32Ptr(0), - Name: to.StringPtr("my-vm_mydisk"), - CreateOption: "Empty", - DiskSizeGB: to.Int32Ptr(64), - }, - }, - }, - OsProfile: &compute.OSProfile{ - ComputerName: to.StringPtr("my-vm"), - AdminUsername: to.StringPtr("capi"), - CustomData: to.StringPtr("fake-bootstrap-data"), - LinuxConfiguration: &compute.LinuxConfiguration{ - DisablePasswordAuthentication: to.BoolPtr(true), - SSH: &compute.SSHConfiguration{ - PublicKeys: &[]compute.SSHPublicKey{ - { - Path: to.StringPtr("/home/capi/.ssh/authorized_keys"), - KeyData: to.StringPtr("fakesshkey\n"), - }, - }, - }, - }, - }, - DiagnosticsProfile: &compute.DiagnosticsProfile{ - BootDiagnostics: &compute.BootDiagnostics{ - Enabled: to.BoolPtr(true), - }, - }, - AvailabilitySet: &compute.SubResource{ - ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Compute/availabilitySets/as-name"), - }, - NetworkProfile: &compute.NetworkProfile{ - NetworkInterfaces: &[]compute.NetworkInterfaceReference{ - { - NetworkInterfaceReferenceProperties: &compute.NetworkInterfaceReferenceProperties{Primary: to.BoolPtr(true)}, - ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/networkInterfaces/my-nic"), - }, - { - NetworkInterfaceReferenceProperties: &compute.NetworkInterfaceReferenceProperties{Primary: to.BoolPtr(false)}, - ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/networkInterfaces/second-nic"), - }, - }, - }, - }, - Resources: nil, - Identity: nil, - ID: nil, - Name: nil, - Type: nil, - Location: to.StringPtr("test-location"), - Tags: map[string]*string{ - "Name": to.StringPtr("my-vm"), - "sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": to.StringPtr("owned"), - "sigs.k8s.io_cluster-api-provider-azure_role": to.StringPtr("control-plane"), - }, - })) - }, - ExpectedError: "", - SetupSKUs: func(svc *Service) { - skus := []compute.ResourceSku{ - { - Name: to.StringPtr("Standard_D2v3"), - Kind: to.StringPtr(string(resourceskus.VirtualMachines)), - Locations: &[]string{ - "test-location", - }, - LocationInfo: &[]compute.ResourceSkuLocationInfo{ - { - Location: to.StringPtr("test-location"), - Zones: &[]string{"1"}, - }, - }, - Capabilities: &[]compute.ResourceSkuCapabilities{ - { - Name: to.StringPtr(resourceskus.VCPUs), - Value: to.StringPtr("2"), - }, - { - Name: to.StringPtr(resourceskus.MemoryGB), - Value: to.StringPtr("4"), - }, - }, - }, - } - resourceSkusCache := resourceskus.NewStaticCache(skus, "") - svc.resourceSKUCache = resourceSkusCache - }, - }, - { - Name: "creating a vm with encryption at host enabled for unsupported VM type fails", - Expect: func(g *WithT, s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) { - s.VMSpec().Return(azure.VMSpec{ - Name: "my-vm", - Role: infrav1.Node, - NICNames: []string{"my-nic"}, - SSHKeyData: "fakesshpublickey", - Size: "Standard_D2v3", - OSDisk: infrav1.OSDisk{}, - SecurityProfile: &infrav1.SecurityProfile{EncryptionAtHost: to.BoolPtr(true)}, - }) - s.ResourceGroup().AnyTimes().Return("my-rg") - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.GetVMImage().AnyTimes().Return(&infrav1.Image{ - Marketplace: &infrav1.AzureMarketplaceImage{ - Publisher: "fake-publisher", - Offer: "my-offer", - SKU: "sku-id", - Version: "1.0", - }, - }, nil) - s.Location().Return("test-location").AnyTimes() - s.ProviderID().Return("") - m.Get(gomockinternal.AContext(), "my-rg", "my-vm"). - Return(compute.VirtualMachine{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - }, - ExpectedError: "reconcile error that cannot be recovered occurred: encryption at host is not supported for VM type Standard_D2v3. Object will not be requeued", - SetupSKUs: func(svc *Service) { - skus := []compute.ResourceSku{ - { - Name: to.StringPtr("Standard_D2v3"), - Kind: to.StringPtr(string(resourceskus.VirtualMachines)), - Locations: &[]string{ - "test-location", - }, - LocationInfo: &[]compute.ResourceSkuLocationInfo{ - { - Location: to.StringPtr("test-location"), - Zones: &[]string{"1"}, - }, - }, - Capabilities: &[]compute.ResourceSkuCapabilities{ - { - Name: to.StringPtr(resourceskus.VCPUs), - Value: to.StringPtr("2"), - }, - { - Name: to.StringPtr(resourceskus.MemoryGB), - Value: to.StringPtr("4"), - }, - }, - }, - } - - svc.resourceSKUCache = resourceskus.NewStaticCache(skus, "") - }, - }, - { - Name: "vm creation fails", - Expect: func(g *WithT, s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) { - s.VMSpec().Return(azure.VMSpec{ - Name: "my-vm", - Role: infrav1.ControlPlane, - NICNames: []string{"my-nic"}, - SSHKeyData: "fakesshpublickey", - Size: "Standard_D2v3", - Zone: "1", - Identity: "", - OSDisk: infrav1.OSDisk{}, - DataDisks: nil, - UserAssignedIdentities: nil, - SpotVMOptions: nil, - }) - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("my-rg") - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.AdditionalTags() - s.Location().Return("test-location").AnyTimes() - s.ClusterName().Return("my-cluster") - s.ProviderID().Return("") - m.Get(gomockinternal.AContext(), "my-rg", "my-vm"). - Return(compute.VirtualMachine{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - s.GetVMImage().AnyTimes().Return(&infrav1.Image{ - Marketplace: &infrav1.AzureMarketplaceImage{ - Publisher: "fake-publisher", - Offer: "my-offer", - SKU: "sku-id", - Version: "1.0", - }, - }, nil) - s.GetBootstrapData(gomockinternal.AContext()).Return("fake-bootstrap-data", nil) - s.AvailabilitySet().Return("", false) - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-vm", gomock.AssignableToTypeOf(compute.VirtualMachine{})).Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) - }, - ExpectedError: "failed to create VM my-vm in resource group my-rg: #: Internal Server Error: StatusCode=500", - SetupSKUs: func(svc *Service) { - skus := []compute.ResourceSku{ - { - Name: to.StringPtr("Standard_D2v3"), - Kind: to.StringPtr(string(resourceskus.VirtualMachines)), - Locations: &[]string{ - "test-location", - }, - LocationInfo: &[]compute.ResourceSkuLocationInfo{ - { - Location: to.StringPtr("test-location"), - Zones: &[]string{"1"}, - }, - }, - Capabilities: &[]compute.ResourceSkuCapabilities{ - { - Name: to.StringPtr(resourceskus.VCPUs), - Value: to.StringPtr("2"), - }, - { - Name: to.StringPtr(resourceskus.MemoryGB), - Value: to.StringPtr("4"), - }, - }, - }, - } - resourceSkusCache := resourceskus.NewStaticCache(skus, "") - svc.resourceSKUCache = resourceSkusCache - }, - }, - { - Name: "cannot create vm if vCPU is less than 2", - Expect: func(g *WithT, s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) { - s.VMSpec().Return(azure.VMSpec{ - Name: "my-vm", - Role: infrav1.ControlPlane, - NICNames: []string{"my-nic", "second-nic"}, - SSHKeyData: "ZmFrZXNzaGtleQo=", - Size: "Standard_D1v3", - Zone: "1", - Identity: infrav1.VMIdentityNone, - OSDisk: infrav1.OSDisk{ - OSType: "Linux", - DiskSizeGB: to.Int32Ptr(128), - ManagedDisk: &infrav1.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - }, - }, - DataDisks: []infrav1.DataDisk{ - { - NameSuffix: "mydisk", - DiskSizeGB: 64, - Lun: to.Int32Ptr(0), - }, - }, - UserAssignedIdentities: nil, - SpotVMOptions: nil, - }) - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("my-rg") - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.ProviderID().Return("") - m.Get(gomockinternal.AContext(), "my-rg", "my-vm"). - Return(compute.VirtualMachine{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - }, - ExpectedError: "reconcile error that cannot be recovered occurred: vm size should be bigger or equal to at least 2 vCPUs. Object will not be requeued", - SetupSKUs: func(svc *Service) { - skus := []compute.ResourceSku{ - { - Name: to.StringPtr("Standard_D1v3"), - Kind: to.StringPtr(string(resourceskus.VirtualMachines)), - Locations: &[]string{ - "test-location", - }, - LocationInfo: &[]compute.ResourceSkuLocationInfo{ - { - Location: to.StringPtr("test-location"), - Zones: &[]string{"1"}, - }, - }, - Capabilities: &[]compute.ResourceSkuCapabilities{ - { - Name: to.StringPtr(resourceskus.VCPUs), - Value: to.StringPtr("1"), - }, - { - Name: to.StringPtr(resourceskus.MemoryGB), - Value: to.StringPtr("4"), - }, - }, - }, - } - resourceSkusCache := resourceskus.NewStaticCache(skus, "") - svc.resourceSKUCache = resourceSkusCache - }, - }, - { - Name: "cannot create vm if memory is less than 2Gi", - Expect: func(g *WithT, s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) { - s.VMSpec().Return(azure.VMSpec{ - Name: "my-vm", - Role: infrav1.ControlPlane, - NICNames: []string{"my-nic", "second-nic"}, - SSHKeyData: "ZmFrZXNzaGtleQo=", - Size: "Standard_D2v3", - Zone: "1", - Identity: infrav1.VMIdentityNone, - OSDisk: infrav1.OSDisk{ - OSType: "Linux", - DiskSizeGB: to.Int32Ptr(128), - ManagedDisk: &infrav1.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - }, - }, - DataDisks: []infrav1.DataDisk{ - { - NameSuffix: "mydisk", - DiskSizeGB: 64, - Lun: to.Int32Ptr(0), - }, - }, - UserAssignedIdentities: nil, - SpotVMOptions: nil, - }) - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("my-rg") - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.ProviderID().Return("") - m.Get(gomockinternal.AContext(), "my-rg", "my-vm"). - Return(compute.VirtualMachine{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - }, - ExpectedError: "reconcile error that cannot be recovered occurred: vm memory should be bigger or equal to at least 2Gi. Object will not be requeued", - SetupSKUs: func(svc *Service) { - skus := []compute.ResourceSku{ - { - Name: to.StringPtr("Standard_D2v3"), - Kind: to.StringPtr(string(resourceskus.VirtualMachines)), - Locations: &[]string{ - "test-location", - }, - LocationInfo: &[]compute.ResourceSkuLocationInfo{ - { - Location: to.StringPtr("test-location"), - Zones: &[]string{"1"}, - }, - }, - Capabilities: &[]compute.ResourceSkuCapabilities{ - { - Name: to.StringPtr(resourceskus.VCPUs), - Value: to.StringPtr("2"), - }, - { - Name: to.StringPtr(resourceskus.MemoryGB), - Value: to.StringPtr("1"), - }, - }, - }, - } - resourceSkusCache := resourceskus.NewStaticCache(skus, "") - svc.resourceSKUCache = resourceSkusCache - }, - }, - { - Name: "cannot create vm if does not support ephemeral os", - Expect: func(g *WithT, s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) { - s.VMSpec().Return(azure.VMSpec{ - Name: "my-vm", - Role: infrav1.ControlPlane, - NICNames: []string{"my-nic", "second-nic"}, - SSHKeyData: "ZmFrZXNzaGtleQo=", - Size: "Standard_D2v3", - Zone: "1", - Identity: infrav1.VMIdentityNone, - OSDisk: infrav1.OSDisk{ - OSType: "Linux", - DiskSizeGB: to.Int32Ptr(128), - ManagedDisk: &infrav1.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - }, - DiffDiskSettings: &infrav1.DiffDiskSettings{ - Option: string(compute.DiffDiskOptionsLocal), - }, - }, - DataDisks: []infrav1.DataDisk{ - { - NameSuffix: "mydisk", - DiskSizeGB: 64, - Lun: to.Int32Ptr(0), - }, - }, - UserAssignedIdentities: nil, - SpotVMOptions: nil, - }) - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("my-rg") - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.ProviderID().Return("") - m.Get(gomockinternal.AContext(), "my-rg", "my-vm"). - Return(compute.VirtualMachine{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - }, - ExpectedError: "reconcile error that cannot be recovered occurred: vm size Standard_D2v3 does not support ephemeral os. select a different vm size or disable ephemeral os. Object will not be requeued", - SetupSKUs: func(svc *Service) { - skus := []compute.ResourceSku{ - { - Name: to.StringPtr("Standard_D2v3"), - Kind: to.StringPtr(string(resourceskus.VirtualMachines)), - Locations: &[]string{ - "test-location", - }, - LocationInfo: &[]compute.ResourceSkuLocationInfo{ - { - Location: to.StringPtr("test-location"), - Zones: &[]string{"1"}, - }, - }, - Capabilities: &[]compute.ResourceSkuCapabilities{ - { - Name: to.StringPtr(resourceskus.EphemeralOSDisk), - Value: to.StringPtr("False"), - }, - { - Name: to.StringPtr(resourceskus.VCPUs), - Value: to.StringPtr("2"), - }, - { - Name: to.StringPtr(resourceskus.MemoryGB), - Value: to.StringPtr("4"), - }, - }, - }, - } - resourceSkusCache := resourceskus.NewStaticCache(skus, "") - svc.resourceSKUCache = resourceSkusCache - }, - }, - { - Name: "can create a vm with EphemeralOSDisk", - Expect: func(g *WithT, s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) { - s.VMSpec().Return(azure.VMSpec{ - Name: "my-vm", - Role: infrav1.ControlPlane, - NICNames: []string{"my-nic", "second-nic"}, - SSHKeyData: "ZmFrZXNzaGtleQo=", - Size: "Standard_D2v3", - Zone: "1", - Identity: infrav1.VMIdentityNone, - OSDisk: infrav1.OSDisk{ - OSType: "Linux", - DiskSizeGB: to.Int32Ptr(128), - ManagedDisk: &infrav1.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - }, - DiffDiskSettings: &infrav1.DiffDiskSettings{ - Option: string(compute.DiffDiskOptionsLocal), - }, - }, - DataDisks: []infrav1.DataDisk{ - { - NameSuffix: "mydisk", - DiskSizeGB: 64, - Lun: to.Int32Ptr(0), - }, - }, - UserAssignedIdentities: nil, - SpotVMOptions: nil, - }) - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("my-rg") - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.AdditionalTags() - s.Location().Return("test-location").AnyTimes() - s.ClusterName().Return("my-cluster") - s.ProviderID().Return("") - m.Get(gomockinternal.AContext(), "my-rg", "my-vm"). - Return(compute.VirtualMachine{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - s.GetVMImage().AnyTimes().Return(&infrav1.Image{ - Marketplace: &infrav1.AzureMarketplaceImage{ - Publisher: "fake-publisher", - Offer: "my-offer", - SKU: "sku-id", - Version: "1.0", - }, - }, nil) - s.GetBootstrapData(gomockinternal.AContext()).Return("fake-bootstrap-data", nil) - s.AvailabilitySet().Return("", false) - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-vm", gomockinternal.DiffEq(compute.VirtualMachine{ - VirtualMachineProperties: &compute.VirtualMachineProperties{ - HardwareProfile: &compute.HardwareProfile{VMSize: "Standard_D2v3"}, - StorageProfile: &compute.StorageProfile{ - ImageReference: &compute.ImageReference{ - Publisher: to.StringPtr("fake-publisher"), - Offer: to.StringPtr("my-offer"), - Sku: to.StringPtr("sku-id"), - Version: to.StringPtr("1.0"), - }, - OsDisk: &compute.OSDisk{ - OsType: "Linux", - Name: to.StringPtr("my-vm_OSDisk"), - CreateOption: "FromImage", - DiskSizeGB: to.Int32Ptr(128), - ManagedDisk: &compute.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - }, - DiffDiskSettings: &compute.DiffDiskSettings{ - Option: compute.DiffDiskOptionsLocal, - }, - }, - DataDisks: &[]compute.DataDisk{ - { - Lun: to.Int32Ptr(0), - Name: to.StringPtr("my-vm_mydisk"), - CreateOption: "Empty", - DiskSizeGB: to.Int32Ptr(64), - }, - }, - }, - DiagnosticsProfile: &compute.DiagnosticsProfile{ - BootDiagnostics: &compute.BootDiagnostics{ - Enabled: to.BoolPtr(true), - }, - }, - OsProfile: &compute.OSProfile{ - ComputerName: to.StringPtr("my-vm"), - AdminUsername: to.StringPtr("capi"), - CustomData: to.StringPtr("fake-bootstrap-data"), - LinuxConfiguration: &compute.LinuxConfiguration{ - DisablePasswordAuthentication: to.BoolPtr(true), - SSH: &compute.SSHConfiguration{ - PublicKeys: &[]compute.SSHPublicKey{ - { - Path: to.StringPtr("/home/capi/.ssh/authorized_keys"), - KeyData: to.StringPtr("fakesshkey\n"), - }, - }, - }, - }, - }, - NetworkProfile: &compute.NetworkProfile{ - NetworkInterfaces: &[]compute.NetworkInterfaceReference{ - { - NetworkInterfaceReferenceProperties: &compute.NetworkInterfaceReferenceProperties{Primary: to.BoolPtr(true)}, - ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/networkInterfaces/my-nic"), - }, - { - NetworkInterfaceReferenceProperties: &compute.NetworkInterfaceReferenceProperties{Primary: to.BoolPtr(false)}, - ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/networkInterfaces/second-nic"), - }, - }, - }, - }, - Resources: nil, - Identity: nil, - Zones: &[]string{"1"}, - ID: nil, - Name: nil, - Type: nil, - Location: to.StringPtr("test-location"), - Tags: map[string]*string{ - "Name": to.StringPtr("my-vm"), - "sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": to.StringPtr("owned"), - "sigs.k8s.io_cluster-api-provider-azure_role": to.StringPtr("control-plane"), - }, - })) - }, - ExpectedError: "", - SetupSKUs: func(svc *Service) { - skus := []compute.ResourceSku{ - { - Name: to.StringPtr("Standard_D2v3"), - Kind: to.StringPtr(string(resourceskus.VirtualMachines)), - Locations: &[]string{ - "test-location", - }, - LocationInfo: &[]compute.ResourceSkuLocationInfo{ - { - Location: to.StringPtr("test-location"), - Zones: &[]string{"1"}, - }, - }, - Capabilities: &[]compute.ResourceSkuCapabilities{ - { - Name: to.StringPtr(resourceskus.EphemeralOSDisk), - Value: to.StringPtr("True"), - }, - { - Name: to.StringPtr(resourceskus.VCPUs), - Value: to.StringPtr("2"), - }, - { - Name: to.StringPtr(resourceskus.MemoryGB), - Value: to.StringPtr("4"), - }, - }, - }, - } - resourceSkusCache := resourceskus.NewStaticCache(skus, "") - svc.resourceSKUCache = resourceSkusCache - }, - }, - { - Name: "can create a vm with a marketplace image using a plan", - Expect: func(g *WithT, s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) { - s.VMSpec().Return(azure.VMSpec{ - Name: "my-vm", - Role: infrav1.ControlPlane, - NICNames: []string{"my-nic", "second-nic"}, - SSHKeyData: "ZmFrZXNzaGtleQo=", - Size: "Standard_D2v3", - Zone: "1", - Identity: infrav1.VMIdentityNone, - OSDisk: infrav1.OSDisk{ - OSType: "Linux", - DiskSizeGB: to.Int32Ptr(128), - ManagedDisk: &infrav1.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - }, - }, - DataDisks: []infrav1.DataDisk{ - { - NameSuffix: "mydisk", - DiskSizeGB: 64, - Lun: to.Int32Ptr(0), - }, - }, - UserAssignedIdentities: nil, - SpotVMOptions: nil, - }) - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("my-rg") - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.AdditionalTags() - s.Location().Return("test-location").AnyTimes() - s.ClusterName().Return("my-cluster") - s.ProviderID().Return("") - m.Get(gomockinternal.AContext(), "my-rg", "my-vm"). - Return(compute.VirtualMachine{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - s.GetVMImage().AnyTimes().Return(&infrav1.Image{ - Marketplace: &infrav1.AzureMarketplaceImage{ - Publisher: "fake-publisher", - Offer: "my-offer", - SKU: "sku-id", - Version: "1.0", - ThirdPartyImage: true, - }, - }, nil) - s.GetBootstrapData(gomockinternal.AContext()).Return("fake-bootstrap-data", nil) - s.AvailabilitySet().Return("", false) - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-vm", gomockinternal.DiffEq(compute.VirtualMachine{ - Plan: &compute.Plan{ - Name: to.StringPtr("sku-id"), - Publisher: to.StringPtr("fake-publisher"), - Product: to.StringPtr("my-offer"), - }, - VirtualMachineProperties: &compute.VirtualMachineProperties{ - HardwareProfile: &compute.HardwareProfile{VMSize: "Standard_D2v3"}, - StorageProfile: &compute.StorageProfile{ - ImageReference: &compute.ImageReference{ - Publisher: to.StringPtr("fake-publisher"), - Offer: to.StringPtr("my-offer"), - Sku: to.StringPtr("sku-id"), - Version: to.StringPtr("1.0"), - }, - OsDisk: &compute.OSDisk{ - OsType: "Linux", - Name: to.StringPtr("my-vm_OSDisk"), - CreateOption: "FromImage", - DiskSizeGB: to.Int32Ptr(128), - ManagedDisk: &compute.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - }, - }, - DataDisks: &[]compute.DataDisk{ - { - Lun: to.Int32Ptr(0), - Name: to.StringPtr("my-vm_mydisk"), - CreateOption: "Empty", - DiskSizeGB: to.Int32Ptr(64), - }, - }, - }, - OsProfile: &compute.OSProfile{ - ComputerName: to.StringPtr("my-vm"), - AdminUsername: to.StringPtr("capi"), - CustomData: to.StringPtr("fake-bootstrap-data"), - LinuxConfiguration: &compute.LinuxConfiguration{ - DisablePasswordAuthentication: to.BoolPtr(true), - SSH: &compute.SSHConfiguration{ - PublicKeys: &[]compute.SSHPublicKey{ - { - Path: to.StringPtr("/home/capi/.ssh/authorized_keys"), - KeyData: to.StringPtr("fakesshkey\n"), - }, - }, - }, - }, - }, - DiagnosticsProfile: &compute.DiagnosticsProfile{ - BootDiagnostics: &compute.BootDiagnostics{ - Enabled: to.BoolPtr(true), - }, - }, - NetworkProfile: &compute.NetworkProfile{ - NetworkInterfaces: &[]compute.NetworkInterfaceReference{ - { - NetworkInterfaceReferenceProperties: &compute.NetworkInterfaceReferenceProperties{Primary: to.BoolPtr(true)}, - ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/networkInterfaces/my-nic"), - }, - { - NetworkInterfaceReferenceProperties: &compute.NetworkInterfaceReferenceProperties{Primary: to.BoolPtr(false)}, - ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/networkInterfaces/second-nic"), - }, - }, - }, - }, - Resources: nil, - Identity: nil, - Zones: &[]string{"1"}, - ID: nil, - Name: nil, - Type: nil, - Location: to.StringPtr("test-location"), - Tags: map[string]*string{ - "Name": to.StringPtr("my-vm"), - "sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": to.StringPtr("owned"), - "sigs.k8s.io_cluster-api-provider-azure_role": to.StringPtr("control-plane"), - }, - })) - }, - ExpectedError: "", - SetupSKUs: func(svc *Service) { - skus := []compute.ResourceSku{ - { - Name: to.StringPtr("Standard_D2v3"), - Kind: to.StringPtr(string(resourceskus.VirtualMachines)), - Locations: &[]string{ - "test-location", - }, - LocationInfo: &[]compute.ResourceSkuLocationInfo{ - { - Location: to.StringPtr("test-location"), - Zones: &[]string{"1"}, - }, - }, - Capabilities: &[]compute.ResourceSkuCapabilities{ - { - Name: to.StringPtr(resourceskus.VCPUs), - Value: to.StringPtr("2"), - }, - { - Name: to.StringPtr(resourceskus.MemoryGB), - Value: to.StringPtr("4"), - }, - }, - }, - } - resourceSkusCache := resourceskus.NewStaticCache(skus, "") - svc.resourceSKUCache = resourceSkusCache - }, - }, - { - Name: "fails when there is a provider id present, but cannot find vm ", - Expect: func(g *WithT, s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) { - s.VMSpec().Return(azure.VMSpec{ - Name: "my-vm", - }) - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("my-rg") - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.ProviderID().Times(2).Return("ExistingVM-ProviderID") + s.VMSpec().AnyTimes().Return(&fakeVMSpec) + s.GetLongRunningOperationState("test-vm", serviceName).Return(nil) + m.DeleteAsync(gomockinternal.AContext(), &fakeVMSpec).Return(nil, nil) s.SetVMState(infrav1.Deleted) - m.Get(gomockinternal.AContext(), "my-rg", "my-vm"). - Return(compute.VirtualMachine{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - }, - ExpectedError: "VM with provider id \"ExistingVM-ProviderID\" has been deleted", - SetupSKUs: func(svc *Service) {}, - }, - { - Name: "can create a vm with a SIG image using a plan", - Expect: func(g *WithT, s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) { - s.VMSpec().Return(azure.VMSpec{ - Name: "my-vm", - Role: infrav1.ControlPlane, - NICNames: []string{"my-nic", "second-nic"}, - SSHKeyData: "ZmFrZXNzaGtleQo=", - Size: "Standard_D2v3", - Zone: "1", - Identity: infrav1.VMIdentityNone, - OSDisk: infrav1.OSDisk{ - OSType: "Linux", - DiskSizeGB: to.Int32Ptr(128), - ManagedDisk: &infrav1.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - }, - }, - DataDisks: []infrav1.DataDisk{ - { - NameSuffix: "mydisk", - DiskSizeGB: 64, - Lun: to.Int32Ptr(0), - }, - }, - UserAssignedIdentities: nil, - SpotVMOptions: nil, - }) - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("my-rg") - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.AdditionalTags() - s.Location().Return("test-location").AnyTimes() - s.ClusterName().Return("my-cluster") - s.ProviderID().Return("") - m.Get(gomockinternal.AContext(), "my-rg", "my-vm"). - Return(compute.VirtualMachine{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - s.GetVMImage().AnyTimes().Return(&infrav1.Image{ - SharedGallery: &infrav1.AzureSharedGalleryImage{ - SubscriptionID: "fake-sub-id", - ResourceGroup: "fake-rg", - Gallery: "fake-gallery", - Name: "fake-name", - Version: "1.0", - Publisher: to.StringPtr("fake-publisher"), - Offer: to.StringPtr("my-offer"), - SKU: to.StringPtr("sku-id"), - }, - }, nil) - s.GetBootstrapData(gomockinternal.AContext()).Return("fake-bootstrap-data", nil) - s.AvailabilitySet().Return("", false) - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-vm", gomockinternal.DiffEq(compute.VirtualMachine{ - Plan: &compute.Plan{ - Name: to.StringPtr("sku-id"), - Publisher: to.StringPtr("fake-publisher"), - Product: to.StringPtr("my-offer"), - }, - VirtualMachineProperties: &compute.VirtualMachineProperties{ - HardwareProfile: &compute.HardwareProfile{VMSize: "Standard_D2v3"}, - StorageProfile: &compute.StorageProfile{ - ImageReference: &compute.ImageReference{ - ID: to.StringPtr("/subscriptions/fake-sub-id/resourceGroups/fake-rg/providers/Microsoft.Compute/galleries/fake-gallery/images/fake-name/versions/1.0"), - }, - OsDisk: &compute.OSDisk{ - OsType: "Linux", - Name: to.StringPtr("my-vm_OSDisk"), - CreateOption: "FromImage", - DiskSizeGB: to.Int32Ptr(128), - ManagedDisk: &compute.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - }, - }, - DataDisks: &[]compute.DataDisk{ - { - Lun: to.Int32Ptr(0), - Name: to.StringPtr("my-vm_mydisk"), - CreateOption: "Empty", - DiskSizeGB: to.Int32Ptr(64), - }, - }, - }, - OsProfile: &compute.OSProfile{ - ComputerName: to.StringPtr("my-vm"), - AdminUsername: to.StringPtr("capi"), - CustomData: to.StringPtr("fake-bootstrap-data"), - LinuxConfiguration: &compute.LinuxConfiguration{ - DisablePasswordAuthentication: to.BoolPtr(true), - SSH: &compute.SSHConfiguration{ - PublicKeys: &[]compute.SSHPublicKey{ - { - Path: to.StringPtr("/home/capi/.ssh/authorized_keys"), - KeyData: to.StringPtr("fakesshkey\n"), - }, - }, - }, - }, - }, - DiagnosticsProfile: &compute.DiagnosticsProfile{ - BootDiagnostics: &compute.BootDiagnostics{ - Enabled: to.BoolPtr(true), - }, - }, - NetworkProfile: &compute.NetworkProfile{ - NetworkInterfaces: &[]compute.NetworkInterfaceReference{ - { - NetworkInterfaceReferenceProperties: &compute.NetworkInterfaceReferenceProperties{Primary: to.BoolPtr(true)}, - ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/networkInterfaces/my-nic"), - }, - { - NetworkInterfaceReferenceProperties: &compute.NetworkInterfaceReferenceProperties{Primary: to.BoolPtr(false)}, - ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/networkInterfaces/second-nic"), - }, - }, - }, - }, - Resources: nil, - Identity: nil, - Zones: &[]string{"1"}, - ID: nil, - Name: nil, - Type: nil, - Location: to.StringPtr("test-location"), - Tags: map[string]*string{ - "Name": to.StringPtr("my-vm"), - "sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": to.StringPtr("owned"), - "sigs.k8s.io_cluster-api-provider-azure_role": to.StringPtr("control-plane"), - }, - })) - }, - ExpectedError: "", - SetupSKUs: func(svc *Service) { - skus := []compute.ResourceSku{ - { - Name: to.StringPtr("Standard_D2v3"), - Kind: to.StringPtr(string(resourceskus.VirtualMachines)), - Locations: &[]string{ - "test-location", - }, - LocationInfo: &[]compute.ResourceSkuLocationInfo{ - { - Location: to.StringPtr("test-location"), - Zones: &[]string{"1"}, - }, - }, - Capabilities: &[]compute.ResourceSkuCapabilities{ - { - Name: to.StringPtr(resourceskus.VCPUs), - Value: to.StringPtr("2"), - }, - { - Name: to.StringPtr(resourceskus.MemoryGB), - Value: to.StringPtr("4"), - }, - }, - }, - } - resourceSkusCache := resourceskus.NewStaticCache(skus, "") - svc.resourceSKUCache = resourceSkusCache - }, - }, - { - Name: "can create a vm with ultra disk enabled", - Expect: func(g *WithT, s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, - mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) { - s.VMSpec().Return(azure.VMSpec{ - Name: "my-ultra-ssd-vm", - Role: infrav1.ControlPlane, - NICNames: []string{"my-nic", "second-nic"}, - SSHKeyData: "ZmFrZXNzaGtleQo=", - Size: "Standard_D2v3", - Zone: "1", - Identity: infrav1.VMIdentityNone, - OSDisk: infrav1.OSDisk{ - OSType: "Linux", - DiskSizeGB: to.Int32Ptr(128), - ManagedDisk: &infrav1.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - }, - }, - DataDisks: []infrav1.DataDisk{ - { - NameSuffix: "mydisk", - DiskSizeGB: 64, - Lun: to.Int32Ptr(0), - }, - { - NameSuffix: "myDiskWithUltraDisk", - DiskSizeGB: 128, - Lun: to.Int32Ptr(1), - ManagedDisk: &infrav1.ManagedDiskParameters{ - StorageAccountType: "UltraSSD_LRS", - }, - }, - { - NameSuffix: "myDiskWithManagedDisk", - DiskSizeGB: 128, - Lun: to.Int32Ptr(2), - ManagedDisk: &infrav1.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - }, - }, - { - NameSuffix: "managedDiskWithEncryption", - DiskSizeGB: 128, - Lun: to.Int32Ptr(3), - ManagedDisk: &infrav1.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - DiskEncryptionSet: &infrav1.DiskEncryptionSetParameters{ - ID: "my_id", - }, - }, - }, - }, - UserAssignedIdentities: nil, - SpotVMOptions: nil, - }) - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("my-rg") - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.AdditionalTags() - s.Location().Return("test-location").AnyTimes() - s.ClusterName().Return("my-cluster") - s.ProviderID().Return("") - m.Get(gomockinternal.AContext(), "my-rg", "my-ultra-ssd-vm"). - Return(compute.VirtualMachine{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - s.GetVMImage().AnyTimes().Return(&infrav1.Image{ - Marketplace: &infrav1.AzureMarketplaceImage{ - Publisher: "fake-publisher", - Offer: "my-offer", - SKU: "sku-id", - Version: "1.0", - }, - }, nil) - s.GetBootstrapData(gomockinternal.AContext()).Return("fake-bootstrap-data", nil) - s.AvailabilitySet().Return("", false) - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-ultra-ssd-vm", gomockinternal.DiffEq(compute.VirtualMachine{ - VirtualMachineProperties: &compute.VirtualMachineProperties{ - HardwareProfile: &compute.HardwareProfile{VMSize: "Standard_D2v3"}, - StorageProfile: &compute.StorageProfile{ - ImageReference: &compute.ImageReference{ - Publisher: to.StringPtr("fake-publisher"), - Offer: to.StringPtr("my-offer"), - Sku: to.StringPtr("sku-id"), - Version: to.StringPtr("1.0"), - }, - OsDisk: &compute.OSDisk{ - OsType: "Linux", - Name: to.StringPtr("my-ultra-ssd-vm_OSDisk"), - CreateOption: "FromImage", - DiskSizeGB: to.Int32Ptr(128), - ManagedDisk: &compute.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - }, - }, - DataDisks: &[]compute.DataDisk{ - { - Lun: to.Int32Ptr(0), - Name: to.StringPtr("my-ultra-ssd-vm_mydisk"), - CreateOption: "Empty", - DiskSizeGB: to.Int32Ptr(64), - }, - { - Lun: to.Int32Ptr(1), - Name: to.StringPtr("my-ultra-ssd-vm_myDiskWithUltraDisk"), - CreateOption: "Empty", - DiskSizeGB: to.Int32Ptr(128), - ManagedDisk: &compute.ManagedDiskParameters{ - StorageAccountType: "UltraSSD_LRS", - }, - }, - { - Lun: to.Int32Ptr(2), - Name: to.StringPtr("my-ultra-ssd-vm_myDiskWithManagedDisk"), - CreateOption: "Empty", - DiskSizeGB: to.Int32Ptr(128), - ManagedDisk: &compute.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - }, - }, - { - Lun: to.Int32Ptr(3), - Name: to.StringPtr("my-ultra-ssd-vm_managedDiskWithEncryption"), - CreateOption: "Empty", - DiskSizeGB: to.Int32Ptr(128), - ManagedDisk: &compute.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - DiskEncryptionSet: &compute.DiskEncryptionSetParameters{ - ID: to.StringPtr("my_id"), - }, - }, - }, - }, - }, - OsProfile: &compute.OSProfile{ - ComputerName: to.StringPtr("my-ultra-ssd-vm"), - AdminUsername: to.StringPtr("capi"), - CustomData: to.StringPtr("fake-bootstrap-data"), - LinuxConfiguration: &compute.LinuxConfiguration{ - DisablePasswordAuthentication: to.BoolPtr(true), - SSH: &compute.SSHConfiguration{ - PublicKeys: &[]compute.SSHPublicKey{ - { - Path: to.StringPtr("/home/capi/.ssh/authorized_keys"), - KeyData: to.StringPtr("fakesshkey\n"), - }, - }, - }, - }, - }, - DiagnosticsProfile: &compute.DiagnosticsProfile{ - BootDiagnostics: &compute.BootDiagnostics{ - Enabled: to.BoolPtr(true), - }, - }, - NetworkProfile: &compute.NetworkProfile{ - NetworkInterfaces: &[]compute.NetworkInterfaceReference{ - { - NetworkInterfaceReferenceProperties: &compute.NetworkInterfaceReferenceProperties{Primary: to.BoolPtr(true)}, - ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/networkInterfaces/my-nic"), - }, - { - NetworkInterfaceReferenceProperties: &compute.NetworkInterfaceReferenceProperties{Primary: to.BoolPtr(false)}, - ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/networkInterfaces/second-nic"), - }, - }, - }, - AdditionalCapabilities: &compute.AdditionalCapabilities{UltraSSDEnabled: pointer.Bool(true)}, - }, - Resources: nil, - Identity: nil, - Zones: &[]string{"1"}, - ID: nil, - Name: nil, - Type: nil, - Location: to.StringPtr("test-location"), - Tags: map[string]*string{ - "Name": to.StringPtr("my-ultra-ssd-vm"), - "sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": to.StringPtr("owned"), - "sigs.k8s.io_cluster-api-provider-azure_role": to.StringPtr("control-plane"), - }, - })) - }, - ExpectedError: "", - SetupSKUs: func(svc *Service) { - skus := []compute.ResourceSku{ - { - Name: to.StringPtr("Standard_D2v3"), - Kind: to.StringPtr(string(resourceskus.VirtualMachines)), - Locations: &[]string{ - "test-location", - }, - LocationInfo: &[]compute.ResourceSkuLocationInfo{ - { - Location: to.StringPtr("test-location"), - Zones: &[]string{"1"}, - ZoneDetails: &[]compute.ResourceSkuZoneDetails{ - { - Capabilities: &[]compute.ResourceSkuCapabilities{ - { - Name: pointer.String("UltraSSDAvailable"), - Value: pointer.String("True"), - }, - }, - Name: &[]string{"1"}, - }, - }, - }, - }, - Capabilities: &[]compute.ResourceSkuCapabilities{ - { - Name: to.StringPtr(resourceskus.VCPUs), - Value: to.StringPtr("2"), - }, - { - Name: to.StringPtr(resourceskus.MemoryGB), - Value: to.StringPtr("4"), - }, - }, - }, - } - resourceSkusCache := resourceskus.NewStaticCache(skus, "test-location") - svc.resourceSKUCache = resourceSkusCache - }, - }, - { - Name: "fail to create a vm with ultra disk enabled", - Expect: func(g *WithT, s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder, - mnic *mock_networkinterfaces.MockClientMockRecorder, mpip *mock_publicips.MockClientMockRecorder) { - s.VMSpec().Return(azure.VMSpec{ - Name: "my-ultra-ssd-vm", - Role: infrav1.ControlPlane, - NICNames: []string{"my-nic", "second-nic"}, - SSHKeyData: "ZmFrZXNzaGtleQo=", - Size: "Standard_D2v3", - Zone: "1", - Identity: infrav1.VMIdentityNone, - OSDisk: infrav1.OSDisk{ - OSType: "Linux", - DiskSizeGB: to.Int32Ptr(128), - ManagedDisk: &infrav1.ManagedDiskParameters{ - StorageAccountType: "Premium_LRS", - }, - }, - DataDisks: []infrav1.DataDisk{ - { - NameSuffix: "myDiskWithUltraDisk", - DiskSizeGB: 128, - Lun: to.Int32Ptr(1), - ManagedDisk: &infrav1.ManagedDiskParameters{ - StorageAccountType: "UltraSSD_LRS", - }, - }, - }, - UserAssignedIdentities: nil, - SpotVMOptions: nil, - }) - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("my-rg") - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.Location().Return("test-location").AnyTimes() - s.ProviderID().Return("") - m.Get(gomockinternal.AContext(), "my-rg", "my-ultra-ssd-vm"). - Return(compute.VirtualMachine{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - }, - ExpectedError: "reconcile error that cannot be recovered occurred: vm size Standard_D2v3 does not support ultra disks in location test-location. select a different vm size or disable ultra disks. Object will not be requeued", - SetupSKUs: func(svc *Service) { - skus := []compute.ResourceSku{ - { - Name: to.StringPtr("Standard_D2v3"), - Kind: to.StringPtr(string(resourceskus.VirtualMachines)), - Locations: &[]string{ - "test-location", - }, - LocationInfo: &[]compute.ResourceSkuLocationInfo{ - { - Location: to.StringPtr("test-location"), - Zones: &[]string{"1"}, - }, - }, - Capabilities: &[]compute.ResourceSkuCapabilities{ - { - Name: to.StringPtr(resourceskus.VCPUs), - Value: to.StringPtr("2"), - }, - { - Name: to.StringPtr(resourceskus.MemoryGB), - Value: to.StringPtr("4"), - }, - }, - }, - } - resourceSkusCache := resourceskus.NewStaticCache(skus, "") - svc.resourceSKUCache = resourceSkusCache - }, - }, - } - - for _, tc := range testcases { - tc := tc - t.Run(tc.Name, func(t *testing.T) { - g := NewWithT(t) - t.Parallel() - mockCtrl := gomock.NewController(t) - defer mockCtrl.Finish() - - scopeMock := mock_virtualmachines.NewMockVMScope(mockCtrl) - clientMock := mock_virtualmachines.NewMockClient(mockCtrl) - interfaceMock := mock_networkinterfaces.NewMockClient(mockCtrl) - publicIPMock := mock_publicips.NewMockClient(mockCtrl) - availabilitySetsMock := mock_availabilitysets.NewMockClient(mockCtrl) - - tc.Expect(g, scopeMock.EXPECT(), clientMock.EXPECT(), interfaceMock.EXPECT(), publicIPMock.EXPECT()) - - s := &Service{ - Scope: scopeMock, - Client: clientMock, - interfacesClient: interfaceMock, - publicIPsClient: publicIPMock, - availabilitySetsClient: availabilitySetsMock, - resourceSKUCache: resourceskus.NewStaticCache(nil, ""), - } - - 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 TestDeleteVM(t *testing.T) { - testcases := []struct { - name string - expectedError string - expect func(s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder) - }{ - { - name: "successfully delete an existing vm", - expectedError: "", - expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder) { - s.VMSpec().Return(azure.VMSpec{ - Name: "my-existing-vm", - Role: infrav1.ControlPlane, - NICNames: []string{"my-nic"}, - SSHKeyData: "fakesshpublickey", - Size: "Standard_D2v3", - Zone: "", - Identity: "", - OSDisk: infrav1.OSDisk{}, - DataDisks: nil, - UserAssignedIdentities: nil, - SpotVMOptions: nil, - }) - s.ResourceGroup().AnyTimes().Return("my-existing-rg") - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - m.Delete(gomockinternal.AContext(), "my-existing-rg", "my-existing-vm") - }, - }, - { - name: "vm already deleted", - expectedError: "", - expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder) { - s.VMSpec().Return(azure.VMSpec{ - Name: "my-vm", - Role: infrav1.ControlPlane, - NICNames: []string{"my-nic"}, - SSHKeyData: "fakesshpublickey", - Size: "Standard_D2v3", - Zone: "", - Identity: "", - OSDisk: infrav1.OSDisk{}, - DataDisks: nil, - UserAssignedIdentities: nil, - SpotVMOptions: nil, - }) - s.ResourceGroup().AnyTimes().Return("my-rg") - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - m.Delete(gomockinternal.AContext(), "my-rg", "my-vm"). - Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - }, - }, - { - name: "vm deletion fails", - expectedError: "failed to delete VM my-vm in resource group my-rg: #: Internal Server Error: StatusCode=500", - expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder) { - s.VMSpec().Return(azure.VMSpec{ - Name: "my-vm", - Role: infrav1.ControlPlane, - NICNames: []string{"my-nic"}, - SSHKeyData: "fakesshpublickey", - Size: "Standard_D2v3", - Zone: "", - Identity: "", - OSDisk: infrav1.OSDisk{}, - DataDisks: nil, - UserAssignedIdentities: nil, - SpotVMOptions: nil, - }) - s.ResourceGroup().AnyTimes().Return("my-rg") - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - m.Delete(gomockinternal.AContext(), "my-rg", "my-vm"). - Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) + s.UpdateDeleteStatus(infrav1.VMRunningCondition, serviceName, nil) }, }, } diff --git a/azure/types.go b/azure/types.go index 4b9aca372d5..a4a0e62ce02 100644 --- a/azure/types.go +++ b/azure/types.go @@ -145,22 +145,6 @@ type NSGSpec struct { SecurityRules infrav1.SecurityRules } -// VMSpec defines the specification for a Virtual Machine. -type VMSpec struct { - Name string - Role string - NICNames []string - SSHKeyData string - Size string - Zone string - Identity infrav1.VMIdentity - OSDisk infrav1.OSDisk - DataDisks []infrav1.DataDisk - UserAssignedIdentities []infrav1.UserAssignedIdentity - SpotVMOptions *infrav1.SpotVMOptions - SecurityProfile *infrav1.SecurityProfile -} - // BastionSpec defines the specification for the generic bastion feature. type BastionSpec struct { AzureBastion *AzureBastionSpec diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml index 031d91fffea..8e1a84c0b1f 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml @@ -990,10 +990,16 @@ spec: subresources: status: {} - additionalPrinterColumns: - - description: AzureMachine ready status - jsonPath: .status.ready + - jsonPath: .status.conditions[?(@.type=='Ready')].status name: Ready type: string + - jsonPath: .status.conditions[?(@.type=='Ready')].reason + name: Reason + type: string + - jsonPath: .status.conditions[?(@.type=='Ready')].message + name: Message + priority: 1 + type: string - description: Azure VM provisioning state jsonPath: .status.vmState name: State diff --git a/controllers/azurecluster_controller.go b/controllers/azurecluster_controller.go index 115e88f2fec..a5b25345338 100644 --- a/controllers/azurecluster_controller.go +++ b/controllers/azurecluster_controller.go @@ -40,6 +40,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/source" 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/scope" "sigs.k8s.io/cluster-api-provider-azure/pkg/coalescing" "sigs.k8s.io/cluster-api-provider-azure/util/reconciler" @@ -223,6 +224,19 @@ func (acr *AzureClusterReconciler) reconcileNormal(ctx context.Context, clusterS } if err := acs.Reconcile(ctx); err != nil { + // Handle transient errors + var reconcileError azure.ReconcileError + if errors.As(err, &reconcileError) { + if reconcileError.IsTransient() { + if azure.IsOperationNotDoneError(reconcileError) { + clusterScope.V(2).Info(fmt.Sprintf("AzureCluster reconcile not done: %s", reconcileError.Error())) + } else { + clusterScope.V(2).Info("transient failure to reconcile AzureCluster, retrying") + } + return reconcile.Result{RequeueAfter: reconcileError.RequeueAfter()}, nil + } + } + wrappedErr := errors.Wrap(err, "failed to reconcile cluster services") acr.Recorder.Eventf(azureCluster, corev1.EventTypeWarning, "ClusterReconcilerNormalFailed", wrappedErr.Error()) return reconcile.Result{}, wrappedErr @@ -259,6 +273,19 @@ func (acr *AzureClusterReconciler) reconcileDelete(ctx context.Context, clusterS } if err := acs.Delete(ctx); err != nil { + // Handle transient errors + var reconcileError azure.ReconcileError + if errors.As(err, &reconcileError) { + if reconcileError.IsTransient() { + if azure.IsOperationNotDoneError(reconcileError) { + clusterScope.V(2).Info(fmt.Sprintf("AzureCluster delete not done: %s", reconcileError.Error())) + } else { + clusterScope.V(2).Info("transient failure to delete AzureCluster, retrying") + } + return reconcile.Result{RequeueAfter: reconcileError.RequeueAfter()}, nil + } + } + wrappedErr := errors.Wrapf(err, "error deleting AzureCluster %s/%s", azureCluster.Namespace, azureCluster.Name) acr.Recorder.Eventf(azureCluster, corev1.EventTypeWarning, "ClusterReconcilerDeleteFailed", wrappedErr.Error()) conditions.MarkFalse(azureCluster, infrav1.NetworkInfrastructureReadyCondition, clusterv1.DeletionFailedReason, clusterv1.ConditionSeverityWarning, err.Error()) diff --git a/controllers/azuremachine_controller.go b/controllers/azuremachine_controller.go index 7b208a1f50b..61159f81097 100644 --- a/controllers/azuremachine_controller.go +++ b/controllers/azuremachine_controller.go @@ -18,6 +18,7 @@ package controllers import ( "context" + "fmt" "time" "github.com/go-logr/logr" @@ -260,6 +261,7 @@ func (amr *AzureMachineReconciler) reconcileNormal(ctx context.Context, machineS return reconcile.Result{}, err } + // Make sure the Cluster Infrastructure is ready. if !clusterScope.Cluster.Status.InfrastructureReady { machineScope.Info("Cluster infrastructure is not ready yet") conditions.MarkFalse(machineScope.AzureMachine, infrav1.VMRunningCondition, infrav1.WaitingForClusterInfrastructureReason, clusterv1.ConditionSeverityInfo, "") @@ -273,6 +275,12 @@ func (amr *AzureMachineReconciler) reconcileNormal(ctx context.Context, machineS return reconcile.Result{}, nil } + // Initialize the cache to be used by the AzureMachine services. + err := machineScope.InitMachineCache(ctx) + if err != nil { + return reconcile.Result{}, errors.Wrap(err, "failed to init machine scope cache") + } + ams, err := amr.createAzureMachineService(machineScope) if err != nil { return reconcile.Result{}, errors.Wrap(err, "failed to create azure machine service") @@ -283,10 +291,10 @@ func (amr *AzureMachineReconciler) reconcileNormal(ctx context.Context, machineS // In this case, we mark it as failed and leave it to MHC for remediation if errors.As(err, &azure.VMDeletedError{}) { amr.Recorder.Eventf(machineScope.AzureMachine, corev1.EventTypeWarning, "VMDeleted", errors.Wrap(err, "failed to reconcile AzureMachine").Error()) - conditions.MarkFalse(machineScope.AzureMachine, infrav1.VMRunningCondition, infrav1.VMProvisionFailedReason, clusterv1.ConditionSeverityError, err.Error()) machineScope.SetFailureReason(capierrors.UpdateMachineError) machineScope.SetFailureMessage(err) machineScope.SetNotReady() + machineScope.SetVMState(infrav1.Deleted) return reconcile.Result{}, errors.Wrap(err, "failed to reconcile AzureMachine") } @@ -304,13 +312,15 @@ func (amr *AzureMachineReconciler) reconcileNormal(ctx context.Context, machineS } if reconcileError.IsTransient() { - machineScope.Error(err, "transient failure to reconcile AzureMachine, retrying", "name", machineScope.Name()) - machineScope.SetNotReady() + if azure.IsOperationNotDoneError(reconcileError) { + machineScope.V(2).Info(fmt.Sprintf("AzureMachine reconcile not done: %s", reconcileError.Error())) + } else { + machineScope.V(2).Info("transient failure to reconcile AzureMachine, retrying") + } return reconcile.Result{RequeueAfter: reconcileError.RequeueAfter()}, nil } } amr.Recorder.Eventf(machineScope.AzureMachine, corev1.EventTypeWarning, "ReconcileError", errors.Wrapf(err, "failed to reconcile AzureMachine").Error()) - conditions.MarkFalse(machineScope.AzureMachine, infrav1.VMRunningCondition, infrav1.VMProvisionFailedReason, clusterv1.ConditionSeverityError, err.Error()) return reconcile.Result{}, errors.Wrap(err, "failed to reconcile AzureMachine") } @@ -319,41 +329,47 @@ func (amr *AzureMachineReconciler) reconcileNormal(ctx context.Context, machineS return reconcile.Result{}, nil } -func (amr *AzureMachineReconciler) reconcileDelete(ctx context.Context, machineScope *scope.MachineScope, clusterScope *scope.ClusterScope) (_ reconcile.Result, reterr error) { +func (amr *AzureMachineReconciler) reconcileDelete(ctx context.Context, machineScope *scope.MachineScope, clusterScope *scope.ClusterScope) (reconcile.Result, error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "controllers.AzureMachineReconciler.reconcileDelete") defer done() machineScope.Info("Handling deleted AzureMachine") - conditions.MarkFalse(machineScope.AzureMachine, infrav1.VMRunningCondition, clusterv1.DeletingReason, clusterv1.ConditionSeverityInfo, "") if err := machineScope.PatchObject(ctx); err != nil { return reconcile.Result{}, err } - defer func() { - if reterr == nil { - machineScope.Info("Removing finalizer from AzureMachine") - controllerutil.RemoveFinalizer(machineScope.AzureMachine, infrav1.MachineFinalizer) - } - }() - if ShouldDeleteIndividualResources(ctx, clusterScope) { machineScope.Info("Deleting AzureMachine") ams, err := amr.createAzureMachineService(machineScope) if err != nil { - reterr = errors.Wrap(err, "failed to create azure machine service") - return + return reconcile.Result{}, errors.Wrap(err, "failed to create azure machine service") } if err := ams.Delete(ctx); err != nil { - amr.Recorder.Eventf(machineScope.AzureMachine, corev1.EventTypeWarning, "Error deleting AzureMachine", errors.Wrapf(err, "error deleting AzureMachine %s/%s", clusterScope.Namespace(), clusterScope.ClusterName()).Error()) - conditions.MarkFalse(machineScope.AzureMachine, infrav1.VMRunningCondition, clusterv1.DeletionFailedReason, clusterv1.ConditionSeverityWarning, err.Error()) - reterr = errors.Wrapf(err, "error deleting AzureMachine %s/%s", clusterScope.Namespace(), clusterScope.ClusterName()) - return + // Handle transient errors + var reconcileError azure.ReconcileError + if errors.As(err, &reconcileError) { + if reconcileError.IsTransient() { + if azure.IsOperationNotDoneError(reconcileError) { + machineScope.V(2).Info(fmt.Sprintf("AzureMachine delete not done: %s", reconcileError.Error())) + } else { + machineScope.V(2).Info("transient failure to delete AzureMachine, retrying") + } + return reconcile.Result{RequeueAfter: reconcileError.RequeueAfter()}, nil + } + } + + amr.Recorder.Eventf(machineScope.AzureMachine, corev1.EventTypeWarning, "Error deleting AzureMachine", errors.Wrapf(err, "error deleting AzureMachine %s/%s", machineScope.Namespace(), machineScope.Name()).Error()) + return reconcile.Result{}, errors.Wrapf(err, "error deleting AzureMachine %s/%s", machineScope.Namespace(), machineScope.Name()) } } else { machineScope.Info("Skipping AzureMachine Deletion; will delete whole resource group.") } - return reconcile.Result{}, reterr + // we're done deleting this AzureMachine so remove the finalizer. + machineScope.Info("Removing finalizer from AzureMachine") + controllerutil.RemoveFinalizer(machineScope.AzureMachine, infrav1.MachineFinalizer) + + return reconcile.Result{}, nil } diff --git a/controllers/azuremachine_controller_test.go b/controllers/azuremachine_controller_test.go index 370d8183d13..86fefa2b6bd 100644 --- a/controllers/azuremachine_controller_test.go +++ b/controllers/azuremachine_controller_test.go @@ -178,6 +178,7 @@ func TestConditions(t *testing.T) { ClusterScope: clusterScope, Machine: tc.machine, AzureMachine: tc.azureMachine, + Cache: &scope.MachineCache{}, }) g.Expect(err).NotTo(HaveOccurred()) diff --git a/controllers/azuremachine_reconciler.go b/controllers/azuremachine_reconciler.go index 3073421abc0..b8a356dcfe1 100644 --- a/controllers/azuremachine_reconciler.go +++ b/controllers/azuremachine_reconciler.go @@ -64,7 +64,7 @@ func newAzureMachineService(machineScope *scope.MachineScope) (*azureMachineServ scope: machineScope, inboundNatRulesSvc: inboundnatrules.New(machineScope), networkInterfacesSvc: networkinterfaces.New(machineScope, cache), - virtualMachinesSvc: virtualmachines.New(machineScope, cache), + virtualMachinesSvc: virtualmachines.New(machineScope), roleAssignmentsSvc: roleassignments.New(machineScope), disksSvc: disks.New(machineScope), publicIPsSvc: publicips.New(machineScope),