Skip to content

Commit b3214fb

Browse files
michaelhtmmichaelhtm
authored
fix: ensure Filesystem resource is synced when available (#41)
Issue [#2468](aws-controllers-k8s/community#2468) Description of changes: removing the following portion from generator.yaml: ```yaml sdk_update_post_request: code: return desired, nil ``` This portion was returning `desired, nil`, right after an update call was made, without checking if the update call returned an error, nor logging the metrics call. Instead using ```yaml sdk_update_pre_set_output: template_path: ... ``` Which checks for update errors, logs API Metrics, and ensures ko.Status is latest.Status (ensuring we return the updated Status to be patched to k8s) By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent d0bc984 commit b3214fb

File tree

8 files changed

+46
-26
lines changed

8 files changed

+46
-26
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ api_directory_checksum: ebeb6f826282ad9134ca78efd74dc9125898fddf
77
api_version: v1alpha1
88
aws_sdk_go_version: v1.32.6
99
generator_config_info:
10-
file_checksum: 99053458e09d8c915086d82f2f76c55d6374efef
10+
file_checksum: 8f51ccb0fdf00c0900f3cd0641fe851d5909686e
1111
original_file_name: generator.yaml
1212
last_modification:
1313
reason: API generation

‎apis/v1alpha1/generator.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ resources:
5656
hooks:
5757
delta_pre_compare:
5858
code: customPreCompare(delta, a, b)
59-
sdk_update_post_request:
60-
code: return desired, nil
59+
sdk_update_pre_set_output:
60+
template_path: hooks/file_system/sdk_update_pre_set_output.go.tpl
6161
sdk_create_post_build_request:
6262
template_path: hooks/file_system/sdk_create_post_build_request.go.tpl
6363
sdk_read_many_post_set_output:

‎generator.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ resources:
5656
hooks:
5757
delta_pre_compare:
5858
code: customPreCompare(delta, a, b)
59-
sdk_update_post_request:
60-
code: return desired, nil
59+
sdk_update_pre_set_output:
60+
template_path: hooks/file_system/sdk_update_pre_set_output.go.tpl
6161
sdk_create_post_build_request:
6262
template_path: hooks/file_system/sdk_create_post_build_request.go.tpl
6363
sdk_read_many_post_set_output:

‎pkg/resource/file_system/hooks.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func requeueWaitState(r *resource) *ackrequeue.RequeueNeededAfter {
5656
return ackrequeue.NeededAfter(
5757
fmt.Errorf("filesystem in '%s' state, requeuing until filesystem is '%s'",
5858
status, svcapitypes.LifeCycleState_available),
59-
time.Second*10,
59+
time.Second*3,
6060
)
6161
}
6262

‎pkg/resource/file_system/sdk.go

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

‎templates/hooks/file_system/sdk_update_pre_build_request.go.tpl

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,44 @@
1-
1+
res := desired.ko.DeepCopy()
2+
// This step Will ensure that the latest Status
3+
// is patched into k8s, and user will be able
4+
// to see the latest efs filesystem Status
5+
res.Status = latest.ko.Status
26
if delta.DifferentAt("Spec.Tags") {
37
err := syncTags(
4-
ctx, rm.sdkapi, rm.metrics,
5-
string(*desired.ko.Status.ACKResourceMetadata.ARN),
8+
ctx, rm.sdkapi, rm.metrics,
9+
string(*desired.ko.Status.ACKResourceMetadata.ARN),
610
desired.ko.Spec.Tags, latest.ko.Spec.Tags,
711
)
812
if err != nil {
9-
return nil, err
13+
return &resource{res}, err
1014
}
1115
}
1216
if delta.DifferentAt("Spec.Policy") {
1317
err := rm.syncPolicy(ctx, desired)
1418
if err != nil {
15-
return nil, err
19+
return &resource{res}, err
1620
}
1721
}
1822
if delta.DifferentAt("Spec.BackupPolicy") {
1923
err := rm.syncBackupPolicy(ctx, desired)
2024
if err != nil {
21-
return nil, err
25+
return &resource{res}, err
2226
}
2327
}
2428
if delta.DifferentAt("Spec.LifecyclePolicies") {
2529
err := rm.syncLifecyclePolicies(ctx, desired)
2630
if err != nil {
27-
return nil, err
31+
return &resource{res}, err
2832
}
2933
}
3034
if delta.DifferentAt("Spec.FileSystemProtection") {
3135
err := rm.syncFilesystemProtection(ctx, desired)
3236
if err != nil {
33-
return nil, err
37+
return &resource{res}, err
3438
}
3539
}
3640
// To trigger to normal update we need to make sure that at least
3741
// one of the following fields are different.
38-
if !delta.DifferentAt("Spec.ProvisionedThroughputInMiBps") && !delta.DifferentAt("Spec.ThroughputMode") {
39-
return desired, nil
40-
}
42+
if !delta.DifferentAt("Spec.ProvisionedThroughputInMiBps") && !delta.DifferentAt("Spec.ThroughputMode") {
43+
return &resource{res}, nil
44+
}

‎templates/hooks/file_system/sdk_update_pre_set_output.go.tpl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
ko.Status = latest.ko.Status
2+
return &resource{ko}, nil

‎test/e2e/tests/test_file_system.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,12 @@ def simple_file_system(efs_client):
9090
class TestFileSystem:
9191

9292
def test_create_delete(self, efs_client, simple_file_system):
93-
(_, _, file_system_id) = simple_file_system
93+
(ref, _, file_system_id) = simple_file_system
9494
assert file_system_id is not None
9595

9696
validator = EFSValidator(efs_client)
9797
assert validator.file_system_exists(file_system_id)
98+
assert k8s.wait_on_condition(ref, "ACK.ResourceSynced", "True", wait_periods=5)
9899

99100
def test_create_update_delete(self, efs_client, simple_file_system):
100101
(ref, _, file_system_id) = simple_file_system
@@ -116,6 +117,7 @@ def test_create_update_delete(self, efs_client, simple_file_system):
116117

117118
fs = validator.get_file_system(file_system_id)
118119
assert fs is not None
120+
assert k8s.wait_on_condition(ref, "ACK.ResourceSynced", "True", wait_periods=5)
119121
assert fs['FileSystems'][0]['ThroughputMode'] == "elastic"
120122
assert fs['FileSystems'][0]['FileSystemProtection']['ReplicationOverwriteProtection'] == "DISABLED"
121123

@@ -138,6 +140,7 @@ def test_update_policies(self, efs_client, simple_file_system):
138140
time.sleep(CREATE_WAIT_AFTER_SECONDS)
139141

140142
observedPolicy = validator.get_file_system_policy(file_system_id)
143+
assert k8s.wait_on_condition(ref, "ACK.ResourceSynced", "True", wait_periods=5)
141144
assert json.loads(policy) == json.loads(observedPolicy)
142145

143146
updates = {
@@ -169,6 +172,7 @@ def test_update_backup_policy(self, efs_client, simple_file_system):
169172

170173
bp = validator.get_backup_policy(file_system_id)
171174
assert bp is not None
175+
assert k8s.wait_on_condition(ref, "ACK.ResourceSynced", "True", wait_periods=5)
172176
assert bp['Status'] == "DISABLED"
173177

174178
def test_update_lifecycle_policies(self, efs_client, simple_file_system):
@@ -178,6 +182,7 @@ def test_update_lifecycle_policies(self, efs_client, simple_file_system):
178182
validator = EFSValidator(efs_client)
179183
assert validator.file_system_exists(file_system_id)
180184

185+
assert k8s.wait_on_condition(ref, "ACK.ResourceSynced", "True", wait_periods=5)
181186
updates = {
182187
"spec": {
183188
"lifecyclePolicies": [
@@ -190,6 +195,7 @@ def test_update_lifecycle_policies(self, efs_client, simple_file_system):
190195

191196
lfps = validator.get_file_system_lifecycle_policy(file_system_id)
192197
assert lfps is not None
198+
assert k8s.wait_on_condition(ref, "ACK.ResourceSynced", "True", wait_periods=5)
193199
assert lfps[0]['TransitionToIA'] == "AFTER_30_DAYS"
194200

195201
def test_update_tags(self, efs_client, simple_file_system):
@@ -198,6 +204,7 @@ def test_update_tags(self, efs_client, simple_file_system):
198204

199205
validator = EFSValidator(efs_client)
200206
assert validator.file_system_exists(file_system_id)
207+
assert k8s.wait_on_condition(ref, "ACK.ResourceSynced", "True", wait_periods=5)
201208

202209
desired_tags = [{
203210
"key": "Name",
@@ -213,6 +220,7 @@ def test_update_tags(self, efs_client, simple_file_system):
213220

214221
fs = validator.get_file_system(file_system_id)
215222
assert fs is not None
223+
assert k8s.wait_on_condition(ref, "ACK.ResourceSynced", "True", wait_periods=5)
216224
latest_tags = fs['FileSystems'][0]["Tags"]
217225

218226
tags.assert_ack_system_tags(

0 commit comments

Comments
 (0)