Updated cloud tests commit to reflect file move.

Author: Joshua Reed

pkg/cloud/client.go

@@ -54,11 +54,11 @@ type Config struct {
 func NewClient(ccPath string) (Client, error) {
 	c := &client{config: Config{VerifySSL: true}}
 	if rawCfg, err := ini.Load(ccPath); err != nil {
-		return nil, errors.Wrapf(err, "reading config at path %s:", ccPath)
+		return nil, errors.Wrapf(err, "reading config at path %s", ccPath)
 	} else if g := rawCfg.Section("Global"); len(g.Keys()) == 0 {
 		return nil, errors.New("section Global not found")
 	} else if err = rawCfg.Section("Global").StrictMapTo(&c.config); err != nil {
-		return nil, errors.Wrapf(err, "parsing [Global] section from config at path %s:", ccPath)
+		return nil, errors.Wrapf(err, "parsing [Global] section from config at path %s", ccPath)
 	}
 
 	// The client returned from NewAsyncClient works in a synchronous way. On the other hand,
@@ -71,7 +71,7 @@ func NewClient(ccPath string) (Client, error) {
 	if err != nil && strings.Contains(strings.ToLower(err.Error()), "i/o timeout") {
 		return c, errors.Wrap(err, "timeout while checking CloudStack API Client connectivity")
 	}
-	return c, errors.Wrap(err, "checking CloudStack API Client connectivity:")
+	return c, errors.Wrap(err, "checking CloudStack API Client connectivity")
 }
 
 // NewClientFromSpec generates a new client from an existing client.
@@ -94,7 +94,7 @@ func (origC *client) NewClientFromSpec(cfg Config) (Client, error) {
 	if err != nil && strings.Contains(strings.ToLower(err.Error()), "i/o timeout") {
 		return newC, errors.Wrap(err, "timeout while checking CloudStack API Client connectivity")
 	}
-	return newC, errors.Wrap(err, "checking CloudStack API Client connectivity:")
+	return newC, errors.Wrap(err, "checking CloudStack API Client connectivity")
 }
 
 func NewClientFromCSAPIClient(cs *cloudstack.CloudStackClient) Client {

pkg/cloud/helpers.go

@@ -24,6 +24,7 @@ import (
 
 type set func(string)
 type setArray func([]string)
+type setInt func(int64)
 
 func setIfNotEmpty(str string, setFn set) {
 	if str != "" {
@@ -37,6 +38,12 @@ func setArrayIfNotEmpty(strArray []string, setFn setArray) {
 	}
 }
 
+func setIntIfPositive(num int64, setFn setInt) {
+	if num > 0 {
+		setFn(num)
+	}
+}
+
 func CompressAndEncodeString(str string) (string, error) {
 	buf := &bytes.Buffer{}
 	gzipWriter := gzip.NewWriter(buf)

pkg/cloud/instance.go

@@ -139,35 +139,55 @@ func (c *client) ResolveTemplate(
 // disk offering name matches name provided in spec.
 // If disk offering ID is not provided, the disk offering name is used to retrieve disk offering ID.
 func (c *client) ResolveDiskOffering(csMachine *infrav1.CloudStackMachine) (diskOfferingID string, retErr error) {
-	if len(csMachine.Spec.DiskOffering.ID) > 0 {
-		csDiskOffering, count, err := c.cs.DiskOffering.GetDiskOfferingByID(csMachine.Spec.DiskOffering.ID)
+	diskOfferingID = csMachine.Spec.DiskOffering.ID
+	if len(csMachine.Spec.DiskOffering.Name) > 0 {
+		diskID, count, err := c.cs.DiskOffering.GetDiskOfferingID(csMachine.Spec.DiskOffering.Name)
 		if err != nil {
 			return "", multierror.Append(retErr, errors.Wrapf(
-				err, "could not get DiskOffering by ID %s", csMachine.Spec.DiskOffering.ID))
+				err, "could not get DiskOffering ID from %s", csMachine.Spec.DiskOffering.Name))
 		} else if count != 1 {
 			return "", multierror.Append(retErr, errors.Errorf(
-				"expected 1 DiskOffering with UUID %s, but got %d", csMachine.Spec.DiskOffering.ID, count))
-		}
-
-		if len(csMachine.Spec.DiskOffering.Name) > 0 && csMachine.Spec.DiskOffering.Name != csDiskOffering.Name {
+				"expected 1 DiskOffering with name %s, but got %d", csMachine.Spec.DiskOffering.Name, count))
+		} else if len(csMachine.Spec.DiskOffering.ID) > 0 && diskID != csMachine.Spec.DiskOffering.ID {
+			return "", multierror.Append(retErr, errors.Errorf(
+				"diskOffering ID %s does not match ID %s returned using name %s",
+				csMachine.Spec.DiskOffering.ID, diskID, csMachine.Spec.DiskOffering.Name))
+		} else if len(diskID) == 0 {
 			return "", multierror.Append(retErr, errors.Errorf(
-				"diskOffering name %s does not match name %s returned using UUID %s",
-				csMachine.Spec.DiskOffering.Name, csDiskOffering.Name, csMachine.Spec.DiskOffering.ID))
+				"empty diskOffering ID %s returned using name %s",
+				diskID, csMachine.Spec.DiskOffering.Name))
 		}
-		return csMachine.Spec.DiskOffering.ID, nil
+		diskOfferingID = diskID
 	}
-	if len(csMachine.Spec.DiskOffering.Name) == 0 {
+	if len(diskOfferingID) == 0 {
 		return "", nil
 	}
-	diskID, count, err := c.cs.DiskOffering.GetDiskOfferingID(csMachine.Spec.DiskOffering.Name)
+
+	return verifyDiskoffering(csMachine, c, diskOfferingID, retErr)
+}
+
+func verifyDiskoffering(csMachine *infrav1.CloudStackMachine, c *client, diskOfferingID string, retErr error) (string, error) {
+	csDiskOffering, count, err := c.cs.DiskOffering.GetDiskOfferingByID(diskOfferingID)
 	if err != nil {
 		return "", multierror.Append(retErr, errors.Wrapf(
-			err, "could not get DiskOffering ID from %s", csMachine.Spec.DiskOffering.Name))
+			err, "could not get DiskOffering by ID %s", diskOfferingID))
 	} else if count != 1 {
 		return "", multierror.Append(retErr, errors.Errorf(
-			"expected 1 DiskOffering with name %s, but got %d", csMachine.Spec.DiskOffering.Name, count))
+			"expected 1 DiskOffering with UUID %s, but got %d", diskOfferingID, count))
+	}
+
+	if csDiskOffering.Iscustomized && csMachine.Spec.DiskOffering.CustomSize == 0 {
+		return "", multierror.Append(retErr, errors.Errorf(
+			"diskOffering with UUID %s is customized, disk size can not be 0 GB",
+			diskOfferingID))
+	}
+
+	if !csDiskOffering.Iscustomized && csMachine.Spec.DiskOffering.CustomSize > 0 {
+		return "", multierror.Append(retErr, errors.Errorf(
+			"diskOffering with UUID %s is not customized, disk size can not be specified",
+			diskOfferingID))
 	}
