Skip to content

Commit 8bca7c9

Browse files
committed
requeue db cluster while not available
After creating a new DBCluster resource, we were not setting ConditionTypeResourceSynced on the resource nor were we asking to requeue the resource after some time. This meant that the resource's Status was not being updated on the Kubernetes side, even after the DB instance's status transitioned to 'available' on the RDS side. In this patch, we're adding two generic hook implementations for sdk_read_many_post_set_output and sdk_create_post_set_output that will create a ConditionTypeResourceSynced on the resource with a value of ConditionFalse (because the resource isn't actually synced yet). This causes the controller to queue the resource up for a new reconciliation loop, during which the resource's Status will be updated to the latest observed values. Issue aws-controllers-k8s/community#923
1 parent cbe4828 commit 8bca7c9

File tree

8 files changed

+80
-6
lines changed

8 files changed

+80
-6
lines changed
Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
ack_generate_info:
2-
build_date: "2021-10-05T23:44:00Z"
3-
build_hash: 2b0ac062dd6d305434d252b5da5ed718c801b106
4-
go_version: go1.17.1
5-
version: v0.14.1
6-
api_directory_checksum: cd0469d4474d82dbcf9badb97a43cac68ba07044
2+
build_date: "2021-09-01T17:48:47Z"
3+
build_hash: 6f22b7b568e25b4ee007bb1ab5f9338c16daf172
4+
go_version: go1.17 linux/amd64
5+
version: v0.13.0
6+
api_directory_checksum: 387f19255122f7e68716af4d6dbce0e7498cfe37
77
api_version: v1alpha1
88
aws_sdk_go_version: v1.37.10
99
generator_config_info:
10-
file_checksum: ec38b79b167ebd86c08109049864acd6a21e5113
10+
file_checksum: f5d26ad7ae3f2cdc82f2e08c45f5b0aefafb9fa4
1111
original_file_name: generator.yaml
1212
last_modification:
1313
reason: API generation
14+
timestamp: 2021-09-01 17:48:52.072416982 +0000 UTC

apis/v1alpha1/generator.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,10 @@ resources:
6666
# resolved.
6767
custom_method_name: customUpdate
6868
hooks:
69+
sdk_create_post_set_output:
70+
template_path: hooks/db_cluster/sdk_create_post_set_output.go.tpl
71+
sdk_read_many_post_set_output:
72+
template_path: hooks/db_cluster/sdk_read_many_post_set_output.go.tpl
6973
sdk_update_pre_build_request:
7074
template_path: hooks/db_cluster/sdk_update_pre_build_request.go.tpl
7175
sdk_delete_pre_build_request:

generator.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,10 @@ resources:
6666
# resolved.
6767
custom_method_name: customUpdate
6868
hooks:
69+
sdk_create_post_set_output:
70+
template_path: hooks/db_cluster/sdk_create_post_set_output.go.tpl
71+
sdk_read_many_post_set_output:
72+
template_path: hooks/db_cluster/sdk_read_many_post_set_output.go.tpl
6973
sdk_update_pre_build_request:
7074
template_path: hooks/db_cluster/sdk_update_pre_build_request.go.tpl
7175
sdk_delete_pre_build_request:

pkg/resource/db_cluster/hooks.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,16 @@ func clusterHasTerminalStatus(r *resource) bool {
116116
return false
117117
}
118118

119+
// clusterAvailable returns true if the supplied DB cluster is in an
120+
// available status
121+
func clusterAvailable(r *resource) bool {
122+
if r.ko.Status.Status == nil {
123+
return false
124+
}
125+
dbcs := *r.ko.Status.Status
126+
return dbcs == StatusAvailable
127+
}
128+
119129
// clusterCreating returns true if the supplied DB cluster is in the process
120130
// of being created
121131
func clusterCreating(r *resource) bool {

pkg/resource/db_cluster/sdk.go

Lines changed: 17 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// We expect the DB cluster to be in 'creating' status since we just
2+
// issued the call to create it, but I suppose it doesn't hurt to check
3+
// here.
4+
if clusterCreating(&resource{ko}) {
5+
// Setting resource synced condition to false will trigger a requeue of
6+
// the resource. No need to return a requeue error here.
7+
setSyncedCondition(&resource{ko}, corev1.ConditionFalse, nil, nil)
8+
return &resource{ko}, nil
9+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
if !clusterAvailable(&resource{ko}) {
2+
// Setting resource synced condition to false will trigger a requeue of
3+
// the resource. No need to return a requeue error here.
4+
setSyncedCondition(&resource{ko}, corev1.ConditionFalse, nil, nil)
5+
return &resource{ko}, nil
6+
}
7+

test/e2e/tests/test_db_cluster.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@
3535

3636
CREATE_INTERVAL_SLEEP_SECONDS = 15
3737
CREATE_TIMEOUT_SECONDS = 600
38+
# Time we wait after resource becoming available in RDS and checking the CR's
39+
# Status has been updated
40+
CHECK_STATUS_WAIT_SECONDS = 10
3841

3942
MODIFY_WAIT_AFTER_SECONDS = 20
4043

@@ -88,6 +91,9 @@ def test_create_delete_mysql_serverless(
8891
cr = k8s.wait_resource_consumed_by_controller(ref)
8992

9093
assert cr is not None
94+
assert 'status' in cr
95+
assert 'status' in cr['status']
96+
assert cr['status']['status'] == 'creating'
9197

9298
# Let's check that the DB cluster appears in RDS
9399
aws_res = rds_client.describe_db_clusters(DBClusterIdentifier=db_cluster_id)
@@ -107,6 +113,22 @@ def test_create_delete_mysql_serverless(
107113
assert len(aws_res['DBClusters']) == 1
108114
dbc_rec = aws_res['DBClusters'][0]
109115

116+
time.sleep(CHECK_STATUS_WAIT_SECONDS)
117+
118+
# Before we update the DBCluster CR below, let's check to see that the
119+
# Status field in the CR has been updated to something other than
120+
# 'creating', which is what is set after the initial creation. The
121+
# CR's `Status.Status` should be updated because the CR is requeued on
122+
# successful reconciliation loops and subsequent reconciliation loops
123+
# call ReadOne and should update the CR's Status with the latest
124+
# observed information.
125+
# https://github.com/aws-controllers-k8s/community/issues/923
126+
cr = k8s.get_resource(ref)
127+
assert cr is not None
128+
assert 'status' in cr
129+
assert 'status' in cr['status']
130+
assert cr['status']['status'] != 'creating'
131+
110132
# We're now going to modify the CopyTagsToSnapshot field of the DB
111133
# instance, wait some time and verify that the RDS server-side resource
112134
# shows the new value of the field.

0 commit comments

Comments
 (0)