All tests excepting isolated use cases passing.

Author: Joshua Reed

pkg/cloud/affinity_groups_test.go

@@ -49,18 +49,19 @@ var _ = Describe("AffinityGroup Unit Tests", func() {
 	})
 
 	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.CSCluster, dummies.AffinityGroup)).Should(Succeed())
 	})
 	It("creates an affinity group", func() {
-		dummies.AffinityGroup.ID = "FakeID"
-		dummies.CSCluster.Spec.Account = "FakeAccount"
-		dummies.CSCluster.Status.DomainID = "FakeDomainID"
+		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(AccountEquals("FakeAccount"), DomainIDEquals("FakeDomainID")))).
+		ags.EXPECT().CreateAffinityGroup(ParamMatch(
+			And(AccountEquals(dummies.Account), DomainIDEquals(dummies.DomainID)))).
 			Return(&cloudstack.CreateAffinityGroupResponse{}, nil)
 
 		Ω(client.GetOrCreateAffinityGroup(dummies.CSCluster, dummies.AffinityGroup)).Should(Succeed())
@@ -69,24 +70,18 @@ var _ = Describe("AffinityGroup Unit Tests", func() {
 	Context("AffinityGroup Integ Tests", func() {
 		client, connectionErr := cloud.NewClient("../../cloud-config")
 
-		var ( // Declare shared vars.
-			arbitraryAG *cloud.AffinityGroup
-		)
 		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.")
 			}
-			arbitraryAG = &cloud.AffinityGroup{Name: "ArbitraryAffinityGroup", Type: cloud.AffinityGroupType}
+			dummies.AffinityGroup.ID = "" // Force name fetching.
 		})
 		AfterEach(func() {
 			mockCtrl.Finish()
 		})
 
 		It("Creates an affinity group.", func() {
-			Ω(client.GetOrCreateAffinityGroup(dummies.CSCluster, arbitraryAG)).Should(Succeed())
-			arbitraryAG2 := &cloud.AffinityGroup{Name: arbitraryAG.Name}
-			Ω(client.GetOrCreateAffinityGroup(dummies.CSCluster, arbitraryAG2)).Should(Succeed())
-			Ω(arbitraryAG2).Should(Equal(arbitraryAG))
+			Ω(client.GetOrCreateAffinityGroup(dummies.CSCluster, dummies.AffinityGroup)).Should(Succeed())
 		})
 		It("Associates an affinity group.", func() {
 			if err := client.GetOrCreateCluster(dummies.CSCluster); err != nil {
@@ -95,12 +90,12 @@ var _ = Describe("AffinityGroup Unit Tests", func() {
 			if err := client.GetOrCreateVMInstance(dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, ""); err != nil {
 				Skip("Could not create VM." + err.Error())
 			}
-			Ω(client.GetOrCreateAffinityGroup(dummies.CSCluster, arbitraryAG)).Should(Succeed())
-			Ω(client.AssociateAffinityGroup(dummies.CSMachine1, *arbitraryAG)).Should(Succeed())
+			Ω(client.GetOrCreateAffinityGroup(dummies.CSCluster, dummies.AffinityGroup)).Should(Succeed())
+			Ω(client.AssociateAffinityGroup(dummies.CSMachine1, *dummies.AffinityGroup)).Should(Succeed())
 		})
 		It("Deletes an affinity group.", func() {
-			Ω(client.DeleteAffinityGroup(arbitraryAG)).Should(Succeed())
-			Ω(client.FetchAffinityGroup(arbitraryAG)).ShouldNot(Succeed())
+			Ω(client.DeleteAffinityGroup(dummies.AffinityGroup)).Should(Succeed())
+			Ω(client.FetchAffinityGroup(dummies.AffinityGroup)).ShouldNot(Succeed())
 		})
 	})
 })

pkg/cloud/cluster.go

@@ -36,20 +36,17 @@ func (c *client) resolveZones(csCluster *infrav1.CloudStackCluster) (retErr erro
 			retErr = multierror.Append(retErr, errors.Errorf(
 				"expected 1 Zone with name %s, but got %d", specZone.Name, count))
 		} else {
-			csCluster.Status.Zones[specZone.Name] = infrav1.Zone{
-				Name: specZone.Name, ID: zoneID, Network: specZone.Network}
+			specZone.ID = zoneID
 		}
 
