rework Table e2e tests to remove flakiness (#52)

There were two e2e tests that were flaky in
`test/e2e/tests/test_table.py`.

The flakiest test was test_enable_ttl, which often would result in the following:

```
>       assert ttl["TimeToLiveDescription"]["AttributeName"] == "ForumName"
E       KeyError: 'AttributeName'

tests/test_table.py:158: KeyError
```

This was caused by the dynamodb client `describe_time_to_live` returning
a `ResourceNotFoundException` when the Table was not in ACTIVE status
and even when the Table was in ACTIVE status, there is an amount of time
when the DescribeTimeToLive's TimeToLiveDescription output shape is
either empty or is a struct with no fields :(

I updated the test to first wait until the Table becomes ACTIVE and has
valid output responses before attempting to read the TTL information.

The second flaky test was the test_table_update_tags. The sleep after
calling the `kubectl patch` with the updated tags was too short at 5
seconds since it sometimes takes a little while for the tags to appear
in the Tagris APIs. Increasing this sleep to 10 fixes this flakiness.

Issue aws-controllers-k8s/community#1561

Signed-off-by: Jay Pipes <[email protected]>

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

Author: jaypipes

test/e2e/table.py

@@ -0,0 +1,125 @@
+# Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You may
+# not use this file except in compliance with the License. A copy of the
+# License is located at
+#
+#	 http://aws.amazon.com/apache2.0/
+#
+# or in the "license" file accompanying this file. This file is distributed
+# on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+# express or implied. See the License for the specific language governing
+# permissions and limitations under the License.
+
+"""Utilities for working with Table resources"""
+
+import datetime
+import time
+import typing
+
+import boto3
+import pytest
+
+DEFAULT_WAIT_UNTIL_TIMEOUT_SECONDS = 60*10
+DEFAULT_WAIT_UNTIL_INTERVAL_SECONDS = 15
+
+TableMatchFunc = typing.NewType(
+    'TableMatchFunc',
+    typing.Callable[[dict], bool],
+)
+
+class StatusMatcher:
+    def __init__(self, status):
+        self.match_on = status
+
+    def __call__(self, record: dict) -> bool:
+        return ('TableStatus' in record
+                and record['TableStatus'] == self.match_on)
+
+
+def status_matches(status: str) -> TableMatchFunc:
+    return StatusMatcher(status)
+
+
+class TTLAttributeMatcher:
+    def __init__(self, attr_name):
+        self.attr_name = attr_name
+
+    def __call__(self, record: dict) -> bool:
+        if 'TableStatus' in record and record['TableStatus'] != 'ACTIVE':
+            return False
+        table_name = record['TableName']
+        # NOTE(jaypipes): The reason we have to do this craziness is because
+        # DynamoDB's DescribeTimeToLive API is straight up bonkers. If you
+        # update the TTL on a Table, the Table needs to transition to ACTIVE
+        # status before DescribeTimeToLive will return a 200 and even after it
+        # does, you need to wait additional time until the
+        # TimeToLiveDescription response shape contains an AttributeName field
+        # that matches what you set in your update call. The
+        # TimeToLiveDescription field can be empty or can be a blank struct
+        # with no fields in it for a long time after updating TTL on a Table...
+        ttl = get_time_to_live(table_name)
+        if ttl is not None:
+            if 'AttributeName' in ttl:
+                if ttl['AttributeName'] == self.attr_name:
+                    return True
+        return False
+
+
+def ttl_on_attribute_matches(attr_name: str) -> TableMatchFunc:
+    return TTLAttributeMatcher(attr_name)
+
+
+def wait_until(
+        table_name: str,
+        match_fn: TableMatchFunc,
+        timeout_seconds: int = DEFAULT_WAIT_UNTIL_TIMEOUT_SECONDS,
+        interval_seconds: int = DEFAULT_WAIT_UNTIL_INTERVAL_SECONDS,
+    ) -> None:
+    """Waits until a Table with a supplied name is returned from the DynamoDB
+    API and the matching functor returns True.
+
+    Usage:
+        from e2e.table import wait_until, status_matches
+
+        wait_until(
+            table_name,
+            status_matches("ACTIVE"),
+        )
+
+    Raises:
+        pytest.fail upon timeout
+    """
+    now = datetime.datetime.now()
+    timeout = now + datetime.timedelta(seconds=timeout_seconds)
+
+    while not match_fn(get(table_name)):
+        if datetime.datetime.now() >= timeout:
+            pytest.fail("failed to match DBInstance before timeout")
+        time.sleep(interval_seconds)
+
+
+def get(table_name):
+    """Returns a dict containing the Role record from the IAM API.
+
+    If no such Table exists, returns None.
+    """
+    c = boto3.client('dynamodb')
+    try:
+        resp = c.describe_table(TableName=table_name)
+        return resp['Table']
+    except c.exceptions.ResourceNotFoundException:
+        return None
+
+
+def get_time_to_live(table_name):
+    """Returns the TTL specification for the table with a supplied name.
+
+    If no such Table exists, returns None.
+    """
+    c = boto3.client('dynamodb')
+    try:
+        resp = c.describe_time_to_live(TableName=table_name)
+        return resp['TimeToLiveDescription']
+    except c.exceptions.ResourceNotFoundException:
+        return None

test/e2e/tests/test_table.py

@@ -29,12 +29,14 @@
 )
 from e2e.replacement_values import REPLACEMENT_VALUES
 from e2e import condition
+from e2e import table
 from e2e import tag
 
 RESOURCE_PLURAL = "tables"
 
 DELETE_WAIT_AFTER_SECONDS = 15
-MODIFY_WAIT_AFTER_SECONDS = 5
+MODIFY_WAIT_AFTER_SECONDS = 10
+
 
 @pytest.fixture(scope="module")
 def forum_table():
@@ -74,39 +76,29 @@ def forum_table():
     _, deleted = k8s.delete_custom_resource(table_reference, period_length=DELETE_WAIT_AFTER_SECONDS)
     assert deleted
 
+
 @service_marker
 @pytest.mark.canary
 class TestTable:
-    def get_table(self, dynamodb_client, table_name: str) -> dict:
-        try:
-            resp = dynamodb_client.describe_table(
-                TableName=table_name,
-            )
-            return resp["Table"]
-
-        except Exception as e:
-            logging.debug(e)
-            return None
+    def table_exists(self, table_name: str) -> bool:
+        return table.get(table_name) is not None
 
-    def table_exists(self, dynamodb_client, table_name: str) -> bool:
-        return self.get_table(dynamodb_client, table_name) is not None
-
-    def test_create_delete(self, dynamodb_client, forum_table):
+    def test_create_delete(self, forum_table):
         (ref, res) = forum_table
 
         table_name = res["spec"]["tableName"]
         condition.assert_synced(ref)
 
         # Check DynamoDB Table exists
-        assert self.table_exists(dynamodb_client, table_name)
+        assert self.table_exists(table_name)
 
-    def test_table_update_tags(self, dynamodb_client, forum_table):
+    def test_table_update_tags(self, forum_table):
         (ref, res) = forum_table
 
         table_name = res["spec"]["tableName"]
 
         # Check DynamoDB Table exists
-        assert self.table_exists(dynamodb_client, table_name)
+        assert self.table_exists(table_name)
 
         # Get CR latest revision
         cr = k8s.wait_resource_consumed_by_controller(ref)
@@ -129,13 +121,13 @@ def test_table_update_tags(self, dynamodb_client, forum_table):
         assert table_tags[0]['Key'] == tags[0]['key']
         assert table_tags[0]['Value'] == tags[0]['value']
 
-    def test_enable_ttl(self, dynamodb_client, forum_table):
+    def test_enable_ttl(self, forum_table):
         (ref, res) = forum_table
 
         table_name = res["spec"]["tableName"]
 
         # Check DynamoDB Table exists
-        assert self.table_exists(dynamodb_client, table_name)
+        assert self.table_exists(table_name)
 
         # Get CR latest revision
         cr = k8s.wait_resource_consumed_by_controller(ref)
@@ -152,9 +144,14 @@ def test_enable_ttl(self, dynamodb_client, forum_table):
 
         # Patch k8s resource
         k8s.patch_custom_resource(ref, updates)
-        time.sleep(MODIFY_WAIT_AFTER_SECONDS)
 
-        ttl = dynamodb_client.describe_time_to_live(TableName=table_name)
-        assert ttl["TimeToLiveDescription"]["AttributeName"] == "ForumName"
-        assert (ttl["TimeToLiveDescription"]["TimeToLiveStatus"] == "ENABLED" or
-            ttl["TimeToLiveDescription"]["TimeToLiveStatus"] == "ENABLING")
+        table.wait_until(
+            table_name,
+            table.ttl_on_attribute_matches("ForumName"),
+        )
+
+        ttl = table.get_time_to_live(table_name)
+        assert ttl is not None
+        assert ttl["AttributeName"] == "ForumName"
+        ttl_status = ttl["TimeToLiveStatus"]
+        assert ttl_status in ("ENABLED", "ENABLING")