Cherrypick: Refactor e2e deletion and unmark adopted Endpoint Test (#216) (#219)

Issue #, if available:

Description of changes:
Disabling the adopted endpoint test as canary since this does not provide value for a canary, will keep as e2e test. Refactors  
```
if k8s.get_resource_exists(reference): 
 _, deleted = k8s.delete_custom_resource(reference, cfg.JOB_DELETE_WAIT_PERIODS, cfg.JOB_DELETE_WAIT_LENGTH) 
> assert deleted 
```
into a shared method, and adds a retry loop for this assert. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Issue #, if available:

Description of changes:

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

Author: ryansteakley

test/e2e/__init__.py

@@ -23,6 +23,8 @@
 import random
 
 from acktest.k8s import resource as k8s
+from common import config as cfg
+
 
 SERVICE_NAME = "sagemaker"
 CRD_GROUP = "sagemaker.services.k8s.aws"
@@ -372,3 +374,15 @@ def get_sagemaker_pipeline(pipeline_name: str):
             f"SageMaker could not find a pipeline with the name {pipeline_name}. Error {error}"
         )
         return None
+
+
+def delete_custom_resource(
+    reference,
+    wait_period=cfg.JOB_DELETE_WAIT_PERIODS,
+    wait_length=cfg.JOB_DELETE_WAIT_LENGTH,
+):
+    deleted = True
+
+    if k8s.get_resource_exists(reference): 
+        _, deleted = k8s.delete_custom_resource(reference, wait_period, wait_length)
+    return deleted

test/e2e/common/config.py

@@ -36,7 +36,7 @@
 DELETE_WAIT_PERIOD = 4
 DELETE_WAIT_LENGTH = 30
 
-JOB_DELETE_WAIT_PERIODS = 12
+JOB_DELETE_WAIT_PERIODS = 18
 JOB_DELETE_WAIT_LENGTH = 30
 
 TAG_DELAY_SLEEP = 20

test/e2e/common/fixtures.py

@@ -17,6 +17,7 @@
 import logging
 from e2e import (
     create_sagemaker_resource,
+    delete_custom_resource,
     wait_sagemaker_endpoint_status,
 )
 
@@ -91,8 +92,9 @@ def xgboost_churn_endpoint(sagemaker_client):
     yield endpoint_spec
 
     for cr in (model_reference, endpoint_config_reference, endpoint_reference):
-        _, deleted = k8s.delete_custom_resource(cr, cfg.DELETE_WAIT_PERIOD, cfg.DELETE_WAIT_LENGTH)
-        assert deleted
+        assert delete_custom_resource(
+            cr, cfg.DELETE_WAIT_PERIOD, cfg.DELETE_WAIT_LENGTH
+        )
 
 
 @pytest.fixture(scope="module")
@@ -118,6 +120,6 @@ def xgboost_churn_data_quality_job_definition(xgboost_churn_endpoint):
 
     yield (job_definition_reference, resource)
 
-    if k8s.get_resource_exists(job_definition_reference):
-        _, deleted = k8s.delete_custom_resource(job_definition_reference, 3, 10)
-        assert deleted
+    assert delete_custom_resource(
+        job_definition_reference, cfg.DELETE_WAIT_PERIOD, cfg.DELETE_WAIT_LENGTH
+    )

test/e2e/tests/test_adopt_endpoint.py

@@ -25,6 +25,7 @@
     CRD_VERSION,
     create_adopted_resource,
     wait_sagemaker_endpoint_status,
+    delete_custom_resource,
     assert_endpoint_status_in_sync,
     sagemaker_client,
     get_sagemaker_endpoint,
@@ -169,13 +170,12 @@ def adopted_endpoint(sdk_endpoint):
     yield (adopt_model_reference, adopt_config_reference, adopt_endpoint_reference)
 
     for cr in (adopt_model_reference, adopt_config_reference, adopt_endpoint_reference):
