fix(build): cleanup build containers (aws#929)

samcli/lib/build/app_builder.py

@@ -246,31 +246,34 @@ def _build_function_on_container(self,  # pylint: disable=too-many-locals
                                          options=None)
 
         try:
-            self._container_manager.run(container)
-        except docker.errors.APIError as ex:
-            if "executable file not found in $PATH" in str(ex):
-                raise UnsupportedBuilderLibraryVersionError(container.image,
-                                                            "{} executable not found in container"
-                                                            .format(container.executable_name))
-
-        # Container's output provides status of whether the build succeeded or failed
-        # stdout contains the result of JSON-RPC call
-        stdout_stream = io.BytesIO()
-        # stderr contains logs printed by the builder. Stream it directly to terminal
-        stderr_stream = osutils.stderr()
-        container.wait_for_logs(stdout=stdout_stream, stderr=stderr_stream)
-
-        stdout_data = stdout_stream.getvalue().decode('utf-8')
-        LOG.debug("Build inside container returned response %s", stdout_data)
-
-        response = self._parse_builder_response(stdout_data, container.image)
-
-        # Request is successful. Now copy the artifacts back to the host
-        LOG.debug("Build inside container was successful. Copying artifacts from container to host")
-
-        # "/." is a Docker thing that instructions the copy command to download contents of the folder only
-        result_dir_in_container = response["result"]["artifacts_dir"] + "/."
-        container.copy(result_dir_in_container, artifacts_dir)
+            try:
+                self._container_manager.run(container)
+            except docker.errors.APIError as ex:
+                if "executable file not found in $PATH" in str(ex):
+                    raise UnsupportedBuilderLibraryVersionError(container.image,
+                                                                "{} executable not found in container"
+                                                                .format(container.executable_name))
+
+            # Container's output provides status of whether the build succeeded or failed
+            # stdout contains the result of JSON-RPC call
+            stdout_stream = io.BytesIO()
+            # stderr contains logs printed by the builder. Stream it directly to terminal
+            stderr_stream = osutils.stderr()
+            container.wait_for_logs(stdout=stdout_stream, stderr=stderr_stream)
+
+            stdout_data = stdout_stream.getvalue().decode('utf-8')
+            LOG.debug("Build inside container returned response %s", stdout_data)
+
+            response = self._parse_builder_response(stdout_data, container.image)
+
+            # Request is successful. Now copy the artifacts back to the host
+            LOG.debug("Build inside container was successful. Copying artifacts from container to host")
+
+            # "/." is a Docker thing that instructions the copy command to download contents of the folder only
+            result_dir_in_container = response["result"]["artifacts_dir"] + "/."
+            container.copy(result_dir_in_container, artifacts_dir)
+        finally:
+            self._container_manager.stop(container)
 
         LOG.debug("Build inside container succeeded")
         return artifacts_dir

tests/integration/buildcmd/build_integ_base.py

@@ -2,6 +2,8 @@
 import shutil
 import tempfile
 
+import docker
+
 try:
     from pathlib import Path
 except ImportError:
@@ -73,6 +75,12 @@ def get_command_list(self, build_dir=None, base_dir=None, manifest_path=None, us
 
         return command_list
 
+    def verify_docker_container_cleanedup(self, runtime):
+        docker_client = docker.from_env()
+        samcli_containers = \
+            docker_client.containers.list(all=True, filters={"ancestor": "lambci/lambda:build-{}".format(runtime)})
+        self.assertFalse(bool(samcli_containers), "Build containers have not been removed")
+
     def _make_parameter_override_arg(self, overrides):
         return " ".join([
             "ParameterKey={},ParameterValue={}".format(key, value) for key, value in overrides.items()

tests/integration/buildcmd/test_build_cmd.py

@@ -68,6 +68,7 @@ def test_with_default_requirements(self, runtime, use_container):
                                            self.FUNCTION_LOGICAL_ID,
                                            self._make_parameter_override_arg(overrides),
                                            expected)
+        self.verify_docker_container_cleanedup(runtime)
 
     def _verify_invoke_built_function(self, template_path, function_logical_id, overrides, expected_result):
         LOG.info("Invoking built function '{}'", function_logical_id)
@@ -165,6 +166,7 @@ def test_with_default_package_json(self, runtime, use_container):
                                            os.path.normpath(os.path.join(str(self.test_data_path), "SomeRelativePath")),
                                            str(self.default_build_dir))
                                        )
+        self.verify_docker_container_cleanedup(runtime)
 
     def _verify_built_artifact(self, build_dir, function_logical_id, expected_files, expected_modules):
 
@@ -229,6 +231,7 @@ def test_with_default_gemfile(self, runtime, use_container):
                                            os.path.normpath(os.path.join(str(self.test_data_path), "SomeRelativePath")),
                                            str(self.default_build_dir))
                                        )
+        self.verify_docker_container_cleanedup(runtime)
 
     def _verify_built_artifact(self, build_dir, function_logical_id, expected_files, expected_modules):
 

tests/unit/lib/build_module/test_app_builder.py

@@ -318,6 +318,7 @@ def mock_wait_for_logs(stdout, stderr):
         self.builder._parse_builder_response.assert_called_once_with(stdout_data, container_mock.image)
         container_mock.copy.assert_called_with(response["result"]["artifacts_dir"] + "/.",
                                                "artifacts_dir")
+        self.container_manager.stop.assert_called_with(container_mock)
 
     @patch("samcli.lib.build.app_builder.LambdaBuildContainer")
     def test_must_raise_on_unsupported_container(self, LambdaBuildContainerMock):
@@ -343,6 +344,7 @@ def test_must_raise_on_unsupported_container(self, LambdaBuildContainerMock):
               "Reason: 'myexecutable executable not found in container'"
 
         self.assertEquals(str(ctx.exception), msg)
+        self.container_manager.stop.assert_called_with(container_mock)
 
 
 class TestApplicationBuilder_parse_builder_response(TestCase):