Merge branch 'master' into refactor-tensorflow-general

Author: eslesar-aws

CHANGELOG.md

@@ -1,5 +1,18 @@
 # Changelog
 
+## v1.31.0 (2019-06-27)
+
+### Features
+
+ * use deep learning images
+
+### Bug fixes and other changes
+
+ * Update buildspec.yml
+ * allow only one integration test run per time
+ * remove unnecessary P3 tests from TFS integration tests
+ * add pytest.mark.local_mode annotation to broken tests
+
 ## v1.30.0 (2019-06-25)
 
 ### Features

VERSION

@@ -1 +1 @@
-1.30.1.dev0
+1.31.1.dev0

buildspec.yml

@@ -8,20 +8,35 @@ phases:
   build:
     commands:
       # run linters
-      - tox -e flake8,pylint
-
-      # run package and docbuild checks
-      - tox -e twine
+      - TOX_PARALLEL_NO_SPINNER=1
+      - PY_COLORS=0
+      - tox -e flake8,pylint,twine,black-check --parallel all
       - tox -e sphinx
 
-      # run format verification
-      - tox -e black-check
-
       # run unit tests
       - AWS_ACCESS_KEY_ID= AWS_SECRET_ACCESS_KEY= AWS_SESSION_TOKEN=
         AWS_CONTAINER_CREDENTIALS_RELATIVE_URI= AWS_DEFAULT_REGION=
-        tox -e py36,py27 -- tests/unit
+        tox -e py36,py27 --parallel all -- tests/unit
 
+      # local mode tests
+      - |
+        if has-matching-changes "tests/" "src/*.py" "setup.py" "setup.cfg" "buildspec.yml"; then
+          IGNORE_COVERAGE=- tox -e py36 -- tests/integ -m local_mode --durations 50
+          IGNORE_COVERAGE=- tox -e py27 -- tests/integ -m local_mode --durations 50
+        else
+          echo "skipping integration tests"
+        fi
+        
+      # run integration tests
+      - |
+        if has-matching-changes "tests/" "src/*.py" "setup.py" "setup.cfg" "buildspec.yml"; then
+          python3 -u ci-scripts/queue_build.py
+          IGNORE_COVERAGE=- tox -e py36 -- tests/integ -m "not local_mode" -n 48  --reruns 3 --reruns-delay 5 --durations 50
+          IGNORE_COVERAGE=- tox -e py27 -- tests/integ -m "not local_mode" -n 48  --reruns 3 --reruns-delay 5 --durations 50
+        else
+          echo "skipping integration tests"
+        fi
+        
       # run notebook test
       - |
         if has-matching-changes "src/*.py" "setup.py" "setup.cfg" "buildspec.yml"; then
@@ -30,11 +45,9 @@ phases:
          else
            echo "skipping notebook test"
          fi
-
-      # run integration tests
-      - |
-        if has-matching-changes "tests/" "src/*.py" "setup.py" "setup.cfg" "buildspec.yml"; then
-          IGNORE_COVERAGE=- tox -e py36,py27 -- tests/integ -n 24 --boxed --reruns 2
-        else
-          echo "skipping integration tests"
-        fi
+  post_build:
+    finally:
+      - FILENAME=$(ls ci-lock/)
+      - ACCOUNT=$(aws sts get-caller-identity --output text | awk '{print $1}')
+      - S3_BUCKET_DIR=s3://sagemaker-us-west-2-${ACCOUNT}/ci-lock/
+      - aws s3 rm ${S3_BUCKET_DIR}${FILENAME}

ci-scripts/queue_build.py

@@ -0,0 +1,117 @@
+# Copyright 2019 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.
+from __future__ import absolute_import
+
+import os
+import time
+import boto3
+
+account = boto3.client("sts").get_caller_identity()["Account"]
+bucket_name = "sagemaker-us-west-2-%s" % account
+
+
+def queue_build():
+    build_id = os.environ.get("CODEBUILD_BUILD_ID", "CODEBUILD-BUILD-ID")
+    source_version = os.environ.get("CODEBUILD_SOURCE_VERSION", "CODEBUILD-SOURCE-VERSION").replace(
+        "/", "-"
+    )
+    ticket_number = int(1000 * time.time())
+    filename = "%s_%s_%s" % (ticket_number, build_id, source_version)
+
+    print("Created queue ticket %s" % ticket_number)
+
+    _write_ticket(filename)
+    files = _list_tickets()
+    _cleanup_tickets_older_than_8_hours(files)
+    _wait_for_other_builds(files, ticket_number)
+
+
+def _build_info_from_file(file):
+    filename = file.key.split("/")[1]
+    ticket_number, build_id, source_version = filename.split("_")
+    return int(ticket_number), build_id, source_version
+
+
+def _wait_for_other_builds(files, ticket_number):
+    newfiles = list(filter(lambda file: not _file_older_than(file), files))
+    sorted_files = list(sorted(newfiles, key=lambda y: y.key))
+
+    print("build queue status:")
+    print()
+
+    for order, file in enumerate(sorted_files):
+        file_ticket_number, build_id, source_version = _build_info_from_file(file)
+        print(
+            "%s -> %s %s, ticket number: %s" % (order, build_id, source_version, file_ticket_number)
+        )
+
+    for file in sorted_files:
+        file_ticket_number, build_id, source_version = _build_info_from_file(file)
+
+        if file_ticket_number == ticket_number:
+
+            break
+        else:
+            while True:
+                client = boto3.client("codebuild")
+                response = client.batch_get_builds(ids=[build_id])
+                build_status = response["builds"][0]["buildStatus"]
+
+                if build_status == "IN_PROGRESS":
+                    print(
+                        "waiting on build %s %s %s" % (build_id, source_version, file_ticket_number)
+                    )
+                    time.sleep(30)
+                else:
+                    print("build %s finished, deleting lock" % build_id)
+                    file.delete()
+                    break
+
+
+def _cleanup_tickets_older_than_8_hours(files):
+    oldfiles = list(filter(_file_older_than, files))
+    for file in oldfiles:
+        print("object %s older than 8 hours. Deleting" % file.key)
+        file.delete()
+    return files
+
+
+def _list_tickets():
+    s3 = boto3.resource("s3")
+    bucket = s3.Bucket(bucket_name)
+    objects = [file for file in bucket.objects.filter(Prefix="ci-lock/")]
+    files = list(filter(lambda x: x != "ci-lock/", objects))
+    return files
+
+
+def _file_older_than(file):
+    timelimit = 1000 * 60 * 60 * 8
+
+    file_ticket_number, build_id, source_version = _build_info_from_file(file)
+
+    return int(time.time()) - file_ticket_number > timelimit
+
+
+def _write_ticket(ticket_number):
+
+    if not os.path.exists("ci-lock"):
+        os.mkdir("ci-lock")
+
+    filename = "ci-lock/" + ticket_number
+    with open(filename, "w") as file:
+        file.write(ticket_number)
+    boto3.Session().resource("s3").Object(bucket_name, filename).upload_file(filename)
+
+
+if __name__ == "__main__":
+    queue_build()

setup.py

@@ -67,7 +67,7 @@ def read_version():
     install_requires=required_packages,
     extras_require={
         "test": [
-            "tox",
+            "tox==3.13.1",
             "flake8",
             "pytest==4.4.1",
             "pytest-cov",
@@ -79,6 +79,7 @@ def read_version():
             "awslogs",
             "pandas",
             "black==19.3b0 ; python_version >= '3.6'",
+            "stopit==1.1.2",
         ]
     },
     entry_points={"console_scripts": ["sagemaker=sagemaker.cli.main:main"]},

src/sagemaker/fw_utils.py