-        if k8s.get_resource_exists(cr):
-            _, deleted = k8s.delete_custom_resource(cr, 3, 10)
-            assert deleted
+        assert delete_custom_resource(
+            cr, cfg.DELETE_WAIT_PERIOD, cfg.DELETE_WAIT_LENGTH
+        )
 
 
 @service_marker
-@pytest.mark.canary
 class TestAdoptedEndpoint:
     def test_smoke(self, sdk_endpoint, adopted_endpoint):
         (
@@ -264,12 +264,13 @@ 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")
 
         for cr in (model_reference, config_reference, endpoint_reference):
-            _, deleted = k8s.delete_custom_resource(
+            assert delete_custom_resource(
                 cr, cfg.DELETE_WAIT_PERIOD, cfg.DELETE_WAIT_LENGTH
             )
-            assert deleted

test/e2e/tests/test_adopt_model_package.py

@@ -25,6 +25,7 @@
     create_adopted_resource,
     wait_sagemaker_model_package_status,
     assert_model_package_status_in_sync,
+    delete_custom_resource,
     get_sagemaker_model_package_group,
     get_sagemaker_model_package,
     sagemaker_client,
@@ -62,8 +63,12 @@ def sdk_make_model_package(model_package_group_name):
                     "ModelDataUrl": f"s3://{data_bucket}/sagemaker/model/xgboost-mnist-model.tar.gz",
                 }
             ],
-            "SupportedContentTypes": ["text/csv",],
-            "SupportedResponseMIMETypes": ["text/csv",],
+            "SupportedContentTypes": [
+                "text/csv",
+            ],
+            "SupportedResponseMIMETypes": [
+                "text/csv",
+            ],
         },
     }
 
@@ -126,7 +131,9 @@ def adopted_model_package(sdk_model_package):
         adopt_model_package_group_reference,
         _,
         adopt_model_package_group_resource,
-    ) = create_adopted_resource(replacements=replacements,)
+    ) = create_adopted_resource(
+        replacements=replacements,
+    )
     assert adopt_model_package_group_resource is not None
 
     # adopt model package
@@ -144,16 +151,17 @@ def adopted_model_package(sdk_model_package):
         _,
         adopt_model_package_resource,
     ) = create_adopted_resource(
-        replacements=replacements, spec_file="adopted_resource_base_arn",
+        replacements=replacements,
+        spec_file="adopted_resource_base_arn",
     )
     assert adopt_model_package_resource is not None
 
     yield (adopt_model_package_group_reference, adopt_model_package_reference)
 
     for cr in (adopt_model_package_group_reference, adopt_model_package_reference):
-        if k8s.get_resource_exists(cr):
-            _, deleted = k8s.delete_custom_resource(cr, 3, 10)
-            assert deleted
+        assert delete_custom_resource(
+            cr, cfg.DELETE_WAIT_PERIOD, cfg.DELETE_WAIT_LENGTH
+        )
 
 
 @service_marker
@@ -236,14 +244,15 @@ def test_smoke(self, sdk_model_package, adopted_model_package):
         ) == model_package_response.get("ModelPackageArn", None)
 
         assert_model_package_status_in_sync(
-            model_package_arn, model_package_reference, cfg.JOB_STATUS_COMPLETED,
+            model_package_arn,
+            model_package_reference,
+            cfg.JOB_STATUS_COMPLETED,
         )
         assert k8s.wait_on_condition(
             model_package_reference, "ACK.ResourceSynced", "True"
         )
 
         for cr in (model_package_reference, model_package_group_reference):
