Build new docker images in build_model with in-memory Dockerfile

clipper_admin/clipper_admin/clipper_admin.py

@@ -1,20 +1,24 @@
 from __future__ import absolute_import, division, print_function
 import logging
 import docker
+import tempfile
 import requests
 from requests.exceptions import RequestException
 import json
 import pprint
 import time
 import re
 import os
+import tarfile
+import six
 
 from .container_manager import CONTAINERLESS_MODEL_IMAGE
 from .exceptions import ClipperException, UnconnectedException
 from .version import __version__
 
 DEFAULT_LABEL = []
 DEFAULT_PREDICTION_CACHE_SIZE_BYTES = 33554432
+CLIPPER_TEMP_DIR = "/tmp/clipper"
 
 logging.basicConfig(
     format='%(asctime)s %(levelname)-8s [%(filename)s:%(lineno)d] %(message)s',
@@ -231,8 +235,7 @@ def build_and_deploy_model(self,
                                base_image,
                                labels=None,
                                container_registry=None,
-                               num_replicas=1,
-                               force=False):
+                               num_replicas=1):
         """Build a new model container Docker image with the provided data and deploy it as
         a model to Clipper.
 
@@ -275,9 +278,6 @@ def build_and_deploy_model(self,
             The number of replicas of the model to create. The number of replicas
             for a model can be changed at any time with
             :py:meth:`clipper.ClipperConnection.set_num_replicas`.
-        force : bool, optional
-            If True, this method will overwrite any existing Dockerfile in ``model_data_path``
-            when building the new image.
 
         Raises
         ------
@@ -287,13 +287,8 @@ def build_and_deploy_model(self,
 
         if not self.connected:
             raise UnconnectedException()
-        image = self.build_model(
-            name,
-            version,
-            model_data_path,
-            base_image,
-            container_registry,
-            force=force)
+        image = self.build_model(name, version, model_data_path, base_image,
+                                 container_registry)
         self.deploy_model(name, version, input_type, image, labels,
                           num_replicas)
 
@@ -302,8 +297,7 @@ def build_model(self,
                     version,
                     model_data_path,
                     base_image,
-                    container_registry=None,
-                    force=False):
+                    container_registry=None):
         """Build a new model container Docker image with the provided data"
 
         This method builds a new Docker image from the provided base image with the local directory specified by
@@ -337,9 +331,6 @@ def build_model(self,
             The Docker container registry to push the freshly built model to. Note
             that if you are running Clipper on Kubernetes, this registry must be accesible
             to the Kubernetes cluster in order to fetch the container from the registry.
-        force : bool, optional
-            If True, this method will overwrite any existing Dockerfile in ``model_data_path``
-            when building the new image.
 
         Returns
         -------
@@ -363,27 +354,33 @@ def build_model(self,
 
         _validate_versioned_model_name(name, version)
 
-        docker_file_path = os.path.join(model_data_path, "Dockerfile")
-        if os.path.isfile(docker_file_path):
-            if force:
-                logger.warning(
-                    "Found existing Dockerfile in {path}. This file will be overwritten".
-                    format(path=model_data_path))
-            else:
-                raise ClipperException("Found existing Dockerfile in {path}.".
-                                       format(path=model_data_path))
-
-        with open(model_data_path + '/Dockerfile', 'w') as f:
-            f.write("FROM {container_name}\nCOPY . /model/\n".format(
-                container_name=base_image))
-
-        image = "{name}:{version}".format(name=name, version=version)
-        if container_registry is not None:
-            image = "{reg}/{image}".format(reg=container_registry, image=image)
-        docker_client = docker.from_env()
-        logger.info("Building model Docker image with model data from {}".
-                    format(model_data_path))
-        docker_client.images.build(path=model_data_path, tag=image)
+        with tempfile.NamedTemporaryFile(
+                mode="w+b", suffix="tar") as context_file:
+            # Create build context tarfile
+            with tarfile.TarFile(
+                    fileobj=context_file, mode="w") as context_tar:
+                context_tar.add(model_data_path)
+                # From https://stackoverflow.com/a/740854/814642
+                df_contents = six.StringIO(
+                    "FROM {container_name}\nCOPY {data_path} /model/\n".format(
+                        container_name=base_image, data_path=model_data_path))
+                df_tarinfo = tarfile.TarInfo('Dockerfile')
+                df_contents.seek(0, os.SEEK_END)
+                df_tarinfo.size = df_contents.tell()
+                df_contents.seek(0)
+                context_tar.addfile(df_tarinfo, df_contents)
+            # Exit Tarfile context manager to finish the tar file
+            # Seek back to beginning of file for reading
+            context_file.seek(0)
+            image = "{name}:{version}".format(name=name, version=version)
+            if container_registry is not None:
+                image = "{reg}/{image}".format(
+                    reg=container_registry, image=image)
+            docker_client = docker.from_env()
+            logger.info("Building model Docker image with model data from {}".
+                        format(model_data_path))
+            docker_client.images.build(
+                fileobj=context_file, custom_context=True, tag=image)
 
         logger.info("Pushing model Docker image to {}".format(image))
         docker_client.images.push(repository=image)

clipper_admin/clipper_admin/deployers/deployer_utils.py

@@ -3,10 +3,12 @@
 import logging
 from .cloudpickle import CloudPickler
 from .module_dependency import ModuleDependencyAnalyzer
+from ..clipper_admin import CLIPPER_TEMP_DIR
 import six
 import os
 import sys
 import shutil
+import tempfile
 cur_dir = os.path.dirname(os.path.abspath(__file__))
 if sys.version < '3':
     import subprocess32 as subprocess
@@ -21,7 +23,6 @@
 
 
 def save_python_function(name, func):
-    relative_base_serializations_dir = "python_func_serializations"
     predict_fname = "func.pkl"
     environment_fname = "environment.yml"
     conda_dep_fname = "conda_dependencies.txt"
@@ -35,10 +36,10 @@ def save_python_function(name, func):
     serialized_prediction_function = s.getvalue()
 
     # Set up serialization directory
-    serialization_dir = os.path.join('/tmp', relative_base_serializations_dir,
-                                     name)
-    if not os.path.exists(serialization_dir):
-        os.makedirs(serialization_dir)
+    if not os.path.exists(CLIPPER_TEMP_DIR):
+        os.makedirs(CLIPPER_TEMP_DIR)
+    serialization_dir = tempfile.mkdtemp(dir=CLIPPER_TEMP_DIR)
+    logger.info("Saving function to {}".format(serialization_dir))
 
     # Export Anaconda environment
     environment_file_abs_path = os.path.join(serialization_dir,

clipper_admin/clipper_admin/deployers/pyspark.py

@@ -210,16 +210,9 @@ def predict(spark, model, inputs):
     logger.info("Spark model saved")
 
     # Deploy model
-    clipper_conn.build_and_deploy_model(
-        name,
-        version,
-        input_type,
-        serialization_dir,
-        base_image,
-        labels,
-        registry,
-        num_replicas,
-        force=True)
+    clipper_conn.build_and_deploy_model(name, version, input_type,
+                                        serialization_dir, base_image, labels,
+                                        registry, num_replicas)
 
     # Remove temp files
     shutil.rmtree(serialization_dir)

clipper_admin/clipper_admin/deployers/python.py

@@ -165,15 +165,8 @@ def centered_predict(inputs):
     serialization_dir = save_python_function(name, func)
     logger.info("Python closure saved")
     # Deploy function
-    clipper_conn.build_and_deploy_model(
-        name,
-        version,
-        input_type,
-        serialization_dir,
-        base_image,
-        labels,
-        registry,
-        num_replicas,
-        force=True)
+    clipper_conn.build_and_deploy_model(name, version, input_type,
+                                        serialization_dir, base_image, labels,
+                                        registry, num_replicas)
     # Remove temp files
     shutil.rmtree(serialization_dir)

examples/tutorial/tutorial_part_two.ipynb

@@ -268,8 +268,7 @@
     "    input_type=\"doubles\",\n",
     "    model_data_path=os.path.abspath(\"tf_cifar_model\"),\n",
     "    base_image=\"clipper/tf_cifar_container:latest\",\n",
-    "    num_replicas=1,\n",
-    "    force=True\n",
+    "    num_replicas=1\n",
     ")"
    ]
   },

integration-tests/clipper_admin_tests.py

@@ -12,6 +12,8 @@
 import json
 import time
 import requests
+import tempfile
+import shutil
 from argparse import ArgumentParser
 import logging
 from test_utils import get_docker_client, create_docker_connection, fake_model_data
@@ -156,12 +158,19 @@ def test_model_version_sets_correctly(self):
         self.assertTrue(models_list_contains_correct_version)
 
     def test_get_logs_creates_log_files(self):
-        log_file_names = self.clipper_conn.get_clipper_logs()
+        if not os.path.exists(cl.CLIPPER_TEMP_DIR):
+            os.makedirs(cl.CLIPPER_TEMP_DIR)
+        tmp_log_dir = tempfile.mkdtemp(dir=cl.CLIPPER_TEMP_DIR)
+        log_file_names = self.clipper_conn.get_clipper_logs(
+            logging_dir=tmp_log_dir)
         self.assertIsNotNone(log_file_names)
         self.assertGreaterEqual(len(log_file_names), 1)
         for file_name in log_file_names:
             self.assertTrue(os.path.isfile(file_name))
 
+        # Remove temp files
+        shutil.rmtree(tmp_log_dir)
+
     def test_inspect_instance_returns_json_dict(self):
         metrics = self.clipper_conn.inspect_instance()
         self.assertEqual(type(metrics), dict)
@@ -173,12 +182,7 @@ def test_model_deploys_successfully(self):
         container_name = "clipper/noop-container:{}".format(clipper_version)
         input_type = "doubles"
         self.clipper_conn.build_and_deploy_model(
-            model_name,
-            version,
-            input_type,
-            fake_model_data,
-            container_name,
-            force=True)
+            model_name, version, input_type, fake_model_data, container_name)
         model_info = self.clipper_conn.get_model_info(model_name, version)
         self.assertIsNotNone(model_info)
         self.assertEqual(type(model_info), dict)
