address regression with User code, fix RG deletion test

Author: echen-98

pkg/resource/user/delta_util.go

@@ -23,12 +23,10 @@ func filterDelta(
 ) {
 	// the returned AccessString can be different than the specified one; as long as the last requested AccessString
 	// matches the currently desired one, remove this difference from the delta
-	//TODO: revert this call to Spec.AccessString once we have a new implementation of it
-	if delta.DifferentAt("AccessString") {
+	if delta.DifferentAt("Spec.AccessString") {
 		if *desired.ko.Spec.AccessString == *desired.ko.Status.LastRequestedAccessString {
 
-			//TODO: revert the call to Spec.AccessString once removeFromDelta implementation changes
-			removeFromDelta(delta, "AccessString")
+			removeFromDelta(delta, "Spec.AccessString")
 		}
 	}
 }
@@ -43,7 +41,7 @@ func removeFromDelta(
 	differences := delta.Differences
 
 	// identify index of Difference to remove
-	//TODO: change once we get a Path.Equals or similar method
+	//TODO: this could require a stricter Path.Equals down the road
 	var i *int = nil
 	for j, diff := range differences {
 		if diff.Path.Contains(subject) {

pkg/resource/user/post_build_request.go

@@ -26,13 +26,11 @@ func (rm *resourceManager) populateUpdatePayload(
 	r *resource,
 	delta *ackcompare.Delta,
 ) {
-	//TODO: change all calls to DifferentAt to include full path once the implementation is clarified
-
-	if delta.DifferentAt("AccessString") && r.ko.Spec.AccessString != nil {
+	if delta.DifferentAt("Spec.AccessString") && r.ko.Spec.AccessString != nil {
 		input.AccessString = r.ko.Spec.AccessString
 	}
 
-	if delta.DifferentAt("NoPasswordRequired") && r.ko.Spec.NoPasswordRequired != nil {
+	if delta.DifferentAt("Spec.NoPasswordRequired") && r.ko.Spec.NoPasswordRequired != nil {
 		input.NoPasswordRequired = r.ko.Spec.NoPasswordRequired
 	}
 

test/e2e/tests/test_replicationgroup.py

@@ -154,6 +154,24 @@ def rg_cmd_update(rg_cmd_update_input, make_replication_group, rg_deletion_waite
     rg_deletion_waiter.wait(ReplicationGroupId=input_dict["RG_ID"])
 
 
+@pytest.fixture(scope="module")
+def rg_deletion_input(make_rg_name):
+    return {
+        "RG_ID": make_rg_name("rg-delete"),
+        "ENGINE_VERSION": "6.x",
+        "NUM_NODE_GROUPS": "1",
+        "REPLICAS_PER_NODE_GROUP": "1"
+    }
+
+
+@pytest.fixture(scope="module")
+def rg_deletion(rg_deletion_input, make_replication_group):
+    input_dict = rg_deletion_input
+
+    (reference, resource) = make_replication_group("replicationgroup_cmd_update", input_dict, input_dict["RG_ID"])
+    return (reference, resource)  # no teardown, as the teardown is part of the actual test
+
+
 @service_marker
 class TestReplicationGroup:
 
@@ -229,9 +247,8 @@ def test_rg_auth_token(self, rg_auth_token):
         k8s.patch_custom_resource(reference, updated_spec)
         assert k8s.wait_on_condition(reference, "ACK.ResourceSynced", "True", wait_periods=30)
 
-    def test_rg_delete(self, rg_cmd_update_input, rg_deletion_waiter):
-        input_dict = rg_cmd_update_input
-        (reference, _) = make_replication_group("replicationgroup_cmd_update", input_dict, input_dict["RG_ID"])
+    def test_rg_deletion(self, rg_deletion_input, rg_deletion, rg_deletion_waiter):
+        (reference, _) = rg_deletion
         assert k8s.wait_on_condition(reference, "ACK.ResourceSynced", "True", wait_periods=30)
 
         # assertions after initial creation
@@ -244,7 +261,7 @@ def test_rg_delete(self, rg_cmd_update_input, rg_deletion_waiter):
 
         resource = k8s.get_resource(reference)
         assert resource['metadata']['deletionTimestamp'] is not None
-        # uncomment when reconciler->cleanup() invokes patchResource()
+        # TODO: uncomment when reconciler->cleanup() invokes patchResource()
         # assert k8s.wait_on_condition(reference, "ACK.ResourceSynced", "False", wait_periods=1)
 
-        rg_deletion_waiter.wait(ReplicationGroupId=input_dict["RG_ID"])
+        rg_deletion_waiter.wait(ReplicationGroupId=rg_deletion_input["RG_ID"])