-
Notifications
You must be signed in to change notification settings - Fork 465
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
docs: update aws and gcp self-managed page with terraform instructions #30846
base: main
Are you sure you want to change the base?
docs: update aws and gcp self-managed page with terraform instructions #30846
Conversation
|
||
{{< /note >}} | ||
```bash | ||
cd terraform-aws-materialize |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wasn't sure if we wanted people to then go to the examples/simple
directory (or whatever this gets renamed to).
@@ -4,7 +4,16 @@ description: "" | |||
robots: "noindex, nofollow" | |||
--- | |||
|
|||
The following tutorial deploys Materialize onto AWS. | |||
Self-managed Materialize requires: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Install the AWS CLI. For details, see the [AWS | ||
documentation](https://docs.aws.amazon.com/cli/latest/userguide/install-cliv2.html). | ||
|
||
- Configure with your AWS credentials. For details, see the [AWS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wasn't sure if we should just give them the steps like aws configure
or export AWS_ACCESS_KEY_ID
... But can, if you all prefer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
soft preference for what you have here already! seems much simpler to maintain
db_allocated_storage = 20 | ||
db_multi_az = false | ||
|
||
enable_cluster_creator_admin_permissions = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we include:
enable_monitoring = false
metrics_retention_days = 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense. the monitoring doesn't come cheap, though in that case, I wonder if we want the defaults to be off
|
||
```bash | ||
terraform plan -out my-plan.tfplan | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you prefer, I can also add a tip making sure that people did config their aws cli. (it's part of the prereq step above .... but, eh).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems fine 👍
1. Create a terraform plan and review the changes. | ||
|
||
```bash | ||
terraform plan -out my-plan.tfplan |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wasn't sure if we want to create an output file or just do terraform plan
.
cidrs: ["0.0.0.0/0"] | ||
internal: | ||
enabled: true | ||
cidrs: ["0.0.0.0/0"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gah! cursor and its helfpul indentations. Will update. sigh.
|
||
node_group_ami_type = "AL2023_ARM_64_STANDARD" | ||
node_group_instance_types = ["r6g.medium"] | ||
node_group_desired_size = 3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can make these more inline with the values in examples/simple.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I do need to make these like those. Otherwise, Insufficient cpu, 3 Insufficient memory.
- Install `kubectl`. See the [Amazon EKS: install `kubectl` | ||
documentation](https://docs.aws.amazon.com/eks/latest/userguide/install-kubectl.html). | ||
|
||
- Configure `kubectl` to connect to your EKS cluster, replacing: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might pull the config out to a separate step to run after people create their environment.
c696409
to
f2b0def
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Really like how this is shaping up, thank you @kay-kim
module](https://github.com/MaterializeInc/terraform-aws-materialize/blob/main/README.md) | ||
to deploy a sample infrastructure on AWS with the following components: | ||
for evaluation purposes only. The module deploys a sample infrastructure on AWS | ||
(region `us-east-1`) with the following components: | ||
|
||
- EKS component | ||
- Networking component |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure what "networking component" translates to. I think we can drop "component" from each of these lines:
- A dedicated VPC
- A Kubernetes (EKS) cluster
- An S3 bucket
- An RDS Postgres cluster and database
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in patch
@@ -0,0 +1,5 @@ | |||
- A Kubernetes (v1.29+) environment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say cluster over environment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in patch
The sample Terraform module is for **evaluation purposes only** and not intended | ||
for production use. It is provided to help you get started with Materialize for | ||
evaluation purposes only. Materialize does not support nor recommends this | ||
module for production use. Materialize does not guarantee tests for changes to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do love the warning here, but the first three lines are all a little redundant... might be able to pare back
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in patch
for production use. It is provided to help you get started with Materialize for | ||
evaluation purposes only. Materialize does not support nor recommends this | ||
module for production use. Materialize does not guarantee tests for changes to | ||
the module. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure what "guarantee tests for changes" is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in patch
module for production use. Materialize does not guarantee tests for changes to | ||
the module. | ||
|
||
For simplicity, this tutorial stores various secrets in a file as well as print |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: the grammar here seems off
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in patch
@@ -14,101 +19,237 @@ For testing purposes only. For testing purposes only. For testing purposes only. | |||
|
|||
## Prerequisites | |||
|
|||
### Terraform |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't comment on the line direction, but I think the "testing purposes only" above can go, now that terraform-disclaimer
exists
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in patch
```bash | ||
kubectl get nodes | ||
``` | ||
- set your `database_password` to a secure password. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @bobbyiliev is this needed? could we auto-generate a password as part of the TF run?
```bash | ||
cd materialize | ||
``` | ||
include the storage configuration your configuration file. See the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: grammar here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in patch
|
||
1. {{< warning>}} | ||
|
||
{{< self-managed/terraform-disclaimer >}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hammering home the disclaimer is important, but imo we don't need it a second time here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Also, I guess this whole section may go away if we get Terraform to deploy materialize itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in patch
https://preview.materialize.com/materialize/30846/self-managed/installation/install-on-aws/
https://preview.materialize.com/materialize/30846/self-managed/installation/install-on-gcp/
Future updates: