Fix misspellings

Fix a few comments to satisfy golint checker
Pull getAffinityGroupIDs function to reduce cyclomatic complexity of GetOrCreateVMInstance

api/v1beta1/cloudstackcluster_types.go

@@ -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/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/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

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/client.go

@@ -54,6 +54,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}
@@ -76,6 +77,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/instance.go

@@ -46,7 +46,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 +165,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 +200,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)

pkg/cloud/network.go

@@ -171,7 +171,7 @@ func (c *client) ResolvePublicIPDetails(csCluster *infrav1.CloudStackCluster) (*
 		// Ignore already allocated here since the IP was specified.
 		return publicAddresses.PublicIpAddresses[0], nil
 	} else if publicAddresses.Count > 0 { // Endpoint not specified.
-		for _, v := range publicAddresses.PublicIpAddresses { // Pick first availabe address.
+		for _, v := range publicAddresses.PublicIpAddresses { // Pick first available address.
 			if v.Allocated == "" { // Found un-allocated Public IP.
 				return v, nil
 			}