-		if retErr != nil {
-			if resp, count, err := c.cs.Zone.GetZoneByID(specZone.ID); err != nil {
-				return multierror.Append(retErr, errors.Wrapf(err, "could not get Zone by ID %s", specZone.ID))
-			} else if count != 1 {
-				return multierror.Append(retErr, errors.Errorf(
-					"expected 1 Zone with UUID %s, but got %d", specZone.ID, count))
-			} else {
-				csCluster.Status.Zones[resp.Name] = infrav1.Zone{
-					Name: resp.Name, ID: specZone.ID, Network: specZone.Network}
-			}
+		if resp, count, err := c.cs.Zone.GetZoneByID(specZone.ID); err != nil {
+			return multierror.Append(retErr, errors.Wrapf(err, "could not get Zone by ID %s", specZone.ID))
+		} else if count != 1 {
+			return multierror.Append(retErr, errors.Errorf(
+				"expected 1 Zone with UUID %s, but got %d", specZone.ID, count))
+		} else {
+			csCluster.Status.Zones[resp.Id] = infrav1.Zone{
+				Name: resp.Name, ID: specZone.ID, Network: specZone.Network}
 		}
 	}
 

pkg/cloud/cluster_test.go

@@ -18,7 +18,8 @@ package cloud_test
 import (
 	"fmt"
 
-	"github.com/apache/cloudstack-go/v2/cloudstack"
+	csapi "github.com/apache/cloudstack-go/v2/cloudstack"
+	capcv1 "github.com/aws/cluster-api-provider-cloudstack/api/v1beta1"
 	"github.com/aws/cluster-api-provider-cloudstack/pkg/cloud"
 	"github.com/aws/cluster-api-provider-cloudstack/test/dummies"
 	"github.com/golang/mock/gomock"
@@ -31,20 +32,21 @@ var _ = Describe("Cluster", func() {
 	var (
 		client     cloud.Client
 		mockCtrl   *gomock.Controller
-		mockClient *cloudstack.CloudStackClient
-		zs         *cloudstack.MockZoneServiceIface
-		ds         *cloudstack.MockDomainServiceIface
-		ns         *cloudstack.MockNetworkServiceIface
+		mockClient *csapi.CloudStackClient
+		zs         *csapi.MockZoneServiceIface
+		ds         *csapi.MockDomainServiceIface
+		ns         *csapi.MockNetworkServiceIface
 	)
 
 	BeforeEach(func() {
 		mockCtrl = gomock.NewController(GinkgoT())
-		mockClient = cloudstack.NewMockClient(mockCtrl)
-		zs = mockClient.Zone.(*cloudstack.MockZoneServiceIface)
-		ds = mockClient.Domain.(*cloudstack.MockDomainServiceIface)
-		ns = mockClient.Network.(*cloudstack.MockNetworkServiceIface)
+		mockClient = csapi.NewMockClient(mockCtrl)
+		zs = mockClient.Zone.(*csapi.MockZoneServiceIface)
+		ds = mockClient.Domain.(*csapi.MockDomainServiceIface)
+		ns = mockClient.Network.(*csapi.MockNetworkServiceIface)
 		client = cloud.NewClientFromCSAPIClient(mockClient)
 		dummies.SetDummyVars()
+		dummies.SetDummyDomainAndAccount()
 	})
 
 	AfterEach(func() {
@@ -65,23 +67,22 @@ var _ = Describe("Cluster", func() {
 			zs.EXPECT().GetZoneID(dummies.Zone1.Name).Return(dummies.Zone1.ID, 2, nil)
 			zs.EXPECT().GetZoneByID(dummies.Zone1.ID).Return(nil, -1, fmt.Errorf("Not found"))
 
-			err := client.GetOrCreateCluster(dummies.CSCluster)
-			Expect(err.Error()).To(ContainSubstring("Expected 1 Zone with name zoneName, but got 2."))
-			Expect(err.Error()).To(ContainSubstring("Could not get Zone by ID zoneName.: Not found"))
+			Ω(client.GetOrCreateCluster(dummies.CSCluster)).Should(MatchError(And(
+				ContainSubstring("expected 1 Zone with name "+dummies.Zone1.Name+", but got 2"),
+				ContainSubstring("could not get Zone by ID "+dummies.Zone1.ID+": Not found"))))
 		})
 
 		It("translates Domain to DomainID when Domain is set", func() {
 			zs.EXPECT().GetZoneID(dummies.Zone1.Name).Return(dummies.Zone1.ID, 1, nil)
+			zs.EXPECT().GetZoneByID(dummies.Zone1.ID).Return(dummies.CAPCZoneToCSAPIZone(&dummies.Zone1), 1, nil)
 			ds.EXPECT().GetDomainID(dummies.CSCluster.Spec.Domain).Return(dummies.DomainID, 1, nil)
+			ns.EXPECT().GetNetworkByName(dummies.Net1.Name).Return(dummies.CAPCNetToCSAPINet(&dummies.Net1), 1, nil)
 
-			// End the fetching with a fake network error here.
-			// Only trying to test domain functions.
-			// TODO: turn the pkg/cloud/client.go client into a composition of interfaces such that the
-			// individual services can be mocked.
-			ns.EXPECT().GetNetworkID(dummies.Net1.Name).Return("", -1, fmt.Errorf("FakeError"))
-			ns.EXPECT().GetNetworkByID(dummies.Net1.ID).Return(&cloudstack.Network{}, -1, fmt.Errorf("FakeError"))
+			// Limit test to single zone.
+			dummies.CSCluster.Spec.Zones = []capcv1.Zone{dummies.Zone1}
+			dummies.CSCluster.Status.Zones = capcv1.ZoneStatusMap{}
 
-			Ω(client.GetOrCreateCluster(dummies.CSCluster)).ShouldNot(Succeed())
+			Ω(client.GetOrCreateCluster(dummies.CSCluster)).Should(Succeed())
 			Ω(dummies.CSCluster.Status.DomainID).Should(Equal(dummies.DomainID))
 		})
 	})

test/dummies/vars.go

@@ -23,7 +23,9 @@ var ( // Declare exported dummy vars.
 	Net1               capcv1.Network
 	Net2               capcv1.Network
 	ISONet1            capcv1.Network
+	Domain             string
 	DomainID           string
+	Account            string
 	Tags               map[string]string
 	Tag1               map[string]string
 	Tag2               map[string]string
@@ -62,6 +64,13 @@ func CAPCNetToCSAPINet(net *capcv1.Network) *csapi.Network {
 	}
 }
 
+func CAPCZoneToCSAPIZone(net *capcv1.Zone) *csapi.Zone {
+	return &csapi.Zone{
+		Name: net.Name,
+		Id:   net.ID,
+	}
+}
+
 // SetDummyVars sets/resets tag related dummy vars.
 func SetDummyTagVars() {
 	CSClusterTagKey = "CAPC_cluster_" + string(CSCluster.ObjectMeta.UID)
@@ -80,7 +89,6 @@ func SetDummyTagVars() {
 
 // SetDummyClusterSpecVars resets the values in each of the exported CloudStackMachines related dummy variables.
 func SetDummyCSMachineTemplateVars() {
-	DomainID = "FakeDomainId"
 	CSMachineTemplate1 = &capcv1.CloudStackMachineTemplate{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1",
@@ -114,7 +122,6 @@ func SetDummyCSMachineTemplateVars() {
 
 // SetDummyClusterSpecVars resets the values in each of the exported CloudStackMachines related dummy variables.
 func SetDummyCSMachineVars() {
-	DomainID = "FakeDomainId"
 	CSMachine1 = &capcv1.CloudStackMachine{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: CSApiVersion,
@@ -144,13 +151,18 @@ func SetDummyCSMachineVars() {
 // SetDummyClusterSpecVars resets the values in each of the exported CloudStackCluster related dummy variables.
 // It is intended to be called in BeforeEach( functions.
 func SetDummyCAPCClusterVars() {
+	DomainID = "FakeDomainID"
+	Domain = "FakeDomainName"
+	Account = "FakeAccountName"
 	CSApiVersion = "infrastructure.cluster.x-k8s.io/v1beta1"
 	CSClusterKind = "CloudStackCluster"
 	CSClusterName = "test-cluster"
+
 	CSlusterNamespace = "default"
 	AffinityGroup = &cloud.AffinityGroup{
 		Name: "FakeAffinityGroup",
-		Type: cloud.AffinityGroupType}
+		Type: cloud.AffinityGroupType,
+		ID:   "FakeAffinityGroupID"}
 	Net1 = capcv1.Network{Name: "SharedGuestNet1", Type: cloud.NetworkTypeShared, ID: "FakeSharedNetID1"}
 	Net2 = capcv1.Network{Name: "SharedGuestNet2", Type: cloud.NetworkTypeShared, ID: "FakeSharedNetID2"}
 	ISONet1 = capcv1.Network{Name: "IsolatedGuestNet1", Type: cloud.NetworkTypeIsolated, ID: "FakeIsolatedNetID1"}
@@ -179,6 +191,15 @@ func SetDummyCAPCClusterVars() {
 	}
 }
 
+func SetDummyDomainAndAccount() {
+	CSCluster.Spec.Account = Account
+	CSCluster.Spec.Domain = Domain
+}
+
+func SetDummyDomainID() {
+	CSCluster.Status.DomainID = "FakeDomainID"
+}
+
 // SetDummyCapiCluster resets the values in each of the exported CAPICluster related dummy variables.
 func SetDummyCAPIClusterVars() {
 	CAPICluster = &clusterv1.Cluster{