Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix more lint errors from goreportcard #34

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/v1beta1/cloudstackcluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
11 changes: 4 additions & 7 deletions api/v1beta1/cloudstackmachine_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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"`
Expand All @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions api/v1beta1/cloudstackmachinetemplate_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion controllers/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions pkg/cloud/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand All @@ -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
Expand Down
56 changes: 37 additions & 19 deletions pkg/cloud/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {

Expand Down
5 changes: 3 additions & 2 deletions pkg/cloud/instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/cloud/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down