diff --git a/api/v1beta1/cloudstackcluster_types.go b/api/v1beta1/cloudstackcluster_types.go index 5eeec390..7b936014 100644 --- a/api/v1beta1/cloudstackcluster_types.go +++ b/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"` diff --git a/api/v1beta1/cloudstackmachine_types.go b/api/v1beta1/cloudstackmachine_types.go index c200fbc6..8ac9eaf9 100644 --- a/api/v1beta1/cloudstackmachine_types.go +++ b/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) { diff --git a/api/v1beta1/cloudstackmachinetemplate_types.go b/api/v1beta1/cloudstackmachinetemplate_types.go index d8b332df..197e6692 100644 --- a/api/v1beta1/cloudstackmachinetemplate_types.go +++ b/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 diff --git a/controllers/utils/utils.go b/controllers/utils/utils.go index a424a927..b47f84cf 100644 --- a/controllers/utils/utils.go +++ b/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 diff --git a/pkg/cloud/client.go b/pkg/cloud/client.go index 98d13ddd..9a9bf3c7 100644 --- a/pkg/cloud/client.go +++ b/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 diff --git a/pkg/cloud/instance.go b/pkg/cloud/instance.go index b50683f6..a00f1748 100644 --- a/pkg/cloud/instance.go +++ b/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 { diff --git a/pkg/cloud/instance_test.go b/pkg/cloud/instance_test.go index 9808517d..a5a59708 100644 --- a/pkg/cloud/instance_test.go +++ b/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,7 +138,7 @@ 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) @@ -145,7 +146,7 @@ var _ = Describe("Instance", func() { Ω(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) diff --git a/pkg/cloud/network.go b/pkg/cloud/network.go index 91273845..27c421a4 100644 --- a/pkg/cloud/network.go +++ b/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 }