Skip to content

Commit 22abffb

Browse files
authored
Merge pull request #35 from aws/feature/ip_address_tagging
Tagging and cleaning up IP addresses
2 parents cbfa8ca + a8e1c67 commit 22abffb

File tree

7 files changed

+257
-82
lines changed

7 files changed

+257
-82
lines changed

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ See [configuring envtest for integration tests](https://book.kubebuilder.io/refe
2424
However, the makefile is setup to install kubebuilder tools in `$PROJECT_DIR/bin`. So, running `make binaries`, and setting
2525
env var KUBEBUILDER_ASSETS should be enough for envtest to work.
2626

27-
- export PROJECT_DIR=`pwd`
27+
- export PROJECT_DIR=$(pwd)
2828

2929
- copy cloud-config to project dir.
3030

pkg/cloud/affinity_groups.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ type AffinityGroupIface interface {
3737
GetOrCreateAffinityGroup(*infrav1.CloudStackCluster, *AffinityGroup) error
3838
DeleteAffinityGroup(*AffinityGroup) error
3939
AssociateAffinityGroup(*infrav1.CloudStackMachine, AffinityGroup) error
40-
DissassociateAffinityGroup(*infrav1.CloudStackMachine, AffinityGroup) error
40+
DisassociateAffinityGroup(*infrav1.CloudStackMachine, AffinityGroup) error
4141
}
4242

4343
func (c *client) FetchAffinityGroup(group *AffinityGroup) (reterr error) {
@@ -171,7 +171,7 @@ func (c *client) AssociateAffinityGroup(csMachine *infrav1.CloudStackMachine, gr
171171
return c.stopAndModifyAffinityGroups(csMachine, groups)
172172
}
173173

174-
func (c *client) DissassociateAffinityGroup(csMachine *infrav1.CloudStackMachine, group AffinityGroup) (retErr error) {
174+
func (c *client) DisassociateAffinityGroup(csMachine *infrav1.CloudStackMachine, group AffinityGroup) (retErr error) {
175175
groups, err := c.getCurrentAffinityGroups(csMachine)
176176
if err != nil {
177177
return err

pkg/cloud/cluster.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,18 @@ func (c *client) GetOrCreateCluster(csCluster *infrav1.CloudStackCluster) (retEr
9595
}
9696

9797
func (c *client) DisposeClusterResources(csCluster *infrav1.CloudStackCluster) (retError error) {
98-
if err := c.RemoveClusterTagFromNetwork(csCluster); err != nil {
98+
if csCluster.Status.PublicIPID != "" {
99+
if err := c.DeleteClusterTag(ResourceTypeIPAddress, csCluster.Status.PublicIPID, csCluster); err != nil {
100+
return err
101+
}
102+
if err := c.DisassociatePublicIPAddressIfNotInUse(csCluster); err != nil {
103+
return err
104+
}
105+
}
106+
107+
if err := c.DeleteClusterTag(ResourceTypeNetwork, csCluster.Status.NetworkID, csCluster); err != nil {
99108
return err
100109
}
110+
101111
return c.DeleteNetworkIfNotInUse(csCluster)
102112
}

pkg/cloud/network.go

Lines changed: 29 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,9 @@ func (c *client) ResolveNetwork(csCluster *infrav1.CloudStackCluster) (retErr er
5858
return nil
5959
}
6060

61-
func generateNetworkTagName(csCluster *infrav1.CloudStackCluster) string {
62-
return clusterTagNamePrefix + string(csCluster.UID)
63-
}
64-
6561
func (c *client) GetOrCreateNetwork(csCluster *infrav1.CloudStackCluster) (retErr error) {
6662
if retErr = c.ResolveNetwork(csCluster); retErr == nil { // Found network.
67-
return addClusterTags(c, csCluster, false)
63+
return c.AddClusterTag(ResourceTypeNetwork, csCluster.Status.NetworkID, csCluster, false)
6864
} else if !strings.Contains(strings.ToLower(retErr.Error()), "no match found") { // Some other error.
6965
return retErr
7066
} // Network not found.
@@ -90,63 +86,36 @@ func (c *client) GetOrCreateNetwork(csCluster *infrav1.CloudStackCluster) (retEr
9086
csCluster.Status.NetworkID = resp.Id
9187
csCluster.Status.NetworkType = resp.Type
9288

93-
return addClusterTags(c, csCluster, true)
89+
return c.AddClusterTag(ResourceTypeNetwork, csCluster.Status.NetworkID, csCluster, true)
9490
}
9591

96-
func addClusterTags(c *client, csCluster *infrav1.CloudStackCluster, addCreatedByTag bool) error {
97-
clusterTagName := generateNetworkTagName(csCluster)
98-
newTags := map[string]string{}
99-
100-
existingTags, err := c.GetNetworkTags(csCluster.Status.NetworkID)
92+
func (c *client) DisassociatePublicIPAddressIfNotInUse(csCluster *infrav1.CloudStackCluster) (retError error) {
93+
tagsAllowDisposal, err := c.DoClusterTagsAllowDisposal(ResourceTypeIPAddress, csCluster.Status.PublicIPID)
10194
if err != nil {
10295
return err
10396
}
10497

105-
if existingTags[clusterTagName] == "" {
106-
newTags[clusterTagName] = "1"
107-
}
108-
109-
if addCreatedByTag && existingTags[createdByCapcTagName] == "" {
110-
newTags[createdByCapcTagName] = "1"
111-
}
112-
113-
if len(newTags) > 0 {
114-
return c.AddNetworkTags(csCluster.Status.NetworkID, newTags)
115-
}
116-
117-
return nil
118-
}
119-
120-
func (c *client) RemoveClusterTagFromNetwork(csCluster *infrav1.CloudStackCluster) (retError error) {
121-
tags, err := c.GetNetworkTags(csCluster.Status.NetworkID)
98+
// Can't disassociate an address if it's the source NAT address.
99+
publicIP, _, err := c.cs.Address.GetPublicIpAddressByID(csCluster.Status.PublicIPID)
122100
if err != nil {
123101
return err
124102
}
103+
sourceNAT := publicIP != nil && publicIP.Issourcenat
125104

126-
clusterTagName := generateNetworkTagName(csCluster)
127-
if tagValue := tags[clusterTagName]; tagValue != "" {
128-
if err = c.DeleteNetworkTags(csCluster.Status.NetworkID, map[string]string{clusterTagName: tagValue}); err != nil {
129-
return err
130-
}
105+
if tagsAllowDisposal && !sourceNAT {
106+
return c.DisassociatePublicIPAddress(csCluster)
131107
}
132108

133109
return nil
134110
}
135111

136112
func (c *client) DeleteNetworkIfNotInUse(csCluster *infrav1.CloudStackCluster) (retError error) {
137-
tags, err := c.GetNetworkTags(csCluster.Status.NetworkID)
113+
okayToDelete, err := c.DoClusterTagsAllowDisposal(ResourceTypeNetwork, csCluster.Status.NetworkID)
138114
if err != nil {
139115
return err
140116
}
141117

142-
var clusterTagCount int
143-
for tagName := range tags {
144-
if strings.HasPrefix(tagName, clusterTagNamePrefix) {
145-
clusterTagCount++
146-
}
147-
}
148-
149-
if clusterTagCount == 0 && tags[createdByCapcTagName] != "" {
118+
if okayToDelete {
150119
return c.DestroyNetwork(csCluster)
151120
}
152121

@@ -171,12 +140,12 @@ func (c *client) ResolvePublicIPDetails(csCluster *infrav1.CloudStackCluster) (*
171140
// Ignore already allocated here since the IP was specified.
172141
return publicAddresses.PublicIpAddresses[0], nil
173142
} else if publicAddresses.Count > 0 { // Endpoint not specified.
174-
for _, v := range publicAddresses.PublicIpAddresses { // Pick first availabe address.
143+
for _, v := range publicAddresses.PublicIpAddresses { // Pick first available address.
175144
if v.Allocated == "" { // Found un-allocated Public IP.
176145
return v, nil
177146
}
178147
}
179-
return nil, errors.New("all Public IP Adresse(s) found were already allocated")
148+
return nil, errors.New("all Public IP Address(es) found were already allocated")
180149
}
181150
return nil, errors.Errorf(`no public addresses found in network: "%s"`, csCluster.Spec.Network)
182151
}
@@ -190,10 +159,11 @@ func (c *client) AssociatePublicIPAddress(csCluster *infrav1.CloudStackCluster)
190159

191160
csCluster.Spec.ControlPlaneEndpoint.Host = publicAddress.Ipaddress
192161
csCluster.Status.PublicIPID = publicAddress.Id
162+
alreadyAllocated := publicAddress.Allocated != ""
193163

194-
if publicAddress.Allocated != "" && publicAddress.Associatednetworkid == csCluster.Status.NetworkID {
164+
if alreadyAllocated && publicAddress.Associatednetworkid == csCluster.Status.NetworkID {
195165
// Address already allocated to network. Allocated is a timestamp -- not a boolean.
196-
return nil
166+
return c.AddClusterTag(ResourceTypeIPAddress, publicAddress.Id, csCluster, false)
197167
} // Address not yet allocated. Allocate now.
198168

199169
// Public IP found, but not yet allocated to network.
@@ -205,7 +175,19 @@ func (c *client) AssociatePublicIPAddress(csCluster *infrav1.CloudStackCluster)
205175
if _, err := c.cs.Address.AssociateIpAddress(p); err != nil {
206176
return err
207177
}
208-
return nil
178+
return c.AddClusterTag(ResourceTypeIPAddress, publicAddress.Id, csCluster, !alreadyAllocated)
179+
}
180+
181+
func (c *client) DisassociatePublicIPAddress(csCluster *infrav1.CloudStackCluster) (retErr error) {
182+
// Remove the CAPC creation tag, so it won't be there the next time this address is associated.
183+
retErr = c.DeleteCreatedByCAPCTag(ResourceTypeIPAddress, csCluster.Status.PublicIPID)
184+
if retErr != nil {
185+
return retErr
186+
}
187+
188+
p := c.cs.Address.NewDisassociateIpAddressParams(csCluster.Status.PublicIPID)
189+
_, retErr = c.cs.Address.DisassociateIpAddress(p)
190+
return retErr
209191
}
210192

211193
func (c *client) OpenFirewallRules(csCluster *infrav1.CloudStackCluster) (retErr error) {

pkg/cloud/network_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,12 @@ var _ = Describe("Network", func() {
8282
})
8383

8484
// Sets expectations network tag creation. To be used by tests that get/create networks.
85-
expectNetworkTags := func(networkId string) {
85+
expectNetworkTags := func(networkID string) {
8686
listTagsParams := &cloudstack.ListTagsParams{}
8787
createTagsParams := &cloudstack.CreateTagsParams{}
8888
rs.EXPECT().NewListTagsParams().Return(listTagsParams)
8989
rs.EXPECT().ListTags(listTagsParams).Return(&cloudstack.ListTagsResponse{}, nil)
90-
rs.EXPECT().NewCreateTagsParams([]string{networkId}, "network", gomock.Any()).Return(createTagsParams)
90+
rs.EXPECT().NewCreateTagsParams([]string{networkID}, string(cloud.ResourceTypeNetwork), gomock.Any()).Return(createTagsParams)
9191
rs.EXPECT().CreateTags(createTagsParams).Return(&cloudstack.CreateTagsResponse{}, nil)
9292
}
9393

@@ -183,7 +183,7 @@ var _ = Describe("Network", func() {
183183
Ω(csCluster.Status.LBRuleID).Should(Equal(lbRuleID))
184184
})
185185

186-
It("doesn't create a new load blancer rule on create", func() {
186+
It("doesn't create a new load balancer rule on create", func() {
187187
lbs.EXPECT().NewListLoadBalancerRulesParams().Return(&cloudstack.ListLoadBalancerRulesParams{})
188188
lbs.EXPECT().ListLoadBalancerRules(gomock.Any()).
189189
Return(&cloudstack.ListLoadBalancerRulesResponse{

pkg/cloud/tags.go

Lines changed: 107 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,30 +16,116 @@ limitations under the License.
1616

1717
package cloud
1818

19+
import (
20+
infrav1 "github.com/aws/cluster-api-provider-cloudstack/api/v1beta1"
21+
"strings"
22+
)
23+
1924
type TagIface interface {
20-
AddNetworkTags(string, map[string]string) error
21-
GetNetworkTags(string) (map[string]string, error)
22-
DeleteNetworkTags(string, map[string]string) error
25+
AddClusterTag(ResourceType, string, *infrav1.CloudStackCluster, bool) error
26+
DeleteClusterTag(ResourceType, string, *infrav1.CloudStackCluster) error
27+
DeleteCreatedByCAPCTag(ResourceType, string) error
28+
DoClusterTagsAllowDisposal(ResourceType, string) (bool, error)
29+
AddTags(ResourceType, string, map[string]string) error
30+
GetTags(ResourceType, string) (map[string]string, error)
31+
DeleteTags(ResourceType, string, map[string]string) error
2332
}
2433

34+
type ResourceType string
35+
2536
const (
26-
clusterTagNamePrefix = "CAPC_cluster_"
27-
createdByCapcTagName = "created_by_CAPC"
28-
resourceTypeNetwork = "network"
37+
clusterTagNamePrefix = "CAPC_cluster_"
38+
createdByCAPCTagName = "created_by_CAPC"
39+
ResourceTypeNetwork ResourceType = "Network"
40+
ResourceTypeIPAddress ResourceType = "PublicIpAddress"
2941
)
3042

31-
// TagNetwork adds tags to a network by network id.
32-
func (c *client) AddNetworkTags(networkID string, tags map[string]string) error {
33-
p := c.cs.Resourcetags.NewCreateTagsParams([]string{networkID}, resourceTypeNetwork, tags)
43+
// AddClusterTag adds cluster-related tags to a resource. One tag indicates that the resource is used by a given
44+
// cluster. The other tag, if applied, indicates that CAPC created the resource and may dispose of it later.
45+
func (c *client) AddClusterTag(resourceType ResourceType, resourceID string, csCluster *infrav1.CloudStackCluster, addCreatedByCAPCTag bool) error {
46+
clusterTagName := generateClusterTagName(csCluster)
47+
newTags := map[string]string{}
48+
49+
existingTags, err := c.GetTags(resourceType, resourceID)
50+
if err != nil {
51+
return err
52+
}
53+
54+
if existingTags[clusterTagName] == "" {
55+
newTags[clusterTagName] = "1"
56+
}
57+
58+
if addCreatedByCAPCTag && existingTags[createdByCAPCTagName] == "" {
59+
newTags[createdByCAPCTagName] = "1"
60+
}
61+
62+
if len(newTags) > 0 {
63+
return c.AddTags(resourceType, resourceID, newTags)
64+
}
65+
66+
return nil
67+
}
68+
69+
// DeleteClusterTag deletes the tag that associates the resource with a given cluster.
70+
func (c *client) DeleteClusterTag(resourceType ResourceType, resourceID string, csCluster *infrav1.CloudStackCluster) error {
71+
tags, err := c.GetTags(resourceType, resourceID)
72+
if err != nil {
73+
return err
74+
}
75+
76+
clusterTagName := generateClusterTagName(csCluster)
77+
if tagValue := tags[clusterTagName]; tagValue != "" {
78+
return c.DeleteTags(resourceType, resourceID, map[string]string{clusterTagName: tagValue})
79+
}
80+
81+
return nil
82+
}
83+
84+
// DeleteCreatedByCAPCTag deletes the tag that indicates that the resource was created by CAPC. This is useful when a
85+
// resource is disassociated instead of deleted. That way the tag won't cause confusion if the resource is reused later.
86+
func (c *client) DeleteCreatedByCAPCTag(resourceType ResourceType, resourceID string) error {
87+
tags, err := c.GetTags(resourceType, resourceID)
88+
if err != nil {
89+
return err
90+
}
91+
92+
if tagValue := tags[createdByCAPCTagName]; tagValue != "" {
93+
return c.DeleteTags(resourceType, resourceID, map[string]string{createdByCAPCTagName: tagValue})
94+
}
95+
96+
return nil
97+
}
98+
99+
// DoClusterTagsAllowDisposal checks to see if the resource is in a state that makes it eligible for disposal. CAPC can
100+
// dispose of a resource if the tags show it was created by CAPC and isn't being used by any clusters.
101+
func (c *client) DoClusterTagsAllowDisposal(resourceType ResourceType, resourceID string) (bool, error) {
102+
tags, err := c.GetTags(resourceType, resourceID)
103+
if err != nil {
104+
return false, err
105+
}
106+
107+
var clusterTagCount int
108+
for tagName := range tags {
109+
if strings.HasPrefix(tagName, clusterTagNamePrefix) {
110+
clusterTagCount++
111+
}
112+
}
113+
114+
return clusterTagCount == 0 && tags[createdByCAPCTagName] != "", nil
115+
}
116+
117+
// AddTags adds arbitrary tags to a resource.
118+
func (c *client) AddTags(resourceType ResourceType, resourceID string, tags map[string]string) error {
119+
p := c.cs.Resourcetags.NewCreateTagsParams([]string{resourceID}, string(resourceType), tags)
34120
_, err := c.cs.Resourcetags.CreateTags(p)
35121
return err
36122
}
37123

38-
// GetNetworkTags gets tags by network id.
39-
func (c *client) GetNetworkTags(networkID string) (map[string]string, error) {
124+
// GetTags gets all of a resource's tags.
125+
func (c *client) GetTags(resourceType ResourceType, resourceID string) (map[string]string, error) {
40126
p := c.cs.Resourcetags.NewListTagsParams()
41-
p.SetResourceid(networkID)
42-
p.SetResourcetype(resourceTypeNetwork)
127+
p.SetResourceid(resourceID)
128+
p.SetResourcetype(string(resourceType))
43129
listTagResponse, err := c.cs.Resourcetags.ListTags(p)
44130
if err != nil {
45131
return nil, err
@@ -51,10 +137,15 @@ func (c *client) GetNetworkTags(networkID string) (map[string]string, error) {
51137
return tags, nil
52138
}
53139

54-
// DeleteNetworkTags deletes matching tags from a network
55-
func (c *client) DeleteNetworkTags(networkID string, tagsToDelete map[string]string) error {
56-
p := c.cs.Resourcetags.NewDeleteTagsParams([]string{networkID}, resourceTypeNetwork)
140+
// DeleteTags deletes the given tags from a resource. If the tags don't exist, or if the values don't match, it will
141+
// result in an error.
142+
func (c *client) DeleteTags(resourceType ResourceType, resourceID string, tagsToDelete map[string]string) error {
143+
p := c.cs.Resourcetags.NewDeleteTagsParams([]string{resourceID}, string(resourceType))
57144
p.SetTags(tagsToDelete)
58145
_, err := c.cs.Resourcetags.DeleteTags(p)
59146
return err
60147
}
148+
149+
func generateClusterTagName(csCluster *infrav1.CloudStackCluster) string {
150+
return clusterTagNamePrefix + string(csCluster.UID)
151+
}

0 commit comments

Comments
 (0)