Skip to content

Commit 5f6b119

Browse files
nishant221nishant221
authored
fixing dropping of secondary cidr blocks on first apply of VPC (#139)
Issue aws-controllers-k8s/community#1816 Description of changes: Added the fix for a bug where Secondary VPC CIDR blocks are getting dropped in the first apply of VPC resource. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent 124784e commit 5f6b119

File tree

7 files changed

+177
-9
lines changed

7 files changed

+177
-9
lines changed

‎apis/v1alpha1/ack-generate-metadata.yaml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
ack_generate_info:
2-
build_date: "2023-06-14T17:05:27Z"
3-
build_hash: e04fca8e1fa32e988ff4dd9425d758ecc8a2a178
4-
go_version: go1.20.3
5-
version: v0.26.1-6-ge04fca8
6-
api_directory_checksum: a930caa16911411d04c5e7c14b27e077993d93a1
2+
build_date: "2023-06-17T15:45:55Z"
3+
build_hash: e9b68590da73ce9143ba1e4361cebdc1d876c81e
4+
go_version: go1.19
5+
version: v0.26.1-7-ge9b6859
6+
api_directory_checksum: 9452be5c34acba71fcd740b2acf4d053f2eb221b
77
api_version: v1alpha1
88
aws_sdk_go_version: v1.44.93
99
generator_config_info:

‎helm/values.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ deletionPolicy: delete
110110
# controller reconciliation configurations
111111
reconcile:
112112
# The default duration, in seconds, to wait before resyncing desired state of custom resources.
113-
defaultResyncPeriod: 0
113+
defaultResyncPeriod: 36000 # 10 Hours
114114
# An object representing the reconcile resync configuration for each specific resource.
115115
resourceResyncPeriods: {}
116116

‎pkg/resource/vpc/hooks.go

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ func computeStringPDifference(a, b []*string) (aNotB, bNotA []*string) {
148148
}
149149
}
150150
for _, elemB := range b {
151-
if _, found := mapOfB[*elemB]; !found {
151+
if _, found := mapOfA[*elemB]; !found {
152152
bNotA = append(bNotA, elemB)
153153
}
154154
}
@@ -172,9 +172,11 @@ func (rm *resourceManager) syncCIDRBlocks(
172172
latestCIDRs := latest.ko.Spec.CIDRBlocks
173173
latestCIDRStates := latest.ko.Status.CIDRBlockAssociationSet
174174
toAddCIDRs, toDeleteCIDRs := computeStringPDifference(desiredCIDRs, latestCIDRs)
175+
cidrblockassociationset := []*svcapitypes.VPCCIDRBlockAssociation{}
175176

176177
// extract associationID for the DisassociateVpcCidr request
177178
for _, cidr := range toDeleteCIDRs {
179+
178180
input := &svcsdk.DisassociateVpcCidrBlockInput{}
179181
for _, cidrAssociation := range latestCIDRStates {
180182
if *cidr == *cidrAssociation.CIDRBlock {
@@ -184,6 +186,8 @@ func (rm *resourceManager) syncCIDRBlocks(
184186
if err != nil {
185187
return err
186188
}
189+
} else {
190+
cidrblockassociationset = append(cidrblockassociationset, cidrAssociation)
187191
}
188192
}
189193
}
@@ -193,11 +197,39 @@ func (rm *resourceManager) syncCIDRBlocks(
193197
VpcId: latest.ko.Status.VPCID,
194198
CidrBlock: cidr,
195199
}
196-
_, err = rm.sdkapi.AssociateVpcCidrBlockWithContext(ctx, input)
200+
var res *svcsdk.AssociateVpcCidrBlockOutput
201+
cidrblockassociation := &svcapitypes.VPCCIDRBlockAssociation{}
202+
res, err = rm.sdkapi.AssociateVpcCidrBlockWithContext(ctx, input)
197203
rm.metrics.RecordAPICall("UPDATE", "AssociateVpcCidrBlock", err)
198204
if err != nil {
199205
return err
200206
}
207+
if res.CidrBlockAssociation != nil {
208+
if res.CidrBlockAssociation.AssociationId != nil {
209+
cidrblockassociation.AssociationID = res.CidrBlockAssociation.AssociationId
210+
}
211+
if res.CidrBlockAssociation.CidrBlock != nil {
212+
cidrblockassociation.CIDRBlock = res.CidrBlockAssociation.CidrBlock
213+
}
214+
if res.CidrBlockAssociation.CidrBlockState != nil {
215+
cidrblockstate := &svcapitypes.VPCCIDRBlockState{}
216+
if res.CidrBlockAssociation.CidrBlockState.State != nil {
217+
cidrblockstate.State = res.CidrBlockAssociation.CidrBlockState.State
218+
}
219+
if res.CidrBlockAssociation.CidrBlockState.StatusMessage != nil {
220+
cidrblockstate.StatusMessage = res.CidrBlockAssociation.CidrBlockState.StatusMessage
221+
}
222+
}
223+
cidrblockassociationset = append(cidrblockassociationset, cidrblockassociation)
224+
}
225+
}
226+
if cidrblockassociationset != nil {
227+
if toDeleteCIDRs != nil {
228+
latest.ko.Status.CIDRBlockAssociationSet = cidrblockassociationset
229+
} else {
230+
latest.ko.Status.CIDRBlockAssociationSet = append(latest.ko.Status.CIDRBlockAssociationSet, cidrblockassociationset...)
231+
}
232+
201233
}
202234

203235
return nil
@@ -257,6 +289,7 @@ func (rm *resourceManager) customUpdateVPC(
257289
return nil, err
258290
}
259291
}
292+
updated.ko.Status.CIDRBlockAssociationSet = latest.ko.Status.CIDRBlockAssociationSet
260293

261294
if delta.DifferentAt("Spec.EnableDNSSupport") {
262295
if err := rm.syncDNSSupportAttribute(ctx, desired); err != nil {

‎pkg/resource/vpc/sdk.go

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

‎templates/hooks/vpc/sdk_create_post_set_output.go.tpl

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
if resp.Vpc.CidrBlock != nil {
2+
ko.Spec.CIDRBlocks = make([]*string, 1)
3+
ko.Spec.CIDRBlocks[0] = resp.Vpc.CidrBlock
4+
}
5+
rm.syncCIDRBlocks(ctx, desired, &resource{ko})
6+
17
rm.setSpecCIDRs(ko)
28
err = rm.createAttributes(ctx, &resource{ko})
39
if err != nil {

‎test/e2e/resources/vpc_multicidr.yaml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
apiVersion: ec2.services.k8s.aws/v1alpha1
2+
kind: VPC
3+
metadata:
4+
name: $VPC_NAME
5+
spec:
6+
cidrBlocks:
7+
- $PRIMARY_CIDR_BLOCK
8+
- $SECONDARY_CIDR_BLOCK
9+
enableDNSSupport: $ENABLE_DNS_SUPPORT
10+
enableDNSHostnames: $ENABLE_DNS_HOSTNAMES
11+
tags:
12+
- key: $TAG_KEY
13+
value: $TAG_VALUE

‎test/e2e/tests/test_vpc.py

Lines changed: 111 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,4 +264,114 @@ def test_terminal_condition_invalid_parameter_value(self):
264264
# An error occurred (InvalidParameterValue) when calling the CreateVpc operation:
265265
# Value (dsfre) for parameter cidrBlock is invalid.
266266
# This is not a valid CIDR block.
267-
assert expected_msg in terminal_condition['message']
267+
assert expected_msg in terminal_condition['message']
268+
269+
def test_vpc_creation_multiple_cidr(self,ec2_client):
270+
resource_name = random_suffix_name("vpc-ack-multicidr", 24)
271+
replacements = REPLACEMENT_VALUES.copy()
272+
replacements["VPC_NAME"] = resource_name
273+
replacements["PRIMARY_CIDR_BLOCK"] = PRIMARY_CIDR_DEFAULT
274+
replacements["SECONDARY_CIDR_BLOCK"] = "10.2.0.0/16"
275+
replacements["ENABLE_DNS_SUPPORT"] = "False"
276+
replacements["ENABLE_DNS_HOSTNAMES"] = "False"
277+
278+
# Load VPC CR
279+
resource_data = load_ec2_resource(
280+
"vpc_multicidr",
281+
additional_replacements=replacements,
282+
)
283+
logging.debug(resource_data)
284+
285+
# Create k8s resource
286+
ref = k8s.CustomResourceReference(
287+
CRD_GROUP, CRD_VERSION, RESOURCE_PLURAL,
288+
resource_name, namespace="default",
289+
)
290+
k8s.create_custom_resource(ref, resource_data)
291+
time.sleep(CREATE_WAIT_AFTER_SECONDS)
292+
cr = k8s.wait_resource_consumed_by_controller(ref)
293+
294+
assert cr is not None
295+
296+
resource_id = cr["status"]["vpcID"]
297+
298+
# Check VPC exists in AWS
299+
ec2_validator = EC2Validator(ec2_client)
300+
ec2_validator.assert_vpc(resource_id)
301+
302+
# Validate CIDR Block
303+
vpc = ec2_validator.get_vpc(resource_id)
304+
assert len(vpc['CidrBlockAssociationSet']) == 2
305+
assert vpc['CidrBlockAssociationSet'][0]['CidrBlock'] == PRIMARY_CIDR_DEFAULT
306+
assert vpc['CidrBlockAssociationSet'][1]['CidrBlock'] == "10.2.0.0/16"
307+
308+
# Delete k8s resource
309+
_, deleted = k8s.delete_custom_resource(ref, 3, 10)
310+
assert deleted is True
311+
312+
time.sleep(DELETE_WAIT_AFTER_SECONDS)
313+
314+
# Check VPC no longer exists in AWS
315+
ec2_validator.assert_vpc(resource_id, exists=False)
316+
317+
def test_vpc_updation_multiple_cidr(self,ec2_client):
318+
resource_name = random_suffix_name("vpc-ack-multicidr", 24)
319+
replacements = REPLACEMENT_VALUES.copy()
320+
replacements["VPC_NAME"] = resource_name
321+
replacements["PRIMARY_CIDR_BLOCK"] = PRIMARY_CIDR_DEFAULT
322+
replacements["SECONDARY_CIDR_BLOCK"] = "10.2.0.0/16"
323+
replacements["ENABLE_DNS_SUPPORT"] = "False"
324+
replacements["ENABLE_DNS_HOSTNAMES"] = "False"
325+
326+
# Load VPC CR
327+
resource_data = load_ec2_resource(
328+
"vpc_multicidr",
329+
additional_replacements=replacements,
330+
)
331+
logging.debug(resource_data)
332+
333+
# Create k8s resource
334+
ref = k8s.CustomResourceReference(
335+
CRD_GROUP, CRD_VERSION, RESOURCE_PLURAL,
336+
resource_name, namespace="default",
337+
)
338+
k8s.create_custom_resource(ref, resource_data)
339+
time.sleep(CREATE_WAIT_AFTER_SECONDS)
340+
cr = k8s.wait_resource_consumed_by_controller(ref)
341+
342+
assert cr is not None
343+
344+
resource_id = cr["status"]["vpcID"]
345+
346+
# Check VPC exists in AWS
347+
ec2_validator = EC2Validator(ec2_client)
348+
ec2_validator.assert_vpc(resource_id)
349+
350+
# Validate CIDR Block
351+
vpc = ec2_validator.get_vpc(resource_id)
352+
assert len(vpc['CidrBlockAssociationSet']) == 2
353+
assert vpc['CidrBlockAssociationSet'][0]['CidrBlock'] == PRIMARY_CIDR_DEFAULT
354+
assert vpc['CidrBlockAssociationSet'][1]['CidrBlock'] == "10.2.0.0/16"
355+
356+
357+
# Remove SECONDARY_CIDR_BLOCK
358+
updates = {
359+
"spec": {"cidrBlocks": [PRIMARY_CIDR_DEFAULT]}
360+
}
361+
k8s.patch_custom_resource(ref, updates)
362+
time.sleep(MODIFY_WAIT_AFTER_SECONDS)
363+
364+
# Re-Validate CIDR Blocks State
365+
vpc = ec2_validator.get_vpc(resource_id)
366+
assert vpc['CidrBlockAssociationSet'][0]['CidrBlockState']['State'] == "associated"
367+
assert vpc['CidrBlockAssociationSet'][1]['CidrBlockState']['State'] == "disassociated"
368+
369+
370+
# Delete k8s resource
371+
_, deleted = k8s.delete_custom_resource(ref, 3, 10)
372+
assert deleted is True
373+
374+
time.sleep(DELETE_WAIT_AFTER_SECONDS)
375+
376+
# Check VPC no longer exists in AWS
377+
ec2_validator.assert_vpc(resource_id, exists=False)

0 commit comments

Comments
 (0)