Skip to content

Commit 391c59e

Browse files
author
Joshua Reed
committed
Rebased on tag update.
1 parent 13aa77a commit 391c59e

File tree

5 files changed

+155
-78
lines changed

5 files changed

+155
-78
lines changed

‎pkg/cloud/cluster.go

Lines changed: 21 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -72,47 +72,40 @@ func (c *client) GetOrCreateCluster(csCluster *infrav1.CloudStackCluster) (retEr
7272
}
7373
}
7474

75-
if len(csCluster.Spec.Zones) > 1 { // Multizone network flow. No Isolated network use.
76-
} else { // Single zone. May need to handle isolated network case.
77-
}
78-
7975
// Get or create network and needed network constructs.
8076
if retErr = c.GetOrCreateNetworks(csCluster); retErr != nil {
8177
return retErr
8278
}
8379

84-
// if csCluster.Status.NetworkType == NetworkTypeIsolated {
85-
// if retErr = c.OpenFirewallRules(csCluster); retErr != nil {
86-
// return retErr
87-
// }
88-
// if csCluster.Status.PublicIPID == "" { // Don't try to get public IP again it's already been fetched.
89-
// if retErr = c.AssociatePublicIpAddress(csCluster); retErr != nil {
90-
// return retErr
91-
// }
92-
// }
93-
// if retErr = c.GetOrCreateLoadBalancerRule(csCluster); retErr != nil {
94-
// return retErr
95-
// }
96-
// }
80+
if len(csCluster.Spec.Zones) > 1 { // Ignore isolated network use case if spec indicates multiple zones.
81+
for _, zone := range csCluster.Status.Zones { // Get the only zone from the status zone map.
82+
if zone.Network.Type == NetworkTypeIsolated {
83+
if retErr = c.OpenFirewallRules(csCluster); retErr != nil {
84+
return retErr
85+
}
86+
if csCluster.Status.PublicIPID == "" { // Don't try to get public IP again it's already been fetched.
87+
if retErr = c.AssociatePublicIpAddress(csCluster); retErr != nil {
88+
return retErr
89+
}
90+
}
91+
if retErr = c.GetOrCreateLoadBalancerRule(csCluster); retErr != nil {
92+
return retErr
93+
}
94+
}
95+
}
96+
}
9797

9898
// Set cluster to ready to indicate readiness to CAPI.
9999
csCluster.Status.Ready = true
100100
return nil
101101
}
102102

