Skip to content

Commit 248ca6a

Browse files
wanyufewanyufe
committed
merge from main
2 parents 576a122 + 465c0fa commit 248ca6a

File tree

4 files changed

+62
-27
lines changed

4 files changed

+62
-27
lines changed

‎pkg/cloud/instance.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -155,11 +155,10 @@ func (c *client) GetOrCreateVMInstance(
155155
}
156156

157157
// Create VM instance.
158-
p := c.cs.VirtualMachine.NewDeployVirtualMachineParams(offeringID, templateID, csMachine.Status.ZoneID)
159-
zone := csCluster.Status.Zones[csMachine.Status.ZoneID]
160-
p.SetNetworkids([]string{zone.Network.ID})
161-
setIfNotEmpty(csMachine.Name, p.SetName)
162-
setIfNotEmpty(csMachine.Name, p.SetDisplayname)
158+
p := c.cs.VirtualMachine.NewDeployVirtualMachineParams(offeringID, templateID, csCluster.Status.ZoneID)
159+
p.SetNetworkids([]string{csCluster.Status.NetworkID})
160+
setIfNotEmpty(capiMachine.Name, p.SetName)
161+
setIfNotEmpty(capiMachine.Name, p.SetDisplayname)
163162
setIfNotEmpty(csMachine.Spec.SSHKey, p.SetKeypair)
164163

165164
compressedAndEncodedUserData, err := CompressAndEncodeString(userData)

‎pkg/cloud/network_test.go

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -67,34 +67,52 @@ var _ = Describe("Network", func() {
6767
mockCtrl.Finish()
6868
})
6969

70-
// // Sets expectations network tag creation. To be used by tests that get/create networks.
71-
// expectNetworkTags := func(networkID string) {
72-
// listTagsParams := &csapi.ListTagsParams{}
73-
// createTagsParams := &csapi.CreateTagsParams{}
74-
// rs.EXPECT().NewListTagsParams().Return(listTagsParams)
75-
// rs.EXPECT().ListTags(listTagsParams).Return(&csapi.ListTagsResponse{}, nil)
76-
// rs.EXPECT().NewCreateTagsParams([]string{networkID}, string(cloud.ResourceTypeNetwork), gomock.Any()).Return(createTagsParams)
77-
// rs.EXPECT().CreateTags(createTagsParams).Return(&csapi.CreateTagsResponse{}, nil)
78-
// }
70+
// Sets expectations network tag creation. To be used by tests that get/create networks.
71+
expectNetworkTags := func(networkID string, expectTagCreation bool) {
72+
listTagsParams := &cloudstack.ListTagsParams{}
73+
createTagsParams := &cloudstack.CreateTagsParams{}
74+
rs.EXPECT().NewListTagsParams().Return(listTagsParams)
75+
rs.EXPECT().ListTags(listTagsParams).Return(&cloudstack.ListTagsResponse{}, nil)
76+
if expectTagCreation {
77+
rs.EXPECT().NewCreateTagsParams([]string{networkID}, string(cloud.ResourceTypeNetwork), gomock.Any()).Return(createTagsParams)
78+
rs.EXPECT().CreateTags(createTagsParams).Return(&cloudstack.CreateTagsResponse{}, nil)
79+
}
80+
}
7981

