Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use images.build rather than low level build API to catch errors #5399

Merged
merged 12 commits into from
Jul 11, 2023
8 changes: 6 additions & 2 deletions samcli/lib/build/app_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -407,14 +407,18 @@ def _build_lambda_image(self, function_name: str, metadata: Dict, architecture:
"dockerfile": dockerfile,
"tag": docker_tag,
"buildargs": docker_build_args,
"decode": True,
"platform": get_docker_platform(architecture),
"rm": True,
}
if docker_build_target:
build_args["target"] = cast(str, docker_build_target)

build_logs = self._docker_client.api.build(**build_args)
try:
(build_image, build_logs) = self._docker_client.images.build(**build_args)
LOG.debug("%s image is built for %s function", build_image, function_name)
except docker.errors.BuildError as ex:
LOG.error("Failed building function %s", function_name)
raise DockerBuildFailed(str(ex)) from ex

# The Docker-py low level api will stream logs back but if an exception is raised by the api
# this is raised when accessing the generator. So we need to wrap accessing build_logs in a try: except.
Expand Down
34 changes: 34 additions & 0 deletions tests/integration/buildcmd/test_build_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from pathlib import Path
from typing import Set
from unittest import skipIf
from uuid import uuid4

import jmespath
import docker
Expand Down Expand Up @@ -49,6 +50,39 @@
SKIP_SAR_TESTS = RUNNING_ON_CI and RUNNING_TEST_FOR_MASTER_ON_CI and not RUN_BY_CANARY


@skipIf(SKIP_DOCKER_TESTS, SKIP_DOCKER_MESSAGE)
class TestBuildingImageTypeLambdaDockerFileFailures(BuildIntegBase):
template = "template_image.yaml"

def test_with_invalid_dockerfile_location(self):
overrides = {
"Runtime": "3.10",
"Handler": "handler",
"DockerFile": "ThisDockerfileDoesNotExist",
"Tag": uuid4().hex,
}
cmdlist = self.get_command_list(parameter_overrides=overrides)
command_result = run_command(cmdlist, cwd=self.working_dir)

# confirm build failed
self.assertEqual(command_result.process.returncode, 1)
self.assertIn("Cannot locate specified Dockerfile", command_result.stderr.decode())

def test_with_invalid_dockerfile_definition(self):
overrides = {
"Runtime": "3.10",
"Handler": "handler",
"DockerFile": "InvalidDockerfile",
"Tag": uuid4().hex,
}
cmdlist = self.get_command_list(parameter_overrides=overrides)
command_result = run_command(cmdlist, cwd=self.working_dir)

# confirm build failed
self.assertEqual(command_result.process.returncode, 1)
self.assertIn("COPY requires at least two arguments", command_result.stderr.decode())


@skipIf(
# Hits public ECR pull limitation, move it to canary tests
(not RUN_BY_CANARY and not CI_OVERRIDE),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
ARG BASE_RUNTIME

FROM public.ecr.aws/lambda/python:$BASE_RUNTIME

ARG FUNCTION_DIR="/var/task"

RUN mkdir -p $FUNCTION_DIR

# invalid line below
COPY main.py

COPY __init__.py $FUNCTION_DIR
COPY requirements.txt $FUNCTION_DIR

RUN python -m pip install -r $FUNCTION_DIR/requirements.txt -t $FUNCTION_DIR

28 changes: 17 additions & 11 deletions tests/unit/lib/build_module/test_app_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -1510,7 +1510,7 @@ def test_docker_build_raises_DockerBuildFailed_when_error_in_buildlog_stream(sel
"DockerBuildArgs": {"a": "b"},
}

self.docker_client_mock.api.build.return_value = [{"error": "Function building failed"}]
self.docker_client_mock.images.build.return_value = (Mock(), [{"error": "Function building failed"}])

self.builder._build_lambda_image("Name", metadata, X86_64)

Expand All @@ -1530,7 +1530,7 @@ def test_dockerfile_not_in_dockercontext(self):
"Bad Request", response=response_mock, explanation="Cannot locate specified Dockerfile"
)
self.builder._stream_lambda_image_build_logs = error_mock
self.docker_client_mock.api.build.return_value = []
self.docker_client_mock.images.build.return_value = (Mock(), [])

self.builder._build_lambda_image("Name", metadata, X86_64)

Expand All @@ -1545,7 +1545,7 @@ def test_error_rerasises(self):
error_mock = Mock()
error_mock.side_effect = docker.errors.APIError("Bad Request", explanation="Some explanation")
self.builder._stream_lambda_image_build_logs = error_mock
self.docker_client_mock.api.build.return_value = []
self.docker_client_mock.images.build.return_value = (Mock(), [])

self.builder._build_lambda_image("Name", metadata, X86_64)

Expand All @@ -1557,7 +1557,7 @@ def test_can_build_image_function(self):
"DockerBuildArgs": {"a": "b"},
}

self.docker_client_mock.api.build.return_value = []
self.docker_client_mock.images.build.return_value = (Mock(), [])

result = self.builder._build_lambda_image("Name", metadata, X86_64)

Expand Down Expand Up @@ -1598,7 +1598,7 @@ def test_build_image_function_with_empty_metadata_raises_Docker_Build_Failed_Exc
def test_can_build_image_function_without_tag(self):
metadata = {"Dockerfile": "Dockerfile", "DockerContext": "context", "DockerBuildArgs": {"a": "b"}}

self.docker_client_mock.api.build.return_value = []
self.docker_client_mock.images.build.return_value = (Mock(), [])
result = self.builder._build_lambda_image("Name", metadata, X86_64)

self.assertEqual(result, "name:latest")
Expand All @@ -1613,19 +1613,18 @@ def test_can_build_image_function_under_debug(self, mock_os):
"DockerBuildArgs": {"a": "b"},
}

self.docker_client_mock.api.build.return_value = []
self.docker_client_mock.images.build.return_value = (Mock, [])

result = self.builder._build_lambda_image("Name", metadata, X86_64)
self.assertEqual(result, "name:Tag-debug")
self.assertEqual(
self.docker_client_mock.api.build.call_args,
self.docker_client_mock.images.build.call_args,
# NOTE (sriram-mv): path set to ANY to handle platform differences.
call(
path=ANY,
dockerfile="Dockerfile",
tag="name:Tag-debug",
buildargs={"a": "b", "SAM_BUILD_MODE": "debug"},
decode=True,
platform="linux/amd64",
rm=True,
),
Expand All @@ -1642,24 +1641,31 @@ def test_can_build_image_function_under_debug_with_target(self, mock_os):
"DockerBuildTarget": "stage",
}

self.docker_client_mock.api.build.return_value = []
self.docker_client_mock.images.build.return_value = (Mock(), [])

result = self.builder._build_lambda_image("Name", metadata, X86_64)
self.assertEqual(result, "name:Tag-debug")
self.assertEqual(
self.docker_client_mock.api.build.call_args,
self.docker_client_mock.images.build.call_args,
call(
path=ANY,
dockerfile="Dockerfile",
tag="name:Tag-debug",
buildargs={"a": "b", "SAM_BUILD_MODE": "debug"},
decode=True,
target="stage",
platform="linux/amd64",
rm=True,
),
)

def test_can_raise_build_error(self):
self.docker_client_mock.images.build.side_effect = docker.errors.BuildError(
reason="Missing Dockerfile", build_log="Build failed"
)

with self.assertRaises(DockerBuildFailed):
self.builder._build_lambda_image("Name", {}, X86_64)


class TestApplicationBuilder_build_function(TestCase):
def setUp(self):
Expand Down