103103
func (c *client) DisposeClusterResources(csCluster *infrav1.CloudStackCluster) (retError error) {
104-
if csCluster.Status.PublicIPID != "" {
105-
if err := c.DeleteClusterTag(ResourceTypeIPAddress, csCluster.Status.PublicIPID, csCluster); err != nil {
104+
for _, zone := range csCluster.Spec.Zones {
105+
if err := c.RemoveClusterTagFromNetwork(csCluster, zone.Network); err != nil {
106106
return err
107107
}
108-
if err := c.DisassociatePublicIPAddressIfNotInUse(csCluster); err != nil {
109-
return err
110-
}
111-
}
112-
113-
if err := c.DeleteClusterTag(ResourceTypeNetwork, csCluster.Status.NetworkID, csCluster); err != nil {
114-
return err
108+
c.DeleteNetworkIfNotInUse(csCluster, zone.Network)
115109
}
116-
117-
return c.DeleteNetworkIfNotInUse(csCluster)
110+
return nil
118111
}

‎pkg/cloud/network.go

Lines changed: 48 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,13 @@ import (
2727
)
2828

2929
const (
30-
NetOffering = "DefaultIsolatedNetworkOfferingWithSourceNatService"
31-
K8sDefaultAPIPort = 6443
32-
NetworkTypeIsolated = "Isolated"
33-
NetworkTypeShared = "Shared"
34-
NetworkProtocolTCP = "tcp"
30+
NetOffering = "DefaultIsolatedNetworkOfferingWithSourceNatService"
31+
K8sDefaultAPIPort = 6443
32+
NetworkTypeIsolated = "Isolated"
33+
NetworkTypeShared = "Shared"
34+
NetworkProtocolTCP = "tcp"
35+
addCreatedByTag = true
36+
doNotAddCreatedByTag = false
3537
)
3638

3739
// ResolveNetworks fetches networks' Id, Name, and Type.
@@ -93,6 +95,7 @@ func (c *client) GetOrCreateNetworks(csCluster *infrav1.CloudStackCluster) (retE
9395
for _, zoneStatus := range csCluster.Status.Zones {
9496
netStatus := zoneStatus.Network
9597
if retErr = c.ResolveNetwork(csCluster, &zoneStatus.Network); retErr == nil { // Found network
98+
c.addClusterTags(csCluster, zoneStatus.Network, doNotAddCreatedByTag)
9699
continue
97100
} else if !strings.Contains(retErr.Error(), "No match found") { // Some other error.
98101
return retErr
@@ -112,6 +115,7 @@ func (c *client) GetOrCreateNetworks(csCluster *infrav1.CloudStackCluster) (retE
112115
if err != nil {
113116
return err
114117
}
118+
c.addClusterTags(csCluster, zoneStatus.Network, addCreatedByTag)
115119

116120
// Update Zone/Network status accordingly.
117121
netStatus.Id = resp.Id
@@ -120,37 +124,65 @@ func (c *client) GetOrCreateNetworks(csCluster *infrav1.CloudStackCluster) (retE
120124
csCluster.Status.Zones[zoneStatus.Name] = zoneStatus
121125
}
122126

123-
return c.AddClusterTag(ResourceTypeNetwork, csCluster.Status.NetworkID, csCluster, true)
127+
return nil
124128
}
125129

126-
func (c *client) DisassociatePublicIPAddressIfNotInUse(csCluster *infrav1.CloudStackCluster) (retError error) {
127-
tagsAllowDisposal, err := c.DoClusterTagsAllowDisposal(ResourceTypeIPAddress, csCluster.Status.PublicIPID)
130+
func (c *client) addClusterTags(csCluster *infrav1.CloudStackCluster, net infrav1.Network, addCreatedBy bool) error {
131+
clusterTagName := generateNetworkTagName(csCluster)
132+
newTags := map[string]string{}
133+
134+
existingTags, err := c.GetNetworkTags(net.Id)
128135
if err != nil {
129136
return err
130137
}
131138

132-
// Can't disassociate an address if it's the source NAT address.
133-
publicIP, _, err := c.cs.Address.GetPublicIpAddressByID(csCluster.Status.PublicIPID)
139+
if existingTags[clusterTagName] == "" {
140+
newTags[clusterTagName] = "1"
141+
}
142+
143+
if addCreatedBy && existingTags[createdByCapcTagName] == "" {
144+
newTags[createdByCapcTagName] = "1"
145+
}
146+
147+
if len(newTags) > 0 {
148+
return c.AddNetworkTags(net.Id, newTags)
149+
}
150+
151+
return nil
152+
}
153+
154+
func (c *client) RemoveClusterTagFromNetwork(csCluster *infrav1.CloudStackCluster, net infrav1.Network) (retError error) {
155+
tags, err := c.GetNetworkTags(net.Id)
134156
if err != nil {
135157
return err
136158
}
137159
sourceNAT := publicIP != nil && publicIP.Issourcenat
138160

139-
if tagsAllowDisposal && !sourceNAT {
140-
return c.DisassociatePublicIPAddress(csCluster)
161+
clusterTagName := generateNetworkTagName(csCluster)
162+
if tagValue := tags[clusterTagName]; tagValue != "" {
163+
if err = c.DeleteNetworkTags(net.Id, map[string]string{clusterTagName: tagValue}); err != nil {
164+
return err
165+
}
141166
}
142167

143168
return nil
144169
}
145170

146-
func (c *client) DeleteNetworkIfNotInUse(csCluster *infrav1.CloudStackCluster) (retError error) {
147-
okayToDelete, err := c.DoClusterTagsAllowDisposal(ResourceTypeNetwork, csCluster.Status.NetworkID)
171+
func (c *client) DeleteNetworkIfNotInUse(csCluster *infrav1.CloudStackCluster, net infrav1.Network) (retError error) {
172+
tags, err := c.GetNetworkTags(net.Id)
148173
if err != nil {
149174
return err
150175
}
151176

152-
if okayToDelete {
153-
return c.DestroyNetwork(csCluster)
177+
var clusterTagCount int
178+
for tagName := range tags {
179+
if strings.HasPrefix(tagName, clusterTagNamePrefix) {
180+
clusterTagCount++
181+
}
182+
}
183+
184+
if clusterTagCount == 0 && tags[createdByCapcTagName] != "" {
185+
return c.DestroyNetwork(net)
154186
}
155187

156188
return nil

‎pkg/cloud/tags_test.go

Lines changed: 18 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -19,50 +19,25 @@ package cloud_test
1919
import (
2020
infrav1 "github.com/aws/cluster-api-provider-cloudstack/api/v1beta1"
2121
"github.com/aws/cluster-api-provider-cloudstack/pkg/cloud"
22+
"github.com/aws/cluster-api-provider-cloudstack/test/dummies"
2223
. "github.com/onsi/ginkgo"
2324
. "github.com/onsi/gomega"
24-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2525
)
2626

2727
var _ = Describe("Tag Unit Tests", func() {
28-
var (
29-
cluster *infrav1.CloudStackCluster
30-
)
31-
3228
BeforeEach(func() {
33-
cluster = &infrav1.CloudStackCluster{
34-
Spec: infrav1.CloudStackClusterSpec{
35-
Zone: "Zone1", Network: "SharedGuestNet1",
36-
},
37-
ObjectMeta: metav1.ObjectMeta{
38-
UID: "0",
39-
},
40-
}
29+
dummies.SetDummyVars()
4130
})
4231

4332
Context("Tag Integ Tests", func() {
4433
client, connectionErr := cloud.NewClient("../../cloud-config")
4534

46-
const (
47-
tagKey = "test_tag"
48-
tagValue = "arbitrary_value"
49-
clusterID = "123456"
50-
createdByCAPCTag = "created_by_CAPC"
51-
clusterTag = "CAPC_cluster_" + clusterID
52-
)
53-
54-
var (
55-
networkID string
56-
testTags map[string]string
57-
csCluster *infrav1.CloudStackCluster
58-
)
59-
6035
BeforeEach(func() {
6136
if connectionErr != nil { // Only do these tests if an actual ACS instance is available via cloud-config.
6237
Skip("Could not connect to ACS instance.")
6338
}
6439

65-
if err := client.GetOrCreateNetwork(cluster); err != nil {
40+
if err := client.GetOrCreateNetworks(dummies.CSCluster); err != nil {
6641
Skip("Could not find network.")
6742
}
6843

@@ -85,16 +60,25 @@ var _ = Describe("Tag Unit Tests", func() {
8560
It("adds and gets a resource tag", func() {
8661
Ω(client.AddTags(cloud.ResourceTypeNetwork, networkID, testTags)).Should(Succeed())
8762
tags, err := client.GetTags(cloud.ResourceTypeNetwork, networkID)
63+
})
64+
65+
It("Tags a network with an arbitrary tag.", func() {
66+
// Delete the tag if it already exists from a prior test run, otherwise the test will fail.
67+
_ = client.DeleteNetworkTags(dummies.Net1.Id, dummies.Tags)
68+
Ω(client.AddNetworkTags(dummies.Net1.Id, dummies.Tags)).Should(Succeed())
69+
})
70+
71+
It("Fetches said tag.", func() {
72+
tags, err := client.GetNetworkTags(dummies.Net1.Id)
8873
Ω(err).Should(BeNil())
89-
Ω(tags[tagKey]).Should(Equal(tagValue))
74+
Ω(tags[dummies.Tag1Key]).Should(Equal(dummies.Tag1Val))
9075
})
9176

92-
It("deletes a resource tag", func() {
93-
_ = client.AddTags(cloud.ResourceTypeNetwork, networkID, testTags)
94-
Ω(client.DeleteTags(cloud.ResourceTypeNetwork, networkID, testTags)).Should(Succeed())
95-
remainingTags, err := client.GetTags(cloud.ResourceTypeNetwork, networkID)
77+
It("Deletes said tag.", func() {
78+
Ω(client.DeleteNetworkTags(dummies.Net1.Id, dummies.Tags)).Should(Succeed())
79+
remainingTags, err := client.GetNetworkTags(dummies.Net1.Id)
9680
Ω(err).Should(BeNil())
97-
Ω(remainingTags[tagKey]).Should(Equal(""))
81+
Ω(remainingTags[dummies.Tag1Key]).Should(Equal(""))
9882
})
9983

10084
It("returns an error when you delete a tag that doesn't exist", func() {

‎pkg/mocks/mock_client.go

Lines changed: 57 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/dummies/vars.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,16 @@ var ( // Declare exported dummy vars.
1919
Zone1 infrav1.Zone
2020
Net1 infrav1.Network
2121
DomainId string
22+
Tags map[string]string
23+
Tag1Key string
24+
Tag1Val string
2225
)
2326

2427
// SetDummyVars sets/resets all dummy vars.
2528
func SetDummyVars() {
2629
SetDummyCAPCClusterVars()
2730
SetDummyCAPIClusterVars()
31+
SetDummyTagVars()
2832
}
2933

3034
// SetDummyClusterSpecVars resets the values in each of the exported CloudStackCluster related dummy variables.
@@ -71,3 +75,10 @@ func SetDummyCAPIClusterVars() {
7175
},
7276
}
7377
}
78+
79+
// SetDummyTagVars resets the values in each of the exported Tag related dummy variables.
80+
func SetDummyTagVars() {
81+
Tag1Key = "test_tag"
82+
Tag1Val = "arbitrary_value"
83+
Tags = map[string]string{Tag1Key: Tag1Val}
84+
}

0 commit comments

Comments
 (0)