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

Update AWS Lambda SDK to 1.28.0 #120

Merged
merged 5 commits into from
Mar 30, 2022

Conversation

ohookins
Copy link
Contributor

I've been scratching my head the last few days about this. The daemon itself works standalone, and will even do so packaged as a Docker image and used with lambda, but will run immediately and exit - not suitable for lambda usage. If you build the lambda subdirectory directly and produce a binary (presumably what is provided as handler.zip in release) it seems to work when packaged as a zip file but not when packaged as a Docker image.

I couldn't seem to determine what was causing problems but it's clearly:

  • the current codebase
  • when packaged as a Docker image and used as an AWS lambda function
  • the binary itself and the code (e.g. main()) is executed, but the Handler function is never called by the AWS lambda runtime.

After trying a few different things, it seems like updating to a later version of the AWS Lambda SDK solves the issue. I suspect some subtle interoperation between the runtime and the specific version of this SDK is not working - after all, it's almost 4 years old at this point.

@ohookins
Copy link
Contributor Author

For reference, this is how I'm building the Docker image:

FROM public.ecr.aws/lambda/provided:al2 as build

# See https://github.com/buildkite/buildkite-agent-metrics/releases for latest version
ENV LAMBDA_VERSION="5.3.0"

WORKDIR /app
COPY . /app

RUN yum install -y wget tar golang && \
    wget "https://github.com/buildkite/buildkite-agent-metrics/archive/refs/tags/v${LAMBDA_VERSION}.tar.gz" && \
    tar xvzf "v${LAMBDA_VERSION}.tar.gz" && \
    cd "buildkite-agent-metrics-${LAMBDA_VERSION}" && \
    go get ./lambda && \
    CGO_ENABLED=0 go build -o ../handler ./lambda

FROM public.ecr.aws/lambda/provided:al2

COPY --from=build /app/handler ${LAMBDA_TASK_ROOT}

ENTRYPOINT ["./handler"]

@moskyb
Copy link
Contributor

moskyb commented Mar 28, 2022

hi there @ohookins! thanks for contributing.

this looks awesome! great work on the bug hunt - i suspect that the previous version of the lambda-go package we were using predates container lambdas, which i imagine is why it doesn't play nicely with them 😅

i'm happy to merge this as-is, but it seems like the dockerfile you've posted above might be useful to people as well - would you be interested in adding it as lambda.Dockerfile (or something) as part of this PR?

@ohookins
Copy link
Contributor Author

ohookins commented Mar 28, 2022

Yeah, I can add that no problem. I tend to use things like Dockerfile.test so perhaps Dockerfile.lambda ?

Actually there is a small problem with it I realise, AWS's provided:al2 ships with Golang 1.15 so in my local environment I've been using golang:1.17 as the build image, and the go.mod specifies 1.16 minimum version. I'll just retest the instructions and then add it to the branch if that still sounds ok.

@ohookins
Copy link
Contributor Author

@moskyb anything else you need from me on this? How does that Dockerfile look? What's the release process from here?

Copy link
Contributor

@moskyb moskyb left a comment

Choose a reason for hiding this comment

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

i have a couple of comments on the dockerfile, but i'm also happy to merge this as is and make the changes myself - i'm aware of how annoying it might be for someone from some other company to say "yeah include that other thing, that looks great!" and then pick it to pieces at the next turn 😅

Dockerfile.lambda Outdated Show resolved Hide resolved
Dockerfile.lambda Outdated Show resolved Hide resolved
Dockerfile.lambda Outdated Show resolved Hide resolved
@moskyb
Copy link
Contributor

moskyb commented Mar 30, 2022

re: the release process, once i have your okay, i'll merge it and make a release against this repo, which will publish the new versions of the handlers, binaries etc.

@ohookins
Copy link
Contributor Author

You are free to merge it at any time! Up to you whether the LAMBDA_VERSION is a blocker or a nit.

@ohookins
Copy link
Contributor Author

OK should be good to go now.

@moskyb
Copy link
Contributor

moskyb commented Mar 30, 2022

Awesome! Thanks so much for your contribution @ohookins, i'm really happy with where we got to :)

@moskyb moskyb merged commit 9479e3e into buildkite:master Mar 30, 2022
@ohookins ohookins deleted the update-lambda-sdk-dependency branch March 30, 2022 03:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants