fix to reduce test flakiness (#53)

ryansteakley · web-flow · commit 522c147abcca · 2021-07-13T17:15:57.000-07:00
Adds a debug logging statement to see status when status does exist but arn does not. Additionally adds a random sleep in the `create_sagemaker_resource` method. This is because there may be many of the same resource created at the same time and it will take longer than 30 seconds for the `wait_resource_consumed_by_controller `to pass instead of adding extra time. Eventually they will all create but for the tests they will fail at the assert since it only waits 30 seconds.
diff --git a/test/e2e/__init__.py b/test/e2e/__init__.py
@@ -16,6 +16,7 @@
 import time
 import boto3
 from pathlib import Path
+import random
 
 from acktest.k8s import resource as k8s
 
@@ -36,12 +37,22 @@ def sagemaker_client():
 
 
 def create_sagemaker_resource(
-    resource_plural, resource_name, spec_file, replacements, namespace="default"
+    resource_plural,
+    resource_name,
+    spec_file,
+    replacements,
+    namespace="default",
+    wait_period=3,
+    period_length=10,
 ):
     """
     Wrapper around k8s.load_and_create_resource to create a SageMaker resource
     """
-
+    # Add a random sleep to prevent throttling exception before the call to load and create
+    # This is because there may be many of the same resource ex: Multiple Models being created at the same time
+    # If this occurs then a throttling exception may occur and cause the tests to fail, this sleep prevents this from occurring.
+    rand = random.randrange(1, 4)
+    time.sleep(rand)
     reference, spec, resource = k8s.load_and_create_resource(
         resource_directory,
         CRD_GROUP,
@@ -51,6 +62,8 @@ def create_sagemaker_resource(
         spec_file,
         replacements,
         namespace,
+        wait_period,
+        period_length,
     )
 
     return reference, spec, resource
diff --git a/test/e2e/common/fixtures.py b/test/e2e/common/fixtures.py
@@ -14,7 +14,7 @@
 """
 
 import pytest
-
+import logging
 from e2e import (
     create_sagemaker_resource,
     wait_sagemaker_endpoint_status,
@@ -51,6 +51,10 @@ def xgboost_churn_endpoint(sagemaker_client):
         replacements=replacements,
     )
     assert model_resource is not None
+    if k8s.get_resource_arn(model_resource) is None:
+        logging.debug(
+            f"ARN for this resource is None, resource status is: {model_resource['status']}"
+        )
     assert k8s.get_resource_arn(model_resource) is not None
 
     (
@@ -64,6 +68,10 @@ def xgboost_churn_endpoint(sagemaker_client):
         replacements=replacements,
     )
     assert endpoint_config_resource is not None
+    if k8s.get_resource_arn(endpoint_config_resource) is None:
+        logging.debug(
+            f"ARN for this resource is None, resource status is: {endpoint_config_resource['status']}"
+        )
     assert k8s.get_resource_arn(endpoint_config_resource) is not None
 
     endpoint_reference, endpoint_spec, endpoint_resource = create_sagemaker_resource(
@@ -73,6 +81,10 @@ def xgboost_churn_endpoint(sagemaker_client):
         replacements=replacements,
     )
     assert endpoint_resource is not None
+    if k8s.get_resource_arn(endpoint_resource) is None:
+        logging.debug(
+            f"ARN for this resource is None, resource status is: {endpoint_resource['status']}"
+        )
     assert k8s.get_resource_arn(endpoint_resource) is not None
     wait_sagemaker_endpoint_status(replacements["ENDPOINT_NAME"], "InService")
 
diff --git a/test/e2e/requirements.txt b/test/e2e/requirements.txt
@@ -1,2 +1,2 @@
-acktest @ git+https://github.com/aws-controllers-k8s/test-infra.git@5ed60a505afa953096e53c9d3d6779830250915b
+acktest @ git+https://github.com/aws-controllers-k8s/test-infra.git@6518b782a765fb57dda1432482dc79c0711b73c2
 black==20.8b1
diff --git a/test/e2e/service_bootstrap.py b/test/e2e/service_bootstrap.py
@@ -123,8 +123,7 @@ def service_bootstrap() -> dict:
     logging.getLogger().setLevel(logging.INFO)
 
     return TestBootstrapResources(
-        create_data_bucket(),
-        create_execution_role(),
+        create_data_bucket(), create_execution_role(),
     ).__dict__
 
 
diff --git a/test/e2e/tests/test_adopt_endpoint.py b/test/e2e/tests/test_adopt_endpoint.py
@@ -123,7 +123,6 @@ def adopted_endpoint(sdk_endpoint):
     (model_input, _, endpoint_config_input, _, endpoint_input, _) = sdk_endpoint
 
     replacements = REPLACEMENT_VALUES.copy()
-
     # adopt model
     replacements["ADOPTED_RESOURCE_NAME"] = "adopt-" + model_input["ModelName"]
     replacements["TARGET_RESOURCE_AWS"] = replacements[
@@ -260,8 +259,6 @@ def test_smoke(self, sdk_endpoint, adopted_endpoint):
         )
 
         assert_endpoint_status_in_sync(
-            endpoint_name,
-            endpoint_reference,
-            cfg.ENDPOINT_STATUS_INSERVICE,
+            endpoint_name, endpoint_reference, cfg.ENDPOINT_STATUS_INSERVICE,
         )
         assert k8s.wait_on_condition(endpoint_reference, "ACK.ResourceSynced", "True")
diff --git a/test/e2e/tests/test_endpoint.py b/test/e2e/tests/test_endpoint.py
@@ -52,6 +52,10 @@ def single_container_model(name_suffix):
         replacements=replacements,
     )
     assert model_resource is not None
+    if k8s.get_resource_arn(model_resource) is None:
+        logging.debug(
+            f"ARN for this resource is None, resource status is: {model_resource['status']}"
+        )
     assert k8s.get_resource_arn(model_resource) is not None
 
     yield (model_reference, model_resource)
@@ -77,6 +81,10 @@ def multi_variant_config(name_suffix, single_container_model):
         replacements=replacements,
     )
     assert config_resource is not None
+    if k8s.get_resource_arn(config_resource) is None:
+        logging.debug(
+            f"ARN for this resource is None, resource status is: {config_resource['status']}"
+        )
     assert k8s.get_resource_arn(config_resource) is not None
 
     yield (config_reference, config_resource)
@@ -102,6 +110,10 @@ def single_variant_config(name_suffix, single_container_model):
         replacements=replacements,
     )
     assert config_resource is not None
+    if k8s.get_resource_arn(config_resource) is None:
+        logging.debug(
+            f"ARN for this resource is None, resource status is: {config_resource['status']}"
+        )
     assert k8s.get_resource_arn(config_resource) is not None
 
     yield (config_reference, config_resource)
@@ -160,7 +172,10 @@ def faulty_config(name_suffix, single_container_model):
         replacements=replacements,
     )
     assert model_resource is not None
-    model_resource = k8s.get_resource(model_reference)
+    if k8s.get_resource_arn(model_resource) is None:
+        logging.debug(
+            f"ARN for this resource is None, resource status is: {model_resource['status']}"
+        )
     assert k8s.get_resource_arn(model_resource) is not None
     s3.delete_object(model_bucket, model_destination_key)
 
@@ -177,6 +192,10 @@ def faulty_config(name_suffix, single_container_model):
         replacements=replacements,
     )
     assert config_resource is not None
+    if k8s.get_resource_arn(config_resource) is None:
+        logging.debug(
+            f"ARN for this resource is None, resource status is: {config_resource['status']}"
+        )
     assert k8s.get_resource_arn(config_resource) is not None
 
     yield (config_reference, config_resource)
@@ -246,9 +265,7 @@ def update_endpoint_failed_test(
 
         # endpoint transitions Updating -> InService state
         assert_endpoint_status_in_sync(
-            endpoint_reference.name,
-            endpoint_reference,
-            cfg.ENDPOINT_STATUS_UPDATING,
+            endpoint_reference.name, endpoint_reference, cfg.ENDPOINT_STATUS_UPDATING,
         )
         assert k8s.wait_on_condition(endpoint_reference, "ACK.ResourceSynced", "False")
         endpoint_resource = k8s.get_resource(endpoint_reference)
@@ -258,17 +275,12 @@ def update_endpoint_failed_test(
         )
 
         assert_endpoint_status_in_sync(
-            endpoint_reference.name,
-            endpoint_reference,
-            cfg.ENDPOINT_STATUS_INSERVICE,
+            endpoint_reference.name, endpoint_reference, cfg.ENDPOINT_STATUS_INSERVICE,
         )
 
         assert k8s.wait_on_condition(endpoint_reference, "ACK.ResourceSynced", "True")
         assert k8s.assert_condition_state_message(
-            endpoint_reference,
-            "ACK.Terminal",
-            "True",
-            FAIL_UPDATE_ERROR_MESSAGE,
+            endpoint_reference, "ACK.Terminal", "True", FAIL_UPDATE_ERROR_MESSAGE,
         )
 
         endpoint_resource = k8s.get_resource(endpoint_reference)
@@ -279,10 +291,10 @@ def update_endpoint_failed_test(
         current_config_name = endpoint_resource["status"].get(
             "latestEndpointConfigName"
         )
-        assert (
-            current_config_name is not None
-            and current_config_name
-            == old_config_resource["spec"].get("endpointConfigName", None)
+        assert current_config_name is not None and current_config_name == old_config_resource[
+            "spec"
+        ].get(
+            "endpointConfigName", None
         )
 
     def update_endpoint_successful_test(
@@ -306,9 +318,7 @@ def update_endpoint_successful_test(
 
         # endpoint transitions Updating -> InService state
         assert_endpoint_status_in_sync(
-            endpoint_reference.name,
-            endpoint_reference,
-            cfg.ENDPOINT_STATUS_UPDATING,
+            endpoint_reference.name, endpoint_reference, cfg.ENDPOINT_STATUS_UPDATING,
         )
 
         assert k8s.wait_on_condition(endpoint_reference, "ACK.ResourceSynced", "False")
@@ -322,9 +332,7 @@ def update_endpoint_successful_test(
         )
 
         assert_endpoint_status_in_sync(
-            endpoint_reference.name,
-            endpoint_reference,
-            cfg.ENDPOINT_STATUS_INSERVICE,
+            endpoint_reference.name, endpoint_reference, cfg.ENDPOINT_STATUS_INSERVICE,
         )
         assert k8s.wait_on_condition(endpoint_reference, "ACK.ResourceSynced", "True")
         assert k8s.assert_condition_state_message(
diff --git a/test/e2e/tests/test_endpoint_config.py b/test/e2e/tests/test_endpoint_config.py
@@ -13,7 +13,6 @@
 """Integration tests for the SageMaker EndpointConfig API.
 """
 
-from _pytest import config
 import botocore
 import pytest
 import logging
@@ -43,6 +42,10 @@ def single_variant_config():
         replacements=replacements,
     )
     assert model_resource is not None
+    if k8s.get_resource_arn(model_resource) is None:
+        logging.debug(
+            f"ARN for this resource is None, resource status is: {model_resource['status']}"
+        )
     assert k8s.get_resource_arn(model_resource) is not None
 
     config_reference, config_spec, config_resource = create_sagemaker_resource(
diff --git a/test/e2e/tests/test_hpo.py b/test/e2e/tests/test_hpo.py
@@ -45,8 +45,11 @@ def xgboost_hpojob():
         spec_file="xgboost_hpojob",
         replacements=replacements,
     )
-
     assert resource is not None
+    if k8s.get_resource_arn(resource) is None:
+        logging.debug(
+            f"ARN for this resource is None, resource status is: {resource['status']}"
+        )
     assert k8s.get_resource_arn(resource) is not None
 
     yield (reference, resource)
diff --git a/test/e2e/tests/test_model_bias_job_definition.py b/test/e2e/tests/test_model_bias_job_definition.py
@@ -53,11 +53,11 @@ def xgboost_churn_model_bias_job_definition(xgboost_churn_endpoint):
     yield (reference, resource)
 
     if k8s.get_resource_exists(reference):
-        _, deleted = k8s.delete_custom_resource(job_definition_reference, 3, 10)
+        _, deleted = k8s.delete_custom_resource(reference, 3, 10)
         assert deleted
 
 
-def describe_sagemaker_model_bias_job_definition(sagemaker_client, job_definition_name):
+def get_sagemaker_model_bias_job_definition(sagemaker_client, job_definition_name):
     try:
         return sagemaker_client.describe_model_bias_job_definition(
             JobDefinitionName=job_definition_name
@@ -79,7 +79,7 @@ def test_smoke(self, sagemaker_client, xgboost_churn_model_bias_job_definition):
         job_definition_name = resource["spec"].get("jobDefinitionName")
         assert (
             k8s.get_resource_arn(resource)
-            == describe_sagemaker_model_bias_job_definition(
+            == get_sagemaker_model_bias_job_definition(
                 sagemaker_client, job_definition_name
             )["JobDefinitionArn"]
         )
@@ -88,7 +88,7 @@ def test_smoke(self, sagemaker_client, xgboost_churn_model_bias_job_definition):
         _, deleted = k8s.delete_custom_resource(reference, 3, 10)
         assert deleted
         assert (
-            describe_sagemaker_model_bias_job_definition(
+            get_sagemaker_model_bias_job_definition(
                 sagemaker_client, job_definition_name
             )
             is None
diff --git a/test/e2e/tests/test_model_explainability_job_definition.py b/test/e2e/tests/test_model_explainability_job_definition.py
@@ -53,11 +53,11 @@ def xgboost_churn_model_explainability_job_definition(xgboost_churn_endpoint):
     yield (reference, resource)
 
     if k8s.get_resource_exists(reference):
-        _, deleted = k8s.delete_custom_resource(job_definition_reference, 3, 10)
+        _, deleted = k8s.delete_custom_resource(reference, 3, 10)
         assert deleted
 
 
-def describe_sagemaker_model_explainability_job_definition(
+def get_sagemaker_model_explainability_job_definition(
     sagemaker_client, job_definition_name
 ):
     try:
@@ -83,7 +83,7 @@ def test_smoke(
         job_definition_name = resource["spec"].get("jobDefinitionName")
         assert (
             k8s.get_resource_arn(resource)
-            == describe_sagemaker_model_explainability_job_definition(
+            == get_sagemaker_model_explainability_job_definition(
                 sagemaker_client, job_definition_name
             )["JobDefinitionArn"]
         )
@@ -92,7 +92,7 @@ def test_smoke(
         _, deleted = k8s.delete_custom_resource(reference, 3, 10)
         assert deleted
         assert (
-            describe_sagemaker_model_explainability_job_definition(
+            get_sagemaker_model_explainability_job_definition(
                 sagemaker_client, job_definition_name
             )
             is None
diff --git a/test/e2e/tests/test_model_quality_job_definition.py b/test/e2e/tests/test_model_quality_job_definition.py
@@ -53,13 +53,11 @@ def xgboost_churn_model_quality_job_definition(xgboost_churn_endpoint):
     yield (reference, resource)
 
     if k8s.get_resource_exists(reference):
-        _, deleted = k8s.delete_custom_resource(job_definition_reference, 3, 10)
+        _, deleted = k8s.delete_custom_resource(reference, 3, 10)
         assert deleted
 
 
-def describe_sagemaker_model_quality_job_definition(
-    sagemaker_client, job_definition_name
-):
+def get_sagemaker_model_quality_job_definition(sagemaker_client, job_definition_name):
     try:
         return sagemaker_client.describe_model_quality_job_definition(
             JobDefinitionName=job_definition_name
@@ -81,7 +79,7 @@ def test_smoke(self, sagemaker_client, xgboost_churn_model_quality_job_definitio
         job_definition_name = resource["spec"].get("jobDefinitionName")
         assert (
             k8s.get_resource_arn(resource)
-            == describe_sagemaker_model_quality_job_definition(
+            == get_sagemaker_model_quality_job_definition(
                 sagemaker_client, job_definition_name
             )["JobDefinitionArn"]
         )
@@ -90,7 +88,7 @@ def test_smoke(self, sagemaker_client, xgboost_churn_model_quality_job_definitio
         _, deleted = k8s.delete_custom_resource(reference, 3, 10)
         assert deleted
         assert (
-            describe_sagemaker_model_quality_job_definition(
+            get_sagemaker_model_quality_job_definition(
                 sagemaker_client, job_definition_name
             )
             is None
diff --git a/test/e2e/tests/test_monitoring_schedule.py b/test/e2e/tests/test_monitoring_schedule.py
diff --git a/test/e2e/tests/test_processingjob.py b/test/e2e/tests/test_processingjob.py
diff --git a/test/e2e/tests/test_trainingjob.py b/test/e2e/tests/test_trainingjob.py
diff --git a/test/e2e/tests/test_trainingjob_debugger.py b/test/e2e/tests/test_trainingjob_debugger.py
diff --git a/test/e2e/tests/test_transformjob.py b/test/e2e/tests/test_transformjob.py

Original file line number	Diff line number	Diff line change
`@@ -1,2 +1,2 @@`
`1`		`-acktest @ git+https://github.com/aws-controllers-k8s/test-infra.git@5ed60a505afa953096e53c9d3d6779830250915b`
	`1`	`+acktest @ git+https://github.com/aws-controllers-k8s/test-infra.git@6518b782a765fb57dda1432482dc79c0711b73c2`
`2`	`2`	`black==20.8b1`