From f48e050d8edfb42cce54d8bd2e774a7b1f9c1920 Mon Sep 17 00:00:00 2001 From: Hans Rakers Date: Tue, 6 Feb 2024 12:36:43 +0100 Subject: [PATCH] Lint fixes Signed-off-by: Hans Rakers --- .../cloudstackaffinitygroup_conversion.go | 5 +-- .../cloudstackisolatednetwork_conversion.go | 5 +-- api/v1beta1/cloudstackmachine_conversion.go | 5 +-- .../cloudstackmachinetemplate_conversion.go | 5 +-- api/v1beta1/conversion.go | 4 +- .../cloudstackisolatednetwork_controller.go | 2 +- controllers/cloudstackmachine_controller.go | 4 +- .../cloudstackmachine_controller_test.go | 20 ++++----- controllers/utils/utils.go | 2 +- pkg/cloud/affinity_groups_test.go | 2 +- pkg/cloud/instance.go | 20 ++++----- pkg/cloud/instance_test.go | 43 +++++++++---------- pkg/cloud/isolated_network.go | 26 ++++------- pkg/cloud/isolated_network_test.go | 26 +++++------ 14 files changed, 72 insertions(+), 97 deletions(-) diff --git a/api/v1beta1/cloudstackaffinitygroup_conversion.go b/api/v1beta1/cloudstackaffinitygroup_conversion.go index 0d3d98b5..685d1ad9 100644 --- a/api/v1beta1/cloudstackaffinitygroup_conversion.go +++ b/api/v1beta1/cloudstackaffinitygroup_conversion.go @@ -47,10 +47,7 @@ func (dst *CloudStackAffinityGroup) ConvertFrom(srcRaw conversion.Hub) error { / } // Preserve Hub data on down-conversion - if err := utilconversion.MarshalData(src, dst); err != nil { - return err - } - return nil + return utilconversion.MarshalData(src, dst) } func Convert_v1beta3_CloudStackAffinityGroupSpec_To_v1beta1_CloudStackAffinityGroupSpec(in *v1beta3.CloudStackAffinityGroupSpec, out *CloudStackAffinityGroupSpec, s machineryconversion.Scope) error { // nolint diff --git a/api/v1beta1/cloudstackisolatednetwork_conversion.go b/api/v1beta1/cloudstackisolatednetwork_conversion.go index b792ff31..4fe3e577 100644 --- a/api/v1beta1/cloudstackisolatednetwork_conversion.go +++ b/api/v1beta1/cloudstackisolatednetwork_conversion.go @@ -47,10 +47,7 @@ func (dst *CloudStackIsolatedNetwork) ConvertFrom(srcRaw conversion.Hub) error { } // Preserve Hub data on down-conversion - if err := utilconversion.MarshalData(src, dst); err != nil { - return err - } - return nil + return utilconversion.MarshalData(src, dst) } func Convert_v1beta3_CloudStackIsolatedNetworkSpec_To_v1beta1_CloudStackIsolatedNetworkSpec(in *v1beta3.CloudStackIsolatedNetworkSpec, out *CloudStackIsolatedNetworkSpec, s machineryconversion.Scope) error { // nolint diff --git a/api/v1beta1/cloudstackmachine_conversion.go b/api/v1beta1/cloudstackmachine_conversion.go index bc9ab100..55de6480 100644 --- a/api/v1beta1/cloudstackmachine_conversion.go +++ b/api/v1beta1/cloudstackmachine_conversion.go @@ -68,10 +68,7 @@ func (dst *CloudStackMachine) ConvertFrom(srcRaw conversion.Hub) error { // noli } // Preserve Hub data on down-conversion - if err := utilconversion.MarshalData(src, dst); err != nil { - return err - } - return nil + return utilconversion.MarshalData(src, dst) } func Convert_v1beta1_CloudStackMachineSpec_To_v1beta3_CloudStackMachineSpec(in *CloudStackMachineSpec, out *v1beta3.CloudStackMachineSpec, scope machineryconversion.Scope) error { // nolint diff --git a/api/v1beta1/cloudstackmachinetemplate_conversion.go b/api/v1beta1/cloudstackmachinetemplate_conversion.go index f4a3bea0..c90d5b6b 100644 --- a/api/v1beta1/cloudstackmachinetemplate_conversion.go +++ b/api/v1beta1/cloudstackmachinetemplate_conversion.go @@ -53,10 +53,7 @@ func (dst *CloudStackMachineTemplate) ConvertFrom(srcRaw conversion.Hub) error { } // Preserve Hub data on down-conversion - if err := utilconversion.MarshalData(src, dst); err != nil { - return err - } - return nil + return utilconversion.MarshalData(src, dst) } func Convert_v1beta1_CloudStackMachineTemplateSpec_To_v1beta3_CloudStackMachineTemplateSpec(in *CloudStackMachineTemplateSpec, out *v1beta3.CloudStackMachineTemplateSpec, s machineryconversion.Scope) error { // nolint diff --git a/api/v1beta1/conversion.go b/api/v1beta1/conversion.go index 07bdac1d..f78f7c28 100644 --- a/api/v1beta1/conversion.go +++ b/api/v1beta1/conversion.go @@ -91,7 +91,7 @@ func GetFailureDomains(csCluster *CloudStackCluster) ([]v1beta3.CloudStackFailur var failureDomains []v1beta3.CloudStackFailureDomainSpec namespace := csCluster.Namespace for _, zone := range csCluster.Spec.Zones { - name, err := GetDefaultFailureDomainName(namespace, csCluster.Name, zone.ID, zone.Name) + name, err := GetDefaultFailureDomainName(namespace, zone.ID, zone.Name) if err != nil { return nil, err } @@ -123,7 +123,7 @@ func GetFailureDomains(csCluster *CloudStackCluster) ([]v1beta3.CloudStackFailur // When upgrading cluster using eks-a, a secret named global will be created by eks-a, and it is used by following // method to get zoneID by calling cloudstack API. // When upgrading cluster using clusterctl directly, zoneID is fetched directly from kubernetes cluster in cloudstackzones. -func GetDefaultFailureDomainName(namespace string, clusterName string, zoneID string, zoneName string) (string, error) { +func GetDefaultFailureDomainName(namespace string, zoneID string, zoneName string) (string, error) { if len(zoneID) > 0 { return zoneID, nil } diff --git a/controllers/cloudstackisolatednetwork_controller.go b/controllers/cloudstackisolatednetwork_controller.go index a2147965..b9943245 100644 --- a/controllers/cloudstackisolatednetwork_controller.go +++ b/controllers/cloudstackisolatednetwork_controller.go @@ -96,7 +96,7 @@ func (r *CloudStackIsoNetReconciliationRunner) Reconcile() (retRes ctrl.Result, func (r *CloudStackIsoNetReconciliationRunner) ReconcileDelete() (retRes ctrl.Result, retErr error) { r.Log.Info("Deleting IsolatedNetwork.") - if err := r.CSUser.DisposeIsoNetResources(r.FailureDomain, r.ReconciliationSubject, r.CSCluster); err != nil { + if err := r.CSUser.DisposeIsoNetResources(r.ReconciliationSubject, r.CSCluster); err != nil { if !strings.Contains(strings.ToLower(err.Error()), "no match found") { return ctrl.Result{}, err } diff --git a/controllers/cloudstackmachine_controller.go b/controllers/cloudstackmachine_controller.go index 8ad88487..01617476 100644 --- a/controllers/cloudstackmachine_controller.go +++ b/controllers/cloudstackmachine_controller.go @@ -240,7 +240,7 @@ func (r *CloudStackMachineReconciliationRunner) GetOrCreateVMInstance() (retRes } userData := processCustomMetadata(data, r) - err := r.CSUser.GetOrCreateVMInstance(r.ReconciliationSubject, r.CAPIMachine, r.CSCluster, r.FailureDomain, r.AffinityGroup, userData) + err := r.CSUser.GetOrCreateVMInstance(r.ReconciliationSubject, r.CAPIMachine, r.FailureDomain, r.AffinityGroup, userData) if err != nil { r.Log.Error(err, "GetOrCreateVMInstance returned error") r.Recorder.Eventf(r.ReconciliationSubject, "Warning", "Creating", CSMachineCreationFailed, err.Error()) @@ -358,7 +358,7 @@ func (r *CloudStackMachineReconciliationRunner) ReconcileDelete() (retRes ctrl.R // SetupWithManager registers the machine reconciler to the CAPI controller manager. func (reconciler *CloudStackMachineReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opts controller.Options) error { reconciler.Recorder = mgr.GetEventRecorderFor("capc-machine-controller") - CloudStackClusterToCloudStackMachines, err := utils.CloudStackClusterToCloudStackMachines(ctx, reconciler.K8sClient, &infrav1.CloudStackMachineList{}, reconciler.Scheme, ctrl.LoggerFrom(ctx)) + CloudStackClusterToCloudStackMachines, err := utils.CloudStackClusterToCloudStackMachines(reconciler.K8sClient, &infrav1.CloudStackMachineList{}, reconciler.Scheme, ctrl.LoggerFrom(ctx)) if err != nil { return errors.Wrap(err, "failed to create CloudStackClusterToCloudStackMachines mapper") } diff --git a/controllers/cloudstackmachine_controller_test.go b/controllers/cloudstackmachine_controller_test.go index e4195726..4cdbe405 100644 --- a/controllers/cloudstackmachine_controller_test.go +++ b/controllers/cloudstackmachine_controller_test.go @@ -60,8 +60,8 @@ var _ = Describe("CloudStackMachineReconciler", func() { // Mock a call to GetOrCreateVMInstance and set the machine to running. mockCloudClient.EXPECT().GetOrCreateVMInstance( gomock.Any(), gomock.Any(), gomock.Any(), - gomock.Any(), gomock.Any(), gomock.Any()).Do( - func(arg1, _, _, _, _, _ interface{}) { + gomock.Any(), gomock.Any()).Do( + func(arg1, _, _, _, _ interface{}) { arg1.(*infrav1.CloudStackMachine).Status.InstanceState = "Running" }).AnyTimes() @@ -85,8 +85,8 @@ var _ = Describe("CloudStackMachineReconciler", func() { // Mock a call to GetOrCreateVMInstance and set the machine to running. mockCloudClient.EXPECT().GetOrCreateVMInstance( gomock.Any(), gomock.Any(), gomock.Any(), - gomock.Any(), gomock.Any(), gomock.Any()).Do( - func(arg1, _, _, _, _, _ interface{}) { + gomock.Any(), gomock.Any()).Do( + func(arg1, _, _, _, _ interface{}) { arg1.(*infrav1.CloudStackMachine).Status.InstanceState = "Running" controllerutil.AddFinalizer(arg1.(*infrav1.CloudStackMachine), infrav1.MachineFinalizer) }).AnyTimes() @@ -125,8 +125,8 @@ var _ = Describe("CloudStackMachineReconciler", func() { // Mock a call to GetOrCreateVMInstance and set the machine to running. mockCloudClient.EXPECT().GetOrCreateVMInstance( gomock.Any(), gomock.Any(), gomock.Any(), - gomock.Any(), gomock.Any(), gomock.Any()).Do( - func(arg1, _, _, _, _, _ interface{}) { + gomock.Any(), gomock.Any()).Do( + func(arg1, _, _, _, _ interface{}) { arg1.(*infrav1.CloudStackMachine).Status.InstanceState = "Running" controllerutil.AddFinalizer(arg1.(*infrav1.CloudStackMachine), infrav1.MachineFinalizer) }).AnyTimes() @@ -174,8 +174,8 @@ var _ = Describe("CloudStackMachineReconciler", func() { // Mock a call to GetOrCreateVMInstance and set the machine to running. mockCloudClient.EXPECT().GetOrCreateVMInstance( gomock.Any(), gomock.Any(), gomock.Any(), - gomock.Any(), gomock.Any(), gomock.Any()).Do( - func(arg1, _, _, _, _, userdata interface{}) { + gomock.Any(), gomock.Any()).Do( + func(arg1, _, _, _, userdata interface{}) { expectedUserdata := fmt.Sprintf("%s{{%s}}", dummies.CAPIMachine.Name, dummies.CSMachine1.Spec.FailureDomainName) Ω(userdata == expectedUserdata).Should(BeTrue()) arg1.(*infrav1.CloudStackMachine).Status.InstanceState = "Running" @@ -236,8 +236,8 @@ var _ = Describe("CloudStackMachineReconciler", func() { }) mockCloudClient.EXPECT().GetOrCreateVMInstance( gomock.Any(), gomock.Any(), gomock.Any(), - gomock.Any(), gomock.Any(), gomock.Any()).Do( - func(arg1, _, _, _, _, _ interface{}) { + gomock.Any(), gomock.Any()).Do( + func(arg1, _, _, _, _ interface{}) { arg1.(*infrav1.CloudStackMachine).Status.InstanceState = "Running" }).AnyTimes() Ω(fakeCtrlClient.Get(ctx, key, dummies.CSCluster)).Should(Succeed()) diff --git a/controllers/utils/utils.go b/controllers/utils/utils.go index cbf15c11..17f92c4e 100644 --- a/controllers/utils/utils.go +++ b/controllers/utils/utils.go @@ -205,7 +205,7 @@ func GetOwnerClusterName(obj metav1.ObjectMeta) (string, error) { // CloudStackClusterToCloudStackMachines is a handler.ToRequestsFunc to be used to enqeue requests for reconciliation // of CloudStackMachines. -func CloudStackClusterToCloudStackMachines(ctx context.Context, c client.Client, obj runtime.Object, scheme *runtime.Scheme, log logr.Logger) (handler.MapFunc, error) { +func CloudStackClusterToCloudStackMachines(c client.Client, obj runtime.Object, scheme *runtime.Scheme, log logr.Logger) (handler.MapFunc, error) { gvk, err := apiutil.GVKForObject(obj, scheme) if err != nil { return nil, errors.Wrap(err, "failed to find GVK for CloudStackMachine") diff --git a/pkg/cloud/affinity_groups_test.go b/pkg/cloud/affinity_groups_test.go index fa354b9a..11738874 100644 --- a/pkg/cloud/affinity_groups_test.go +++ b/pkg/cloud/affinity_groups_test.go @@ -142,7 +142,7 @@ var _ = Describe("AffinityGroup Unit Tests", func() { dummies.CSMachine1.Spec.DiskOffering.Name = "" Ω(client.GetOrCreateVMInstance( - dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "", + dummies.CSMachine1, dummies.CAPIMachine, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "", )).Should(Succeed()) Ω(client.GetOrCreateAffinityGroup(dummies.AffinityGroup)).Should(Succeed()) diff --git a/pkg/cloud/instance.go b/pkg/cloud/instance.go index fd5937e7..4bd01bae 100644 --- a/pkg/cloud/instance.go +++ b/pkg/cloud/instance.go @@ -35,7 +35,7 @@ import ( ) type VMIface interface { - GetOrCreateVMInstance(*infrav1.CloudStackMachine, *clusterv1.Machine, *infrav1.CloudStackCluster, *infrav1.CloudStackFailureDomain, *infrav1.CloudStackAffinityGroup, string) error + GetOrCreateVMInstance(*infrav1.CloudStackMachine, *clusterv1.Machine, *infrav1.CloudStackFailureDomain, *infrav1.CloudStackAffinityGroup, string) error ResolveVMInstanceDetails(*infrav1.CloudStackMachine) error DestroyVMInstance(*infrav1.CloudStackMachine) error } @@ -117,7 +117,6 @@ func (c *client) ResolveServiceOffering(csMachine *infrav1.CloudStackMachine, zo } func (c *client) ResolveTemplate( - csCluster *infrav1.CloudStackCluster, csMachine *infrav1.CloudStackMachine, zoneID string, ) (templateID string, retErr error) { @@ -211,7 +210,7 @@ func verifyDiskoffering(csMachine *infrav1.CloudStackMachine, c *client, diskOff } // CheckAccountLimits Checks the account's limit of VM, CPU & Memory -func (c *client) CheckAccountLimits(fd *infrav1.CloudStackFailureDomain, offering *cloudstack.ServiceOffering) error { +func (c *client) CheckAccountLimits(offering *cloudstack.ServiceOffering) error { if c.user.Account.CPUAvailable != "Unlimited" { cpuAvailable, err := strconv.ParseInt(c.user.Account.CPUAvailable, 10, 0) if err == nil && int64(offering.Cpunumber) > cpuAvailable { @@ -236,7 +235,7 @@ func (c *client) CheckAccountLimits(fd *infrav1.CloudStackFailureDomain, offerin } // CheckDomainLimits Checks the domain's limit of VM, CPU & Memory -func (c *client) CheckDomainLimits(fd *infrav1.CloudStackFailureDomain, offering *cloudstack.ServiceOffering) error { +func (c *client) CheckDomainLimits(offering *cloudstack.ServiceOffering) error { if c.user.Account.Domain.CPUAvailable != "Unlimited" { cpuAvailable, err := strconv.ParseInt(c.user.Account.Domain.CPUAvailable, 10, 0) if err == nil && int64(offering.Cpunumber) > cpuAvailable { @@ -262,15 +261,14 @@ func (c *client) CheckDomainLimits(fd *infrav1.CloudStackFailureDomain, offering // CheckLimits will check the account & domain limits func (c *client) CheckLimits( - fd *infrav1.CloudStackFailureDomain, offering *cloudstack.ServiceOffering, ) error { - err := c.CheckAccountLimits(fd, offering) + err := c.CheckAccountLimits(offering) if err != nil { return err } - err = c.CheckDomainLimits(fd, offering) + err = c.CheckDomainLimits(offering) if err != nil { return err } @@ -283,13 +281,12 @@ func (c *client) CheckLimits( func (c *client) DeployVM( csMachine *infrav1.CloudStackMachine, capiMachine *clusterv1.Machine, - csCluster *infrav1.CloudStackCluster, fd *infrav1.CloudStackFailureDomain, affinity *infrav1.CloudStackAffinityGroup, offering *cloudstack.ServiceOffering, userData string, ) error { - templateID, err := c.ResolveTemplate(csCluster, csMachine, fd.Spec.Zone.ID) + templateID, err := c.ResolveTemplate(csMachine, fd.Spec.Zone.ID) if err != nil { return err } @@ -363,7 +360,6 @@ func (c *client) DeployVM( func (c *client) GetOrCreateVMInstance( csMachine *infrav1.CloudStackMachine, capiMachine *clusterv1.Machine, - csCluster *infrav1.CloudStackCluster, fd *infrav1.CloudStackFailureDomain, affinity *infrav1.CloudStackAffinityGroup, userData string, @@ -379,12 +375,12 @@ func (c *client) GetOrCreateVMInstance( return err } - err = c.CheckLimits(fd, &offering) + err = c.CheckLimits(&offering) if err != nil { return err } - if err := c.DeployVM(csMachine, capiMachine, csCluster, fd, affinity, &offering, userData); err != nil { + if err := c.DeployVM(csMachine, capiMachine, fd, affinity, &offering, userData); err != nil { return err } diff --git a/pkg/cloud/instance_test.go b/pkg/cloud/instance_test.go index 952ad651..0b4454d1 100644 --- a/pkg/cloud/instance_test.go +++ b/pkg/cloud/instance_test.go @@ -131,14 +131,14 @@ var _ = Describe("Instance", func() { It("doesn't re-create if one already exists.", func() { vms.EXPECT().GetVirtualMachinesMetricByID(*dummies.CSMachine1.Spec.InstanceID).Return(vmMetricResp, -1, nil) Ω(client.GetOrCreateVMInstance( - dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). + dummies.CSMachine1, dummies.CAPIMachine, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). Should(Succeed()) }) It("returns unknown error while fetching VM instance", func() { vms.EXPECT().GetVirtualMachinesMetricByID(*dummies.CSMachine1.Spec.InstanceID).Return(nil, -1, unknownError) Ω(client.GetOrCreateVMInstance( - dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). + dummies.CSMachine1, dummies.CAPIMachine, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). Should(MatchError(unknownErrorMessage)) }) @@ -146,7 +146,7 @@ var _ = Describe("Instance", func() { expectVMNotFound() sos.EXPECT().GetServiceOfferingByName(dummies.CSMachine1.Spec.Offering.Name, gomock.Any()).Return(&cloudstack.ServiceOffering{}, -1, unknownError) Ω(client.GetOrCreateVMInstance( - dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). + dummies.CSMachine1, dummies.CAPIMachine, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). ShouldNot(Succeed()) }) @@ -157,7 +157,7 @@ var _ = Describe("Instance", func() { Name: dummies.CSMachine1.Spec.Offering.Name, }, 2, nil) Ω(client.GetOrCreateVMInstance( - dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). + dummies.CSMachine1, dummies.CAPIMachine, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). ShouldNot(Succeed()) }) @@ -172,7 +172,7 @@ var _ = Describe("Instance", func() { ts.EXPECT().GetTemplateID(dummies.CSMachine1.Spec.Template.Name, executableFilter, dummies.Zone1.ID). Return("", -1, unknownError) Ω(client.GetOrCreateVMInstance( - dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). + dummies.CSMachine1, dummies.CAPIMachine, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). ShouldNot(Succeed()) }) @@ -186,7 +186,7 @@ var _ = Describe("Instance", func() { }, 1, nil) ts.EXPECT().GetTemplateID(dummies.CSMachine1.Spec.Template.Name, executableFilter, dummies.Zone1.ID).Return("", 2, nil) Ω(client.GetOrCreateVMInstance( - dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). + dummies.CSMachine1, dummies.CAPIMachine, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). ShouldNot(Succeed()) }) @@ -201,7 +201,7 @@ var _ = Describe("Instance", func() { ts.EXPECT().GetTemplateID(dummies.CSMachine1.Spec.Template.Name, executableFilter, dummies.Zone1.ID).Return(dummies.CSMachine1.Spec.Template.ID, 1, nil) dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name, gomock.Any()).Return(diskOfferingFakeID, 2, nil) Ω(client.GetOrCreateVMInstance( - dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). + dummies.CSMachine1, dummies.CAPIMachine, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). ShouldNot(Succeed()) }) @@ -216,7 +216,7 @@ var _ = Describe("Instance", func() { dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name, gomock.Any()).Return(diskOfferingFakeID, 1, nil) dos.EXPECT().GetDiskOfferingByID(diskOfferingFakeID).Return(&cloudstack.DiskOffering{Iscustomized: false}, 1, unknownError) Ω(client.GetOrCreateVMInstance( - dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). + dummies.CSMachine1, dummies.CAPIMachine, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). ShouldNot(Succeed()) }) @@ -232,7 +232,7 @@ var _ = Describe("Instance", func() { dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name, gomock.Any()).Return(diskOfferingFakeID, 1, nil) dos.EXPECT().GetDiskOfferingByID(diskOfferingFakeID).Return(&cloudstack.DiskOffering{Iscustomized: false}, 1, nil) Ω(client.GetOrCreateVMInstance( - dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). + dummies.CSMachine1, dummies.CAPIMachine, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). ShouldNot(Succeed()) }) @@ -250,7 +250,7 @@ var _ = Describe("Instance", func() { dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name, gomock.Any()).Return(diskOfferingFakeID, 1, nil) dos.EXPECT().GetDiskOfferingByID(diskOfferingFakeID).Return(&cloudstack.DiskOffering{Iscustomized: true}, 1, nil) Ω(client.GetOrCreateVMInstance( - dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). + dummies.CSMachine1, dummies.CAPIMachine, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). ShouldNot(Succeed()) }) @@ -279,7 +279,7 @@ var _ = Describe("Instance", func() { } c := cloud.NewClientFromCSAPIClient(mockClient, user) Ω(c.GetOrCreateVMInstance( - dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). + dummies.CSMachine1, dummies.CAPIMachine, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). Should(MatchError(MatchRegexp("CPU available .* in account can't fulfil the requirement:.*"))) }) @@ -307,7 +307,7 @@ var _ = Describe("Instance", func() { } c := cloud.NewClientFromCSAPIClient(mockClient, user) Ω(c.GetOrCreateVMInstance( - dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). + dummies.CSMachine1, dummies.CAPIMachine, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). Should(MatchError(MatchRegexp("CPU available .* in domain can't fulfil the requirement:.*"))) }) @@ -335,7 +335,7 @@ var _ = Describe("Instance", func() { } c := cloud.NewClientFromCSAPIClient(mockClient, user) Ω(c.GetOrCreateVMInstance( - dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). + dummies.CSMachine1, dummies.CAPIMachine, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). Should(MatchError(MatchRegexp("memory available .* in account can't fulfil the requirement:.*"))) }) @@ -363,7 +363,7 @@ var _ = Describe("Instance", func() { } c := cloud.NewClientFromCSAPIClient(mockClient, user) Ω(c.GetOrCreateVMInstance( - dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). + dummies.CSMachine1, dummies.CAPIMachine, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). Should(MatchError(MatchRegexp("memory available .* in domain can't fulfil the requirement:.*"))) }) @@ -391,7 +391,7 @@ var _ = Describe("Instance", func() { } c := cloud.NewClientFromCSAPIClient(mockClient, user) Ω(c.GetOrCreateVMInstance( - dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). + dummies.CSMachine1, dummies.CAPIMachine, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). Should(MatchError("VM Limit in account has reached it's maximum value")) }) @@ -419,7 +419,7 @@ var _ = Describe("Instance", func() { } c := cloud.NewClientFromCSAPIClient(mockClient, user) Ω(c.GetOrCreateVMInstance( - dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). + dummies.CSMachine1, dummies.CAPIMachine, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). Should(MatchError("VM Limit in domain has reached it's maximum value")) }) }) @@ -445,7 +445,7 @@ var _ = Describe("Instance", func() { vms.EXPECT().NewListVirtualMachinesParams().Return(&cloudstack.ListVirtualMachinesParams{}) vms.EXPECT().ListVirtualMachines(gomock.Any()).Return(&cloudstack.ListVirtualMachinesResponse{}, nil) Ω(client.GetOrCreateVMInstance( - dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). + dummies.CSMachine1, dummies.CAPIMachine, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). Should(MatchError(unknownErrorMessage)) }) @@ -486,7 +486,7 @@ var _ = Describe("Instance", func() { }).Return(deploymentResp, nil) Ω(client.GetOrCreateVMInstance( - dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, expectUserData)). + dummies.CSMachine1, dummies.CAPIMachine, dummies.CSFailureDomain1, dummies.CSAffinityGroup, expectUserData)). Should(Succeed()) } @@ -628,7 +628,7 @@ var _ = Describe("Instance", func() { sos.EXPECT().GetServiceOfferingByID(dummies.CSMachine1.Spec.Offering.ID).Return(&cloudstack.ServiceOffering{Name: "offering-not-match"}, 1, nil) requiredRegexp := "offering name %s does not match name %s returned using UUID %s" Ω(client.GetOrCreateVMInstance( - dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). + dummies.CSMachine1, dummies.CAPIMachine, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). Should(MatchError(MatchRegexp(requiredRegexp, dummies.CSMachine1.Spec.Offering.Name, "offering-not-match", offeringFakeID))) }) @@ -642,7 +642,7 @@ var _ = Describe("Instance", func() { ts.EXPECT().GetTemplateByID(dummies.CSMachine1.Spec.Template.ID, executableFilter).Return(&cloudstack.Template{Name: "template-not-match"}, 1, nil) requiredRegexp := "template name %s does not match name %s returned using UUID %s" Ω(client.GetOrCreateVMInstance( - dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). + dummies.CSMachine1, dummies.CAPIMachine, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). Should(MatchError(MatchRegexp(requiredRegexp, dummies.CSMachine1.Spec.Template.Name, "template-not-match", templateFakeID))) }) @@ -659,7 +659,7 @@ var _ = Describe("Instance", func() { dos.EXPECT().GetDiskOfferingID(dummies.CSMachine1.Spec.DiskOffering.Name, gomock.Any()).Return(diskOfferingFakeID+"-not-match", 1, nil) requiredRegexp := "diskOffering ID %s does not match ID %s returned using name %s" Ω(client.GetOrCreateVMInstance( - dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). + dummies.CSMachine1, dummies.CAPIMachine, dummies.CSFailureDomain1, dummies.CSAffinityGroup, "")). Should(MatchError(MatchRegexp(requiredRegexp, dummies.CSMachine1.Spec.DiskOffering.ID, diskOfferingFakeID+"-not-match", dummies.CSMachine1.Spec.DiskOffering.Name))) }) }) @@ -724,7 +724,6 @@ var _ = Describe("Instance", func() { err := client.GetOrCreateVMInstance( dummies.CSMachine1, dummies.CAPIMachine, - dummies.CSCluster, dummies.CSFailureDomain1, dummies.CSAffinityGroup, expectUserData, diff --git a/pkg/cloud/isolated_network.go b/pkg/cloud/isolated_network.go index e8386c9d..4a93b3a9 100644 --- a/pkg/cloud/isolated_network.go +++ b/pkg/cloud/isolated_network.go @@ -31,14 +31,14 @@ type IsoNetworkIface interface { GetOrCreateIsolatedNetwork(*infrav1.CloudStackFailureDomain, *infrav1.CloudStackIsolatedNetwork, *infrav1.CloudStackCluster) error AssociatePublicIPAddress(*infrav1.CloudStackFailureDomain, *infrav1.CloudStackIsolatedNetwork, *infrav1.CloudStackCluster) error - GetOrCreateLoadBalancerRule(*infrav1.CloudStackFailureDomain, *infrav1.CloudStackIsolatedNetwork, *infrav1.CloudStackCluster) error + GetOrCreateLoadBalancerRule(*infrav1.CloudStackIsolatedNetwork, *infrav1.CloudStackCluster) error OpenFirewallRules(*infrav1.CloudStackIsolatedNetwork) error - GetPublicIP(*infrav1.CloudStackFailureDomain, *infrav1.CloudStackIsolatedNetwork, *infrav1.CloudStackCluster) (*cloudstack.PublicIpAddress, error) - ResolveLoadBalancerRuleDetails(*infrav1.CloudStackFailureDomain, *infrav1.CloudStackIsolatedNetwork, *infrav1.CloudStackCluster) error + GetPublicIP(*infrav1.CloudStackFailureDomain, *infrav1.CloudStackCluster) (*cloudstack.PublicIpAddress, error) + ResolveLoadBalancerRuleDetails(*infrav1.CloudStackIsolatedNetwork) error AssignVMToLoadBalancerRule(isoNet *infrav1.CloudStackIsolatedNetwork, instanceID string) error DeleteNetwork(infrav1.Network) error - DisposeIsoNetResources(*infrav1.CloudStackFailureDomain, *infrav1.CloudStackIsolatedNetwork, *infrav1.CloudStackCluster) error + DisposeIsoNetResources(*infrav1.CloudStackIsolatedNetwork, *infrav1.CloudStackCluster) error } // getOfferingID fetches an offering id. @@ -60,7 +60,7 @@ func (c *client) AssociatePublicIPAddress( csCluster *infrav1.CloudStackCluster, ) (retErr error) { // Check specified IP address is available or get an unused one if not specified. - publicAddress, err := c.GetPublicIP(fd, isoNet, csCluster) + publicAddress, err := c.GetPublicIP(fd, csCluster) if err != nil { return errors.Wrapf(err, "fetching a public IP address") } @@ -145,7 +145,6 @@ func (c *client) OpenFirewallRules(isoNet *infrav1.CloudStackIsolatedNetwork) (r // GetPublicIP gets a public IP with ID for cluster endpoint. func (c *client) GetPublicIP( fd *infrav1.CloudStackFailureDomain, - isoNet *infrav1.CloudStackIsolatedNetwork, csCluster *infrav1.CloudStackCluster, ) (*cloudstack.PublicIpAddress, error) { ip := csCluster.Spec.ControlPlaneEndpoint.Host @@ -201,9 +200,7 @@ func (c *client) GetIsolatedNetwork(isoNet *infrav1.CloudStackIsolatedNetwork) ( // ResolveLoadBalancerRuleDetails resolves the details of a load balancer rule by PublicIPID and Port. func (c *client) ResolveLoadBalancerRuleDetails( - fd *infrav1.CloudStackFailureDomain, isoNet *infrav1.CloudStackIsolatedNetwork, - csCluster *infrav1.CloudStackCluster, ) error { p := c.cs.LoadBalancer.NewListLoadBalancerRulesParams() p.SetPublicipid(isoNet.Status.PublicIPID) @@ -224,12 +221,11 @@ func (c *client) ResolveLoadBalancerRuleDetails( // GetOrCreateLoadBalancerRule Create a load balancer rule that can be assigned to instances. func (c *client) GetOrCreateLoadBalancerRule( - fd *infrav1.CloudStackFailureDomain, isoNet *infrav1.CloudStackIsolatedNetwork, csCluster *infrav1.CloudStackCluster, ) (retErr error) { // Check if rule exists. - if err := c.ResolveLoadBalancerRuleDetails(fd, isoNet, csCluster); err == nil || + if err := c.ResolveLoadBalancerRuleDetails(isoNet); err == nil || !strings.Contains(strings.ToLower(err.Error()), "no load balancer rule found") { return errors.Wrap(err, "resolving load balancer rule details") } @@ -290,7 +286,7 @@ func (c *client) GetOrCreateIsolatedNetwork( } // Setup a load balancing rule to map VMs to Public IP. - if err := c.GetOrCreateLoadBalancerRule(fd, isoNet, csCluster); err != nil { + if err := c.GetOrCreateLoadBalancerRule(isoNet, csCluster); err != nil { return errors.Wrap(err, "getting or creating load balancing rule") } } @@ -332,7 +328,6 @@ func (c *client) DeleteNetwork(net infrav1.Network) error { // DisposeIsoNetResources cleans up isolated network resources. func (c *client) DisposeIsoNetResources( - zone *infrav1.CloudStackFailureDomain, isoNet *infrav1.CloudStackIsolatedNetwork, csCluster *infrav1.CloudStackCluster, ) (retError error) { @@ -347,15 +342,12 @@ func (c *client) DisposeIsoNetResources( if err := c.RemoveClusterTagFromNetwork(csCluster, *isoNet.Network()); err != nil { return err } - if err := c.DeleteNetworkIfNotInUse(csCluster, *isoNet.Network()); err != nil { - return err - } - return nil + return c.DeleteNetworkIfNotInUse(*isoNet.Network()) } // DeleteNetworkIfNotInUse deletes an isolated network if the network is no longer in use (indicated by in use tags). -func (c *client) DeleteNetworkIfNotInUse(csCluster *infrav1.CloudStackCluster, net infrav1.Network) (retError error) { +func (c *client) DeleteNetworkIfNotInUse(net infrav1.Network) (retError error) { tags, err := c.GetTags(ResourceTypeNetwork, net.ID) if err != nil { return err diff --git a/pkg/cloud/isolated_network_test.go b/pkg/cloud/isolated_network_test.go index 358c0768..77d77f95 100644 --- a/pkg/cloud/isolated_network_test.go +++ b/pkg/cloud/isolated_network_test.go @@ -196,7 +196,7 @@ var _ = Describe("Network", func() { Count: 1, PublicIpAddresses: []*csapi.PublicIpAddress{{Id: "PublicIPID", Ipaddress: ipAddress}}, }, nil) - publicIPAddress, err := client.GetPublicIP(dummies.CSFailureDomain1, dummies.CSISONet1, dummies.CSCluster) + publicIPAddress, err := client.GetPublicIP(dummies.CSFailureDomain1, dummies.CSCluster) Ω(err).Should(Succeed()) Ω(publicIPAddress).ShouldNot(BeNil()) Ω(publicIPAddress.Ipaddress).Should(Equal(ipAddress)) @@ -211,7 +211,7 @@ var _ = Describe("Network", func() { Count: 0, PublicIpAddresses: []*csapi.PublicIpAddress{}, }, nil) - publicIPAddress, err := client.GetPublicIP(dummies.CSFailureDomain1, dummies.CSISONet1, dummies.CSCluster) + publicIPAddress, err := client.GetPublicIP(dummies.CSFailureDomain1, dummies.CSCluster) Ω(publicIPAddress).Should(BeNil()) Ω(err.Error()).Should(ContainSubstring("no public addresses found in available networks")) }) @@ -232,7 +232,7 @@ var _ = Describe("Network", func() { Associatednetworkid: "1", }}, }, nil) - publicIPAddress, err := client.GetPublicIP(dummies.CSFailureDomain1, dummies.CSISONet1, dummies.CSCluster) + publicIPAddress, err := client.GetPublicIP(dummies.CSFailureDomain1, dummies.CSCluster) Ω(publicIPAddress).Should(BeNil()) Ω(err.Error()).Should(ContainSubstring("all Public IP Address(es) found were already allocated")) }) @@ -285,7 +285,7 @@ var _ = Describe("Network", func() { {Publicport: strconv.Itoa(int(dummies.EndPointPort)), Id: dummies.LBRuleID}}}, nil) dummies.CSISONet1.Status.LBRuleID = "" - Ω(client.ResolveLoadBalancerRuleDetails(dummies.CSFailureDomain1, dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) + Ω(client.ResolveLoadBalancerRuleDetails(dummies.CSISONet1)).Should(Succeed()) Ω(dummies.CSISONet1.Status.LBRuleID).Should(Equal(dummies.LBRuleID)) }) @@ -296,7 +296,7 @@ var _ = Describe("Network", func() { {Publicport: "differentPublicPort", Id: dummies.LBRuleID}}}, nil) dummies.CSISONet1.Status.LBRuleID = "" - Ω(client.ResolveLoadBalancerRuleDetails(dummies.CSFailureDomain1, dummies.CSISONet1, dummies.CSCluster).Error()). + Ω(client.ResolveLoadBalancerRuleDetails(dummies.CSISONet1).Error()). Should(Equal("no load balancer rule found")) }) @@ -306,7 +306,7 @@ var _ = Describe("Network", func() { nil, fakeError) dummies.CSISONet1.Status.LBRuleID = "" - Ω(client.ResolveLoadBalancerRuleDetails(dummies.CSFailureDomain1, dummies.CSISONet1, dummies.CSCluster).Error()). + Ω(client.ResolveLoadBalancerRuleDetails(dummies.CSISONet1).Error()). Should(ContainSubstring("listing load balancer rules")) }) @@ -317,7 +317,7 @@ var _ = Describe("Network", func() { LoadBalancerRules: []*csapi.LoadBalancerRule{ {Publicport: strconv.Itoa(int(dummies.EndPointPort)), Id: dummies.LBRuleID}}}, nil) - Ω(client.GetOrCreateLoadBalancerRule(dummies.CSFailureDomain1, dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) + Ω(client.GetOrCreateLoadBalancerRule(dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) Ω(dummies.CSISONet1.Status.LBRuleID).Should(Equal(dummies.LBRuleID)) }) }) @@ -376,7 +376,7 @@ var _ = Describe("Network", func() { lbs.EXPECT().CreateLoadBalancerRule(gomock.Any()). Return(&csapi.CreateLoadBalancerRuleResponse{Id: "2ndLBRuleID"}, nil) - Ω(client.GetOrCreateLoadBalancerRule(dummies.CSFailureDomain1, dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) + Ω(client.GetOrCreateLoadBalancerRule(dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) Ω(dummies.CSISONet1.Status.LBRuleID).Should(Equal("2ndLBRuleID")) }) @@ -384,7 +384,7 @@ var _ = Describe("Network", func() { lbs.EXPECT().NewListLoadBalancerRulesParams().Return(&csapi.ListLoadBalancerRulesParams{}) lbs.EXPECT().ListLoadBalancerRules(gomock.Any()). Return(nil, fakeError) - err := client.GetOrCreateLoadBalancerRule(dummies.CSFailureDomain1, dummies.CSISONet1, dummies.CSCluster) + err := client.GetOrCreateLoadBalancerRule(dummies.CSISONet1, dummies.CSCluster) Ω(err).ShouldNot(Succeed()) Ω(err.Error()).Should(ContainSubstring(errorMessage)) }) @@ -398,7 +398,7 @@ var _ = Describe("Network", func() { Return(&csapi.CreateLoadBalancerRuleParams{}) lbs.EXPECT().CreateLoadBalancerRule(gomock.Any()). Return(nil, fakeError) - err := client.GetOrCreateLoadBalancerRule(dummies.CSFailureDomain1, dummies.CSISONet1, dummies.CSCluster) + err := client.GetOrCreateLoadBalancerRule(dummies.CSISONet1, dummies.CSCluster) Ω(err).ShouldNot(Succeed()) Ω(err.Error()).Should(Equal(errorMessage)) @@ -432,7 +432,7 @@ var _ = Describe("Network", func() { rs.EXPECT().ListTags(rtlp).Return(&csapi.ListTagsResponse{}, nil).Times(4) as.EXPECT().GetPublicIpAddressByID(dummies.CSISONet1.Status.PublicIPID).Return(&csapi.PublicIpAddress{}, 1, nil) - Ω(client.DisposeIsoNetResources(dummies.CSFailureDomain1, dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) + Ω(client.DisposeIsoNetResources(dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) }) It("delete all isolated network resources when managed by CAPC", func() { @@ -450,7 +450,7 @@ var _ = Describe("Network", func() { as.EXPECT().NewDisassociateIpAddressParams(dummies.CSISONet1.Status.PublicIPID).Return(dap) as.EXPECT().DisassociateIpAddress(dap).Return(&csapi.DisassociateIpAddressResponse{}, nil) - Ω(client.DisposeIsoNetResources(dummies.CSFailureDomain1, dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) + Ω(client.DisposeIsoNetResources(dummies.CSISONet1, dummies.CSCluster)).Should(Succeed()) }) It("disassociate IP address fails due to failure in deleting a resource i.e., disassociate Public IP", func() { @@ -467,7 +467,7 @@ var _ = Describe("Network", func() { as.EXPECT().NewDisassociateIpAddressParams(dummies.CSISONet1.Status.PublicIPID).Return(dap) as.EXPECT().DisassociateIpAddress(dap).Return(nil, fakeError) - Ω(client.DisposeIsoNetResources(dummies.CSFailureDomain1, dummies.CSISONet1, dummies.CSCluster)).ShouldNot(Succeed()) + Ω(client.DisposeIsoNetResources(dummies.CSISONet1, dummies.CSCluster)).ShouldNot(Succeed()) }) })