-
Notifications
You must be signed in to change notification settings - Fork 11
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
Updates Terraform and K8s to support deploy to AWS QA env #1534
Conversation
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.
Reviewed 9 of 9 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @YuhongWang-Amazon)
docs/eks/duchy-deployment.md
line 348 at r2 (raw file):
Follow this [instruction](https://docs.aws.amazon.com/athena/latest/ug/connectors-postgresql.html) to create an athena_postgres_connector. This instruction will guide u to:
nit
Suggestion:
you
docs/eks/metrics-deployment.md
line 59 at r2 (raw file):
* the ADOT addon to the existing EKS cluster. (using console to install the version `v0.80.0-eksbuild.2`. There were some breaking changes in later versions.) * a service account that allows ADOT addon to publish metrics to Amazon Managed Prometheus
Should this be managed by Terraform?
If not, you'll want to indent the code fence so it is inside the bullet point
src/main/terraform/aws/cmms/main.tf
line 28 at r2 (raw file):
vpc_subnet_tags = {for name in local.duchy_names : "kubernetes.io/cluster/${name}-duchy" => "shared"} vpc_name = var.aws_project_env bucket_name = "${var.aws_project_env}-bucket"
I believe the bucket name comes from the AWS_S3_BUCKET
GitHub Environment variable rather than being derived from the account name.
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.
Reviewable status: 6 of 11 files reviewed, 2 unresolved discussions (waiting on @SanjayVas)
docs/eks/metrics-deployment.md
line 59 at r2 (raw file):
Previously, SanjayVas (Sanjay Vasandani) wrote…
Should this be managed by Terraform?
If not, you'll want to indent the code fence so it is inside the bullet point
Done.
src/main/terraform/aws/cmms/main.tf
line 28 at r2 (raw file):
Previously, SanjayVas (Sanjay Vasandani) wrote…
I believe the bucket name comes from the
AWS_S3_BUCKET
GitHub Environment variable rather than being derived from the account name.
Updated to use the Env Var.
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.
Reviewed 5 of 5 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @YuhongWang-Amazon)
No description provided.