@@ -54,6 +54,56 @@
 VALID_EIA_FRAMEWORKS = ["tensorflow", "tensorflow-serving", "mxnet", "mxnet-serving"]
 VALID_ACCOUNTS_BY_REGION = {"us-gov-west-1": "246785580436", "us-iso-east-1": "744548109606"}
 
+MERGED_FRAMEWORKS_REPO_MAP = {
+    "tensorflow-scriptmode": "tensorflow-training",
+    "mxnet": "mxnet-training",
+    "tensorflow-serving": "tensorflow-inference",
+    "mxnet-serving": "mxnet-inference",
+}
+
+MERGED_FRAMEWORKS_LOWEST_VERSIONS = {
+    "tensorflow-scriptmode": [1, 13, 1],
+    "mxnet": [1, 4, 1],
+    "tensorflow-serving": [1, 13, 0],
+    "mxnet-serving": [1, 4, 1],
+}
+
+
+def is_version_equal_or_higher(lowest_version, framework_version):
+    """Determine whether the ``framework_version`` is equal to or higher than ``lowest_version``
+
+    Args:
+        lowest_version (List[int]): lowest version represented in an integer list
+        framework_version (str): framework version string
+
+    Returns:
+        bool: Whether or not framework_version is equal to or higher than lowest_version
+    """
+    version_list = [int(s) for s in framework_version.split(".")]
+    return version_list >= lowest_version[0 : len(version_list)]
+
+
+def _is_merged_versions(framework, framework_version):
+    lowest_version_list = MERGED_FRAMEWORKS_LOWEST_VERSIONS.get(framework)
+    if lowest_version_list:
+        return is_version_equal_or_higher(lowest_version_list, framework_version)
+    else:
+        return False
+
+
+def _using_merged_images(region, framework, py_version, accelerator_type, framework_version):
+    is_gov_region = region in VALID_ACCOUNTS_BY_REGION
+    is_py3 = py_version == "py3" or py_version is None
+    is_merged_versions = _is_merged_versions(framework, framework_version)
+    return (not is_gov_region) and is_merged_versions and is_py3 and accelerator_type is None
+
+
+def _registry_id(region, framework, py_version, account, accelerator_type, framework_version):
+    if _using_merged_images(region, framework, py_version, accelerator_type, framework_version):
+        return "763104351884"
+    else:
+        return VALID_ACCOUNTS_BY_REGION.get(region, account)
+
 
 def create_image_uri(
     region,
@@ -86,8 +136,15 @@ def create_image_uri(
     if py_version and py_version not in VALID_PY_VERSIONS:
         raise ValueError("invalid py_version argument: {}".format(py_version))
 
-    # Handle Account Number for Gov Cloud
-    account = VALID_ACCOUNTS_BY_REGION.get(region, account)
+    # Handle Account Number for Gov Cloud and frameworks with DLC merged images
+    account = _registry_id(
+        region=region,
+        framework=framework,
+        py_version=py_version,
+        account=account,
+        accelerator_type=accelerator_type,
+        framework_version=framework_version,
+    )
 
     # Handle Local Mode
     if instance_type.startswith("local"):
@@ -121,7 +178,14 @@ def create_image_uri(
     ):
         framework += "-eia"
 
-    return "{}/sagemaker-{}:{}".format(get_ecr_image_uri_prefix(account, region), framework, tag)
+    if _using_merged_images(region, framework, py_version, accelerator_type, framework_version):
+        return "{}/{}:{}".format(
+            get_ecr_image_uri_prefix(account, region), MERGED_FRAMEWORKS_REPO_MAP[framework], tag
+        )
+    else:
+        return "{}/sagemaker-{}:{}".format(
+            get_ecr_image_uri_prefix(account, region), framework, tag
+        )
 
 
 def _accelerator_type_valid_for_framework(
@@ -264,7 +328,7 @@ def framework_name_from_image(image_name):
         # extract framework, python version and image tag
         # We must support both the legacy and current image name format.
         name_pattern = re.compile(
-            r"^sagemaker(?:-rl)?-(tensorflow|mxnet|chainer|pytorch|scikit-learn)(?:-)?(scriptmode)?:(.*)-(.*?)-(py2|py3)$"  # noqa: E501
+            r"^(?:sagemaker(?:-rl)?-)?(tensorflow|mxnet|chainer|pytorch|scikit-learn)(?:-)?(scriptmode|training)?:(.*)-(.*?)-(py2|py3)$"  # noqa: E501
         )
         legacy_name_pattern = re.compile(r"^sagemaker-(tensorflow|mxnet)-(py2|py3)-(cpu|gpu):(.*)$")
 

tests/integ/test_local_mode.py

@@ -20,9 +20,10 @@
 import pytest
 import tempfile
 
+import stopit
+
 import tests.integ.lock as lock
 from tests.integ import DATA_DIR, PYTHON_VERSION
-from tests.integ.timeout import timeout
 
 from sagemaker.local import LocalSession, LocalSagemakerRuntimeClient, LocalSagemakerClient
 from sagemaker.mxnet import MXNet
@@ -86,7 +87,7 @@ def _create_model(output_path):
 @pytest.mark.local_mode
 @pytest.mark.skipif(PYTHON_VERSION != "py2", reason="TensorFlow image supports only python 2.")
 def test_tf_local_mode(sagemaker_local_session):
-    with timeout(minutes=5):
+    with stopit.ThreadingTimeout(5 * 60, swallow_exc=False):
         script_path = os.path.join(DATA_DIR, "iris", "iris-dnn-classifier.py")
 
         estimator = TensorFlow(
@@ -129,7 +130,7 @@ def test_tf_local_mode(sagemaker_local_session):
 @pytest.mark.local_mode
 @pytest.mark.skipif(PYTHON_VERSION != "py2", reason="TensorFlow image supports only python 2.")
 def test_tf_distributed_local_mode(sagemaker_local_session):
-    with timeout(minutes=5):
+    with stopit.ThreadingTimeout(5 * 60, swallow_exc=False):
         script_path = os.path.join(DATA_DIR, "iris", "iris-dnn-classifier.py")
 
         estimator = TensorFlow(
@@ -171,7 +172,7 @@ def test_tf_distributed_local_mode(sagemaker_local_session):
 @pytest.mark.local_mode
 @pytest.mark.skipif(PYTHON_VERSION != "py2", reason="TensorFlow image supports only python 2.")
 def test_tf_local_data(sagemaker_local_session):
-    with timeout(minutes=5):
+    with stopit.ThreadingTimeout(5 * 60, swallow_exc=False):
         script_path = os.path.join(DATA_DIR, "iris", "iris-dnn-classifier.py")
 
         estimator = TensorFlow(
@@ -212,7 +213,7 @@ def test_tf_local_data(sagemaker_local_session):
 @pytest.mark.local_mode
 @pytest.mark.skipif(PYTHON_VERSION != "py2", reason="TensorFlow image supports only python 2.")
 def test_tf_local_data_local_script():
-    with timeout(minutes=5):
+    with stopit.ThreadingTimeout(5 * 60, swallow_exc=False):
         script_path = os.path.join(DATA_DIR, "iris", "iris-dnn-classifier.py")
 
         estimator = TensorFlow(
@@ -391,7 +392,7 @@ def test_local_transform_mxnet(sagemaker_local_session, tmpdir, mxnet_full_versi
         path=os.path.join(data_path, "test"), key_prefix="integ-test-data/mxnet_mnist/test"
     )
 
-    with timeout(minutes=15):
+    with stopit.ThreadingTimeout(5 * 60, swallow_exc=False):
         mx.fit({"train": train_input, "test": test_input})
 
     transform_input_path = os.path.join(data_path, "transform")