From 45fb80d54ee124928f4a713a6fdd866f25e4d913 Mon Sep 17 00:00:00 2001 From: Cecile Robert-Michon Date: Tue, 3 Nov 2020 15:29:52 -0800 Subject: [PATCH] :gem: cleanup: VM and VNet spec no longer return arrays --- cloud/scope/cluster.go | 14 +- cloud/scope/machine.go | 32 +- cloud/scope/managedcontrolplane.go | 14 +- .../virtualmachines_mock.go | 14 +- cloud/services/virtualmachines/service.go | 2 +- .../virtualmachines/virtualmachines.go | 234 ++++---- .../virtualmachines/virtualmachines_test.go | 538 ++++++++---------- .../virtualnetworks_mock.go | 14 +- cloud/services/virtualnetworks/service.go | 2 +- .../virtualnetworks/virtualnetworks.go | 102 ++-- .../virtualnetworks/virtualnetworks_test.go | 120 ++-- 11 files changed, 510 insertions(+), 576 deletions(-) diff --git a/cloud/scope/cluster.go b/cloud/scope/cluster.go index 687ba280432..93d77acbed5 100644 --- a/cloud/scope/cluster.go +++ b/cloud/scope/cluster.go @@ -229,14 +229,12 @@ func (s *ClusterScope) SubnetSpecs() []azure.SubnetSpec { } } -// VNetSpecs returns the virtual network specs. -func (s *ClusterScope) VNetSpecs() []azure.VNetSpec { - return []azure.VNetSpec{ - { - ResourceGroup: s.Vnet().ResourceGroup, - Name: s.Vnet().Name, - CIDRs: s.Vnet().CIDRBlocks, - }, +// VNetSpec returns the virtual network spec. +func (s *ClusterScope) VNetSpec() azure.VNetSpec { + return azure.VNetSpec{ + ResourceGroup: s.Vnet().ResourceGroup, + Name: s.Vnet().Name, + CIDRs: s.Vnet().CIDRBlocks, } } diff --git a/cloud/scope/machine.go b/cloud/scope/machine.go index 2f47164689a..6eda53027d1 100644 --- a/cloud/scope/machine.go +++ b/cloud/scope/machine.go @@ -88,23 +88,21 @@ type MachineScope struct { AzureMachine *infrav1.AzureMachine } -// VMSpecs returns the VM specs. -func (m *MachineScope) VMSpecs() []azure.VMSpec { - return []azure.VMSpec{ - { - Name: m.Name(), - Role: m.Role(), - NICNames: m.NICNames(), - SSHKeyData: m.AzureMachine.Spec.SSHPublicKey, - Size: m.AzureMachine.Spec.VMSize, - OSDisk: m.AzureMachine.Spec.OSDisk, - DataDisks: m.AzureMachine.Spec.DataDisks, - Zone: m.AvailabilityZone(), - Identity: m.AzureMachine.Spec.Identity, - UserAssignedIdentities: m.AzureMachine.Spec.UserAssignedIdentities, - SpotVMOptions: m.AzureMachine.Spec.SpotVMOptions, - SecurityProfile: m.AzureMachine.Spec.SecurityProfile, - }, +// VMSpec returns the VM spec. +func (m *MachineScope) VMSpec() azure.VMSpec { + return azure.VMSpec{ + Name: m.Name(), + Role: m.Role(), + NICNames: m.NICNames(), + SSHKeyData: m.AzureMachine.Spec.SSHPublicKey, + Size: m.AzureMachine.Spec.VMSize, + OSDisk: m.AzureMachine.Spec.OSDisk, + DataDisks: m.AzureMachine.Spec.DataDisks, + Zone: m.AvailabilityZone(), + Identity: m.AzureMachine.Spec.Identity, + UserAssignedIdentities: m.AzureMachine.Spec.UserAssignedIdentities, + SpotVMOptions: m.AzureMachine.Spec.SpotVMOptions, + SecurityProfile: m.AzureMachine.Spec.SecurityProfile, } } diff --git a/cloud/scope/managedcontrolplane.go b/cloud/scope/managedcontrolplane.go index 74f554183bb..827f8240458 100644 --- a/cloud/scope/managedcontrolplane.go +++ b/cloud/scope/managedcontrolplane.go @@ -157,14 +157,12 @@ func (s *ManagedControlPlaneScope) Vnet() *infrav1.VnetSpec { } } -// VNetSpecs returns the virtual network specs. -func (s *ManagedControlPlaneScope) VNetSpecs() []azure.VNetSpec { - return []azure.VNetSpec{ - { - ResourceGroup: s.Vnet().ResourceGroup, - Name: s.Vnet().Name, - CIDRs: s.Vnet().CIDRBlocks, - }, +// VNetSpec returns the virtual network spec. +func (s *ManagedControlPlaneScope) VNetSpec() azure.VNetSpec { + return azure.VNetSpec{ + ResourceGroup: s.Vnet().ResourceGroup, + Name: s.Vnet().Name, + CIDRs: s.Vnet().CIDRBlocks, } } diff --git a/cloud/services/virtualmachines/mock_virtualmachines/virtualmachines_mock.go b/cloud/services/virtualmachines/mock_virtualmachines/virtualmachines_mock.go index 634db755f4c..4643dcb6366 100644 --- a/cloud/services/virtualmachines/mock_virtualmachines/virtualmachines_mock.go +++ b/cloud/services/virtualmachines/mock_virtualmachines/virtualmachines_mock.go @@ -302,18 +302,18 @@ func (mr *MockVMScopeMockRecorder) AdditionalTags() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AdditionalTags", reflect.TypeOf((*MockVMScope)(nil).AdditionalTags)) } -// VMSpecs mocks base method. -func (m *MockVMScope) VMSpecs() []azure.VMSpec { +// VMSpec mocks base method. +func (m *MockVMScope) VMSpec() azure.VMSpec { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "VMSpecs") - ret0, _ := ret[0].([]azure.VMSpec) + ret := m.ctrl.Call(m, "VMSpec") + ret0, _ := ret[0].(azure.VMSpec) return ret0 } -// VMSpecs indicates an expected call of VMSpecs. -func (mr *MockVMScopeMockRecorder) VMSpecs() *gomock.Call { +// VMSpec indicates an expected call of VMSpec. +func (mr *MockVMScopeMockRecorder) VMSpec() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "VMSpecs", reflect.TypeOf((*MockVMScope)(nil).VMSpecs)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "VMSpec", reflect.TypeOf((*MockVMScope)(nil).VMSpec)) } // GetBootstrapData mocks base method. diff --git a/cloud/services/virtualmachines/service.go b/cloud/services/virtualmachines/service.go index cf01fa30608..a18efdc60a7 100644 --- a/cloud/services/virtualmachines/service.go +++ b/cloud/services/virtualmachines/service.go @@ -33,7 +33,7 @@ import ( type VMScope interface { logr.Logger azure.ClusterDescriber - VMSpecs() []azure.VMSpec + VMSpec() azure.VMSpec GetBootstrapData(ctx context.Context) (string, error) GetVMImage() (*infrav1.Image, error) SetAnnotation(string, string) diff --git a/cloud/services/virtualmachines/virtualmachines.go b/cloud/services/virtualmachines/virtualmachines.go index d33935b558f..e69c835fb6c 100644 --- a/cloud/services/virtualmachines/virtualmachines.go +++ b/cloud/services/virtualmachines/virtualmachines.go @@ -40,131 +40,130 @@ func (s *Service) Reconcile(ctx context.Context) error { ctx, span := tele.Tracer().Start(ctx, "virtualmachines.Service.Reconcile") defer span.End() - for _, vmSpec := range s.Scope.VMSpecs() { - existingVM, err := s.getExisting(ctx, vmSpec.Name) - switch { - 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(fmt.Sprintf("azure:///%s", existingVM.ID)) - s.Scope.SetAnnotation("cluster-api-provider-azure", "true") - s.Scope.SetAddresses(existingVM.Addresses) - s.Scope.SetVMState(existingVM.State) - 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 errors.Wrapf(err, "failed to get find vm sku %s in compute api", vmSpec.Size) - } + vmSpec := s.Scope.VMSpec() + existingVM, err := s.getExisting(ctx, vmSpec.Name) + switch { + 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(fmt.Sprintf("azure:///%s", existingVM.ID)) + s.Scope.SetAnnotation("cluster-api-provider-azure", "true") + s.Scope.SetAddresses(existingVM.Addresses) + s.Scope.SetVMState(existingVM.State) + 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 errors.Wrapf(err, "failed to get find vm sku %s in compute api", vmSpec.Size) + } - storageProfile, err := s.generateStorageProfile(ctx, vmSpec, sku) - if err != nil { - return err - } + storageProfile, err := s.generateStorageProfile(ctx, vmSpec, sku) + if err != nil { + return err + } - securityProfile, err := getSecurityProfile(vmSpec, sku) - if err != nil { - return err - } + securityProfile, err := getSecurityProfile(vmSpec, sku) + if err != nil { + return err + } - 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), - }, - } + 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 := getSpotVMOptions(vmSpec.SpotVMOptions) - if err != nil { - return errors.Wrapf(err, "failed to get Spot VM options") - } + priority, evictionPolicy, billingProfile, err := getSpotVMOptions(vmSpec.SpotVMOptions) + if err != nil { + return errors.Wrapf(err, "failed to get Spot VM options") + } - sshKey, err := base64.StdEncoding.DecodeString(vmSpec.SSHKeyData) - if err != nil { - return errors.Wrapf(err, "failed to decode ssh public key") - } - bootstrapData, err := s.Scope.GetBootstrapData(ctx) - if err != nil { - return errors.Wrap(err, "failed to retrieve bootstrap data") - } + sshKey, err := base64.StdEncoding.DecodeString(vmSpec.SSHKeyData) + if err != nil { + return errors.Wrapf(err, "failed to decode ssh public key") + } + bootstrapData, err := s.Scope.GetBootstrapData(ctx) + if err != nil { + return errors.Wrap(err, "failed to retrieve bootstrap data") + } - 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: &compute.OSProfile{ - ComputerName: to.StringPtr(vmSpec.Name), - AdminUsername: to.StringPtr(azure.DefaultUserName), - CustomData: to.StringPtr(bootstrapData), - 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)), - }, + 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: &compute.OSProfile{ + ComputerName: to.StringPtr(vmSpec.Name), + AdminUsername: to.StringPtr(azure.DefaultUserName), + CustomData: to.StringPtr(bootstrapData), + 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)), }, }, }, }, - NetworkProfile: &compute.NetworkProfile{ - NetworkInterfaces: &nicRefs, - }, - Priority: priority, - EvictionPolicy: evictionPolicy, - BillingProfile: billingProfile, - DiagnosticsProfile: &compute.DiagnosticsProfile{ - BootDiagnostics: &compute.BootDiagnostics{ - Enabled: to.BoolPtr(true), - }, + }, + NetworkProfile: &compute.NetworkProfile{ + NetworkInterfaces: &nicRefs, + }, + Priority: priority, + EvictionPolicy: evictionPolicy, + BillingProfile: billingProfile, + DiagnosticsProfile: &compute.DiagnosticsProfile{ + BootDiagnostics: &compute.BootDiagnostics{ + Enabled: to.BoolPtr(true), }, }, - } + }, + } - if vmSpec.Zone != "" { - zones := []string{vmSpec.Zone} - virtualMachine.Zones = &zones - } + 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, - } + if vmSpec.Identity == infrav1.VMIdentitySystemAssigned { + virtualMachine.Identity = &compute.VirtualMachineIdentity{ + Type: compute.ResourceIdentityTypeSystemAssigned, } - - 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()) + } 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, } + } - s.Scope.V(2).Info("successfully created VM", "vm", vmSpec.Name) + 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) } return nil @@ -175,19 +174,18 @@ func (s *Service) Delete(ctx context.Context) error { ctx, span := tele.Tracer().Start(ctx, "virtualmachines.Service.Delete") defer span.End() - for _, vmSpec := range s.Scope.VMSpecs() { - 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 - continue - } - 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) + 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 } diff --git a/cloud/services/virtualmachines/virtualmachines_test.go b/cloud/services/virtualmachines/virtualmachines_test.go index a480956ab5b..33507ab6cf9 100644 --- a/cloud/services/virtualmachines/virtualmachines_test.go +++ b/cloud/services/virtualmachines/virtualmachines_test.go @@ -284,32 +284,30 @@ func TestReconcileVM(t *testing.T) { { 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.VMSpecs().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: 128, - ManagedDisk: infrav1.ManagedDisk{ - StorageAccountType: "Premium_LRS", - }, - }, - DataDisks: []infrav1.DataDisk{ - { - NameSuffix: "mydisk", - DiskSizeGB: 64, - Lun: to.Int32Ptr(0), - }, + 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: 128, + ManagedDisk: infrav1.ManagedDisk{ + StorageAccountType: "Premium_LRS", }, - UserAssignedIdentities: nil, - SpotVMOptions: nil, }, + 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") @@ -439,20 +437,18 @@ func TestReconcileVM(t *testing.T) { { 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.VMSpecs().Return([]azure.VMSpec{ - { - Name: "my-vm", - Role: infrav1.Node, - NICNames: []string{"my-nic"}, - SSHKeyData: "fakesshpublickey", - Size: "Standard_D2v3", - Zone: "", - Identity: infrav1.VMIdentitySystemAssigned, - OSDisk: infrav1.OSDisk{}, - DataDisks: nil, - UserAssignedIdentities: nil, - SpotVMOptions: nil, - }, + s.VMSpec().Return(azure.VMSpec{ + Name: "my-vm", + Role: infrav1.Node, + NICNames: []string{"my-nic"}, + SSHKeyData: "fakesshpublickey", + Size: "Standard_D2v3", + Zone: "", + Identity: infrav1.VMIdentitySystemAssigned, + OSDisk: infrav1.OSDisk{}, + DataDisks: nil, + UserAssignedIdentities: nil, + SpotVMOptions: nil, }) s.SubscriptionID().AnyTimes().Return("123") s.ResourceGroup().AnyTimes().Return("my-rg") @@ -511,20 +507,18 @@ func TestReconcileVM(t *testing.T) { { 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.VMSpecs().Return([]azure.VMSpec{ - { - Name: "my-vm", - Role: infrav1.Node, - NICNames: []string{"my-nic"}, - SSHKeyData: "fakesshpublickey", - Size: "Standard_D2v3", - Zone: "", - Identity: infrav1.VMIdentityUserAssigned, - OSDisk: infrav1.OSDisk{}, - DataDisks: nil, - UserAssignedIdentities: []infrav1.UserAssignedIdentity{{ProviderID: "my-user-id"}}, - SpotVMOptions: nil, - }, + s.VMSpec().Return(azure.VMSpec{ + Name: "my-vm", + Role: infrav1.Node, + NICNames: []string{"my-nic"}, + SSHKeyData: "fakesshpublickey", + Size: "Standard_D2v3", + Zone: "", + 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") @@ -583,20 +577,18 @@ func TestReconcileVM(t *testing.T) { { 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.VMSpecs().Return([]azure.VMSpec{ - { - Name: "my-vm", - Role: infrav1.Node, - NICNames: []string{"my-nic"}, - SSHKeyData: "fakesshpublickey", - Size: "Standard_D2v3", - Zone: "", - Identity: "", - OSDisk: infrav1.OSDisk{}, - DataDisks: nil, - UserAssignedIdentities: nil, - SpotVMOptions: &infrav1.SpotVMOptions{}, - }, + s.VMSpec().Return(azure.VMSpec{ + Name: "my-vm", + Role: infrav1.Node, + NICNames: []string{"my-nic"}, + SSHKeyData: "fakesshpublickey", + Size: "Standard_D2v3", + Zone: "", + Identity: "", + OSDisk: infrav1.OSDisk{}, + DataDisks: nil, + UserAssignedIdentities: nil, + SpotVMOptions: &infrav1.SpotVMOptions{}, }) s.SubscriptionID().AnyTimes().Return("123") s.ResourceGroup().AnyTimes().Return("my-rg") @@ -656,26 +648,24 @@ func TestReconcileVM(t *testing.T) { { 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.VMSpecs().Return([]azure.VMSpec{ - { - Name: "my-vm", - Role: infrav1.Node, - NICNames: []string{"my-nic"}, - SSHKeyData: "fakesshpublickey", - Size: "Standard_D2v3", - Zone: "", - Identity: "", - OSDisk: infrav1.OSDisk{ - ManagedDisk: infrav1.ManagedDisk{ - StorageAccountType: "Premium_LRS", - DiskEncryptionSet: &infrav1.DiskEncryptionSetParameters{ - ID: "my-diskencryptionset-id", - }, + s.VMSpec().Return(azure.VMSpec{ + Name: "my-vm", + Role: infrav1.Node, + NICNames: []string{"my-nic"}, + SSHKeyData: "fakesshpublickey", + Size: "Standard_D2v3", + Zone: "", + Identity: "", + OSDisk: infrav1.OSDisk{ + ManagedDisk: infrav1.ManagedDisk{ + StorageAccountType: "Premium_LRS", + DiskEncryptionSet: &infrav1.DiskEncryptionSetParameters{ + ID: "my-diskencryptionset-id", }, }, - DataDisks: nil, - UserAssignedIdentities: nil, }, + DataDisks: nil, + UserAssignedIdentities: nil, }) s.SubscriptionID().AnyTimes().Return("123") s.ResourceGroup().AnyTimes().Return("my-rg") @@ -734,16 +724,14 @@ func TestReconcileVM(t *testing.T) { { 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.VMSpecs().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.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.SubscriptionID().AnyTimes().Return("123") s.ResourceGroup().AnyTimes().Return("my-rg") @@ -806,16 +794,14 @@ func TestReconcileVM(t *testing.T) { { 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.VMSpecs().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.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()) @@ -864,20 +850,18 @@ func TestReconcileVM(t *testing.T) { { 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.VMSpecs().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.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.SubscriptionID().AnyTimes().Return("123") s.ResourceGroup().AnyTimes().Return("my-rg") @@ -933,32 +917,30 @@ func TestReconcileVM(t *testing.T) { { 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.VMSpecs().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: 128, - ManagedDisk: infrav1.ManagedDisk{ - StorageAccountType: "Premium_LRS", - }, - }, - DataDisks: []infrav1.DataDisk{ - { - NameSuffix: "mydisk", - DiskSizeGB: 64, - Lun: to.Int32Ptr(0), - }, + 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: 128, + ManagedDisk: infrav1.ManagedDisk{ + StorageAccountType: "Premium_LRS", }, - UserAssignedIdentities: nil, - SpotVMOptions: nil, }, + 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") @@ -1001,32 +983,30 @@ func TestReconcileVM(t *testing.T) { { 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.VMSpecs().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: 128, - ManagedDisk: infrav1.ManagedDisk{ - StorageAccountType: "Premium_LRS", - }, - }, - DataDisks: []infrav1.DataDisk{ - { - NameSuffix: "mydisk", - DiskSizeGB: 64, - Lun: to.Int32Ptr(0), - }, + 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: 128, + ManagedDisk: infrav1.ManagedDisk{ + StorageAccountType: "Premium_LRS", + }, + }, + DataDisks: []infrav1.DataDisk{ + { + NameSuffix: "mydisk", + DiskSizeGB: 64, + Lun: to.Int32Ptr(0), }, - UserAssignedIdentities: nil, - SpotVMOptions: nil, }, + UserAssignedIdentities: nil, + SpotVMOptions: nil, }) s.SubscriptionID().AnyTimes().Return("123") s.ResourceGroup().AnyTimes().Return("my-rg") @@ -1069,35 +1049,33 @@ func TestReconcileVM(t *testing.T) { { 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.VMSpecs().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: 128, - ManagedDisk: infrav1.ManagedDisk{ - StorageAccountType: "Premium_LRS", - }, - DiffDiskSettings: &infrav1.DiffDiskSettings{ - Option: string(compute.Local), - }, - }, - DataDisks: []infrav1.DataDisk{ - { - NameSuffix: "mydisk", - DiskSizeGB: 64, - Lun: to.Int32Ptr(0), - }, + 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: 128, + ManagedDisk: infrav1.ManagedDisk{ + StorageAccountType: "Premium_LRS", + }, + DiffDiskSettings: &infrav1.DiffDiskSettings{ + Option: string(compute.Local), }, - UserAssignedIdentities: nil, - SpotVMOptions: nil, }, + 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") @@ -1144,35 +1122,33 @@ func TestReconcileVM(t *testing.T) { { 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.VMSpecs().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: 128, - ManagedDisk: infrav1.ManagedDisk{ - StorageAccountType: "Premium_LRS", - }, - DiffDiskSettings: &infrav1.DiffDiskSettings{ - Option: string(compute.Local), - }, - }, - DataDisks: []infrav1.DataDisk{ - { - NameSuffix: "mydisk", - DiskSizeGB: 64, - Lun: to.Int32Ptr(0), - }, + 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: 128, + ManagedDisk: infrav1.ManagedDisk{ + StorageAccountType: "Premium_LRS", + }, + DiffDiskSettings: &infrav1.DiffDiskSettings{ + Option: string(compute.Local), }, - UserAssignedIdentities: nil, - SpotVMOptions: nil, }, + 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") @@ -1309,32 +1285,30 @@ func TestReconcileVM(t *testing.T) { { 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.VMSpecs().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: 128, - ManagedDisk: infrav1.ManagedDisk{ - StorageAccountType: "Premium_LRS", - }, - }, - DataDisks: []infrav1.DataDisk{ - { - NameSuffix: "mydisk", - DiskSizeGB: 64, - Lun: to.Int32Ptr(0), - }, + 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: 128, + ManagedDisk: infrav1.ManagedDisk{ + StorageAccountType: "Premium_LRS", + }, + }, + DataDisks: []infrav1.DataDisk{ + { + NameSuffix: "mydisk", + DiskSizeGB: 64, + Lun: to.Int32Ptr(0), }, - UserAssignedIdentities: nil, - SpotVMOptions: nil, }, + UserAssignedIdentities: nil, + SpotVMOptions: nil, }) s.SubscriptionID().AnyTimes().Return("123") s.ResourceGroup().AnyTimes().Return("my-rg") @@ -1514,20 +1488,18 @@ func TestDeleteVM(t *testing.T) { name: "successfully delete an existing vm", expectedError: "", expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder) { - s.VMSpecs().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.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()) @@ -1538,20 +1510,18 @@ func TestDeleteVM(t *testing.T) { name: "vm already deleted", expectedError: "", expect: func(s *mock_virtualmachines.MockVMScopeMockRecorder, m *mock_virtualmachines.MockClientMockRecorder) { - s.VMSpecs().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.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()) @@ -1563,20 +1533,18 @@ func TestDeleteVM(t *testing.T) { 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.VMSpecs().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.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()) diff --git a/cloud/services/virtualnetworks/mock_virtualnetworks/virtualnetworks_mock.go b/cloud/services/virtualnetworks/mock_virtualnetworks/virtualnetworks_mock.go index 2257e1a8cf4..58a6ea98dbd 100644 --- a/cloud/services/virtualnetworks/mock_virtualnetworks/virtualnetworks_mock.go +++ b/cloud/services/virtualnetworks/mock_virtualnetworks/virtualnetworks_mock.go @@ -314,16 +314,16 @@ func (mr *MockVNetScopeMockRecorder) Vnet() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Vnet", reflect.TypeOf((*MockVNetScope)(nil).Vnet)) } -// VNetSpecs mocks base method. -func (m *MockVNetScope) VNetSpecs() []azure.VNetSpec { +// VNetSpec mocks base method. +func (m *MockVNetScope) VNetSpec() azure.VNetSpec { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "VNetSpecs") - ret0, _ := ret[0].([]azure.VNetSpec) + ret := m.ctrl.Call(m, "VNetSpec") + ret0, _ := ret[0].(azure.VNetSpec) return ret0 } -// VNetSpecs indicates an expected call of VNetSpecs. -func (mr *MockVNetScopeMockRecorder) VNetSpecs() *gomock.Call { +// VNetSpec indicates an expected call of VNetSpec. +func (mr *MockVNetScopeMockRecorder) VNetSpec() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "VNetSpecs", reflect.TypeOf((*MockVNetScope)(nil).VNetSpecs)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "VNetSpec", reflect.TypeOf((*MockVNetScope)(nil).VNetSpec)) } diff --git a/cloud/services/virtualnetworks/service.go b/cloud/services/virtualnetworks/service.go index c81b1bf1bdf..dd677ed435d 100644 --- a/cloud/services/virtualnetworks/service.go +++ b/cloud/services/virtualnetworks/service.go @@ -28,7 +28,7 @@ type VNetScope interface { logr.Logger azure.ClusterDescriber Vnet() *infrav1.VnetSpec - VNetSpecs() []azure.VNetSpec + VNetSpec() azure.VNetSpec } // Service provides operations on azure resources diff --git a/cloud/services/virtualnetworks/virtualnetworks.go b/cloud/services/virtualnetworks/virtualnetworks.go index ef4b7868bb0..daef44ff562 100644 --- a/cloud/services/virtualnetworks/virtualnetworks.go +++ b/cloud/services/virtualnetworks/virtualnetworks.go @@ -42,44 +42,43 @@ func (s *Service) Reconcile(ctx context.Context) error { // * Control Plane NSG // * Node NSG // * Node Route Table - for _, vnetSpec := range s.Scope.VNetSpecs() { - existingVnet, err := s.getExisting(ctx, vnetSpec) - - switch { - case err != nil && !azure.ResourceNotFound(err): - return errors.Wrapf(err, "failed to get VNet %s", vnetSpec.Name) - - case err == nil: - // vnet already exists, cannot update since it's immutable - if !existingVnet.IsManaged(s.Scope.ClusterName()) { - s.Scope.V(2).Info("Working on custom VNet", "vnet-id", existingVnet.ID) - } - existingVnet.DeepCopyInto(s.Scope.Vnet()) - - default: - s.Scope.V(2).Info("creating VNet", "VNet", vnetSpec.Name) - - vnetProperties := network.VirtualNetwork{ - Tags: converters.TagsToMap(infrav1.Build(infrav1.BuildParams{ - ClusterName: s.Scope.ClusterName(), - Lifecycle: infrav1.ResourceLifecycleOwned, - Name: to.StringPtr(vnetSpec.Name), - Role: to.StringPtr(infrav1.CommonRole), - Additional: s.Scope.AdditionalTags(), - })), - Location: to.StringPtr(s.Scope.Location()), - VirtualNetworkPropertiesFormat: &network.VirtualNetworkPropertiesFormat{ - AddressSpace: &network.AddressSpace{ - AddressPrefixes: &vnetSpec.CIDRs, - }, + vnetSpec := s.Scope.VNetSpec() + existingVnet, err := s.getExisting(ctx, vnetSpec) + + switch { + case err != nil && !azure.ResourceNotFound(err): + return errors.Wrapf(err, "failed to get VNet %s", vnetSpec.Name) + + case err == nil: + // vnet already exists, cannot update since it's immutable + if !existingVnet.IsManaged(s.Scope.ClusterName()) { + s.Scope.V(2).Info("Working on custom VNet", "vnet-id", existingVnet.ID) + } + existingVnet.DeepCopyInto(s.Scope.Vnet()) + + default: + s.Scope.V(2).Info("creating VNet", "VNet", vnetSpec.Name) + + vnetProperties := network.VirtualNetwork{ + Tags: converters.TagsToMap(infrav1.Build(infrav1.BuildParams{ + ClusterName: s.Scope.ClusterName(), + Lifecycle: infrav1.ResourceLifecycleOwned, + Name: to.StringPtr(vnetSpec.Name), + Role: to.StringPtr(infrav1.CommonRole), + Additional: s.Scope.AdditionalTags(), + })), + Location: to.StringPtr(s.Scope.Location()), + VirtualNetworkPropertiesFormat: &network.VirtualNetworkPropertiesFormat{ + AddressSpace: &network.AddressSpace{ + AddressPrefixes: &vnetSpec.CIDRs, }, - } - err = s.Client.CreateOrUpdate(ctx, vnetSpec.ResourceGroup, vnetSpec.Name, vnetProperties) - if err != nil { - return errors.Wrapf(err, "failed to create virtual network %s", vnetSpec.Name) - } - s.Scope.V(2).Info("successfully created VNet", "VNet", vnetSpec.Name) + }, } + err = s.Client.CreateOrUpdate(ctx, vnetSpec.ResourceGroup, vnetSpec.Name, vnetProperties) + if err != nil { + return errors.Wrapf(err, "failed to create virtual network %s", vnetSpec.Name) + } + s.Scope.V(2).Info("successfully created VNet", "VNet", vnetSpec.Name) } return nil @@ -90,24 +89,23 @@ func (s *Service) Delete(ctx context.Context) error { ctx, span := tele.Tracer().Start(ctx, "virtualnetworks.Service.Delete") defer span.End() - for _, vnetSpec := range s.Scope.VNetSpecs() { - if !s.Scope.Vnet().IsManaged(s.Scope.ClusterName()) { - s.Scope.V(4).Info("Skipping VNet deletion in custom vnet mode") - continue - } - - s.Scope.V(2).Info("deleting VNet", "VNet", vnetSpec.Name) - err := s.Client.Delete(ctx, vnetSpec.ResourceGroup, vnetSpec.Name) - if err != nil && azure.ResourceNotFound(err) { - // already deleted - continue - } - if err != nil { - return errors.Wrapf(err, "failed to delete VNet %s in resource group %s", vnetSpec.Name, vnetSpec.ResourceGroup) - } + vnetSpec := s.Scope.VNetSpec() + if !s.Scope.Vnet().IsManaged(s.Scope.ClusterName()) { + s.Scope.V(4).Info("Skipping VNet deletion in custom vnet mode") + return nil + } - s.Scope.V(2).Info("successfully deleted VNet", "VNet", vnetSpec.Name) + s.Scope.V(2).Info("deleting VNet", "VNet", vnetSpec.Name) + err := s.Client.Delete(ctx, vnetSpec.ResourceGroup, vnetSpec.Name) + if err != nil && azure.ResourceNotFound(err) { + // already deleted + return nil } + if err != nil { + return errors.Wrapf(err, "failed to delete VNet %s in resource group %s", vnetSpec.Name, vnetSpec.ResourceGroup) + } + + s.Scope.V(2).Info("successfully deleted VNet", "VNet", vnetSpec.Name) return nil } diff --git a/cloud/services/virtualnetworks/virtualnetworks_test.go b/cloud/services/virtualnetworks/virtualnetworks_test.go index bbd78434671..0f95401dd61 100644 --- a/cloud/services/virtualnetworks/virtualnetworks_test.go +++ b/cloud/services/virtualnetworks/virtualnetworks_test.go @@ -47,12 +47,10 @@ func TestReconcileVnet(t *testing.T) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) s.ClusterName().AnyTimes().Return("fake-cluster") s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "vnet-exists"}) - s.VNetSpecs().Return([]azure.VNetSpec{ - { - ResourceGroup: "my-rg", - Name: "vnet-exists", - CIDRs: []string{"10.0.0.0/8"}, - }, + s.VNetSpec().Return(azure.VNetSpec{ + ResourceGroup: "my-rg", + Name: "vnet-exists", + CIDRs: []string{"10.0.0.0/8"}, }) m.Get(gomockinternal.AContext(), "my-rg", "vnet-exists"). Return(network.VirtualNetwork{ @@ -78,12 +76,10 @@ func TestReconcileVnet(t *testing.T) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) s.ClusterName().AnyTimes().Return("fake-cluster") s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "vnet-exists"}) - s.VNetSpecs().Return([]azure.VNetSpec{ - { - ResourceGroup: "my-rg", - Name: "ipv6-vnet-exists", - CIDRs: []string{"10.0.0.0/8", "2001:1234:5678:9a00::/56"}, - }, + s.VNetSpec().Return(azure.VNetSpec{ + ResourceGroup: "my-rg", + Name: "ipv6-vnet-exists", + CIDRs: []string{"10.0.0.0/8", "2001:1234:5678:9a00::/56"}, }) m.Get(gomockinternal.AContext(), "my-rg", "ipv6-vnet-exists"). Return(network.VirtualNetwork{ @@ -114,12 +110,10 @@ func TestReconcileVnet(t *testing.T) { s.Location().AnyTimes().Return("fake-location") s.AdditionalTags().AnyTimes().Return(infrav1.Tags{}) s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "vnet-new"}) - s.VNetSpecs().Return([]azure.VNetSpec{ - { - ResourceGroup: "my-rg", - Name: "vnet-new", - CIDRs: []string{"10.0.0.0/8"}, - }, + s.VNetSpec().Return(azure.VNetSpec{ + ResourceGroup: "my-rg", + Name: "vnet-new", + CIDRs: []string{"10.0.0.0/8"}, }) m.Get(gomockinternal.AContext(), "my-rg", "vnet-new"). Return(network.VirtualNetwork{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) @@ -136,12 +130,10 @@ func TestReconcileVnet(t *testing.T) { s.Location().AnyTimes().Return("fake-location") s.AdditionalTags().AnyTimes().Return(infrav1.Tags{}) s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "vnet-new"}) - s.VNetSpecs().Return([]azure.VNetSpec{ - { - ResourceGroup: "my-rg", - Name: "vnet-ipv6-new", - CIDRs: []string{"10.0.0.0/8", "2001:1234:5678:9a00::/56"}, - }, + s.VNetSpec().Return(azure.VNetSpec{ + ResourceGroup: "my-rg", + Name: "vnet-ipv6-new", + CIDRs: []string{"10.0.0.0/8", "2001:1234:5678:9a00::/56"}, }) m.Get(gomockinternal.AContext(), "my-rg", "vnet-ipv6-new"). Return(network.VirtualNetwork{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) @@ -172,12 +164,10 @@ func TestReconcileVnet(t *testing.T) { s.ClusterName().AnyTimes().Return("fake-cluster") s.Location().AnyTimes().Return("fake-location") s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "custom-vnet"}) - s.VNetSpecs().Return([]azure.VNetSpec{ - { - ResourceGroup: "custom-vnet-rg", - Name: "custom-vnet", - CIDRs: []string{"10.0.0.0/16"}, - }, + s.VNetSpec().Return(azure.VNetSpec{ + ResourceGroup: "custom-vnet-rg", + Name: "custom-vnet", + CIDRs: []string{"10.0.0.0/16"}, }) m.Get(gomockinternal.AContext(), "custom-vnet-rg", "custom-vnet"). Return(network.VirtualNetwork{ @@ -203,12 +193,10 @@ func TestReconcileVnet(t *testing.T) { s.Location().AnyTimes().Return("fake-location") s.AdditionalTags().AnyTimes().Return(infrav1.Tags{}) s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "custom-vnet"}) - s.VNetSpecs().Return([]azure.VNetSpec{ - { - ResourceGroup: "custom-vnet-rg", - Name: "custom-vnet", - CIDRs: []string{"10.0.0.0/16"}, - }, + s.VNetSpec().Return(azure.VNetSpec{ + ResourceGroup: "custom-vnet-rg", + Name: "custom-vnet", + CIDRs: []string{"10.0.0.0/16"}, }) m.Get(gomockinternal.AContext(), "custom-vnet-rg", "custom-vnet"). Return(network.VirtualNetwork{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) @@ -222,12 +210,10 @@ func TestReconcileVnet(t *testing.T) { expect: func(s *mock_virtualnetworks.MockVNetScopeMockRecorder, m *mock_virtualnetworks.MockClientMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) s.ClusterName().AnyTimes().Return("fake-cluster") - s.VNetSpecs().Return([]azure.VNetSpec{ - { - ResourceGroup: "custom-vnet-rg", - Name: "custom-vnet", - CIDRs: []string{"10.0.0.0/16"}, - }, + s.VNetSpec().Return(azure.VNetSpec{ + ResourceGroup: "custom-vnet-rg", + Name: "custom-vnet", + CIDRs: []string{"10.0.0.0/16"}, }) m.Get(gomockinternal.AContext(), "custom-vnet-rg", "custom-vnet"). Return(network.VirtualNetwork{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) @@ -242,12 +228,10 @@ func TestReconcileVnet(t *testing.T) { s.Location().AnyTimes().Return("fake-location") s.AdditionalTags().AnyTimes().Return(infrav1.Tags{}) s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "custom-vnet"}) - s.VNetSpecs().Return([]azure.VNetSpec{ - { - ResourceGroup: "custom-vnet-rg", - Name: "custom-vnet", - CIDRs: []string{"10.0.0.0/16"}, - }, + s.VNetSpec().Return(azure.VNetSpec{ + ResourceGroup: "custom-vnet-rg", + Name: "custom-vnet", + CIDRs: []string{"10.0.0.0/16"}, }) m.Get(gomockinternal.AContext(), "custom-vnet-rg", "custom-vnet"). Return(network.VirtualNetwork{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) @@ -305,12 +289,10 @@ func TestDeleteVnet(t *testing.T) { "sigs.k8s.io_cluster-api-provider-azure_role": "common", }) s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "vnet-exists"}) - s.VNetSpecs().Return([]azure.VNetSpec{ - { - ResourceGroup: "my-rg", - Name: "vnet-exists", - CIDRs: []string{"10.0.0.0/16"}, - }, + s.VNetSpec().Return(azure.VNetSpec{ + ResourceGroup: "my-rg", + Name: "vnet-exists", + CIDRs: []string{"10.0.0.0/16"}, }) m.Delete(gomockinternal.AContext(), "my-rg", "vnet-exists") }, @@ -328,12 +310,10 @@ func TestDeleteVnet(t *testing.T) { "sigs.k8s.io_cluster-api-provider-azure_role": "common", }) s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "vnet-exists"}) - s.VNetSpecs().Return([]azure.VNetSpec{ - { - ResourceGroup: "my-rg", - Name: "vnet-exists", - CIDRs: []string{"10.0.0.0/16"}, - }, + s.VNetSpec().Return(azure.VNetSpec{ + ResourceGroup: "my-rg", + Name: "vnet-exists", + CIDRs: []string{"10.0.0.0/16"}, }) m.Delete(gomockinternal.AContext(), "my-rg", "vnet-exists"). Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) @@ -348,12 +328,10 @@ func TestDeleteVnet(t *testing.T) { s.Location().AnyTimes().Return("fake-location") s.AdditionalTags().AnyTimes().Return(infrav1.Tags{}) s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{ResourceGroup: "my-rg", Name: "my-vnet", ID: "azure/custom-vnet/id"}) - s.VNetSpecs().Return([]azure.VNetSpec{ - { - ResourceGroup: "my-rg", - Name: "my-vnet", - CIDRs: []string{"10.0.0.0/16"}, - }, + s.VNetSpec().Return(azure.VNetSpec{ + ResourceGroup: "my-rg", + Name: "my-vnet", + CIDRs: []string{"10.0.0.0/16"}, }) }, }, @@ -370,12 +348,10 @@ func TestDeleteVnet(t *testing.T) { "sigs.k8s.io_cluster-api-provider-azure_role": "common", }) s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "vnet-exists"}) - s.VNetSpecs().Return([]azure.VNetSpec{ - { - ResourceGroup: "my-rg", - Name: "vnet-exists", - CIDRs: []string{"10.0.0.0/16"}, - }, + s.VNetSpec().Return(azure.VNetSpec{ + ResourceGroup: "my-rg", + Name: "vnet-exists", + CIDRs: []string{"10.0.0.0/16"}, }) m.Delete(gomockinternal.AContext(), "my-rg", "vnet-exists"). Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Honk Server"))