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

Bug: sam build doesn't return an error when it should #5026

Closed
aidansteele opened this issue Apr 17, 2023 · 4 comments
Closed

Bug: sam build doesn't return an error when it should #5026

aidansteele opened this issue Apr 17, 2023 · 4 comments
Labels
area/docker stage/waiting-for-release Fix has been merged to develop and is waiting for a release type/bug

Comments

@aidansteele
Copy link

Description:

I am trying to build and deploy an image-based Lambda function using the SAM CLI. sam build succeeds even when the image failed to build, which causes a lot of confusion.

I think the cause is the underlying docker-py library not raising an exception when it should. I filed an issue in that repo too: docker/docker-py#3108. The code in question hasn't changed between 4.2.0 (which SAM uses) and the head of that repo.

Steps to reproduce:

Have the following template:

Transform: AWS::Serverless-2016-10-31

Resources:
  Repro:
    Type: AWS::Serverless::Function
    Metadata:
      DockerContext: example
      Dockerfile: Dockerfile
    Properties:
      PackageType: Image

Make sure there is an example directory without a Dockerfile inside it. Now run sam build. This should fail. It fails (as expected) on Linux, but it succeeds on MacOS.

Observed result:

aidan@Aidans-MacBook-Pro samcli-issue-repro % sam build --debug
2023-04-17 15:20:56,189 | Config file location: /Users/aidan/dev/tmp/samcli-issue-repro/samconfig.toml
2023-04-17 15:20:56,189 | Config file '/Users/aidan/dev/tmp/samcli-issue-repro/samconfig.toml' does not exist
2023-04-17 15:20:56,190 | Using SAM Template at /Users/aidan/dev/tmp/samcli-issue-repro/template.yml
2023-04-17 15:20:56,236 | Using config file: samconfig.toml, config environment: default
2023-04-17 15:20:56,236 | Expand command line arguments to:
2023-04-17 15:20:56,236 | --template_file=/Users/aidan/dev/tmp/samcli-issue-repro/template.yml --mount_with=READ --build_dir=.aws-sam/build --cache_dir=.aws-sam/cache 
2023-04-17 15:20:56,587 | 'build' command is called
2023-04-17 15:20:56,588 | No Parameters detected in the template
2023-04-17 15:20:56,609 | There is no customer defined id or cdk path defined for resource Repro, so we will use the resource logical id as the resource id
2023-04-17 15:20:56,609 | 0 stacks found in the template
2023-04-17 15:20:56,609 | No Parameters detected in the template
2023-04-17 15:20:56,618 | There is no customer defined id or cdk path defined for resource Repro, so we will use the resource logical id as the resource id
2023-04-17 15:20:56,619 | 1 resources found in the stack 
2023-04-17 15:20:56,619 | Found Serverless function with name='Repro' and ImageUri='None'
2023-04-17 15:20:56,619 | --base-dir is not presented, adjusting uri example relative to /Users/aidan/dev/tmp/samcli-issue-repro/template.yml
2023-04-17 15:20:56,619 | --base-dir is not presented, adjusting uri . relative to /Users/aidan/dev/tmp/samcli-issue-repro/template.yml
2023-04-17 15:20:56,620 | 1 resources found in the stack 
2023-04-17 15:20:56,620 | Found Serverless function with name='Repro' and ImageUri='None'
2023-04-17 15:20:56,621 | Error occurred while trying to track an event: Event 'BuildFunctionRuntime' does not accept value 'None'.
2023-04-17 15:20:56,621 | Instantiating build definitions
2023-04-17 15:20:56,623 | Unique function build definition found, adding as new (Function Build Definition: BuildDefinition(None, /Users/aidan/dev/tmp/samcli-issue-repro, Image, , 42499c64-f556-4788-ae4a-e03973bece35, {'DockerContext': '/Users/aidan/dev/tmp/samcli-issue-repro/example', 'Dockerfile': 'Dockerfile'}, {}, x86_64, []), Function: Function(function_id='Repro', name='Repro', functionname='Repro', runtime=None, memory=None, timeout=None, handler=None, imageuri=None, packagetype='Image', imageconfig=None, codeuri='/Users/aidan/dev/tmp/samcli-issue-repro', environment=None, rolearn=None, layers=[], events=None, metadata={'DockerContext': '/Users/aidan/dev/tmp/samcli-issue-repro/example', 'Dockerfile': 'Dockerfile', 'SamResourceId': 'Repro'}, inlinecode=None, codesign_config_arn=None, architectures=None, function_url_config=None, stack_path='', runtime_management_config=None))
2023-04-17 15:20:56,625 | Building codeuri: /Users/aidan/dev/tmp/samcli-issue-repro runtime: None metadata: {'DockerContext': '/Users/aidan/dev/tmp/samcli-issue-repro/example', 'Dockerfile': 'Dockerfile'} architecture: x86_64 functions: Repro
2023-04-17 15:20:56,625 | Building to following folder /Users/aidan/dev/tmp/samcli-issue-repro/.aws-sam/build/Repro
2023-04-17 15:20:56,625 | Building image for Repro function
2023-04-17 15:20:56,666 | Setting DockerBuildArgs: {} for Repro function

2023-04-17 15:20:58,282 | There is no customer defined id or cdk path defined for resource Repro, so we will use the resource logical id as the resource id
2023-04-17 15:20:58,282 | 1 resources found in the stack 
2023-04-17 15:20:58,282 | Found Serverless function with name='Repro' and ImageUri='None'

Build Succeeded

Built Artifacts  : .aws-sam/build
Built Template   : .aws-sam/build/template.yaml