-            _, deleted = k8s.delete_custom_resource(
-                cr, cfg.JOB_DELETE_WAIT_PERIODS, cfg.JOB_DELETE_WAIT_LENGTH
+            assert delete_custom_resource(
+                cr, cfg.DELETE_WAIT_PERIOD, cfg.DELETE_WAIT_LENGTH
             )
-            assert deleted

test/e2e/tests/test_cross_region.py

@@ -24,6 +24,7 @@
 from e2e import (
     service_marker,
     create_sagemaker_resource,
+    delete_custom_resource,
     get_sagemaker_model,
     sagemaker_client,
 )
@@ -54,9 +55,9 @@ def cross_region_model():
     yield (reference, resource)
 
     # Delete the k8s resource if not already deleted by tests
-    if k8s.get_resource_exists(reference):
-        _, deleted = k8s.delete_custom_resource(reference, 3, 10)
-        assert deleted
+    assert delete_custom_resource(
+        reference, cfg.DELETE_WAIT_PERIOD, cfg.DELETE_WAIT_LENGTH
+    )
 
 
 def get_cross_region():
@@ -79,7 +80,8 @@ def test_create_cross_region_model(self, cross_region_model):
         assert k8s.get_resource_arn(resource) == cross_region_model_arn
 
         # Delete the k8s resource.
-        _, deleted = k8s.delete_custom_resource(reference, 3, 10)
-        assert deleted
+        assert delete_custom_resource(
+            reference, cfg.DELETE_WAIT_PERIOD, cfg.DELETE_WAIT_LENGTH
+        )
 
         assert get_sagemaker_model(model_name, sm_client) is None

test/e2e/tests/test_data_quality_job_definition.py

@@ -25,6 +25,7 @@
 )
 from e2e.common import config as cfg
 from acktest.k8s import resource as k8s
+from e2e import delete_custom_resource
 
 # Access variable so it is loaded as a fixture
 _accessed = xgboost_churn_data_quality_job_definition, xgboost_churn_endpoint
