refactor tests and bug fixes (#28)

* refactor tests and bug fixes

* fix wait_resource

* log describe error, increase delete timeout, refractor tests

* add owners alias

Author: surajkota

OWNERS_ALIASES

@@ -9,4 +9,7 @@ aliases:
     - RedbackThomson
     - vijtrip2
   # TODO: Add your team members to your team controller alias
-  service-team: []
+  service-team:
+    - akartsky
+    - mbaijal
+    - surajkota

test/e2e/common/fixtures.py

@@ -81,7 +81,7 @@ 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)
+        _, deleted = k8s.delete_custom_resource(cr, 3, 10)
         assert deleted
 
 
@@ -109,5 +109,5 @@ 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)
+        _, deleted = k8s.delete_custom_resource(job_definition_reference, 3, 10)
         assert deleted

test/e2e/tests/test_adopt_endpoint.py

@@ -159,7 +159,7 @@ def adopted_endpoint(sdk_endpoint):
 
     for cr in (adopt_model_reference, adopt_config_reference, adopt_endpoint_reference):
         if k8s.get_resource_exists(cr):
-            _, deleted = k8s.delete_custom_resource(cr)
+            _, deleted = k8s.delete_custom_resource(cr, 3, 10)
             assert deleted
 
 
@@ -201,7 +201,7 @@ def test_smoke(self, sdk_endpoint, adopted_endpoint):
         model_reference = k8s.create_reference(
             CRD_GROUP, CRD_VERSION, cfg.MODEL_RESOURCE_PLURAL, model_name, namespace
         )
-        model_resource = k8s.get_resource(model_reference)
+        model_resource = k8s.wait_resource_consumed_by_controller(model_reference)
         assert model_resource is not None
 
         assert model_resource["spec"].get("modelName", None) == model_name
@@ -221,7 +221,7 @@ def test_smoke(self, sdk_endpoint, adopted_endpoint):
             endpoint_config_name,
             namespace,
         )