8082
Context("for an existing network", func() {
8183
It("resolves network by ID", func() {
8284
ns.EXPECT().GetNetworkByName(dummies.ISONet1.Name).Return(nil, 0, nil)
8385
ns.EXPECT().GetNetworkByID(dummies.ISONet1.ID).Return(dummies.CAPCNetToCSAPINet(&dummies.ISONet1), 1, nil)
8486

85-
Ω(client.ResolveNetwork(dummies.CSCluster, &dummies.ISONet1)).Should(Succeed())
87+
It("does not call to create a new network via GetOrCreateNetwork", func() {
88+
ns.EXPECT().GetNetworkID(fakeNetName).Return(fakeNetID, 1, nil)
89+
ns.EXPECT().GetNetworkByID(fakeNetID).Return(&cloudstack.Network{Type: isolatedNetworkType}, 1, nil)
90+
expectNetworkTags(fakeNetID, false)
91+
92+
Ω(client.GetOrCreateNetwork(csCluster)).Should(Succeed())
8693
})
8794

88-
It("resolves network by Name", func() {
89-
ns.EXPECT().GetNetworkByName(dummies.ISONet1.Name).Return(dummies.CAPCNetToCSAPINet(&dummies.ISONet1), 1, nil)
95+
It("resolves network details with network ID instead of network name", func() {
96+
ns.EXPECT().GetNetworkID(gomock.Any()).Return("", -1, errors.New("no match found for blah"))
97+
ns.EXPECT().GetNetworkByID(fakeNetID).Return(&cloudstack.Network{Type: isolatedNetworkType}, 1, nil)
98+
expectNetworkTags(fakeNetID, false)
9099

91100
Ω(client.ResolveNetwork(dummies.CSCluster, &dummies.ISONet1)).Should(Succeed())
92101
})
93102

94-
It("resolves network details in cluster status", func() {
95-
dummies.SetDummyZoneStatus()
96-
// Gets Net1 by Name.
97-
ns.EXPECT().GetNetworkByName(dummies.Net1.Name).Return(dummies.CAPCNetToCSAPINet(&dummies.Net1), 1, nil)
103+
Context("for a non-existent network", func() {
104+
It("when GetOrCreateNetwork is called it calls CloudStack to create a network", func() {
105+
ns.EXPECT().GetNetworkID(gomock.Any()).Return("", -1, errors.New("no match found for blah"))
106+
ns.EXPECT().GetNetworkByID(gomock.Any()).Return(nil, -1, errors.New("no match found for blah"))
107+
nos.EXPECT().GetNetworkOfferingID(gomock.Any()).Return("someOfferingID", 1, nil)
108+
ns.EXPECT().NewCreateNetworkParams(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
109+
Return(&cloudstack.CreateNetworkParams{})
110+
ns.EXPECT().CreateNetwork(gomock.Any()).Return(&cloudstack.CreateNetworkResponse{Id: netID}, nil)
111+
expectNetworkTags(netID, true)
112+
113+
Ω(client.GetOrCreateNetwork(csCluster)).Should(Succeed())
114+
})
115+
})
98116

99117
// Trys to get Net2 by name and doesn't find it. Then finds Net2 via ID.
100118
ns.EXPECT().GetNetworkByName(dummies.Net2.Name).Return(nil, 0, nil)

‎pkg/cloud/tags.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,24 @@ func ignoreAlreadyPresentErrors(err error, rType ResourceType, rID string) error
5151
if err != nil && !strings.Contains(strings.ToLower(err.Error()), matchSubString) {
5252
return err
5353
}
54+
55+
// Don't tag resources unless they are now being created by CAPC, or were previously created by CAPC.
56+
if !addCreatedByCAPCTag && existingTags[createdByCAPCTagName] == "" {
57+
return nil
58+
}
59+
60+
if existingTags[clusterTagName] == "" {
61+
newTags[clusterTagName] = "1"
62+
}
63+
64+
if addCreatedByCAPCTag && existingTags[createdByCAPCTagName] == "" {
65+
newTags[createdByCAPCTagName] = "1"
66+
}
67+
68+
if len(newTags) > 0 {
69+
return c.AddTags(resourceType, resourceID, newTags)
70+
}
71+
5472
return nil
5573
}
5674

‎pkg/cloud/tags_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ var _ = Describe("Tag Unit Tests", func() {
4444
if err != nil {
4545
Fail("Failed to get existing tags. Error: " + err.Error())
4646
}
47-
if len(existingTags) != 0 {
48-
err = client.DeleteTags(cloud.ResourceTypeNetwork, dummies.Net1.ID, existingTags)
47+
if len(existingTags) > 0 {
48+
err = client.DeleteTags(cloud.ResourceTypeNetwork, networkID, existingTags)
4949
if err != nil {
5050
Fail("Failed to delete existing tags. Error: " + err.Error())
5151
}
@@ -83,14 +83,14 @@ var _ = Describe("Tag Unit Tests", func() {
8383
Ω(client.AddClusterTag(cloud.ResourceTypeNetwork, dummies.Net1.ID, dummies.CSCluster)).Should(Succeed())
8484
})
8585

86-
It("adds the tags for a cluster (resource NOT created by CAPC)", func() {
87-
Ω(client.AddClusterTag(cloud.ResourceTypeNetwork, dummies.Net1.ID, dummies.CSCluster)).Should(Succeed())
86+
It("doesn't add the tags for a cluster (resource NOT created by CAPC)", func() {
87+
Ω(client.AddClusterTag(cloud.ResourceTypeNetwork, networkID, csCluster, false)).Should(Succeed())
8888

8989
// Verify tags
9090
tags, err := client.GetTags(cloud.ResourceTypeNetwork, dummies.Net1.ID)
9191
Ω(err).Should(BeNil())
92-
Ω(tags[dummies.CreatedByCapcKey]).Should(Equal(""))
93-
Ω(tags[dummies.CSClusterTagKey]).Should(Equal("1"))
92+
Ω(tags[createdByCAPCTag]).Should(Equal(""))
93+
Ω(tags[clusterTag]).Should(Equal(""))
9494
})
9595

9696
It("deletes a cluster tag", func() {

0 commit comments

Comments
 (0)