@@ -63,8 +64,9 @@ def test_smoke(self, sagemaker_client, xgboost_churn_data_quality_job_definition
         resource_tags = resource["spec"].get("tags", None)
         assert_tags_in_sync(job_definition_arn, resource_tags)
         # Delete the k8s resource.
-        _, deleted = k8s.delete_custom_resource(reference, 3, 10)
-        assert deleted
+        assert delete_custom_resource(
+            reference, cfg.DELETE_WAIT_PERIOD, cfg.DELETE_WAIT_LENGTH
+        )
         assert (
             describe_sagemaker_data_quality_job_definition(
                 sagemaker_client, job_definition_name

test/e2e/tests/test_endpoint.py

@@ -25,6 +25,7 @@
 from e2e import (
     service_marker,
     create_sagemaker_resource,
+    delete_custom_resource,
     assert_endpoint_status_in_sync,
     assert_tags_in_sync,
     get_sagemaker_endpoint,
@@ -65,7 +66,9 @@ def single_container_model(name_suffix):
 
     yield (model_reference, model_resource)
 
-    _, deleted = k8s.delete_custom_resource(model_reference, 3, 10)
+    _, deleted = k8s.delete_custom_resource(
+        model_reference, cfg.DELETE_WAIT_PERIOD, cfg.DELETE_WAIT_LENGTH
+    )
     assert deleted
 
 
@@ -94,7 +97,9 @@ def multi_variant_config(name_suffix, single_container_model):
 
     yield (config_reference, config_resource)
 
-    _, deleted = k8s.delete_custom_resource(config_reference, 3, 10)
+    _, deleted = k8s.delete_custom_resource(
+        config_reference, cfg.DELETE_WAIT_PERIOD, cfg.DELETE_WAIT_LENGTH
+    )
     assert deleted
 
 
@@ -123,7 +128,9 @@ def single_variant_config(name_suffix, single_container_model):
 
     yield (config_reference, config_resource)
 
-    _, deleted = k8s.delete_custom_resource(config_reference, 3, 10)
+    _, deleted = k8s.delete_custom_resource(
+        config_reference, cfg.DELETE_WAIT_PERIOD, cfg.DELETE_WAIT_LENGTH
+    )
     assert deleted
 
 
@@ -149,10 +156,7 @@ def xgboost_endpoint(name_suffix, single_variant_config):
     yield (reference, resource, spec)
 
     # Delete the k8s resource if not already deleted by tests
-    if k8s.get_resource_exists(reference):
-        # longer wait incase endpoint is in creating/updating status
-        _, deleted = k8s.delete_custom_resource(reference, 40, cfg.DELETE_WAIT_LENGTH)
-        assert deleted
+    assert delete_custom_resource(reference, 40, cfg.DELETE_WAIT_LENGTH)
 
 
 @pytest.fixture(scope="module")
@@ -207,7 +211,9 @@ def faulty_config(name_suffix, single_container_model):
     yield (config_reference, config_resource)
 
     for cr in (model_reference, config_reference):
-        _, deleted = k8s.delete_custom_resource(cr, 3, 10)
+        _, deleted = k8s.delete_custom_resource(
+            cr, cfg.DELETE_WAIT_PERIOD, cfg.DELETE_WAIT_LENGTH
+        )
         assert deleted
 
 
@@ -255,7 +261,9 @@ 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)
@@ -264,7 +272,9 @@ def update_endpoint_failed_test(
         assert annotations[LAST_ENDPOINTCONFIG_UPDATE_ANNOTATION] == faulty_config_name
 
         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", "False")
@@ -302,7 +312,9 @@ def update_endpoint_successful_test(self, multi_variant_config, xgboost_endpoint
 
         # 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")
@@ -313,7 +325,9 @@ def update_endpoint_successful_test(self, multi_variant_config, xgboost_endpoint
         assert annotations[LAST_ENDPOINTCONFIG_UPDATE_ANNOTATION] == new_config_name
 
         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.get_resource_condition(endpoint_reference, "ACK.Terminal") is None
@@ -336,10 +350,9 @@ def delete_endpoint_test(self, xgboost_endpoint):
         (reference, resource, _) = xgboost_endpoint
         endpoint_name = resource["spec"].get("endpointName", None)
 
-        _, deleted = k8s.delete_custom_resource(
+        assert delete_custom_resource(
             reference, cfg.DELETE_WAIT_PERIOD, cfg.DELETE_WAIT_LENGTH
         )
-        assert deleted
 
         assert get_sagemaker_endpoint(endpoint_name) is None
 

test/e2e/tests/test_endpoint_config.py

@@ -24,6 +24,7 @@
 from e2e import (
     service_marker,
     create_sagemaker_resource,
+    delete_custom_resource,
     assert_tags_in_sync,
     get_sagemaker_endpoint_config,
 )
@@ -63,11 +64,13 @@ def single_variant_config():
 
     yield (config_reference, config_resource)
 
-    k8s.delete_custom_resource(model_reference, 3, 10)
+    k8s.delete_custom_resource(
+        model_reference, cfg.DELETE_WAIT_PERIOD, cfg.DELETE_WAIT_LENGTH
+    )
     # Delete the k8s resource if not already deleted by tests
-    if k8s.get_resource_exists(config_reference):
-        _, deleted = k8s.delete_custom_resource(config_reference, 3, 10)
-        assert deleted
+    assert delete_custom_resource(
+        config_reference, cfg.DELETE_WAIT_PERIOD, cfg.DELETE_WAIT_LENGTH
+    )
 
 
 @service_marker
@@ -87,7 +90,8 @@ def test_create_endpoint_config(self, single_variant_config):
         resource_tags = resource["spec"].get("tags", None)
         assert_tags_in_sync(endpoint_arn, resource_tags)
         # Delete the k8s resource.
-        _, deleted = k8s.delete_custom_resource(reference, 3, 10)
-        assert deleted
+        assert delete_custom_resource(
+            reference, cfg.DELETE_WAIT_PERIOD, cfg.DELETE_WAIT_LENGTH
+        )
 
         assert get_sagemaker_endpoint_config(config_name) is None