Fix more lint errors from goreportcard

Makefile

@@ -109,9 +109,10 @@ vet: ## Run go vet on the whole project.
 	go vet ./...
 
 .PHONY: lint
-lint: bin/golangci-lint generate-mocks ## Run linting for the project.
+lint: bin/golangci-lint bin/golint generate-mocks ## Run linting for the project.
 	go fmt ./...
 	go vet ./...
+	golint ./...
 	golangci-lint run -v --timeout 360s ./...
 	@ # The below string of commands checks that ginkgo isn't present in the controllers.
 	@(grep ginkgo ${PROJECT_DIR}/controllers/cloudstack*_controller.go && \
@@ -141,6 +142,8 @@ bin/ginkgo: ## Install ginkgo to bin.
 	GOBIN=$(PROJECT_DIR)/bin go install github.com/onsi/ginkgo/ginkgo@v1.16.5
 bin/mockgen:
 	GOBIN=$(PROJECT_DIR)/bin go install github.com/golang/mock/mockgen@v1.6.0
+bin/golint:
+	GOBIN=$(PROJECT_DIR)/bin go install golang.org/x/lint/golint
 bin/kustomize: ## Install kustomize to bin.
 	@mkdir -p bin
 	cd bin && curl -s "https://raw.githubusercontent.com/kubernetes-sigs/kustomize/master/hack/install_kustomize.sh"  | bash

api/v1beta1/cloudstackcluster_types.go

@@ -22,7 +22,7 @@ import (
 )
 
 const (
-	// The presence of a finalizer prevents CAPI from deleting the corresponding CAPI data.
+	// ClusterFinalizer prevents CAPI from deleting the corresponding CAPI data.
 	ClusterFinalizer       = "cloudstackcluster.infrastructure.cluster.x-k8s.io"
 	defaultIdentityRefKind = "Secret"
 )
@@ -63,7 +63,7 @@ type CloudStackClusterSpec struct {
 	IdentityRef *CloudStackIdentityReference `json:"identityRef,omitempty"`
 }
 
-// The status of the abstract CS k8s (not an actual Cloudstack Cluster) cluster.
+// CloudStackClusterStatus is the status of the abstract CS k8s (not an actual Cloudstack Cluster) cluster.
 type CloudStackClusterStatus struct {
 	// Reflects the readiness of the CS cluster.
 	Ready bool `json:"ready"`

api/v1beta1/cloudstackcluster_webhook.go

@@ -31,6 +31,7 @@ import (
 // log is for logging in this package.
 var cloudstackclusterlog = logf.Log.WithName("cloudstackcluster-resource")
 
+// SetupWebhookWithManager creates a new webhook managed by passed manager
 func (r *CloudStackCluster) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(r).

api/v1beta1/cloudstackmachine_types.go

@@ -28,7 +28,7 @@ import (
 )
 
 const (
-	// The presence of a finalizer prevents CAPI from deleting the corresponding CAPI data.
+	// MachineFinalizer prevents CAPI from deleting the corresponding CAPI data.
 	MachineFinalizer = "cloudstackmachine.infrastructure.cluster.x-k8s.io"
 )
 
@@ -69,17 +69,14 @@ type CloudStackMachineSpec struct {
 	IdentityRef *CloudStackIdentityReference `json:"identityRef,omitempty"`
 }
 
-// TODO: Review the use of this field/type.
-type InstanceState string
-
-// Type pulled mostly from the CloudStack API.
+// CloudStackMachineStatus type pulled mostly from the CloudStack API.
 type CloudStackMachineStatus struct {
 	// Addresses contains a CloudStack VM instance's IP addresses.
 	Addresses []corev1.NodeAddress `json:"addresses,omitempty"`
 
 	// InstanceState is the state of the CloudStack instance for this machine.
 	// +optional
-	InstanceState InstanceState `json:"instanceState,omitempty"`
+	InstanceState string `json:"instanceState,omitempty"`
 
 	// Ready indicates the readiness of the provider resource.
 	Ready bool `json:"ready"`
@@ -104,7 +101,7 @@ type CloudStackMachine struct {
 	Status CloudStackMachineStatus `json:"status,omitempty"`
 }
 
-// The computed affinity group name relevant to this machine.
+// AffinityGroupName generates the affinity group name relevant to this machine.
 func (csm CloudStackMachine) AffinityGroupName(
 	capiMachine *capiv1.Machine,
 ) (string, error) {

api/v1beta1/cloudstackmachine_webhook.go

@@ -32,6 +32,7 @@ import (
 // log is for logging in this package.
 var cloudstackmachinelog = logf.Log.WithName("cloudstackmachine-resource")
 
+// SetupWebhookWithManager creates a new webhook managed by passed manager
 func (r *CloudStackMachine) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(r).

api/v1beta1/cloudstackmachinetemplate_types.go

@@ -20,6 +20,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+// CloudStackMachineTemplateResource defines the desired spec of CloudStackMachine
 type CloudStackMachineTemplateResource struct {
 	// Standard object's metadata.
 	// More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata

api/v1beta1/cloudstackmachinetemplate_webhook.go

@@ -33,6 +33,7 @@ import (
 // log is for logging in this package.
 var cloudstackmachinetemplatelog = logf.Log.WithName("cloudstackmachinetemplate-resource")
 
+// SetupWebhookWithManager creates a new webhook managed by passed manager
 func (r *CloudStackMachineTemplate) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(r).

controllers/cloudstackcluster_controller.go

@@ -153,7 +153,7 @@ func (r *CloudStackClusterReconciler) reconcileDelete(
 	return ctrl.Result{}, nil
 }
 
-// Called in main, this registers the cluster reconciler to the CAPI controller manager.
+// SetupWithManager is called in main, this registers the cluster reconciler to the CAPI controller manager.
 func (r *CloudStackClusterReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	controller, err := ctrl.NewControllerManagedBy(mgr).
 		For(&infrav1.CloudStackCluster{}).

controllers/cloudstackmachine_controller.go

@@ -236,7 +236,7 @@ func (r *CloudStackMachineReconciler) reconcileDelete(
 	return ctrl.Result{}, nil
 }
 
-// Called in main, this registers the machine reconciler to the CAPI controller manager.
+// SetupWithManager is called in main, this registers the machine reconciler to the CAPI controller manager.
 func (r *CloudStackMachineReconciler) SetupWithManager(mgr ctrl.Manager) error {
 
 	controller, err := ctrl.NewControllerManagedBy(mgr).

controllers/utils/utils.go

@@ -89,7 +89,7 @@ func getKubeadmControlPlaneFromCAPIMachine(
 // IsOwnerDeleted returns a boolean if the owner of the CAPI machine has been deleted.
 func IsOwnerDeleted(ctx context.Context, client clientPkg.Client, capiMachine *capiv1.Machine) (bool, error) {
 	if util.IsControlPlaneMachine(capiMachine) {
-		// The controlplane sticks around after deletion pending the deletion of its machiens.
+		// The controlplane sticks around after deletion pending the deletion of its machines.
 		// As such, need to check the deletion timestamp thereof.
 		if cp, err := getKubeadmControlPlaneFromCAPIMachine(ctx, client, capiMachine); cp != nil && cp.DeletionTimestamp == nil {
 			return false, nil

pkg/cloud/affinity_groups.go

@@ -21,17 +21,14 @@ import (
 	"github.com/pkg/errors"
 )
 
-const (
-	AntiAffinityGroupType = "host anti-affinity"
-	AffinityGroupType     = "host affinity"
-)
-
+// AffinityGroup type
 type AffinityGroup struct {
 	Type string
 	Name string
 	ID   string
 }
 
+// AffinityGroupIface contains the collection of functions for AffinityGroup
 type AffinityGroupIface interface {
 	FetchAffinityGroup(*AffinityGroup) error
 	GetOrCreateAffinityGroup(*infrav1.CloudStackCluster, *AffinityGroup) error

pkg/cloud/affinity_groups_test.go

@@ -18,6 +18,7 @@ package cloud_test
 
 import (
 	"errors"
+
 	"github.com/apache/cloudstack-go/v2/cloudstack"
 	infrav1 "github.com/aws/cluster-api-provider-cloudstack/api/v1beta1"
 	"github.com/aws/cluster-api-provider-cloudstack/pkg/cloud"
@@ -47,7 +48,7 @@ var _ = Describe("AffinityGroup Unit Tests", func() {
 		client = cloud.NewClientFromCSAPIClient(mockClient)
 		fakeAG = &cloud.AffinityGroup{
 			Name: "FakeAffinityGroup",
-			Type: cloud.AffinityGroupType}
+			Type: "host affinity"}
 		cluster = &infrav1.CloudStackCluster{Spec: infrav1.CloudStackClusterSpec{
 			Zone: "Zone1", Network: "SharedGuestNet1"}}
 		cluster.ObjectMeta.SetUID("0")
@@ -89,7 +90,7 @@ var _ = Describe("AffinityGroup Unit Tests", func() {
 			if connectionErr != nil { // Only do these tests if an actual ACS instance is available via cloud-config.
 				Skip("Could not connect to ACS instance.")
 			}
-			arbitraryAG = &cloud.AffinityGroup{Name: "ArbitraryAffinityGroup", Type: cloud.AffinityGroupType}
+			arbitraryAG = &cloud.AffinityGroup{Name: "ArbitraryAffinityGroup", Type: "host affinity"}
 		})
 		AfterEach(func() {
 			mockCtrl.Finish()

pkg/cloud/client.go

@@ -20,22 +20,17 @@ import (
 	"strings"
 
 	"github.com/apache/cloudstack-go/v2/cloudstack"
-	infrav1 "github.com/aws/cluster-api-provider-cloudstack/api/v1beta1"
 	"github.com/pkg/errors"
 	"gopkg.in/ini.v1"
 )
 
 //go:generate mockgen -destination=../mocks/mock_client.go -package=mocks github.com/aws/cluster-api-provider-cloudstack/pkg/cloud Client
 
+// Client contains the collection of interfaces for CloudStack API
 type Client interface {
 	ClusterIface
 	VMIface
-	ResolveNetwork(*infrav1.CloudStackCluster) error
-	GetOrCreateNetwork(*infrav1.CloudStackCluster) error
-	OpenFirewallRules(*infrav1.CloudStackCluster) error
-	ResolvePublicIPDetails(*infrav1.CloudStackCluster) (*cloudstack.PublicIpAddress, error)
-	ResolveLoadBalancerRuleDetails(*infrav1.CloudStackCluster) error
-	GetOrCreateLoadBalancerRule(*infrav1.CloudStackCluster) error
+	NetworkIface
 	AffinityGroupIface
 	TagIface
 }
@@ -53,6 +48,7 @@ type config struct {
 	VerifySSL bool   `ini:"verify-ssl"`
 }
 
+// NewClient creates the CloudStack client interface using the cloud-config file
 func NewClient(ccPath string) (Client, error) {
 	c := &client{}
 	cfg := &config{VerifySSL: true}
@@ -73,6 +69,7 @@ func NewClient(ccPath string) (Client, error) {
 	return c, errors.Wrap(err, "Error encountered while checking CloudStack API Client connectivity.")
 }
 
+// NewClientFromCSAPIClient is used to test with a mock client
 func NewClientFromCSAPIClient(cs *cloudstack.CloudStackClient) Client {
 	c := &client{cs: cs}
 	return c

pkg/cloud/cluster.go

@@ -22,6 +22,7 @@ import (
 	"github.com/pkg/errors"
 )
 
+// ClusterIface contains the collection of functions for get/create/delete a cluster
 type ClusterIface interface {
 	GetOrCreateCluster(*infrav1.CloudStackCluster) error
 	DisposeClusterResources(cluster *infrav1.CloudStackCluster) error

pkg/cloud/cluster_test.go

@@ -17,6 +17,7 @@ package cloud_test
 
 import (
 	"fmt"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/apache/cloudstack-go/v2/cloudstack"

pkg/cloud/helpers.go

@@ -30,6 +30,7 @@ func setIfNotEmpty(str string, setFn set) {
 	}
 }
 
+// CompressAndEncodeString compresses and encodes a string
 func CompressAndEncodeString(str string) (string, error) {
 	buf := &bytes.Buffer{}
 	gzipWriter := gzip.NewWriter(buf)

pkg/cloud/helpers_test.go

@@ -33,7 +33,7 @@ import (
 )
 
 const (
-	FixturePath = "test/fixtures/cloud-config-files"
+	fixturePath = "test/fixtures/cloud-config-files"
 )
 
 var _ = Describe("Helpers", func() {
@@ -65,7 +65,7 @@ var _ = Describe("Helpers", func() {
 
 func getConfigPath(filename string) string {
 	dir, _ := os.Getwd()
-	return path.Join(dir, FixturePath, filename)
+	return path.Join(dir, fixturePath, filename)
 }
 
 // This matcher is used to make gomega matching compatible with gomock parameter matching.

pkg/cloud/instance.go

@@ -32,8 +32,13 @@ import (
 	"k8s.io/utils/pointer"
 )
 
-const antiAffinityValue = "anti"
+const (
+	antiAffinityGroupType = "host anti-affinity"
+	affinityGroupType     = "host affinity"
+	antiAffinityValue     = "anti"
+)
 
+// VMIface contains the collection of functions for get/create/delete a VM instance
 type VMIface interface {
 	GetOrCreateVMInstance(*infrav1.CloudStackMachine, *capiv1.Machine, *infrav1.CloudStackCluster, string) error
 	ResolveVMInstanceDetails(*infrav1.CloudStackMachine) error
@@ -46,7 +51,7 @@ func setMachineDataFromVMMetrics(vmResponse *cloudstack.VirtualMachinesMetric, c
 	csMachine.Spec.ProviderID = pointer.StringPtr(fmt.Sprintf("cloudstack:///%s", vmResponse.Id))
 	csMachine.Spec.InstanceID = pointer.StringPtr(vmResponse.Id)
 	csMachine.Status.Addresses = []corev1.NodeAddress{{Type: corev1.NodeInternalIP, Address: vmResponse.Ipaddress}}
-	csMachine.Status.InstanceState = infrav1.InstanceState(vmResponse.State)
+	csMachine.Status.InstanceState = vmResponse.State
 }
 
 // ResolveVMInstanceDetails Retrieves VM instance details by csMachine.Spec.InstanceID or csMachine.Name, and
@@ -165,27 +170,17 @@ func (c *client) GetOrCreateVMInstance(
 		return err
 	}
 	setIfNotEmpty(compressedAndEncodedUserData, p.SetUserdata)
-
-	if len(csMachine.Spec.AffinityGroupIds) > 0 {
-		p.SetAffinitygroupids(csMachine.Spec.AffinityGroupIds)
-	} else if strings.ToLower(csMachine.Spec.Affinity) != "no" && csMachine.Spec.Affinity != "" {
-		affinityType := AffinityGroupType
-		if strings.ToLower(csMachine.Spec.Affinity) == antiAffinityValue {
-			affinityType = AntiAffinityGroupType
-		}
-		name, err := csMachine.AffinityGroupName(capiMachine)
-		if err != nil {
-			return err
-		}
-		group := &AffinityGroup{Name: name, Type: affinityType}
-		if err := c.GetOrCreateAffinityGroup(csCluster, group); err != nil {
-			return err
-		}
-		p.SetAffinitygroupids([]string{group.ID})
-	}
 	setIfNotEmpty(csCluster.Spec.Account, p.SetAccount)
 	setIfNotEmpty(csCluster.Status.DomainID, p.SetDomainid)
 
+	affinityGroups, err := c.getAffinityGroupIDs(csMachine, capiMachine, csCluster)
+	if err != nil {
+		return err
+	}
+	if affinityGroups != nil {
+		p.SetAffinitygroupids(affinityGroups)
+	}
+
 	// If this VM instance is a control plane, consider setting its IP.
 	_, isControlPlanceMachine := capiMachine.ObjectMeta.Labels["cluster.x-k8s.io/control-plane"]
 	if isControlPlanceMachine && csCluster.Status.NetworkType == NetworkTypeShared {
@@ -210,6 +205,34 @@ func (c *client) GetOrCreateVMInstance(
 
 }
 
+func (c *client) getAffinityGroupIDs(
+	csMachine *infrav1.CloudStackMachine,
+	capiMachine *capiv1.Machine,
+	csCluster *infrav1.CloudStackCluster) ([]string, error) {
+
+	if len(csMachine.Spec.AffinityGroupIds) > 0 {
+		return csMachine.Spec.AffinityGroupIds, nil
+	}
+
+	affinity := strings.ToLower(csMachine.Spec.Affinity)
+	if affinity != "no" && affinity != "" {
+		affinityType := affinityGroupType
+		if affinity == antiAffinityValue {
+			affinityType = antiAffinityGroupType
+		}
+		name, err := csMachine.AffinityGroupName(capiMachine)
+		if err != nil {
+			return nil, err
+		}
+		group := &AffinityGroup{Name: name, Type: affinityType}
+		if err := c.GetOrCreateAffinityGroup(csCluster, group); err != nil {
+			return nil, err
+		}
+		return []string{group.ID}, nil
+	}
+	return nil, nil
+}
+
 // DestroyVMInstance Destroy a VM instance. Assumes machine has been fetched prior and has an instance ID.
 func (c *client) DestroyVMInstance(csMachine *infrav1.CloudStackMachine) error {
 

pkg/cloud/instance_test.go

@@ -18,6 +18,7 @@ package cloud_test
 
 import (
 	"fmt"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	capiv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -137,15 +138,15 @@ var _ = Describe("Instance", func() {
 			Ω(client.GetOrCreateVMInstance(csMachine, machine, csCluster, "")).Should(MatchError(unknownErrorMessage))
 		})
 
-		It("returns errors occuring while fetching sevice offering information", func() {
+		It("returns errors occurring while fetching service offering information", func() {
 			vms.EXPECT().GetVirtualMachinesMetricByID(*csMachine.Spec.InstanceID).Return(nil, -1, notFoundError)
 			vms.EXPECT().GetVirtualMachinesMetricByName(csMachine.Name).Return(nil, -1, notFoundError)
 			sos.EXPECT().GetServiceOfferingID(csMachine.Spec.Offering).Return("", -1, unknownError)
 			sos.EXPECT().GetServiceOfferingByID(csMachine.Spec.Offering).Return(nil, -1, unknownError)
 			Ω(client.GetOrCreateVMInstance(csMachine, machine, csCluster, "")).ShouldNot(Succeed())
 		})
 
-		It("returns errors if more than one sevice offering found", func() {
+		It("returns errors if more than one service offering found", func() {
 			vms.EXPECT().GetVirtualMachinesMetricByID(*csMachine.Spec.InstanceID).Return(nil, -1, notFoundError)
 			vms.EXPECT().GetVirtualMachinesMetricByName(csMachine.Name).Return(nil, -1, notFoundError)
 			sos.EXPECT().GetServiceOfferingID(csMachine.Spec.Offering).Return("", 2, nil)