Commands you can use next
=========================
[*] Validate SAM template: sam validate
[*] Invoke Function: sam local invoke
[*] Test Function in the Cloud: sam sync --stack-name {{stack-name}} --watch
[*] Deploy: sam deploy --guided
2023-04-17 15:20:58,286 | Telemetry endpoint configured to be https://aws-serverless-tools-telemetry.us-west-2.amazonaws.com/metrics
2023-04-17 15:20:58,334 | Sending Telemetry: {'metrics': [{'commandRun': {'requestId': '2de2ab17-40f4-4b58-b7dd-228765c40aea', 'installationId': '70bca007-b2e7-41ee-8bbd-6a5df3cbd156', 'sessionId': '607d1848-d6fc-4b57-a9cf-e477253f2a1d', 'executionEnvironment': 'CLI', 'ci': False, 'pyversion': '3.8.16', 'samcliVersion': '1.79.0', 'awsProfileProvided': False, 'debugFlagProvided': True, 'region': '', 'commandName': 'sam build', 'metricSpecificAttributes': {'projectType': 'CFN', 'gitOrigin': None, 'projectName': '3777e6677d14521a71e0dc5415f6de986cd07b338510396df556cd5f6cfe5fc1', 'initialCommit': None}, 'duration': 2050, 'exitReason': 'success', 'exitCode': 0}}]}
2023-04-17 15:20:59,043 | HTTPSConnectionPool(host='aws-serverless-tools-telemetry.us-west-2.amazonaws.com', port=443): Read timed out. (read timeout=0.1)
aidan@Aidans-MacBook-Pro samcli-issue-repro % 

Expected result:

I expect the build to fail, like it does on Linux.

Additional environment details (Ex: Windows, Mac, Amazon Linux etc)

{
  "version": "1.79.0",
  "system": {
    "python": "3.8.16",
    "os": "macOS-13.3.1-arm64-arm-64bit"
  },
  "additional_dependencies": {
    "docker_engine": "20.10.23",
    "aws_cdk": "Not available",
    "terraform": "1.4.5"
  }
}
@aidansteele aidansteele added the stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. label Apr 17, 2023
@jfuss
Copy link
Contributor

jfuss commented Apr 17, 2023

@aidansteele Thanks for reporting both to us and Docker. We do need to upgrade Docker regardless (we are too far out of date).

@jfuss jfuss added area/docker type/bug and removed stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. labels Apr 17, 2023
@mndeveci
Copy link
Contributor

As @jfuss recommended, I've tried to update dockerpy to v6 but unfortunately this is still happening.

What I've found is that, when we call build API, it only returns the log stream there is no information about the success of the call. What we are doing now is to check keys in that response, and error is in that response, we raise an exception and fail the build (see here and here).

There are two options for fixing this issue;

  1. We can check for the message key in the log stream and if there is one, we can fail the build. But unfortunately, Docker API doesn't have specific documentation about it, and if they are using message key for something else, this might break some customers.
  2. We can check existince of Dockerfile before starting the build. Somewhere here, we can create the Dockerfile path with docker_context_dir.joinpath(dockerfile) and check existence of that path before moving forward.

I will be discussing this with the team about which approach would be better.

@efekarakus
Copy link
Contributor

Hi! 👋

I also encountered the same issue, I'm just sharing here my setup if it helps validate that the error is part of the message key:

Reproduction steps

  1. I initialized a sample application with the sam init command and option 7: Serverless API.

  2. Then I wrote the following the following problematic Dockerfile:

    FROM public.ecr.aws/lambda/nodejs:18
    COPY package.json
    COPY src/handlers/get-all-items.mjs ${LAMBDA_TASK_ROOT}
    WORKDIR ${LAMBDA_TASK_ROOT}
    RUN npm install
    
    CMD ["get-all-items.getAllItemsHandler"]

    The issue was line 2. I forgot to add a destination path for the COPY instruction.

  3. This was the log of output from sam build:

    $ sam build
    Starting Build use cache                                                                                                                                                                                     
    Building codeuri: /Users/karakuse/Development/playground/deleteme/helloworld-sam/helloworldsam runtime: None metadata: {'DockerTag': 'nihao', 'DockerContext': '/Users/karakuse/Development/playground/deleteme/helloworld-sam/helloworldsam', 'Dockerfile': 'Dockerfile'} architecture: x86_64 functions: getAllItemsFunction                                              
    Building image for getAllItemsFunction function                                                                                                                                                              
    Manifest is not changed for (getByIdFunction, putItemFunction), running incremental build                                                                                                                    
    Building codeuri: /Users/karakuse/Development/playground/deleteme/helloworld-sam/helloworldsam runtime: nodejs18.x metadata: {} architecture: x86_64 functions: getByIdFunction, putItemFunction             
    Setting DockerBuildArgs: {} for getAllItemsFunction function                                                                                                                                                 
    Running NodejsNpmBuilder:NpmPack                                                                                                                                                                             
    Running NodejsNpmBuilder:CopyNpmrcAndLockfile                                                                                                                                                                
    Running NodejsNpmBuilder:CopySource                                                                                                                                                                          
    Running NodejsNpmBuilder:CopySource                                                                                                                                                                          
    Running NodejsNpmBuilder:CleanUpNpmrc                                                                                                                                                                        
    Running NodejsNpmBuilder:LockfileCleanUp                                                                                                                                                                     
    Running NodejsNpmBuilder:LockfileCleanUp                                                                                                                                                                     
    
    Build Succeeded

@mndeveci mndeveci added the stage/waiting-for-release Fix has been merged to develop and is waiting for a release label Jul 11, 2023
@github-actions
Copy link
Contributor

Patch is released in v1.91.0. Closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docker stage/waiting-for-release Fix has been merged to develop and is waiting for a release type/bug
Projects
None yet
Development

No branches or pull requests

4 participants