-	return diskID, nil
+	return diskOfferingID, nil
 }
 
 // GetOrCreateVMInstance CreateVMInstance will fetch or create a VM instance, and
@@ -205,6 +225,7 @@ func (c *client) GetOrCreateVMInstance(
 	setIfNotEmpty(csMachine.Name, p.SetName)
 	setIfNotEmpty(csMachine.Name, p.SetDisplayname)
 	setIfNotEmpty(diskOfferingID, p.SetDiskofferingid)
+	setIntIfPositive(csMachine.Spec.DiskOffering.CustomSize, p.SetSize)
 
 	setIfNotEmpty(csMachine.Spec.SSHKey, p.SetKeypair)
 

pkg/cloud/isolated_network.go

@@ -22,7 +22,6 @@ import (
 
 	"github.com/apache/cloudstack-go/v2/cloudstack"
 	capcv1 "github.com/aws/cluster-api-provider-cloudstack/api/v1beta1"
-	infrav1 "github.com/aws/cluster-api-provider-cloudstack/api/v1beta1"
 	"github.com/hashicorp/go-multierror"
 	"github.com/pkg/errors"
 )
@@ -38,7 +37,7 @@ type IsoNetworkIface interface {
 
 	AssignVMToLoadBalancerRule(isoNet *capcv1.CloudStackIsolatedNetwork, instanceID string) error
 	DeleteNetwork(capcv1.Network) error
-	DisposeIsoNetResources(*capcv1.CloudStackZone, *infrav1.CloudStackIsolatedNetwork, *infrav1.CloudStackCluster) error
+	DisposeIsoNetResources(*capcv1.CloudStackZone, *capcv1.CloudStackIsolatedNetwork, *capcv1.CloudStackCluster) error
 }
 
 // getOfferingID fetches an offering id.
@@ -61,7 +60,7 @@ func (c *client) AssociatePublicIPAddress(
 	// Check specified IP address is available or get an unused one if not specified.
 	publicAddress, err := c.GetPublicIP(zone, isoNet, csCluster)
 	if err != nil {
-		return errors.Wrapf(err, "fetching a public IP address:")
+		return errors.Wrapf(err, "fetching a public IP address")
 	}
 	isoNet.Spec.ControlPlaneEndpoint.Host = publicAddress.Ipaddress
 	csCluster.Spec.ControlPlaneEndpoint.Host = publicAddress.Ipaddress
@@ -78,14 +77,14 @@ func (c *client) AssociatePublicIPAddress(
 	p.SetNetworkid(isoNet.Spec.ID)
 	if _, err := c.cs.Address.AssociateIpAddress(p); err != nil {
 		return errors.Wrapf(err,
-			"associating public IP address with ID %s to netowrk with ID %s:",
+			"associating public IP address with ID %s to network with ID %s",
 			publicAddress.Id, isoNet.Spec.ID)
 	} else if err := c.AddClusterTag(ResourceTypeIPAddress, publicAddress.Id, csCluster); err != nil {
 		return errors.Wrapf(err,
-			"adding tag to public IP address with ID %s:", publicAddress.Id)
+			"adding tag to public IP address with ID %s", publicAddress.Id)
 	} else if err := c.AddCreatedByCAPCTag(ResourceTypeIPAddress, isoNet.Status.PublicIPID); err != nil {
 		return errors.Wrapf(err,
-			"adding tag to public IP address with ID %s:", publicAddress.Id)
+			"adding tag to public IP address with ID %s", publicAddress.Id)
 	}
 	return nil
 }
@@ -102,7 +101,7 @@ func (c *client) CreateIsolatedNetwork(zone *capcv1.CloudStackZone, isoNet *capc
 	p := c.cs.Network.NewCreateNetworkParams(isoNet.Spec.Name, isoNet.Spec.Name, offeringID, zone.Spec.ID)
 	resp, err := c.cs.Network.CreateNetwork(p)
 	if err != nil {
-		return errors.Wrapf(err, "creating network with name %s:", isoNet.Spec.Name)
+		return errors.Wrapf(err, "creating network with name %s", isoNet.Spec.Name)
 	}
 	isoNet.Spec.ID = resp.Id
 	return c.AddCreatedByCAPCTag(ResourceTypeNetwork, isoNet.Spec.ID)
@@ -186,7 +185,7 @@ func (c *client) ResolveLoadBalancerRuleDetails(
 	p.SetPublicipid(isoNet.Status.PublicIPID)
 	loadBalancerRules, err := c.cs.LoadBalancer.ListLoadBalancerRules(p)
 	if err != nil {
-		return errors.Wrap(err, "listing load balancer rules:")
+		return errors.Wrap(err, "listing load balancer rules")
 	}
 	for _, rule := range loadBalancerRules.LoadBalancerRules {
 		if rule.Publicport == strconv.Itoa(int(isoNet.Spec.ControlPlaneEndpoint.Port)) {
@@ -218,7 +217,7 @@ func (c *client) GetOrCreateLoadBalancerRule(
 	// Check if rule exists.
 	if err := c.ResolveLoadBalancerRuleDetails(zone, isoNet, csCluster); err == nil ||
 		!strings.Contains(strings.ToLower(err.Error()), "no load balancer rule found") {
-		return errors.Wrap(err, "resolving load balancer rule details:")
+		return errors.Wrap(err, "resolving load balancer rule details")
 	}
 
 	p := c.cs.LoadBalancer.NewCreateLoadBalancerRuleParams(
@@ -240,35 +239,35 @@ func (c *client) GetOrCreateLoadBalancerRule(
 func (c *client) GetOrCreateIsolatedNetwork(
 	zone *capcv1.CloudStackZone,
 	isoNet *capcv1.CloudStackIsolatedNetwork,
-	csCluster *infrav1.CloudStackCluster,
+	csCluster *capcv1.CloudStackCluster,
 ) error {
 	// Get or create the isolated network itself and resolve details into passed custom resources.
 	net := isoNet.Network()
 	if err := c.ResolveNetwork(net); err != nil { // Doesn't exist, create isolated network.
 		if err = c.CreateIsolatedNetwork(zone, isoNet); err != nil {
-			return errors.Wrap(err, "creating a new isolated network:")
+			return errors.Wrap(err, "creating a new isolated network")
 		}
 	}
 	isoNet.Spec.ID = net.ID
 
 	// Tag the created network.
 	networkID := isoNet.Spec.ID
 	if err := c.AddClusterTag(ResourceTypeNetwork, networkID, csCluster); err != nil {
-		return errors.Wrapf(err, "tagging network with id %s:", networkID)
+		return errors.Wrapf(err, "tagging network with id %s", networkID)
 	}
 
 	// Associate Public IP with CloudStackIsolatedNetwork
 	if err := c.AssociatePublicIPAddress(zone, isoNet, csCluster); err != nil {
-		return errors.Wrapf(err, "associating public IP address to csCluster:")
+		return errors.Wrapf(err, "associating public IP address to csCluster")
 	}
 
 	// Setup a load balancing rule to map VMs to Public IP.
 	if err := c.GetOrCreateLoadBalancerRule(zone, isoNet, csCluster); err != nil {
-		return errors.Wrap(err, "getting or creating load balancing rule:")
+		return errors.Wrap(err, "getting or creating load balancing rule")
 	}
 
 	//  Open the Isolated Network on endopint port.
-	return errors.Wrap(c.OpenFirewallRules(isoNet), "opening the isolated network's firewall:")
+	return errors.Wrap(c.OpenFirewallRules(isoNet), "opening the isolated network's firewall")
 }
 
 // AssignVMToLoadBalancerRule assigns a VM instance to a load balancing rule (specifying lb membership).
@@ -296,14 +295,14 @@ func (c *client) AssignVMToLoadBalancerRule(isoNet *capcv1.CloudStackIsolatedNet
 // DeleteNetwork deletes an isolated network.
 func (c *client) DeleteNetwork(net capcv1.Network) error {
 	_, err := c.cs.Network.DeleteNetwork(c.cs.Network.NewDeleteNetworkParams(net.ID))
-	return errors.Wrapf(err, "deleting network with id %s:", net.ID)
+	return errors.Wrapf(err, "deleting network with id %s", net.ID)
 }
 
 // DisposeIsoNetResources cleans up isolated network resources.
 func (c *client) DisposeIsoNetResources(
-	zone *infrav1.CloudStackZone,
-	isoNet *infrav1.CloudStackIsolatedNetwork,
-	csCluster *infrav1.CloudStackCluster,
+	zone *capcv1.CloudStackZone,
+	isoNet *capcv1.CloudStackIsolatedNetwork,
+	csCluster *capcv1.CloudStackCluster,
 ) (retError error) {
 	if isoNet.Status.PublicIPID != "" {
 		if err := c.DeleteClusterTag(ResourceTypeIPAddress, csCluster.Status.PublicIPID, csCluster); err != nil {

pkg/cloud/tags.go

@@ -58,7 +58,7 @@ func (c *client) IsCapcManaged(resourceType ResourceType, resourceID string) (bo
 	tags, err := c.GetTags(resourceType, resourceID)
 	if err != nil {
 		return false, errors.Wrapf(err,
-			"checking if %s with ID: %s is tagged as CAPC managed:", resourceType, resourceID)
+			"checking if %s with ID: %s is tagged as CAPC managed", resourceType, resourceID)
 	}
 	_, CreatedByCAPC := tags[CreatedByCAPCTagName]
 	return CreatedByCAPC, nil

pkg/cloud/user_credentials.go

@@ -74,8 +74,8 @@ func (c *client) ResolveDomain(domain *Domain) error {
 			tokens = append([]string{rootDomain}, tokens...)
 		} else {
 			tokens[0] = rootDomain
-			domain.Path = strings.Join(tokens, domainDelimiter)
 		}
+		domain.Path = strings.Join(tokens, domainDelimiter)
 	}
 
 	// Set present search/list parameters.
@@ -130,7 +130,7 @@ func (c *client) ResolveDomain(domain *Domain) error {
 func (c *client) ResolveAccount(account *Account) error {
 	// Resolve domain prior to any account resolution activity.
 	if err := c.ResolveDomain(&account.Domain); err != nil {
-		return errors.Wrapf(err, "resolving domain %s details:", account.Domain.Name)
+		return errors.Wrapf(err, "resolving domain %s details", account.Domain.Name)
 	}
 
 	p := c.cs.Account.NewListAccountsParams()

test/unit/cloud/affinity_groups_test.go

@@ -0,0 +1,99 @@
+/*
+Copyright 2022 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package cloud_test
+
+import (
+	"errors"
+
+	"github.com/apache/cloudstack-go/v2/cloudstack"
+	"github.com/aws/cluster-api-provider-cloudstack/pkg/cloud"
+	"github.com/aws/cluster-api-provider-cloudstack/test/dummies"
+	"github.com/golang/mock/gomock"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("AffinityGroup Unit Tests", func() {
+	var ( // Declare shared vars.
+		mockCtrl   *gomock.Controller
+		mockClient *cloudstack.CloudStackClient
+		ags        *cloudstack.MockAffinityGroupServiceIface
+		client     cloud.Client
+	)
+
+	BeforeEach(func() {
+		// Setup new mock services.
+		mockCtrl = gomock.NewController(GinkgoT())
+		mockClient = cloudstack.NewMockClient(mockCtrl)
+		ags = mockClient.AffinityGroup.(*cloudstack.MockAffinityGroupServiceIface)
+		client = cloud.NewClientFromCSAPIClient(mockClient)
+		dummies.SetDummyVars()
+	})
+
+	AfterEach(func() {
+		mockCtrl.Finish()
+	})
+
+	It("fetches an affinity group", func() {
+		dummies.AffinityGroup.ID = "" // Force name fetching.
+		ags.EXPECT().GetAffinityGroupByName(dummies.AffinityGroup.Name).Return(&cloudstack.AffinityGroup{}, 1, nil)
+
+		Ω(client.GetOrCreateAffinityGroup(dummies.AffinityGroup)).Should(Succeed())
+	})
+	It("creates an affinity group", func() {
+		dummies.SetDummyDomainAndAccount()
+		dummies.SetDummyDomainID()
+		ags.EXPECT().GetAffinityGroupByID(dummies.AffinityGroup.ID).Return(nil, -1, errors.New("FakeError"))
+		ags.EXPECT().NewCreateAffinityGroupParams(dummies.AffinityGroup.Name, dummies.AffinityGroup.Type).
+			Return(&cloudstack.CreateAffinityGroupParams{})
+		ags.EXPECT().CreateAffinityGroup(ParamMatch(And(NameEquals(dummies.AffinityGroup.Name)))).
+			Return(&cloudstack.CreateAffinityGroupResponse{}, nil)
+
+		Ω(client.GetOrCreateAffinityGroup(dummies.AffinityGroup)).Should(Succeed())
+	})
+
+	Context("AffinityGroup Integ Tests", func() {
+		client, connectionErr := cloud.NewClient("../../cloud-config")
+
+		BeforeEach(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.")
+			}
+			dummies.AffinityGroup.ID = "" // Force name fetching.
+		})
+		AfterEach(func() {
+			mockCtrl.Finish()
+		})
+
+		It("Creates an affinity group.", func() {
+			Ω(client.GetOrCreateAffinityGroup(dummies.AffinityGroup)).Should(Succeed())
+		})
+		It("Associates an affinity group.", func() {
+			if err := client.GetOrCreateVMInstance(
+				dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, dummies.CSZone1, dummies.CSAffinityGroup, "",
+			); err != nil {
+				Skip("Could not create VM." + err.Error())
+			}
+			Ω(client.GetOrCreateAffinityGroup(dummies.AffinityGroup)).Should(Succeed())
+			Ω(client.AssociateAffinityGroup(dummies.CSMachine1, *dummies.AffinityGroup)).Should(Succeed())
+		})
+		It("Deletes an affinity group.", func() {
+			Ω(client.DeleteAffinityGroup(dummies.AffinityGroup)).Should(Succeed())
+			Ω(client.FetchAffinityGroup(dummies.AffinityGroup)).ShouldNot(Succeed())
+		})
+	})
+})