-        config_resource = k8s.get_resource(config_reference)
+        config_resource = k8s.wait_resource_consumed_by_controller(config_reference)
         assert config_resource is not None
 
         assert (
@@ -236,7 +236,7 @@ def test_smoke(self, sdk_endpoint, adopted_endpoint):
         endpoint_reference = k8s.create_reference(
             CRD_GROUP, CRD_VERSION, cfg.ENDPOINT_RESOURCE_PLURAL, endpoint_name, namespace
         )
-        endpoint_resource = k8s.get_resource(endpoint_reference)
+        endpoint_resource = k8s.wait_resource_consumed_by_controller(endpoint_reference)
         assert endpoint_resource is not None
 
         assert endpoint_resource["spec"].get("endpointName", None) == endpoint_name

test/e2e/tests/test_data_quality_job_definition.py

@@ -15,6 +15,7 @@
 
 import pytest
 import logging
+import botocore
 
 from e2e import service_marker
 from e2e.common.fixtures import (
@@ -34,9 +35,9 @@ def describe_sagemaker_data_quality_job_definition(
         return sagemaker_client.describe_data_quality_job_definition(
             JobDefinitionName=job_definition_name
         )
-    except BaseException:
+    except botocore.exceptions.ClientError as error:
         logging.error(
-            f"Could not find Data Quality Job Definition with name {job_definition_name}"
+            f"Could not find Data Quality Job Definition with name {job_definition_name}. Error {error}"
         )
         return None
 
@@ -57,7 +58,7 @@ def test_smoke(self, sagemaker_client, xgboost_churn_data_quality_job_definition
         )
 
         # Delete the k8s resource.
-        _, deleted = k8s.delete_custom_resource(reference)
+        _, deleted = k8s.delete_custom_resource(reference, 3, 10)
         assert deleted
         assert (
             describe_sagemaker_data_quality_job_definition(

test/e2e/tests/test_endpoint.py

@@ -13,7 +13,7 @@
 """Integration tests for the SageMaker Endpoint API.
 """
 
-import boto3
+import botocore
 import pytest
 import logging
 import time
@@ -56,7 +56,7 @@ def single_container_model(name_suffix):
 
     yield (model_reference, model_resource)
 
-    _, deleted = k8s.delete_custom_resource(model_reference)
+    _, deleted = k8s.delete_custom_resource(model_reference, 3, 10)
     assert deleted
 
 
@@ -81,7 +81,7 @@ def multi_variant_config(name_suffix, single_container_model):
 
     yield (config_reference, config_resource)
 
-    _, deleted = k8s.delete_custom_resource(config_reference)
+    _, deleted = k8s.delete_custom_resource(config_reference, 3, 10)
     assert deleted
 
 
@@ -106,7 +106,7 @@ def single_variant_config(name_suffix, single_container_model):
 
     yield (config_reference, config_resource)
 
-    _, deleted = k8s.delete_custom_resource(config_reference)
+    _, deleted = k8s.delete_custom_resource(config_reference, 3, 10)
     assert deleted
 
 
@@ -133,7 +133,7 @@ def xgboost_endpoint(name_suffix, single_variant_config):
 
     # Delete the k8s resource if not already deleted by tests
     if k8s.get_resource_exists(reference):
-        _, deleted = k8s.delete_custom_resource(reference)
+        _, deleted = k8s.delete_custom_resource(reference, 3, 10)
         assert deleted
 
 
@@ -182,7 +182,7 @@ 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)
+        _, deleted = k8s.delete_custom_resource(cr, 3, 10)
         assert deleted
 
 
@@ -199,9 +199,9 @@ def _get_resource_endpoint_arn(self, resource: Dict):
     def _describe_sagemaker_endpoint(self, sagemaker_client, endpoint_name: str):
         try:
             return sagemaker_client.describe_endpoint(EndpointName=endpoint_name)
-        except BaseException:
+        except botocore.exceptions.ClientError as error:
             logging.error(
-                f"SageMaker could not find a endpoint with the name {endpoint_name}"
+                f"SageMaker could not find a endpoint with the name {endpoint_name}. Error {error}"
             )
             return None
 
@@ -349,7 +349,7 @@ def delete_endpoint_test(self, sagemaker_client, xgboost_endpoint):
         (reference, resource, _) = xgboost_endpoint
         endpoint_name = resource["spec"].get("endpointName", None)
 
-        _, deleted = k8s.delete_custom_resource(reference)
+        _, deleted = k8s.delete_custom_resource(reference, 3, 10)
         assert deleted
 
         # resource is removed from management from controller side if call to deleteEndpoint succeeds.

test/e2e/tests/test_endpoint_config.py

@@ -13,7 +13,8 @@
 """Integration tests for the SageMaker EndpointConfig API.
 """
 
-import boto3
+from _pytest import config
+import botocore
 import pytest
 import logging
 from typing import Dict
@@ -24,6 +25,7 @@
 from e2e import (
     service_marker,
     create_sagemaker_resource,
+    sagemaker_client
 )
 from e2e.replacement_values import REPLACEMENT_VALUES
 from e2e.common import config as cfg
@@ -57,59 +59,38 @@ def single_variant_config():
 
     yield (config_reference, config_resource)
 
-    k8s.delete_custom_resource(model_reference)
+    k8s.delete_custom_resource(model_reference, 3, 10)
     # Delete the k8s resource if not already deleted by tests
     if k8s.get_resource_exists(config_reference):
-        k8s.delete_custom_resource(config_reference)
+        _, deleted = k8s.delete_custom_resource(config_reference, 3, 10)
+        assert deleted
+
+def get_sagemaker_endpoint_config(config_name: str):
+    try:
+        return sagemaker_client().describe_endpoint_config(
+            EndpointConfigName=config_name
+        )
+    except botocore.exceptions.ClientError as error:
+        logging.error(
+            f"SageMaker could not find a config with the name {config_name}. Error {error}"
+        )
+        return None
 
 
 @service_marker
 @pytest.mark.canary
 class TestEndpointConfig:
-    def _get_resource_endpoint_config_arn(self, resource: Dict):
-        assert (
-            "ackResourceMetadata" in resource["status"]
-            and "arn" in resource["status"]["ackResourceMetadata"]
-        )
-        return resource["status"]["ackResourceMetadata"]["arn"]
-
-    def _get_sagemaker_endpoint_config_arn(self, sagemaker_client, config_name: str):
-        try:
-            response = sagemaker_client.describe_endpoint_config(
-                EndpointConfigName=config_name
-            )
-            return response["EndpointConfigArn"]
-        except BaseException:
-            logging.error(
-                f"SageMaker could not find a config with the name {config_name}"
-            )
-            return None
-
     def test_create_endpoint_config(self, single_variant_config):
         (reference, resource) = single_variant_config
         assert k8s.get_resource_exists(reference)
 
-    def test_config_has_correct_arn(self, sagemaker_client, single_variant_config):
-        (reference, _) = single_variant_config
-        resource = k8s.get_resource(reference)
         config_name = resource["spec"].get("endpointConfigName", None)
 
-        assert config_name is not None
-
-        assert self._get_resource_endpoint_config_arn(
-            resource
-        ) == self._get_sagemaker_endpoint_config_arn(sagemaker_client, config_name)
-
-    def test_config_is_deleted(self, sagemaker_client, single_variant_config):
-        (reference, _) = single_variant_config
-        resource = k8s.get_resource(reference)
-        config_name = resource["spec"].get("endpointConfigName", None)
+        assert k8s.get_resource_arn(resource) == get_sagemaker_endpoint_config(config_name)["EndpointConfigArn"]
 
         # Delete the k8s resource.
-        _, deleted = k8s.delete_custom_resource(reference)
-        assert deleted is True
+        _, deleted = k8s.delete_custom_resource(reference, 3, 10)
+        assert deleted
+
+        assert get_sagemaker_endpoint_config(config_name) is None
 
-        assert (
-            self._get_sagemaker_endpoint_config_arn(sagemaker_client, config_name)
-            is None
-        )

test/e2e/tests/test_hpo.py

@@ -13,7 +13,7 @@
 """Integration tests for the SageMaker HyperParameterTuning API.
 """
 
-import boto3
+import botocore
 import pytest
 import logging
 from typing import Dict
@@ -52,7 +52,7 @@ def xgboost_hpojob():
     yield (reference, resource)
 
     if k8s.get_resource_exists(reference):
-        _, deleted = k8s.delete_custom_resource(reference)
+        _, deleted = k8s.delete_custom_resource(reference, 3, 10)
         assert deleted
 
 
@@ -62,9 +62,9 @@ def get_sagemaker_hpo_job(hpo_job_name: str):
             HyperParameterTuningJobName=hpo_job_name
         )
         return hpo_desc
-    except BaseException:
+    except botocore.exceptions.ClientError as error:
         logging.error(
-            f"SageMaker could not find an hpo job with the name {hpo_job_name}"
+            f"SageMaker could not find an hpo job with the name {hpo_job_name}. Error {error}"
         )
         return None
 
@@ -139,7 +139,7 @@ def test_stopped(self, xgboost_hpojob):
         )
 
         # Delete the k8s resource.
-        _, deleted = k8s.delete_custom_resource(reference)
+        _, deleted = k8s.delete_custom_resource(reference, 3, 10)
         assert deleted is True
 
         hpo_sm_desc = get_sagemaker_hpo_job(hpo_job_name)
@@ -167,5 +167,5 @@ def test_completed(self, xgboost_hpojob):
         assert k8s.wait_on_condition(reference, "ACK.ResourceSynced", "True")
 
         # Check that you can delete a completed resource from k8s
-        _, deleted = k8s.delete_custom_resource(reference)
+        _, deleted = k8s.delete_custom_resource(reference, 3, 10)
         assert deleted is True

test/e2e/tests/test_model.py

@@ -13,7 +13,7 @@
 """Integration tests for the SageMaker Model API.
 """
 
-import boto3
+import botocore
 import pytest
 import logging
 from typing import Dict
@@ -23,6 +23,7 @@
 from e2e import (
     service_marker,
     create_sagemaker_resource,
+    sagemaker_client,
 )
 from e2e.replacement_values import REPLACEMENT_VALUES
 from e2e.common import config as cfg
@@ -47,51 +48,31 @@ def xgboost_model():
 
     # Delete the k8s resource if not already deleted by tests
     if k8s.get_resource_exists(reference):
-        k8s.delete_custom_resource(reference)
-
-
+        _, deleted = k8s.delete_custom_resource(reference, 3, 10)
+        assert deleted
+
+def get_sagemaker_model(model_name: str):
+    try:
+        return sagemaker_client().describe_model(ModelName=model_name)
+    except botocore.exceptions.ClientError as error:
+        logging.error(
+            f"SageMaker could not find a model with the name {model_name}. Error {error}"
+        )
+        return None
 @service_marker
 @pytest.mark.canary
 class TestModel:
-    def _get_resource_model_arn(self, resource: Dict):
-        assert (
-            "ackResourceMetadata" in resource["status"]
-            and "arn" in resource["status"]["ackResourceMetadata"]
-        )
-        return resource["status"]["ackResourceMetadata"]["arn"]
-
-    def _get_sagemaker_model_arn(self, sagemaker_client, model_name: str):
-        try:
-            model = sagemaker_client.describe_model(ModelName=model_name)
-            return model["ModelArn"]
-        except BaseException:
-            logging.error(
-                f"SageMaker could not find a model with the name {model_name}"
-            )
-            return None
-
     def test_create_model(self, xgboost_model):
         (reference, resource) = xgboost_model
         assert k8s.get_resource_exists(reference)
 
-    def test_model_has_correct_arn(self, sagemaker_client, xgboost_model):
-        (reference, _) = xgboost_model
-        resource = k8s.get_resource(reference)
         model_name = resource["spec"].get("modelName", None)
 
-        assert model_name is not None
-
-        assert self._get_resource_model_arn(resource) == self._get_sagemaker_model_arn(
-            sagemaker_client, model_name
-        )
-
-    def test_model_is_deleted(self, sagemaker_client, xgboost_model):
-        (reference, _) = xgboost_model
-        resource = k8s.get_resource(reference)
-        model_name = resource["spec"].get("modelName", None)
+        assert k8s.get_resource_arn(resource) == get_sagemaker_model(model_name)["ModelArn"]
 
         # Delete the k8s resource.
-        _, deleted = k8s.delete_custom_resource(reference)
-        assert deleted is True
+        _, deleted = k8s.delete_custom_resource(reference, 3, 10)
+        assert deleted
+
+        assert get_sagemaker_model(model_name) is None
 
-        assert self._get_sagemaker_model_arn(sagemaker_client, model_name) is None