Use the CreationTime to update LastModifiedTime (#49)

Issue #, if available:
Fixing [comment](#40 (review)) on previous PR #40 

Description of changes:
 - Use the CreationTime to update LastModifiedTime 
 - Add associated test to check LastModifiedTime

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

Author: mbaijal

apis/v1alpha1/ack-generate-metadata.yaml

@@ -1,13 +1,13 @@
 ack_generate_info:
-  build_date: "2021-09-16T00:00:15Z"
+  build_date: "2021-09-16T00:51:44Z"
   build_hash: 29a44ff03ae7d871faa8603c6ee9885cab5abd92
   go_version: go1.14.14
   version: v0.14.0
 api_directory_checksum: a13caf20935ebb6193efdee1ab377cae33311ad7
 api_version: v1alpha1
 aws_sdk_go_version: v1.37.10
 generator_config_info:
-  file_checksum: b9cb2cfa6c027466a00a98d2031047d9edaa648a
+  file_checksum: baa788a6de90973eba7fbee7ee1f146c803186f8
   original_file_name: generator.yaml
 last_modification:
   reason: API generation

apis/v1alpha1/generator.yaml

@@ -18,14 +18,14 @@ operations:
 resources:
   ScalableTarget:
     hooks:
-      sdk_create_post_set_output:
-        code: rm.customSetLastModifiedTime(ko)
+      sdk_read_many_post_set_output:
+        code: rm.customSetLastModifiedTimeToCreationTime(ko)
       sdk_read_many_post_build_request:
         code: rm.customDescribeScalableTarget(ctx, r, input)
       delta_pre_compare:
         code: customSetDefaults(a, b)
       sdk_update_post_set_output:
-        code: rm.customSetLastModifiedTime(ko)
+        code: rm.customSetLastModifiedTimeToCurrentTime(ko)
     fields:
       ResourceID:
         is_primary_key: true
@@ -41,10 +41,10 @@ resources:
           path: ScalableTargets..CreationTime
   ScalingPolicy:
     hooks:
-      sdk_create_post_set_output:
-        code: rm.customSetLastModifiedTime(ko)
+      sdk_read_many_post_set_output:
+        code: rm.customSetLastModifiedTimeToCreationTime(ko)
       sdk_update_post_set_output:
-        code: rm.customSetLastModifiedTime(ko)
+        code: rm.customSetLastModifiedTimeToCurrentTime(ko)
     fields:
       ResourceID:
         is_primary_key: true

generator.yaml

@@ -18,14 +18,14 @@ operations:
 resources:
   ScalableTarget:
     hooks:
-      sdk_create_post_set_output:
-        code: rm.customSetLastModifiedTime(ko)
+      sdk_read_many_post_set_output:
+        code: rm.customSetLastModifiedTimeToCreationTime(ko)
       sdk_read_many_post_build_request:
         code: rm.customDescribeScalableTarget(ctx, r, input)
       delta_pre_compare:
         code: customSetDefaults(a, b)
       sdk_update_post_set_output:
-        code: rm.customSetLastModifiedTime(ko)
+        code: rm.customSetLastModifiedTimeToCurrentTime(ko)
     fields:
       ResourceID:
         is_primary_key: true
@@ -41,10 +41,10 @@ resources:
           path: ScalableTargets..CreationTime
   ScalingPolicy:
     hooks:
-      sdk_create_post_set_output:
-        code: rm.customSetLastModifiedTime(ko)
+      sdk_read_many_post_set_output:
+        code: rm.customSetLastModifiedTimeToCreationTime(ko)
       sdk_update_post_set_output:
-        code: rm.customSetLastModifiedTime(ko)
+        code: rm.customSetLastModifiedTimeToCurrentTime(ko)
     fields:
       ResourceID:
         is_primary_key: true

pkg/resource/scalable_target/custom_api.go

@@ -35,8 +35,15 @@ func (rm *resourceManager) customDescribeScalableTarget(
 	}
 }
 
-// customSetOutputUpdate sets the LastModifiedTime field to the current time post an update
-func (rm *resourceManager) customSetLastModifiedTime(ko *svcapitypes.ScalableTarget) {
+// customSetLastModifiedTimeToCreationTime sets the LastModifiedTime field to the creationTime
+func (rm *resourceManager) customSetLastModifiedTimeToCreationTime(ko *svcapitypes.ScalableTarget) {
+	if ko.Status.CreationTime != nil && ko.Status.LastModifiedTime == nil {
+		ko.Status.LastModifiedTime = ko.Status.CreationTime
+	}
+}
+
+// customSetLastModifiedTimeToCurrentTime sets the LastModifiedTime field to the current time post an update
+func (rm *resourceManager) customSetLastModifiedTimeToCurrentTime(ko *svcapitypes.ScalableTarget) {
 	currentTime := metav1.Time{Time: time.Now().UTC()}
 	ko.Status.LastModifiedTime = &currentTime
 }

pkg/resource/scalable_target/sdk.go

@@ -19,8 +19,15 @@ import (
 	"time"
 )
 
-// customSetOutputUpdate sets the LastModifiedTime field to the current time post an update
-func (rm *resourceManager) customSetLastModifiedTime(ko *svcapitypes.ScalingPolicy) {
+// customSetLastModifiedTimeToCreationTime sets the LastModifiedTime field to the creationTime
+func (rm *resourceManager) customSetLastModifiedTimeToCreationTime(ko *svcapitypes.ScalingPolicy) {
+	if ko.Status.CreationTime != nil && ko.Status.LastModifiedTime == nil {
+		ko.Status.LastModifiedTime = ko.Status.CreationTime
+	}
+}
+
+// customSetLastModifiedTimeToCurrentTime sets the LastModifiedTime field to the current time post an update
+func (rm *resourceManager) customSetLastModifiedTimeToCurrentTime(ko *svcapitypes.ScalingPolicy) {
 	currentTime := metav1.Time{Time: time.Now().UTC()}
 	ko.Status.LastModifiedTime = &currentTime
 }

pkg/resource/scaling_policy/custom_api.go

@@ -143,7 +143,9 @@ def adopt_scaling_policy(adopt_scalable_target):
 @service_marker
 @pytest.mark.canary
 class TestAdopted:
-    def test_sagemaker_endpoint_autoscaling(self, put_scaling_policy, adopt_scaling_policy):
+    def test_sagemaker_endpoint_autoscaling(
+        self, put_scaling_policy, adopt_scaling_policy
+    ):
         sdk_resource_id = put_scaling_policy
 
         (

pkg/resource/scaling_policy/sdk.go

@@ -16,6 +16,7 @@
 import boto3
 import botocore
 import pytest
+import datetime
 import logging
 from typing import Dict, Tuple
 
@@ -83,7 +84,11 @@ def generate_sagemaker_target(sagemaker_endpoint):
     replacements["SCALABLETARGET_NAME"] = target_resource_name
     replacements["RESOURCE_ID"] = resource_id
 
-    target_reference, target_spec, target_resource = create_applicationautoscaling_resource(
+    (
+        target_reference,
+        target_spec,
+        target_resource,
+    ) = create_applicationautoscaling_resource(
         resource_plural=TARGET_RESOURCE_PLURAL,
         resource_name=target_resource_name,
         spec_file="sagemaker_endpoint_autoscaling_target",
@@ -92,7 +97,7 @@ def generate_sagemaker_target(sagemaker_endpoint):
 
     assert target_resource is not None
 
-    yield (resource_id, target_spec, target_reference)
+    yield (resource_id, target_reference, target_spec, target_resource)
 
     if k8s.get_resource_exists(target_reference):
         _, deleted = k8s.delete_custom_resource(target_reference)
@@ -101,14 +106,23 @@ def generate_sagemaker_target(sagemaker_endpoint):
 
 @pytest.fixture(scope="module")
 def generate_sagemaker_policy(generate_sagemaker_target):
-    resource_id, target_spec, target_reference = generate_sagemaker_target
+    (
+        resource_id,
+        target_reference,
+        target_spec,
+        target_resource,
+    ) = generate_sagemaker_target
     policy_resource_name = random_suffix_name("sagemaker-scaling-policy", 32)
 
     replacements = REPLACEMENT_VALUES.copy()
     replacements["SCALINGPOLICY_NAME"] = policy_resource_name
     replacements["RESOURCE_ID"] = resource_id
 
-    policy_reference, policy_spec, policy_resource = create_applicationautoscaling_resource(
+    (
+        policy_reference,
+        policy_spec,
+        policy_resource,
+    ) = create_applicationautoscaling_resource(
         resource_plural=POLICY_RESOURCE_PLURAL,
         resource_name=policy_resource_name,
         spec_file="sagemaker_endpoint_autoscaling_policy",
@@ -117,7 +131,15 @@ def generate_sagemaker_policy(generate_sagemaker_target):
 
     assert policy_resource is not None
 
-    yield (resource_id, target_spec, target_reference, policy_resource, policy_spec, policy_reference)
+    yield (
+        resource_id,
+        target_reference,
+        target_spec,
+        target_resource,
+        policy_resource,
+        policy_spec,
+        policy_reference,
+    )
 
     if k8s.get_resource_exists(policy_reference):
         _, deleted = k8s.delete_custom_resource(policy_reference)
@@ -127,6 +149,23 @@ def generate_sagemaker_policy(generate_sagemaker_target):
 @service_marker
 @pytest.mark.canary
 class TestSageMakerEndpointAutoscaling:
+    def wait_until_update(
+        self, reference, previous_modified_time, wait_period=2, wait_time=30
+    ):
+        for i in range(wait_period):
+            resource = k8s.get_resource(reference)
+            assert resource is not None
+            assert "lastModifiedTime" in resource["status"]
+            last_modified_time = resource["status"]["lastModifiedTime"]
+            d1 = datetime.datetime.strptime(last_modified_time, "%Y-%m-%dT%H:%M:%SZ")
+            d2 = datetime.datetime.strptime(
+                previous_modified_time, "%Y-%m-%dT%H:%M:%SZ"
+            )
+            if d1 > d2:
+                return True
+            sleep(wait_time)
+        return False
+
     def get_sagemaker_scalable_target_description(
         self, applicationautoscaling_client, resource_id: str, expectedTargets: int
     ):
@@ -163,8 +202,9 @@ def get_sagemaker_scaling_policy_description(
     def test_create(self, applicationautoscaling_client, generate_sagemaker_policy):
         (
             resource_id,
-            target_spec,
             target_reference,
+            target_spec,
+            target_resource,
             policy_resource,
             policy_spec,
             policy_reference,
@@ -189,46 +229,56 @@ def test_create(self, applicationautoscaling_client, generate_sagemaker_policy):
     def test_update(self, applicationautoscaling_client, generate_sagemaker_policy):
         (
             resource_id,
-            target_spec,
             target_reference,
+            target_spec,
+            target_resource,
             policy_resource,
             policy_spec,
             policy_reference,
         ) = generate_sagemaker_policy
 
         updatedMaxCapacity = 4
         updatedTargetValue = 120
-        
+
         # Update the ScalableTarget
         target_spec["spec"]["maxCapacity"] = updatedMaxCapacity
+        assert "lastModifiedTime" in target_resource["status"]
+        last_modified_time = target_resource["status"]["lastModifiedTime"]
         k8s.patch_custom_resource(target_reference, target_spec)
-        sleep(5)
-        
+        assert self.wait_until_update(target_reference, last_modified_time) == True
+
         updated_target_description = self.get_sagemaker_scalable_target_description(
             applicationautoscaling_client, resource_id, 1
         )
         assert updated_target_description is not None
-        assert (
-            updated_target_description[0]["MaxCapacity"] == updatedMaxCapacity
-        )
+        assert updated_target_description[0]["MaxCapacity"] == updatedMaxCapacity
 
         # Update the ScalingPolicy
-        policy_spec["spec"]["targetTrackingScalingPolicyConfiguration"]["targetValue"] = updatedTargetValue
+        policy_spec["spec"]["targetTrackingScalingPolicyConfiguration"][
+            "targetValue"
+        ] = updatedTargetValue
+        assert "lastModifiedTime" in policy_resource["status"]
+        last_modified_time = policy_resource["status"]["lastModifiedTime"]
         k8s.patch_custom_resource(policy_reference, policy_spec)
-        sleep(5)
+        assert self.wait_until_update(policy_reference, last_modified_time) == True
+
         updated_policy_description = self.get_sagemaker_scaling_policy_description(
             applicationautoscaling_client, resource_id, 1
         )
         assert updated_policy_description is not None
         assert (
-            updated_policy_description[0]["TargetTrackingScalingPolicyConfiguration"]["TargetValue"] == updatedTargetValue
+            updated_policy_description[0]["TargetTrackingScalingPolicyConfiguration"][
+                "TargetValue"
+            ]
+            == updatedTargetValue
         )
 
     def test_delete(self, applicationautoscaling_client, generate_sagemaker_policy):
         (
             resource_id,
-            target_spec,
             target_reference,
+            target_spec,
+            target_resource,
             policy_resource,
             policy_spec,
             policy_reference,
@@ -249,4 +299,4 @@ def test_delete(self, applicationautoscaling_client, generate_sagemaker_policy):
         # TODO: Ideally this check should pass after line 188 itself; but it requires the scalabletarget to be deleted too.
         policy_description = self.get_sagemaker_scaling_policy_description(
             applicationautoscaling_client, resource_id, 0
-        )
+        )