From 913cb1eaec26707cd3a2fc0db250440f829e0adf Mon Sep 17 00:00:00 2001 From: Eric Chen Date: Wed, 16 Jun 2021 16:00:36 -0700 Subject: [PATCH] address regression with User code, fix RG deletion test --- pkg/resource/user/delta_util.go | 8 +++----- pkg/resource/user/post_build_request.go | 6 ++---- test/e2e/tests/test_replicationgroup.py | 27 ++++++++++++++++++++----- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/pkg/resource/user/delta_util.go b/pkg/resource/user/delta_util.go index b981597f..783a3bdf 100644 --- a/pkg/resource/user/delta_util.go +++ b/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) { diff --git a/pkg/resource/user/post_build_request.go b/pkg/resource/user/post_build_request.go index 6e3f12a4..40e2c4e3 100644 --- a/pkg/resource/user/post_build_request.go +++ b/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 } diff --git a/test/e2e/tests/test_replicationgroup.py b/test/e2e/tests/test_replicationgroup.py index 8bd7bccc..6fa1a511 100644 --- a/test/e2e/tests/test_replicationgroup.py +++ b/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"])