Skip to content

Commit f533e3b

Browse files
author
Joshua Reed
committed
Merged against main.
2 parents 069a106 + 465c0fa commit f533e3b

File tree

4 files changed

+66
-27
lines changed

4 files changed

+66
-27
lines changed

‎pkg/cloud/network.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ func (c *client) ResolveNetwork(csCluster *capcv1.CloudStackCluster, net *capcv1
100100
}
101101

102102
func generateNetworkTagName(csCluster *capcv1.CloudStackCluster) string {
103-
return clusterTagNamePrefix + string(csCluster.UID)
103+
return ClusterTagNamePrefix + string(csCluster.UID)
104104
}
105105

106106
// getOfferingID fetches an offering id.
@@ -169,9 +169,9 @@ func (c *client) RemoveClusterTagFromNetwork(csCluster *capcv1.CloudStackCluster
169169
return err
170170
}
171171

172-
clusterTagName := generateNetworkTagName(csCluster)
173-
if tagValue := tags[clusterTagName]; tagValue != "" {
174-
if err = c.DeleteTags(ResourceTypeNetwork, net.ID, map[string]string{clusterTagName: tagValue}); err != nil {
172+
ClusterTagName := generateNetworkTagName(csCluster)
173+
if tagValue := tags[ClusterTagName]; tagValue != "" {
174+
if err = c.DeleteTags(ResourceTypeNetwork, net.ID, map[string]string{ClusterTagName: tagValue}); err != nil {
175175
return err
176176
}
177177
}
@@ -187,12 +187,12 @@ func (c *client) DeleteNetworkIfNotInUse(csCluster *capcv1.CloudStackCluster, ne
187187

188188
var clusterTagCount int
189189
for tagName := range tags {
190-
if strings.HasPrefix(tagName, clusterTagNamePrefix) {
190+
if strings.HasPrefix(tagName, ClusterTagNamePrefix) {
191191
clusterTagCount++
192192
}
193193
}
194194

195-
if clusterTagCount == 0 && tags[createdByCAPCTagName] != "" {
195+
if clusterTagCount == 0 && tags[CreatedByCAPCTagName] != "" {
196196
return c.DestroyNetwork(net)
197197
}
198198

‎pkg/cloud/network_test.go

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,15 @@ var _ = Describe("Network", func() {
9999
It("correctly identifies an existing network from a network status", func() {
100100
Ω(cloud.NetworkExists(dummies.CSCluster.Status.Zones.GetOne().Network)).Should(BeTrue())
101101
})
102+
103+
// It("resolves network details with network ID instead of network name", func() {
104+
// ns.EXPECT().GetNetworkID(gomock.Any()).Return("", -1, errors.New("no match found for blah"))
105+
// ns.EXPECT().GetNetworkByID(fakeNetID).Return(&cloudstack.Network{Type: isolatedNetworkType}, 1, nil)
106+
// expectNetworkTags(fakeNetID, false)
107+
108+
// csCluster.Spec.Network = fakeNetID
109+
// Ω(client.GetOrCreateNetwork(csCluster)).Should(Succeed())
110+
// })
102111
})
103112

104113
Context("for a non-existent network", func() {
@@ -128,9 +137,18 @@ var _ = Describe("Network", func() {
128137
as.EXPECT().NewAssociateIpAddressParams().Return(&csapi.AssociateIpAddressParams{})
129138
as.EXPECT().AssociateIpAddress(gomock.Any())
130139

131-
createTagsParams := &csapi.CreateTagsParams{}
132-
rs.EXPECT().NewCreateTagsParams(gomock.Any(), gomock.Any(), gomock.Any()).Return(createTagsParams).Times(4)
133-
rs.EXPECT().CreateTags(createTagsParams).Return(&csapi.CreateTagsResponse{}, nil).Times(4)
140+
// Will add cluster tag once to Network and once to PublicIP.
141+
createdByResponse := &csapi.ListTagsResponse{Tags: []*csapi.Tag{{Key: cloud.CreatedByCAPCTagName, Value: "1"}}}
142+
gomock.InOrder(
143+
rs.EXPECT().NewListTagsParams().Return(&csapi.ListTagsParams{}),
144+
rs.EXPECT().ListTags(gomock.Any()).Return(createdByResponse, nil),
145+
rs.EXPECT().NewListTagsParams().Return(&csapi.ListTagsParams{}),
146+
rs.EXPECT().ListTags(gomock.Any()).Return(createdByResponse, nil))
147+
148+
// Will add creation and cluster tags to network and PublicIP.
149+
rs.EXPECT().NewCreateTagsParams(gomock.Any(), gomock.Any(), gomock.Any()).
150+
Return(&csapi.CreateTagsParams{}).Times(4)
151+
rs.EXPECT().CreateTags(gomock.Any()).Return(&csapi.CreateTagsResponse{}, nil).Times(4)
134152

135153
lbs.EXPECT().NewListLoadBalancerRulesParams().Return(&csapi.ListLoadBalancerRulesParams{})
136154
lbs.EXPECT().ListLoadBalancerRules(gomock.Any()).Return(

‎pkg/cloud/tags.go

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ type TagIface interface {
3939
type ResourceType string
4040

4141
const (
42-
clusterTagNamePrefix = "CAPC_cluster_"
43-
createdByCAPCTagName = "created_by_CAPC"
42+
ClusterTagNamePrefix = "CAPC_cluster_"
43+
CreatedByCAPCTagName = "created_by_CAPC"
4444
ResourceTypeNetwork ResourceType = "Network"
4545
ResourceTypeIPAddress ResourceType = "PublicIpAddress"
4646
)
@@ -54,28 +54,47 @@ func ignoreAlreadyPresentErrors(err error, rType ResourceType, rID string) error
5454
return nil
5555
}
5656

57+
func (c *client) IsCapcManaged(resourceType ResourceType, resourceID string) (bool, error) {
58+
tags, err := c.GetTags(resourceType, resourceID)
59+
if err != nil {
60+
return false, errors.Wrapf(err,
61+
"error encountered while checking if %s with ID: %s is tagged as CAPC managed", resourceType, resourceID)
62+
}
63+
_, CreatedByCAPC := tags[CreatedByCAPCTagName]
64+
return CreatedByCAPC, nil
65+
}
66+
5767
// AddClusterTag adds cluster tag to a resource. This tag indicates the resource is used by a given the cluster.
5868
func (c *client) AddClusterTag(rType ResourceType, rID string, csCluster *infrav1.CloudStackCluster) error {
59-
clusterTagName := generateClusterTagName(csCluster)
60-
return c.AddTags(rType, rID, map[string]string{clusterTagName: "1"})
69+
if managedByCAPC, err := c.IsCapcManaged(rType, rID); err != nil {
70+
return err
71+
} else if managedByCAPC {
72+
ClusterTagName := generateClusterTagName(csCluster)
73+
return c.AddTags(rType, rID, map[string]string{ClusterTagName: "1"})
74+
}
75+
return nil
6176
}
6277

6378
// DeleteClusterTag deletes the tag that associates the resource with a given cluster.
6479
func (c *client) DeleteClusterTag(rType ResourceType, rID string, csCluster *infrav1.CloudStackCluster) error {
65-
clusterTagName := generateClusterTagName(csCluster)
66-
return c.DeleteTags(rType, rID, map[string]string{clusterTagName: "1"})
80+
if managedByCAPC, err := c.IsCapcManaged(rType, rID); err != nil {
81+
return err
82+
} else if managedByCAPC {
83+
ClusterTagName := generateClusterTagName(csCluster)
84+
return c.DeleteTags(rType, rID, map[string]string{ClusterTagName: "1"})
85+
}
86+
return nil
6787
}
6888

69-
// AddCreatedByCAPCTag deletes the tag that indicates that the resource was created by CAPC. This is useful when a
70-
// resource is disassociated instead of deleted. That way the tag won't cause confusion if the resource is reused later.
89+
// AddCreatedByCAPCTag adds the tag that indicates that the resource was created by CAPC.
90+
// This is useful when a resource is disassociated but not deleted.
7191
func (c *client) AddCreatedByCAPCTag(rType ResourceType, rID string) error {
72-
return c.AddTags(rType, rID, map[string]string{createdByCAPCTagName: "1"})
92+
return c.AddTags(rType, rID, map[string]string{CreatedByCAPCTagName: "1"})
7393
}
7494

75-
// DeleteCreatedByCAPCTag deletes the tag that indicates that the resource was created by CAPC. This is useful when a
76-
// resource is disassociated instead of deleted. That way the tag won't cause confusion if the resource is reused later.
95+
// DeleteCreatedByCAPCTag deletes the tag that indicates that the resource was created by CAPC.
7796
func (c *client) DeleteCreatedByCAPCTag(rType ResourceType, rID string) error {
78-
return c.DeleteTags(rType, rID, map[string]string{createdByCAPCTagName: "1"})
97+
return c.DeleteTags(rType, rID, map[string]string{CreatedByCAPCTagName: "1"})
7998
}
8099

81100
// DoClusterTagsAllowDisposal checks to see if the resource is in a state that makes it eligible for disposal. CAPC can
@@ -88,12 +107,12 @@ func (c *client) DoClusterTagsAllowDisposal(resourceType ResourceType, resourceI
88107

89108
var clusterTagCount int
90109
for tagName := range tags {
91-
if strings.HasPrefix(tagName, clusterTagNamePrefix) {
110+
if strings.HasPrefix(tagName, ClusterTagNamePrefix) {
92111
clusterTagCount++
93112
}
94113
}
95114

96-
return clusterTagCount == 0 && tags[createdByCAPCTagName] != "", nil
115+
return clusterTagCount == 0 && tags[CreatedByCAPCTagName] != "", nil
97116
}
98117

99118
// AddTags adds arbitrary tags to a resource.
@@ -140,5 +159,5 @@ func (c *client) DeleteTags(resourceType ResourceType, resourceID string, tagsTo
140159
}
141160

142161
func generateClusterTagName(csCluster *infrav1.CloudStackCluster) string {
143-
return clusterTagNamePrefix + string(csCluster.UID)
162+
return ClusterTagNamePrefix + string(csCluster.UID)
144163
}

‎pkg/cloud/tags_test.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ 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 {
47+
if len(existingTags) > 0 {
4848
err = client.DeleteTags(cloud.ResourceTypeNetwork, dummies.Net1.ID, existingTags)
4949
if err != nil {
5050
Fail("Failed to delete existing tags. Error: " + err.Error())
@@ -68,6 +68,8 @@ var _ = Describe("Tag Unit Tests", func() {
6868
})
6969

7070
It("adds the tags for a cluster (resource created by CAPC)", func() {
71+
Ω(client.AddCreatedByCAPCTag(cloud.ResourceTypeNetwork, dummies.Net1.ID)).
72+
Should(Succeed())
7173
Ω(client.AddClusterTag(cloud.ResourceTypeNetwork, dummies.Net1.ID, dummies.CSCluster)).
7274
Should(Succeed())
7375

@@ -83,14 +85,14 @@ var _ = Describe("Tag Unit Tests", func() {
8385
Ω(client.AddClusterTag(cloud.ResourceTypeNetwork, dummies.Net1.ID, dummies.CSCluster)).Should(Succeed())
8486
})
8587

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

8991
// Verify tags
9092
tags, err := client.GetTags(cloud.ResourceTypeNetwork, dummies.Net1.ID)
9193
Ω(err).Should(BeNil())
9294
Ω(tags[dummies.CreatedByCapcKey]).Should(Equal(""))
93-
Ω(tags[dummies.CSClusterTagKey]).Should(Equal("1"))
95+
Ω(tags[dummies.CSClusterTagKey]).Should(Equal(""))
9496
})
9597

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

0 commit comments

Comments
 (0)