Skip to content

Conversation

@bmoffatt
Copy link
Contributor

@bmoffatt bmoffatt commented Sep 13, 2022

Issue #, if available:

N/A

Description of changes:

I ran into this when trying to deploy a go function with the arm64 platform. go1.x is x86_64 only, so this requires the provided.al2 runtime. The docs for the go runtime demonstrate using the Makefile builder for using provided.al2. This works, but requires an extra project file compared to go1.x. I also saw in the docs that BuildMethod can be set the the value of a runtime, so tried that!

from: https://docs.aws.amazon.com/serverless-application-model/latest/developerguide/sam-cli-command-reference-sam-build.html

If a resource includes a Metadata resource attribute with a BuildMethod entry, sam build builds that resource according to the value of the BuildMethod entry. Valid values for BuildMethod are 1) One of the identifiers for a Lambda runtime, or 2) The makefile identifier.

Here's my test template.yaml:

AWSTemplateFormatVersion: '2010-09-09'
Transform: AWS::Serverless-2016-10-31
Resources:
  Hello:
    Type: AWS::Serverless::Function 
    Metadata:
      BuildMethod: go1.x
    Properties:
      CodeUri: .
      Handler: bootstrap
      Runtime: provided.al2
      Architectures:
      - arm64
      FunctionUrlConfig:
        AuthType: NONE
Outputs:
  HelloEndpoint:
    Description: "The Function URL!"
    Value:
      Fn::GetAtt: HelloUrl.FunctionUrl

And the error sam build throws:

$ sam build
Building codeuri: /Users/moffattb/sam-app runtime: provided.al2 metadata: {'BuildMethod': 'go1.x'} architecture: arm64 functions: Hello
Traceback (most recent call last):
  File "/usr/local/bin/sam", line 8, in <module>
    sys.exit(cli())
  File "/usr/local/Cellar/aws-sam-cli/1.56.1/libexec/lib/python3.8/site-packages/click/core.py", line 829, in __call__
    return self.main(*args, **kwargs)
  File "/usr/local/Cellar/aws-sam-cli/1.56.1/libexec/lib/python3.8/site-packages/click/core.py", line 782, in main
    rv = self.invoke(ctx)
  File "/usr/local/Cellar/aws-sam-cli/1.56.1/libexec/lib/python3.8/site-packages/click/core.py", line 1259, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/usr/local/Cellar/aws-sam-cli/1.56.1/libexec/lib/python3.8/site-packages/click/core.py", line 1066, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/local/Cellar/aws-sam-cli/1.56.1/libexec/lib/python3.8/site-packages/click/core.py", line 610, in invoke
    return callback(*args, **kwargs)
  File "/usr/local/Cellar/aws-sam-cli/1.56.1/libexec/lib/python3.8/site-packages/click/decorators.py", line 73, in new_func
    return ctx.invoke(f, obj, *args, **kwargs)
  File "/usr/local/Cellar/aws-sam-cli/1.56.1/libexec/lib/python3.8/site-packages/click/core.py", line 610, in invoke
    return callback(*args, **kwargs)
  File "/usr/local/Cellar/aws-sam-cli/1.56.1/libexec/lib/python3.8/site-packages/samcli/lib/telemetry/metric.py", line 183, in wrapped
    raise exception  # pylint: disable=raising-bad-type
  File "/usr/local/Cellar/aws-sam-cli/1.56.1/libexec/lib/python3.8/site-packages/samcli/lib/telemetry/metric.py", line 129, in wrapped
    return_value = func(*args, **kwargs)
  File "/usr/local/Cellar/aws-sam-cli/1.56.1/libexec/lib/python3.8/site-packages/samcli/lib/utils/version_checker.py", line 41, in wrapped
    actual_result = func(*args, **kwargs)
  File "/usr/local/Cellar/aws-sam-cli/1.56.1/libexec/lib/python3.8/site-packages/samcli/cli/main.py", line 87, in wrapper
    return func(*args, **kwargs)
  File "/usr/local/Cellar/aws-sam-cli/1.56.1/libexec/lib/python3.8/site-packages/samcli/commands/build/command.py", line 182, in cli
    do_cli(
  File "/usr/local/Cellar/aws-sam-cli/1.56.1/libexec/lib/python3.8/site-packages/samcli/commands/build/command.py", line 262, in do_cli
    ctx.run()
  File "/usr/local/Cellar/aws-sam-cli/1.56.1/libexec/lib/python3.8/site-packages/samcli/commands/build/build_context.py", line 252, in run
    build_result = builder.build()
  File "/usr/local/Cellar/aws-sam-cli/1.56.1/libexec/lib/python3.8/site-packages/samcli/lib/build/app_builder.py", line 221, in build
    return ApplicationBuildResult(build_graph, build_strategy.build())
  File "/usr/local/Cellar/aws-sam-cli/1.56.1/libexec/lib/python3.8/site-packages/samcli/lib/build/build_strategy.py", line 80, in build
    result.update(self._build_functions(self._build_graph))
  File "/usr/local/Cellar/aws-sam-cli/1.56.1/libexec/lib/python3.8/site-packages/samcli/lib/build/build_strategy.py", line 90, in _build_functions
    function_build_results.update(self.build_single_function_definition(build_definition))
  File "/usr/local/Cellar/aws-sam-cli/1.56.1/libexec/lib/python3.8/site-packages/samcli/lib/build/build_strategy.py", line 163, in build_single_function_definition
    result = self._build_function(
  File "/usr/local/Cellar/aws-sam-cli/1.56.1/libexec/lib/python3.8/site-packages/samcli/lib/build/app_builder.py", line 657, in _build_function
    return self._build_function_in_process(
  File "/usr/local/Cellar/aws-sam-cli/1.56.1/libexec/lib/python3.8/site-packages/samcli/lib/build/app_builder.py", line 746, in _build_function_in_process
    builder.build(
  File "/usr/local/Cellar/aws-sam-cli/1.56.1/libexec/lib/python3.8/site-packages/aws_lambda_builders/builder.py", line 164, in build
    return workflow.run()
  File "/usr/local/Cellar/aws-sam-cli/1.56.1/libexec/lib/python3.8/site-packages/aws_lambda_builders/workflow.py", line 65, in wrapper
    valid_path = binary_checker.validator.validate(executable_path)
  File "/usr/local/Cellar/aws-sam-cli/1.56.1/libexec/lib/python3.8/site-packages/aws_lambda_builders/workflows/go_modules/validator.py", line 55, in validate
    expected_major_version = int(self.runtime.replace(self.LANGUAGE, "").split(".")[0])
ValueError: invalid literal for int() with base 10: 'provided'

From a quick read, the runtime validator for the go modules builder seems to conflate the runtime enumeration value with the lookup path for the go compiler. This of course barfs if the runtime is anything other than go*.

I manually tested this fix by following CONTRIBUTING.md to setup samdev.

(samcli) ~/sam-app
$ samdev build && samdev deploy
Building codeuri: /Users/moffattb/sam-app runtime: provided.al2 metadata: {'BuildMethod': 'go1.x'} architecture: arm64 functions: Hello
Running GoModulesBuilder:Build

Build Succeeded
...
Initiating deployment
=====================
...
Successfully created/updated stack - sam-app-hello in us-west-2
...
(samcli) ~/sam-app
$ curl https://<redacted>.lambda-url.us-west-2.on.aws/
Hello World!%

Happy to follow any guidance for updating this project's testing for additional coverage, I didn't get a chance to dig deeper into what's already present.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added pr/external stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. area/workflow/go_modules labels Sep 13, 2022
@sriram-mv
Copy link
Contributor

Interesting, thanks for diving into this and giving a working example! I wonder if it might be better if we looked up extra options that were being passed in from build method to determine which workflow is chosen and set that runtime dynamically, instead of it being statically set within the workflow (though this works).

@bmoffatt
Copy link
Contributor Author

bmoffatt commented Oct 4, 2022

I wonder if it might be better if we looked up extra options that were being passed in from build method

I'm not SAM expert enough to know what you meant by this :) This referring to options that a user can define in the template, or stuff internal to the builders lib?

@sriram-mv
Copy link
Contributor

Something like referenced here: https://github.com/aws/aws-lambda-builders/blob/develop/aws_lambda_builders/workflows/go_modules/workflow.py#L29 (pass in something like build_runtime with options).

This is then something that can be passed in within the template under a BuildProperties section of the Metadata, and all the places where we validate with a runtime we can substitute with the value of the BuildProperties.

AWSTemplateFormatVersion: '2010-09-09'
Transform: AWS::Serverless-2016-10-31
Resources:
  Hello:
    Type: AWS::Serverless::Function 
    Metadata:
      BuildMethod: go1.x
      BuildProperties:
          BuildRuntime: go1.x
    Properties:
      CodeUri: .
      Handler: bootstrap
      Runtime: provided.al2
      Architectures:
      - arm64
      FunctionUrlConfig:
        AuthType: NONE
Outputs:
  HelloEndpoint:
    Description: "The Function URL!"
    Value:
      Fn::GetAtt: HelloUrl.FunctionUrl

This way it will be generic for all builders? I understand this looks to be duplicated, this is because of the selection of the builders associated within AWS SAM CLI is based on the runtime. But in the future we could decouple the BuildMethod being specific to a runtime.

I'm open to suggestions, just trying to see how we can make this generic across other runtimes, another example creating your pypy runtime but you want to use pip builder, I would want that case to also work.

@bmoffatt
Copy link
Contributor Author

You mean as a way to parameterize the builder with the (infeered) toolchain path, right?

So, using your python example:

Metadata:
  BuildMethod: pip
  BuildProperties:
    # Will cause builder to resolve to toolchain on PATH
    # ex: $HOME/.pyenv/versions/pypy3.9-7.3.8/bin/pypy
    BuildRuntime: pypy 
Properties:
  Runtime: provided.al2
  Layers:
  - arn:blahblah:pypy:9001

or the golang

Metadata:
  BuildMethod: go_modules
  BuildProperties:
    # Will cause builder to resolve to explicit toolchain on PATH
    # ref: https://go.dev/doc/manage-install to install
    # ex: toolchain at $HOME/go/bin/go1.19.2 
    BuildRuntime: go1.19.2 
Properties:
  Runtime: provided.al2

If that's what you mean, totally agree that's a nice looking feature to build. It wouldn't make sense to me though without doing the BuildMethod decoupling at the same time, otherwise one could write something goofy like this:

Metadata:
  BuildMethod: Python3.9 # HACK: SAM requires arbitrary python version here to trigger the pip builder
  BuildProperties:
    # use toolchain at $HOME/.pyenv/versions/pypy2.7-7.3.9/bin/pypy2.7
    BuildRuntime: pypy2.7
Properties:
  Runtime: provided.al2
  Layers:
  - arn:blahblah:pypy:9001

@bmoffatt
Copy link
Contributor Author

bmoffatt commented Oct 11, 2022

In re-reading, I saw that the validator isn't involved in constructing the toolchain path, which is what I thought when I posted this PR. It's used to validate the toolchain path already selected. 05b184f is equivalent to my initial commit, in that it doesen't barf when Runtime: provided.al2, but I think makes it clearer that the validator is simply checking that thego on the PATH is indeed go, and of version >= 1.11

Copy link
Contributor

@sriram-mv sriram-mv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving with the current changes.

@mndeveci mndeveci removed the stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. label Oct 21, 2022
@mndeveci
Copy link
Contributor

Thanks for your contribution @bmoffatt !

There are some failures with Windows CI jobs, do you know what could be the reason with them?

_______________________________________________________________________________________ TestGoWorkflow.test_fails_if_modules_cannot_resolve_dependencies _______________________________________________________________________________________
self = <tests.integration.workflows.go_modules.test_go.TestGoWorkflow testMethod=test_fails_if_modules_cannot_resolve_dependencies>
    def test_fails_if_modules_cannot_resolve_dependencies(self):
        source_dir = os.path.join(self.TEST_DATA_FOLDER, "broken-deps")
        with self.assertRaises(WorkflowFailedError) as ctx:
            self.builder.build(
                source_dir,
                self.artifacts_dir,
                self.scratch_dir,
                os.path.join(source_dir, "go.mod"),
                runtime=self.runtime,
                options={"artifact_executable_name": "failed"},
            )
>       self.assertIn("GoModulesBuilder:Build - Builder Failed: ", str(ctx.exception))
E       AssertionError: 'GoModulesBuilder:Build - Builder Failed: ' not found in 'GoModulesBuilder:Resolver - Path resolution for runtime: go1.x of binary: go was not successful'
tests/integration/workflows/go_modules/test_go.py:76: AssertionError

@bmoffatt
Copy link
Contributor Author

My first guess is that it looks like go disappeared from the PATH of AppVeyor's Ubuntu environment. I'm not reproducing the same errors when running the tests on my laptop.

@bmoffatt
Copy link
Contributor Author

yeah that looks to be what's happened

from my last good build: https://ci.appveyor.com/project/AWSSAMCLI/aws-lambda-builders/builds/44768289/job/580kni5mitk2o876

echo $PATH
/home/appveyor/venv3.6.15/bin:/usr/lib/jvm/java-11-openjdk-amd64/bin:/home/appveyor/.rvm/gems/ruby-2.7.2/bin:/home/appveyor/.rvm/gems/ruby-2.7.2@global/bin:/home/appveyor/.rvm/rubies/ruby-2.7.2/bin:/home/appveyor/.gvm/pkgsets/go1.19/global/bin:/home/appveyor/.gvm/gos/go1.19/bin:/home/appveyor/.gvm/pkgsets/go1.19/global/overlay/bin:/home/appveyor/.gvm/bin:/home/appveyor/.gvm/bin:/usr/lib/jvm/java-9-openjdk-amd64/bin:/home/appveyor/.nvm/versions/node/v14.17.6/bin:/opt/appveyor/build-agent:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin:/home/appveyor/.dotnet/tools:/home/appveyor/.rvm/bin:/opt/mssql-tools/bin:/home/appveyor/vcpkg

and my latest failing build: https://ci.appveyor.com/project/AWSSAMCLI/aws-lambda-builders/builds/45120452/job/5u9ch5ef0hv2qo5y

echo $PATH
/home/appveyor/venv3.6.15/bin:/usr/lib/jvm/java-11-openjdk-amd64/bin:/home/appveyor/.rvm/gems/ruby-2.7.2/bin:/home/appveyor/.rvm/gems/ruby-2.7.2@global/bin:/home/appveyor/.rvm/rubies/ruby-2.7.2/bin:/home/appveyor/.gvm/bin:/usr/lib/jvm/java-9-openjdk-amd64/bin:/home/appveyor/.nvm/versions/node/v14.17.6/bin:/opt/appveyor/build-agent:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin:/home/appveyor/.dotnet/tools:/home/appveyor/.rvm/bin:/opt/mssql-tools/bin:/home/appveyor/vcpkg

notice: /home/appveyor/.gvm/pkgsets/go1.19/global/bin:/home/appveyor/.gvm/gos/go1.19/bin:/home/appveyor/.gvm/pkgsets/go1.19/global/overlay/bin:/home/appveyor/.gvm/bin: went missing

@bmoffatt
Copy link
Contributor Author

@mndeveci and it passes now... seems I was unlucky and triggered CI during a bad ~week.

@mndeveci mndeveci merged commit d8c7a19 into aws:develop Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants