requeue db instance while not available (#28)

After creating a new DBInstance 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.

Fixes Issue aws-controllers-k8s/community#923

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license.

Author: jaypipes

apis/v1alpha1/ack-generate-metadata.yaml

@@ -1,14 +1,14 @@
 ack_generate_info:
-  build_date: "2021-09-01T12:27:55Z"
+  build_date: "2021-09-01T14:48:37Z"
   build_hash: 6f22b7b568e25b4ee007bb1ab5f9338c16daf172
   go_version: go1.17 linux/amd64
   version: v0.13.0
-api_directory_checksum: 387f19255122f7e68716af4d6dbce0e7498cfe37
+api_directory_checksum: 183f450ec5188cdb1cf76da50f21933f6b4cd7e3
 api_version: v1alpha1
 aws_sdk_go_version: v1.37.10
 generator_config_info:
-  file_checksum: b592c20bd6f321ab727cc08d77bcae382b5c1bd8
+  file_checksum: f62cefeb4c379bb1cc428d6685a4e5e4e182eb92
   original_file_name: generator.yaml
 last_modification:
   reason: API generation
-  timestamp: 2021-09-01 12:27:59.692202323 +0000 UTC
+  timestamp: 2021-09-01 14:48:42.205143913 +0000 UTC

apis/v1alpha1/generator.yaml

@@ -122,6 +122,10 @@ resources:
           path: Parameters
   DBInstance:
     hooks:
+      sdk_create_post_set_output:
+        template_path: hooks/db_instance/sdk_create_post_set_output.go.tpl
+      sdk_read_many_post_set_output:
+        template_path: hooks/db_instance/sdk_read_many_post_set_output.go.tpl
       sdk_update_pre_build_request:
         template_path: hooks/db_instance/sdk_update_pre_build_request.go.tpl
       sdk_delete_pre_build_request:

generator.yaml

@@ -122,6 +122,10 @@ resources:
           path: Parameters
   DBInstance:
     hooks:
+      sdk_create_post_set_output:
+        template_path: hooks/db_instance/sdk_create_post_set_output.go.tpl
+      sdk_read_many_post_set_output:
+        template_path: hooks/db_instance/sdk_read_many_post_set_output.go.tpl
       sdk_update_pre_build_request:
         template_path: hooks/db_instance/sdk_update_pre_build_request.go.tpl
       sdk_delete_pre_build_request:

pkg/resource/db_instance/hooks.go

@@ -125,6 +125,16 @@ func instanceHasTerminalStatus(r *resource) bool {
 	return false
 }
 
+// instanceAvailable returns true if the supplied DB instance is in an
+// available status
+func instanceAvailable(r *resource) bool {
+	if r.ko.Status.DBInstanceStatus == nil {
+		return false
+	}
+	dbis := *r.ko.Status.DBInstanceStatus
+	return dbis == StatusAvailable
+}
+
 // instanceCreating returns true if the supplied DB instance is in the process
 // of being created
 func instanceCreating(r *resource) bool {

pkg/resource/db_instance/sdk.go

@@ -0,0 +1,9 @@
+	// We expect the DB instance to be in 'creating' status since we just
+	// issued the call to create it, but I suppose it doesn't hurt to check
+	// here.
+	if instanceCreating(&resource{ko}) {
+		// Setting resource synced condition to false will trigger a requeue of
+		// the resource. No need to return a requeue error here.
+		setSyncedCondition(&resource{ko}, corev1.ConditionFalse, nil, nil)
+		return &resource{ko}, nil
+	}

templates/hooks/db_instance/sdk_create_post_set_output.go.tpl

@@ -0,0 +1,6 @@
+	if !instanceAvailable(&resource{ko}) {
+		// Setting resource synced condition to false will trigger a requeue of
+		// the resource. No need to return a requeue error here.
+		setSyncedCondition(&resource{ko}, corev1.ConditionFalse, nil, nil)
+		return &resource{ko}, nil
+	}

templates/hooks/db_instance/sdk_read_many_post_set_output.go.tpl

@@ -39,6 +39,9 @@
 CREATE_INTERVAL_SLEEP_SECONDS = 15
 # Time to wait before we get to an expected `available` state.
 CREATE_TIMEOUT_SECONDS = 600
+# Time we wait after resource becoming available in RDS and checking the CR's
+# Status has been updated
+CHECK_STATUS_WAIT_SECONDS = 10
 
 MODIFY_WAIT_AFTER_SECONDS = 20
 
@@ -91,6 +94,9 @@ def test_crud_postgres13_t3_micro(
         cr = k8s.wait_resource_consumed_by_controller(ref)
 
         assert cr is not None
+        assert 'status' in cr
+        assert 'dbInstanceStatus' in cr['status']
+        assert cr['status']['dbInstanceStatus'] == 'creating'
 
         # Let's check that the DB instance appears in RDS
         aws_res = rds_client.describe_db_instances(DBInstanceIdentifier=db_id)
@@ -110,6 +116,22 @@ def test_crud_postgres13_t3_micro(
             assert len(aws_res['DBInstances']) == 1
             dbi_rec = aws_res['DBInstances'][0]
 
+        time.sleep(CHECK_STATUS_WAIT_SECONDS)
+
+        # Before we update the DBInstance CR below, let's check to see that the
+        # DbInstanceStatus field in the CR has been updated to something other
+        # than 'creating', which is what is set after the initial creation.
+        # The CR's `Status.DBInstanceStatus` should be updated because the CR
+        # is requeued on successful reconciliation loops and subsequent
+        # reconciliation loops call ReadOne and should update the CR's Status
+        # with the latest observed information.
+        # https://github.com/aws-controllers-k8s/community/issues/923
+        cr = k8s.get_resource(ref)
+        assert cr is not None
+        assert 'status' in cr
+        assert 'dbInstanceStatus' in cr['status']
+        assert cr['status']['dbInstanceStatus'] != 'creating'
+
         # We're now going to modify the CopyTagsToSnapshot field of the DB
         # instance, wait some time and verify that the RDS server-side resource
         # shows the new value of the field.