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

Serverless aws deploy #1338

Closed
wants to merge 13 commits into from
Closed

Conversation

zalegrala
Copy link
Contributor

@zalegrala zalegrala commented Mar 9, 2022

What this PR does:

Here we a to build the serverless components for AWS lambda with the main tempo build.

  • remove serverless lambda go module
  • build lambda as part of the main build
  • include lambda dependency

Checklist

  • Test Docker image deloyment
  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

.drone/drone.yml Outdated Show resolved Hide resolved
.drone/drone.yml Outdated Show resolved Hide resolved
@zalegrala zalegrala force-pushed the serverlessAwsDeploy branch 5 times, most recently from 76989e4 to e5820f2 Compare March 17, 2022 21:33
go.mod Outdated Show resolved Hide resolved
@@ -25,18 +25,6 @@ build-gcf-zip:
$(IN_CLOUD_FUNCTIONS) zip tempo-serverless-$(VERSION).zip ./* -r
$(IN_CLOUD_FUNCTIONS) rm -rf vendor

#
Copy link
Member

Choose a reason for hiding this comment

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

we need some way to build the docker image from the makefile. also we should clean up build-lambda-zip if we don't intend to use it anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The docker build for lambda has been moved to the root Makefile, but I was leaving build-lambda-zip in place while the #1347 is in flight.

Makefile Outdated
@@ -55,6 +55,10 @@ tempo-cli:
tempo-vulture:
GO111MODULE=on CGO_ENABLED=0 go build $(GO_OPT) -o ./bin/$(GOOS)/tempo-vulture-$(GOARCH) $(BUILD_INFO) ./cmd/tempo-vulture

.PHONY: tempo-serverless/lambda
Copy link
Member

Choose a reason for hiding this comment

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

we should have a way to build all of the serverless stuff in the cmd/tempo-serverless makefile. having some commands there and some here is messy.

also there is already a ### serverless section these could go in. i'm not married to it, but we need to organize ourselves somehow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree we'll want to organize, and I'd favor this file over the nested one if we can. I'm mostly paying attention to the AWS bits currently because it seems more easily workable into our existing CI.

@@ -0,0 +1,44 @@
#
Copy link
Member

Choose a reason for hiding this comment

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

if this is intended to be a proxy image only we could clean it up quite a bit. there's no need to put the lambda binary in here.

also, i'm not seeing changes to the e2e tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll be looking at cleaning up the proxy file, and maybe relocating it. I'm thinking we want to produce a proxy docker image for the e2e environment.

FROM alpine:3.9 as certs
Copy link
Member

Choose a reason for hiding this comment

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

does this work? doesn't a custom docker image require specific base images? or some kind of lambda runtime?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this will come up in the testing. I believe you might be right though if we want to test outside of AWS, we need to have the proper environment variables in place.

@joe-elliott
Copy link
Member

Also note that you'll need to update the docs merged here:
#1347

@zalegrala
Copy link
Contributor Author

I prefer #1357 for the e2e parts, and for deploy, I'll follow up with the s3 approach. Building out the docker image as I attempted here doesn't gain us anything over the s3 approach in that it won't be able to provide an image for the community to use, since users will need to copy this to their own ecr repo, which has slightly more complexity than the bucket + access approach we can take with s3.

@zalegrala zalegrala closed this Apr 19, 2022
@zalegrala zalegrala deleted the serverlessAwsDeploy branch April 19, 2022 14:06
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