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

Adding Terraform deployment option #54

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

marcusit
Copy link

I ran through the step by step instructions and created Terraform code to perform a full deployment, including a separate VPC and subnets.

==================
Running instructions:

You will need to have AWS authentication configured.
The easiest way is to add your access key and secret into file ~/.aws/credentials
https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-files.html

Requires a recent Terraform version, tested on 1.2.8.
CLI commands:

cd terraform
terraform init
terraform apply # <- you will be prompted to enter region, server name and DNS domain.

@doctorray117
Copy link
Owner

Hi sorry for the delay, I've been crazy on work. I will review this all over the weekend and let you know if I have any questions!

@joshuacherry
Copy link

This is great, I would suggesting adding a few additional variables so this is in parity with what's supported in the cdk. (see .env.sample )

For example, users may want to change the mineraft edition, task memory & cpu, and container environmental variables.

@md5
Copy link

md5 commented Feb 20, 2023

It looks like the handling of DNS in the PR differs from what the CDK version is doing.

The CDK version assumes that there is an existing Route 53 hosted zone and it adds the server name as a subdomain of that zone with an apex A record, along with NS glue records in the existing parent zone. The Terraform version is just creating a hosted zone directly along with a non-apex A record. Unless I'm just misunderstanding what's being done here, someone would have to manually add the NS glue records either to an existing (parent) zone of their own or with a registrar in order for this Terraform version to work.

In terms of maintainability, it also feels like the Terraform configuration should not have a different copy of the lambda code used to update desiredCount, but should instead leverage the code that the CDK version uses so that any feature changes in that lambda would be reflected regardless of the chosen deployment method.

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.

4 participants