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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deploy VAST to AWS Lambda #2108

Merged
merged 26 commits into from Mar 3, 2022
Merged

Deploy VAST to AWS Lambda #2108

merged 26 commits into from Mar 3, 2022

Conversation

rdettai
Copy link
Contributor

@rdettai rdettai commented Feb 23, 2022

This changes adds the possibility to deploy VAST on AWS Lambda and Fargate. This is a more interesting feature set as it will enable connecting to the VAST server on Fargate through client VAST instances on Lambda

馃摑 Checklist

  • All user-facing changes have changelog entries.
  • The changes are reflected on docs.tenzir.com/vast, if necessary.
  • The PR description contains instructions for the reviewer, if necessary.

馃幆 Review Instructions

This PR can be broken down into:

  • the terraform scripts to deploy fargate task definition and the lambda function. It will create a subnet in a provided vpc where all this tooling will execute
  • a wrapper image around VAST for the lambda function, containing the VAST CLI and the AWS Lambda runtime interface (Dockerfile + python handler)
  • a makefile to simplify the interaction with the deployment script and test the deployment

@rdettai rdettai mentioned this pull request Feb 23, 2022
3 tasks
@mavam mavam added this to the VAST on AWS milestone Feb 28, 2022
@dominiklohmann dominiklohmann linked an issue Mar 1, 2022 that may be closed by this pull request
2 tasks
Copy link
Contributor

@dispanser dispanser left a comment

Choose a reason for hiding this comment

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

Great work, thank you so much.

I've tested it following the readme, and was able to deploy vast, import data, query data, ... so everything works as expected.

Two points:

  1. I'm not sure the approach using environment variables / default.env is optimal. In my previous usages of terraform, I always relied on tfvars files to inject behavior (and some defaults for the variables). I can see how the approach taken here makes working with the Makefile easier, but it stops us from using terraform directly.
  2. Now that you added the ability to directly execute commands on the ecs task using make execute-command, the existence of lambda functions is not strictly necessary any more. It's a nice showcase for how to integrate, and lambda is practically free, so it's not really a problem.

My suggestion would be to move forward as is, and see how this actually gets used and adapt it from these learnings.

cloud/aws/README.md Outdated Show resolved Hide resolved
cloud/aws/README.md Outdated Show resolved Hide resolved
cloud/aws/README.md Outdated Show resolved Hide resolved
@rdettai
Copy link
Contributor Author

rdettai commented Mar 2, 2022

@dispanser

I'm not sure the approach using environment variables / default.env is optimal. In my previous usages of terraform, I always relied on tfvars files to inject behavior (and some defaults for the variables). I can see how the approach taken here makes working with the Makefile easier, but it stops us from using terraform directly.

I don't love this make import either because it is kind of a hack, but it works well in practice. You can still use tfvars to provide the variables if you want to interact with terraform directly: terraform apply -var-file="yourfile.tfvars"

Now that you added the ability to directly execute commands on the ecs task using make execute-command, the existence of lambda functions is not strictly necessary any more. It's a nice showcase for how to integrate, and lambda is practically free, so it's not really a problem.

Yeah, I think I over-engineered this a bit 馃槄. But I believe it can be very useful if you want to add extra tooling around vast import while keeping the server image to its bare minimum. Ideally, the vast server should be able to run on a much slimmer image (maybe scratch?), and in that case, going through lambda would be a very elegant way to keep the linux flavor you need/like to use the client CLI.

@dominiklohmann dominiklohmann added the feature New functionality label Mar 3, 2022
Copy link
Member

@dominiklohmann dominiklohmann left a comment

Choose a reason for hiding this comment

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

Thanks for incorporating the CI changes as well.

@dominiklohmann
Copy link
Member

@rdettai we require all commits to have a valid signature, which is why your PR cannot be merged currently. Do you have the setup for signing commits locally so you can rebase, or should I rebase this PR leaving you as the author of the commits and me as the (signing) committer?

@rdettai
Copy link
Contributor Author

rdettai commented Mar 3, 2022

I'll setup the signature and rebase

@rdettai
Copy link
Contributor Author

rdettai commented Mar 3, 2022

@dominiklohmann done! you can re-activate auto-merge ;-)

@dominiklohmann dominiklohmann merged commit 06512dc into tenzir:master Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality
Projects
None yet
4 participants