From 114619934092be908a4fd42c65389e7dddb93986 Mon Sep 17 00:00:00 2001 From: Hans Rakers Date: Thu, 16 May 2024 13:28:41 +0200 Subject: [PATCH] chore(lint): Linting fixes --- .golangci.yml | 15 +++++++++++---- api/v1beta2/cloudstackmachine_types_test.go | 6 +++--- .../cloudstackfailuredomain_controller_test.go | 18 +++++++++--------- controllers/cloudstackmachine_controller.go | 6 +++--- .../cloudstackmachine_controller_test.go | 4 ++-- controllers/controllers_suite_test.go | 8 +++----- main.go | 8 ++------ pkg/cloud/instance.go | 12 ++++++------ pkg/cloud/instance_test.go | 12 ++++++------ test/dummies/v1beta1/vars.go | 6 +++--- test/dummies/v1beta2/vars.go | 6 +++--- test/dummies/v1beta3/vars.go | 6 +++--- 12 files changed, 54 insertions(+), 53 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 5cf6680a..a077f4d3 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -21,7 +21,14 @@ linters-settings: limitations under the License. gocyclo: min-complexity: 15 - + revive: + rules: + - name: dot-imports + arguments: + # dot import should be ONLY allowed for ginkgo testing packages + allowedPackages: + - "github.com/onsi/ginkgo/v2" + - "github.com/onsi/gomega" linters: enable: - gosec @@ -32,12 +39,12 @@ linters: run: issues-exit-code: 1 - skip-dirs: - - pkg/mocks - - test issues: # Excluding configuration per-path, per-linter, per-text and per-source + exclude-dirs: + - pkg/mocks + - test exclude-rules: # Exclude some linters from running on tests files. - path: _test\.go diff --git a/api/v1beta2/cloudstackmachine_types_test.go b/api/v1beta2/cloudstackmachine_types_test.go index 3d3750db..1186a79a 100644 --- a/api/v1beta2/cloudstackmachine_types_test.go +++ b/api/v1beta2/cloudstackmachine_types_test.go @@ -17,7 +17,7 @@ limitations under the License. package v1beta2_test import ( - "k8s.io/utils/pointer" + pointer "k8s.io/utils/ptr" capcv1 "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta2" . "github.com/onsi/ginkgo/v2" @@ -43,7 +43,7 @@ var _ = Describe("CloudStackMachineConfig_CompressUserdata", func() { Name: "is false when uncompressed user data is true", Machine: capcv1.CloudStackMachine{ Spec: capcv1.CloudStackMachineSpec{ - UncompressedUserData: pointer.Bool(true), + UncompressedUserData: pointer.To(true), }, }, Expect: false, @@ -52,7 +52,7 @@ var _ = Describe("CloudStackMachineConfig_CompressUserdata", func() { Name: "Is false when uncompressed user data is false", Machine: capcv1.CloudStackMachine{ Spec: capcv1.CloudStackMachineSpec{ - UncompressedUserData: pointer.Bool(false), + UncompressedUserData: pointer.To(false), }, }, Expect: true, diff --git a/controllers/cloudstackfailuredomain_controller_test.go b/controllers/cloudstackfailuredomain_controller_test.go index 3c3b6898..c7c4b155 100644 --- a/controllers/cloudstackfailuredomain_controller_test.go +++ b/controllers/cloudstackfailuredomain_controller_test.go @@ -21,7 +21,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/utils/pointer" + pointer "k8s.io/utils/ptr" infrav1 "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta3" "sigs.k8s.io/cluster-api-provider-cloudstack/pkg/cloud" dummies "sigs.k8s.io/cluster-api-provider-cloudstack/test/dummies/v1beta3" @@ -117,17 +117,17 @@ var _ = Describe("CloudStackFailureDomainReconciler", func() { }, // should delete - simulate owner is kubeadmcontrolplane - Entry("Should delete machine if spec.replicas > 1", true, pointer.Int32(2), pointer.Int32(2), pointer.Int32(2), pointer.Bool(true), true), + Entry("Should delete machine if spec.replicas > 1", true, pointer.To(int32(2)), pointer.To(int32(2)), pointer.To(int32(2)), pointer.To(true), true), // should delete - simulate owner is etcdadmcluster - Entry("Should delete machine if status.readyReplica does not exist", true, pointer.Int32(2), pointer.Int32(2), nil, pointer.Bool(true), true), + Entry("Should delete machine if status.readyReplica does not exist", true, pointer.To(int32(2)), pointer.To(int32(2)), nil, pointer.To(true), true), // should delete - simulate owner is machineset - Entry("Should delete machine if status.ready does not exist", true, pointer.Int32(2), pointer.Int32(2), pointer.Int32(2), nil, true), + Entry("Should delete machine if status.ready does not exist", true, pointer.To(int32(2)), pointer.To(int32(2)), pointer.To(int32(2)), nil, true), // should not delete if condition not met - Entry("Should not delete machine if cluster control plane not ready", false, pointer.Int32(2), pointer.Int32(2), pointer.Int32(2), pointer.Bool(true), false), - Entry("Should not delete machine if status.replicas < spec.replicas", false, pointer.Int32(2), pointer.Int32(1), pointer.Int32(1), pointer.Bool(true), true), - Entry("Should not delete machine if spec.replicas < 2", false, pointer.Int32(1), pointer.Int32(1), pointer.Int32(1), pointer.Bool(true), true), - Entry("Should not delete machine if status.ready is false", false, pointer.Int32(2), pointer.Int32(2), pointer.Int32(2), pointer.Bool(false), true), - Entry("Should not delete machine if status.readyReplicas <> status.replicas", false, pointer.Int32(2), pointer.Int32(2), pointer.Int32(1), pointer.Bool(true), true), + Entry("Should not delete machine if cluster control plane not ready", false, pointer.To(int32(2)), pointer.To(int32(2)), pointer.To(int32(2)), pointer.To(true), false), + Entry("Should not delete machine if status.replicas < spec.replicas", false, pointer.To(int32(2)), pointer.To(int32(1)), pointer.To(int32(1)), pointer.To(true), true), + Entry("Should not delete machine if spec.replicas < 2", false, pointer.To(int32(1)), pointer.To(int32(1)), pointer.To(int32(1)), pointer.To(true), true), + Entry("Should not delete machine if status.ready is false", false, pointer.To(int32(2)), pointer.To(int32(2)), pointer.To(int32(2)), pointer.To(false), true), + Entry("Should not delete machine if status.readyReplicas <> status.replicas", false, pointer.To(int32(2)), pointer.To(int32(2)), pointer.To(int32(1)), pointer.To(true), true), ) }) }) diff --git a/controllers/cloudstackmachine_controller.go b/controllers/cloudstackmachine_controller.go index 01617476..312be5b1 100644 --- a/controllers/cloudstackmachine_controller.go +++ b/controllers/cloudstackmachine_controller.go @@ -24,7 +24,7 @@ import ( "regexp" "time" - "k8s.io/utils/pointer" + pointer "k8s.io/utils/ptr" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" @@ -327,8 +327,8 @@ func (r *CloudStackMachineReconciliationRunner) ReconcileDelete() (retRes ctrl.R // ResolveVMInstanceDetails can get InstanceID by CS machine name err := r.CSClient.ResolveVMInstanceDetails(r.ReconciliationSubject) if err != nil { - r.ReconciliationSubject.Status.Status = pointer.String(metav1.StatusFailure) - r.ReconciliationSubject.Status.Reason = pointer.String(err.Error() + + r.ReconciliationSubject.Status.Status = pointer.To(metav1.StatusFailure) + r.ReconciliationSubject.Status.Reason = pointer.To(err.Error() + fmt.Sprintf(" If this VM has already been deleted, please remove the finalizer named %s from object %s", "cloudstackmachine.infrastructure.cluster.x-k8s.io", r.ReconciliationSubject.Name)) // Cloudstack VM may be not found or more than one found by name diff --git a/controllers/cloudstackmachine_controller_test.go b/controllers/cloudstackmachine_controller_test.go index 4cdbe405..83ef7be2 100644 --- a/controllers/cloudstackmachine_controller_test.go +++ b/controllers/cloudstackmachine_controller_test.go @@ -26,7 +26,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - "k8s.io/utils/pointer" + pointer "k8s.io/utils/ptr" infrav1 "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta3" dummies "sigs.k8s.io/cluster-api-provider-cloudstack/test/dummies/v1beta3" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" @@ -121,7 +121,7 @@ var _ = Describe("CloudStackMachineReconciler", func() { }) It("Should call ResolveVMInstanceDetails when CS machine without instanceID deleted", func() { - instanceID := pointer.String("instance-id-123") + instanceID := pointer.To("instance-id-123") // Mock a call to GetOrCreateVMInstance and set the machine to running. mockCloudClient.EXPECT().GetOrCreateVMInstance( gomock.Any(), gomock.Any(), gomock.Any(), diff --git a/controllers/controllers_suite_test.go b/controllers/controllers_suite_test.go index 0525e2ed..d36206f1 100644 --- a/controllers/controllers_suite_test.go +++ b/controllers/controllers_suite_test.go @@ -32,18 +32,16 @@ import ( "k8s.io/client-go/tools/record" "sigs.k8s.io/cluster-api-provider-cloudstack/test/fakes" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/klog/v2" - "k8s.io/klog/v2/klogr" - "github.com/apache/cloudstack-go/v2/cloudstack" "github.com/go-logr/logr" "github.com/golang/mock/gomock" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/pkg/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" + "k8s.io/klog/v2" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -154,7 +152,7 @@ var _ = BeforeSuite(func() { Ω(flag.Lookup("v").Value.Set("1")).Should(Succeed()) flag.Parse() - logger = klogr.New() + logger = klog.Background() }) // A mock fo the CloudClient interface used in controller utils. diff --git a/main.go b/main.go index 288bfa66..ed2fd083 100644 --- a/main.go +++ b/main.go @@ -19,10 +19,8 @@ package main import ( "context" "fmt" - "os" - "k8s.io/klog/v2" - "k8s.io/klog/v2/klogr" + "os" flag "github.com/spf13/pflag" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" @@ -67,8 +65,6 @@ var ( ) func init() { - klog.InitFlags(nil) - utilruntime.Must(clientgoscheme.AddToScheme(scheme)) utilruntime.Must(clusterv1.AddToScheme(scheme)) utilruntime.Must(infrav1b1.AddToScheme(scheme)) @@ -188,7 +184,7 @@ func main() { os.Exit(1) } - ctrl.SetLogger(klogr.New()) + ctrl.SetLogger(klog.Background()) tlsOptionOverrides, err := flags.GetTLSOptionOverrideFuncs(tlsOptions) if err != nil { diff --git a/pkg/cloud/instance.go b/pkg/cloud/instance.go index a18c39cf..1acdbce0 100644 --- a/pkg/cloud/instance.go +++ b/pkg/cloud/instance.go @@ -30,7 +30,7 @@ import ( "github.com/hashicorp/go-multierror" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" - "k8s.io/utils/pointer" + pointer "k8s.io/utils/ptr" infrav1 "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta3" ) @@ -42,9 +42,9 @@ type VMIface interface { // Set infrastructure spec and status from the CloudStack API's virtual machine metrics type. func setMachineDataFromVMMetrics(vmResponse *cloudstack.VirtualMachinesMetric, csMachine *infrav1.CloudStackMachine) { - csMachine.Spec.ProviderID = pointer.String(fmt.Sprintf("cloudstack:///%s", vmResponse.Id)) + csMachine.Spec.ProviderID = pointer.To(fmt.Sprintf("cloudstack:///%s", vmResponse.Id)) // InstanceID is later used as required parameter to destroy VM. - csMachine.Spec.InstanceID = pointer.String(vmResponse.Id) + csMachine.Spec.InstanceID = pointer.To(vmResponse.Id) csMachine.Status.Addresses = []corev1.NodeAddress{{Type: corev1.NodeInternalIP, Address: vmResponse.Ipaddress}} newInstanceState := vmResponse.State if newInstanceState != csMachine.Status.InstanceState || (newInstanceState != "" && csMachine.Status.InstanceStateLastUpdated.IsZero()) { @@ -351,14 +351,14 @@ func (c *client) DeployVM( return err } - csMachine.Spec.InstanceID = pointer.String(vm.Id) + csMachine.Spec.InstanceID = pointer.To(vm.Id) csMachine.Status.InstanceState = vm.State return fmt.Errorf("incomplete vm deployment (vm_id=%v): %w", vm.Id, err) } - csMachine.Spec.InstanceID = pointer.String(deployVMResp.Id) - csMachine.Status.Status = pointer.String(metav1.StatusSuccess) + csMachine.Spec.InstanceID = pointer.To(deployVMResp.Id) + csMachine.Status.Status = pointer.To(metav1.StatusSuccess) return nil } diff --git a/pkg/cloud/instance_test.go b/pkg/cloud/instance_test.go index 0b4454d1..07cc17df 100644 --- a/pkg/cloud/instance_test.go +++ b/pkg/cloud/instance_test.go @@ -30,7 +30,7 @@ import ( . "github.com/onsi/gomega" "github.com/pkg/errors" - "k8s.io/utils/pointer" + pointer "k8s.io/utils/ptr" ) var _ = Describe("Instance", func() { @@ -89,8 +89,8 @@ var _ = Describe("Instance", func() { vmsResp := &cloudstack.VirtualMachinesMetric{Id: *dummies.CSMachine1.Spec.InstanceID} vms.EXPECT().GetVirtualMachinesMetricByID(*dummies.CSMachine1.Spec.InstanceID).Return(vmsResp, 1, nil) Ω(client.ResolveVMInstanceDetails(dummies.CSMachine1)).Should(Succeed()) - Ω(dummies.CSMachine1.Spec.ProviderID).Should(Equal(pointer.String("cloudstack:///" + vmsResp.Id))) - Ω(dummies.CSMachine1.Spec.InstanceID).Should(Equal(pointer.String(vmsResp.Id))) + Ω(dummies.CSMachine1.Spec.ProviderID).Should(Equal(pointer.To("cloudstack:///" + vmsResp.Id))) + Ω(dummies.CSMachine1.Spec.InstanceID).Should(Equal(pointer.To(vmsResp.Id))) }) It("handles an unknown error when fetching by name", func() { @@ -115,8 +115,8 @@ var _ = Describe("Instance", func() { Ω(client.ResolveVMInstanceDetails(dummies.CSMachine1)).Should(Succeed()) Ω(dummies.CSMachine1.Spec.ProviderID).Should(Equal( - pointer.String(fmt.Sprintf("cloudstack:///%s", *dummies.CSMachine1.Spec.InstanceID)))) - Ω(dummies.CSMachine1.Spec.InstanceID).Should(Equal(pointer.String(*dummies.CSMachine1.Spec.InstanceID))) + pointer.To(fmt.Sprintf("cloudstack:///%s", *dummies.CSMachine1.Spec.InstanceID)))) + Ω(dummies.CSMachine1.Spec.InstanceID).Should(Equal(pointer.To(*dummies.CSMachine1.Spec.InstanceID))) }) }) @@ -670,7 +670,7 @@ var _ = Describe("Instance", func() { dummies.CSMachine1.Spec.Template.ID = "" dummies.CSMachine1.Spec.Offering.Name = "offering" dummies.CSMachine1.Spec.Template.Name = "template" - dummies.CSMachine1.Spec.UncompressedUserData = pointer.Bool(true) + dummies.CSMachine1.Spec.UncompressedUserData = pointer.To(true) vms.EXPECT(). GetVirtualMachinesMetricByID(*dummies.CSMachine1.Spec.InstanceID). diff --git a/test/dummies/v1beta1/vars.go b/test/dummies/v1beta1/vars.go index 06d51160..9edb69d2 100644 --- a/test/dummies/v1beta1/vars.go +++ b/test/dummies/v1beta1/vars.go @@ -8,7 +8,7 @@ import ( "github.com/smallfish/simpleyaml" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/pointer" + pointer "k8s.io/utils/ptr" capcv1 "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta1" "sigs.k8s.io/cluster-api-provider-cloudstack/pkg/cloud" capiv1 "sigs.k8s.io/cluster-api/api/v1beta1" @@ -199,7 +199,7 @@ func SetDummyCSMachineVars() { Kind: "Secret", Name: "IdentitySecret", }, - InstanceID: pointer.String("Instance1"), + InstanceID: pointer.To("Instance1"), Template: capcv1.CloudStackResourceIdentifier{ Name: GetYamlVal("CLOUDSTACK_TEMPLATE_NAME"), }, @@ -344,7 +344,7 @@ func SetClusterSpecToNet(net *capcv1.Network) { func SetDummyCAPIMachineVars() { CAPIMachine = &capiv1.Machine{ - Spec: capiv1.MachineSpec{FailureDomain: pointer.String(Zone1.ID)}, + Spec: capiv1.MachineSpec{FailureDomain: pointer.To(Zone1.ID)}, } } diff --git a/test/dummies/v1beta2/vars.go b/test/dummies/v1beta2/vars.go index 6e7d3d47..1619517a 100644 --- a/test/dummies/v1beta2/vars.go +++ b/test/dummies/v1beta2/vars.go @@ -8,7 +8,7 @@ import ( "github.com/smallfish/simpleyaml" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/pointer" + pointer "k8s.io/utils/ptr" infrav1 "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta2" "sigs.k8s.io/cluster-api-provider-cloudstack/pkg/cloud" "sigs.k8s.io/cluster-api-provider-cloudstack/test/fakes" @@ -228,7 +228,7 @@ func SetDummyCSMachineVars() { }, Spec: infrav1.CloudStackMachineSpec{ Name: "test-machine-1", - InstanceID: pointer.String("Instance1"), + InstanceID: pointer.To("Instance1"), FailureDomainName: GetYamlVal("CLOUDSTACK_FD1_NAME"), Template: infrav1.CloudStackResourceIdentifier{ Name: GetYamlVal("CLOUDSTACK_TEMPLATE_NAME"), @@ -451,7 +451,7 @@ func SetDummyCAPIMachineVars() { }, Spec: clusterv1.MachineSpec{ ClusterName: ClusterName, - FailureDomain: pointer.String("fd1"), + FailureDomain: pointer.To("fd1"), InfrastructureRef: corev1.ObjectReference{ APIVersion: infrav1.GroupVersion.String(), Kind: "CloudStackMachine", diff --git a/test/dummies/v1beta3/vars.go b/test/dummies/v1beta3/vars.go index 2c950123..a5281770 100644 --- a/test/dummies/v1beta3/vars.go +++ b/test/dummies/v1beta3/vars.go @@ -8,7 +8,7 @@ import ( "github.com/smallfish/simpleyaml" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/pointer" + pointer "k8s.io/utils/ptr" infrav1 "sigs.k8s.io/cluster-api-provider-cloudstack/api/v1beta3" "sigs.k8s.io/cluster-api-provider-cloudstack/pkg/cloud" "sigs.k8s.io/cluster-api-provider-cloudstack/test/fakes" @@ -224,7 +224,7 @@ func SetDummyCSMachineVars() { }, Spec: infrav1.CloudStackMachineSpec{ Name: "test-machine-1", - InstanceID: pointer.String("Instance1"), + InstanceID: pointer.To("Instance1"), FailureDomainName: GetYamlVal("CLOUDSTACK_FD1_NAME"), Template: infrav1.CloudStackResourceIdentifier{ Name: GetYamlVal("CLOUDSTACK_TEMPLATE_NAME"), @@ -447,7 +447,7 @@ func SetDummyCAPIMachineVars() { }, Spec: clusterv1.MachineSpec{ ClusterName: ClusterName, - FailureDomain: pointer.String("fd1"), + FailureDomain: pointer.To("fd1"), InfrastructureRef: corev1.ObjectReference{ APIVersion: infrav1.GroupVersion.String(), Kind: "CloudStackMachine",