@@ -187,34 +191,14 @@ def test_model_deploys_successfully(self):
             filters={"ancestor": container_name})
         self.assertEqual(len(containers), 1)
 
-    def test_dont_overwrite_docker_file(self):
-        model_name = "m"
-        version = "v1"
-        container_name = "clipper/noop-container:{}".format(clipper_version)
-        input_type = "doubles"
-        # Force this one to make sure there's a docker file in the directory
-        self.clipper_conn.build_model(
-            model_name, version, fake_model_data, container_name, force=True)
-
-        # This call to build_model should cause an error
-        with self.assertRaises(cl.ClipperException) as context:
-            self.clipper_conn.build_model(model_name, version, fake_model_data,
-                                          container_name)
-        self.assertTrue("Found existing Dockerfile" in str(context.exception))
-
     def test_set_num_replicas_for_deployed_model_succeeds(self):
         model_name = "set-num-reps-model"
         input_type = "doubles"
         version = "v1"
         container_name = "clipper/noop-container:{}".format(clipper_version)
         input_type = "doubles"
         self.clipper_conn.build_and_deploy_model(
-            model_name,
-            version,
-            input_type,
-            fake_model_data,
-            container_name,
-            force=True)
+            model_name, version, input_type, fake_model_data, container_name)
 
         # Version defaults to current version
         self.clipper_conn.set_num_replicas(model_name, 4)
@@ -237,8 +221,7 @@ def test_remove_inactive_containers_succeeds(self):
             input_type,
             fake_model_data,
             container_name,
-            num_replicas=2,
-            force=True)
+            num_replicas=2)
         docker_client = get_docker_client()
         containers = docker_client.containers.list(
             filters={"ancestor": container_name})
@@ -250,8 +233,7 @@ def test_remove_inactive_containers_succeeds(self):
             input_type,
             fake_model_data,
             container_name,
-            num_replicas=3,
-            force=True)
+            num_replicas=3)
         containers = docker_client.containers.list(
             filters={"ancestor": container_name})
         self.assertEqual(len(containers), 5)
@@ -395,12 +377,8 @@ def test_deployed_model_queried_successfully(self):
         model_version = 1
         container_name = "clipper/noop-container:{}".format(clipper_version)
         self.clipper_conn.build_and_deploy_model(
-            self.model_name_2,
-            model_version,
-            self.input_type,
-            fake_model_data,
-            container_name,
-            force=True)
+            self.model_name_2, model_version, self.input_type, fake_model_data,
+            container_name)
 
         self.clipper_conn.link_model_to_app(self.app_name_2, self.model_name_2)
         time.sleep(30)
@@ -463,7 +441,6 @@ def predict_func(inputs):
     'test_link_registered_model_to_app_succeeds',
     'get_app_info_for_registered_app_returns_info_dictionary',
     'get_app_info_for_nonexistent_app_returns_none',
-    'test_dont_overwrite_docker_file',
     'test_set_num_replicas_for_external_model_fails',
     'test_model_version_sets_correctly',
     'test_get_logs_creates_log_files',

integration-tests/kubernetes_integration_test.py

@@ -2,6 +2,8 @@
 import os
 import sys
 import requests
+import tempfile
+import shutil
 import json
 import numpy as np
 import time
@@ -10,7 +12,7 @@
                         fake_model_data, headers, log_clipper_state)
 cur_dir = os.path.dirname(os.path.abspath(__file__))
 sys.path.insert(0, os.path.abspath("%s/../clipper_admin" % cur_dir))
-from clipper_admin import __version__ as clipper_version
+from clipper_admin import __version__ as clipper_version, CLIPPER_TEMP_DIR
 
 logging.basicConfig(
     format='%(asctime)s %(levelname)-8s [%(filename)s:%(lineno)d] %(message)s',
@@ -33,8 +35,7 @@ def deploy_model(clipper_conn, name, version):
         "clipper/noop-container:{}".format(clipper_version),
         num_replicas=1,
         container_registry=
-        "568959175238.dkr.ecr.us-west-1.amazonaws.com/clipper",
-        force=True)
+        "568959175238.dkr.ecr.us-west-1.amazonaws.com/clipper")
     time.sleep(10)
 
     clipper_conn.link_model_to_app(app_name, model_name)
@@ -116,7 +117,13 @@ def create_and_test_app(clipper_conn, name, num_models):
                         (num_apps, num_models))
             for a in range(num_apps):
                 create_and_test_app(clipper_conn, "testapp%s" % a, num_models)
-            logger.info(clipper_conn.get_clipper_logs())
+
+            if not os.path.exists(CLIPPER_TEMP_DIR):
+                os.makedirs(CLIPPER_TEMP_DIR)
+            tmp_log_dir = tempfile.mkdtemp(dir=CLIPPER_TEMP_DIR)
+            logger.info(clipper_conn.get_clipper_logs(tmp_log_dir))
+            # Remove temp files
+            shutil.rmtree(tmp_log_dir)
             log_clipper_state(clipper_conn)
             logger.info("SUCCESS")
             clipper_conn.stop_all()