[terraform] #874: terraform module for AWS #942

barroco · 2023-01-30T18:29:37Z

This PR introduces a terraform module (terraform-aws-dss) to deploy the DSS to AWS using Elastic Kubernetes Service.

Summary of changes:

A new dependency has been added to provision the infrastructure to AWS: dependencies/terraform-aws-kubernetes. See terraform-aws-kubernetes/README for design notes.
terraform-commons-dss: introduction of two variables (gateway_cert_name and workload_subnet) to support the new cloud provider and generate the correct tanka configuration.
Tanka manifests have been updated to support google and aws specific kubernetes resources. Note that the changes to the metadata variables should be backward compatible.
AWS unique identifier for cluster context uses urn which includes special characters. (ie : and /) Scripts in build/ have been updated to replace it by _.
google_kubernetes_storage_class variable has been removed from terraform-google-kubernetes since it is not required by the dependency.

The PR is quiet big, so please let me know if it is not manageable and I will break it down.

…se it is not required in the dependency.

…e AZ

…nka templates

…context

… EBS add-on

…ource and worker node group

…ual dns configuration

barroco · 2023-01-30T22:15:54Z

terraform-aws-dss (dns configured automatically and manually) and terraform-google-dss (dns configured automatically) successfully deployed to dedicated cloud accounts (no pooling). Resulting clusters were successfully probed individually.

BenjaminPelletier

Looks good, though I don't have the capability currently to personally verify. Comments are mostly minor.

...oy/infrastructure/dependencies/terraform-aws-kubernetes/AWSLoadBalancerControllerPolicy.json

deploy/infrastructure/dependencies/terraform-aws-kubernetes/README.md

deploy/infrastructure/modules/terraform-aws-dss/README.md

BenjaminPelletier · 2023-01-31T01:47:58Z

deploy/infrastructure/modules/terraform-google-dss/TFVARS.md

@@ -119,7 +120,7 @@ of this .pem file here as /public-certs/YOUR-KEY-NAME.pem replacing YOUR-KEY-NAM
 if using the provided us-demo.pem, use the path /public-certs/us-demo.pem. Note that your .pem file should built
 in the docker image or mounted manually.
 Example:
-```json
+```


nit: I'd recommend (substituting backticks where appropriate)

Example 1 (dummy auth): '''json { public_key_pem_path = "/test-certs/auth2.pem" } ''' Example 2: '''json { public_key_pem_path = "/jwt-public-certs/us-demo.pem" } '''

They have to be backticks though (not apostrophes); I didn't put backticks in my recommendation because then the preview wouldn't have rendered correctly :)

deploy/infrastructure/dependencies/terraform-commons-dss/default_latest.tf

deploy/infrastructure/modules/terraform-aws-dss/terraform.dev.example.tfvars

deploy/infrastructure/utils/variables.py

hrishiballal · 2023-01-31T16:48:39Z

deploy/infrastructure/dependencies/terraform-commons-dss/variables.tf

@@ -147,23 +150,3 @@ variable "kubernetes_namespace" {
  }
 }

-variable "app_hostname" {


This whole block with app_hostname and crdb_hostname_suffix needs to be re-added back, this breaks terraform plan command.

Thank you @hrishiballal for the feedback, may I kindly ask you to try again with the latest version of this branch. I just pushed a fix. The issue was introduced with 695e252. The problem should be solved. Tested planning with terraform-aws-dss and terraform-google-dss

BenjaminPelletier

Just one minor formatting issue remaining, but everything looks good to merge from my perspective.

marcadamsge · 2023-02-02T07:20:41Z

deploy/infrastructure/modules/terraform-aws-dss/README.md

@@ -0,0 +1,74 @@
+# terraform-aws-dss


I'm not sure if we already talked about this or not, but these READMEs seem a little hidden down in the tree of directories. This might be for another PR, but I think there should be a README in the top level deploy directory that:

quickly explains the infrastructure directory contains terraform code for setting up k8s in different clouds

points to:

deploy/infrastructure/modules/terraform-aws-dss/README.md

deploy/infrastructure/modules/terraform-google-dss/README.md

Sounds like the right moment to add the README indeed.

marcadamsge · 2023-02-02T07:26:14Z

deploy/infrastructure/modules/terraform-aws-dss/README.md

+1. Install [terraform](https://developer.hashicorp.com/terraform/downloads).
+2. Install tools from [Prerequisites](../../../../build/README.md)
+3. Install provider specific tools:
+    1. [Amazon Web Services](./README.md#amazon-web-services)


Isn't that a broken link? Where is it supposed to go?

It is actually correct. Markdown creates an anchor for each titles and it can be reference using the url's fragment. In this case, it points to the next section in the same document.
This prepares the ground for an aggregated README with all cloud providers.

marcadamsge · 2023-02-02T10:00:11Z

deploy/infrastructure/modules/terraform-aws-dss/TFVARS.md

+If providing the access token public key via JWKS, do not provide this parameter.
+If providing a .pem file directly as the public key to validate incoming access tokens, specify the name
+of this .pem file here as /public-certs/YOUR-KEY-NAME.pem replacing YOUR-KEY-NAME as appropriate. For instance,
+if using the provided us-demo.pem, use the path /public-certs/us-demo.pem. Note that your .pem file should built


I think this should be:

Suggested change

if using the provided us-demo.pem, use the path /public-certs/us-demo.pem. Note that your .pem file should built

if using the provided us-demo.pem, use the path /public-certs/us-demo.pem. Note that your .pem file should be built

marcadamsge · 2023-02-02T10:03:50Z

deploy/infrastructure/modules/terraform-aws-dss/terraform.dev.example.tfvars

@@ -0,0 +1,27 @@
+# This file is an example, please adapt it to your configuration.
+# See TFVARS.md for full set of variables and related descriptions.


I think this should be:

Suggested change

# See TFVARS.md for full set of variables and related descriptions.

# See TFVARS.md for the full set of variables and related descriptions.

hrishiballal · 2023-02-02T13:24:14Z

deploy/infrastructure/modules/terraform-aws-dss/README.md

+6. In the new directory (ie /deploy/infrastructure/personal/terraform-aws-dss-dev), initialize terraform: `terraform init`.
+7. Run `terraform plan` to check that the configuration is valid. It will display the resources which will be provisioned.
+8. Run `terraform apply` to deploy the cluster. (This operation may take up to 15 min.)
+9. If `aws_route53_zone_id` was not provided, configure the DNS resolution to the public ip addresses and for SSL certificate validation.  


I would recommend slightly rewording this: if aws_route53_zone_id is provided, please make sure that the domain or sub-domain in the Route 53 zone is mapped to Route53 via NS entries, e.g. see instuctitions for a sub-domain

Thanks, I modified the description to point directly to the Setup DNS documentation and added a reference to domain delegation instructions (see aba1ddb)

hrishiballal · 2023-02-02T13:51:53Z

deploy/infrastructure/modules/terraform-aws-dss/README.md

+2. Run `./get-credentials.sh` to login to kubernetes. You can now access the cluster with `kubectl`.
+3. Generate the certificates using `./make-certs.sh`. Follow script instructions if you are not initializing the cluster.
+4. Deploy the certificates using `./apply-certs.sh`.
+5. Run `tk apply .` to deploy the services to kubernetes. (This may take up to 30 min)


I am getting a Kubernetes version error here. Maybe AWS expects a particular version:

The output of kubectl version --client is below:
Client Version: version.Info{Major:"1", Minor:"26", GitVersion:"v1.26.1", GitCommit:"8f94681cd294aa8cfd3407b8191f6c70214973a4", GitTreeState:"clean", BuildDate:"2023-01-18T15:58:16Z", GoVersion:"go1.19.5", Compiler:"gc", Platform:"linux/amd64"} Kustomize Version: v4.5.7

When I install kubectl version as 1.24.10-00 in Ubuntu it works no problems.

hrishiballal · 2023-02-03T13:18:11Z

I can confirm that DSS installation works and having run the prober tests, they pass / work as well.

hrishiballal · 2023-02-06T10:59:13Z

deploy/infrastructure/modules/terraform-aws-dss/README.md

+## Clean up
+
+1. Note that the following operations can't be reverted and all data will be lost.
+2. To delete all resources, run `tk destroy .` in the workspace folder.


There is no command called tk destroy . , did you mean tk delete . ?

For some reason, tanka is generating an error when I do tk delete .

The deletion process works its just that tanka gives a error.

Thanks, fixed in the documentation

marcadamsge

I deployed the DSS on AWS following these steps as is, it worked without issue, good job! 👍

barroco added 23 commits January 30, 2023 15:01

[terraform] Limit google_kubernetes_storage_class to the module becau…

989a7f7

…se it is not required in the dependency.

[terraform] Initial AWS terraform dependency

da1bb1d

[terraform] terraform-aws-kubernetes variables

029d6cb

[terraform] add aws_route53_zone_id variable

2dcc233

[terraform] terraform-aws-kubernetes: Load Balancer

db72ce3

[terraform] terraform-aws-kubernetes: Documentation WIP

1c27d3f

[terraform] terraform-aws-kubernetes: Limit the deployment to a singl…

260fdfb

…e AZ

[tanka] Update tanka manifests to support multiple cloud providers

6e02411

[terraform] terraform-commons-dss: Add new cloud provider field to ta…

c33c897

…nka templates

[terraform-aws-dss] Update scripts to support arn format for cluster …

d472b6c

…context

[tanka] Add AWS resources

0308656

[terraform-aws-kubernetes] Elastic Block storage configuration

eaca312

[terraform-aws-dss] Add variables subnet and cert_name

19b3a8d

[terraform] fix authorization description.

865da83

[terraform-aws-dss] Add new module

d0f7f48

[terraform-aws-dss] Propagate subnet to tanka and use tf to provision…

12155e2

… EBS add-on

Delete inappropriate file

a658bb3

[terraform-aws-dss] Fix gcp missing service and certname variable

b307c62

[terraform-aws-dss] Finalize DNS setup for CRDB and documentation

86c9c53

Format

1d591c7

[terraform-aws-kubernetes] Rename AWSLoadBalancerControllerPolicy res…

a98017f

…ource and worker node group

Clean up

715feda

[terraform-aws-kubernetes] Handle manual dns setup

5b70f79

barroco requested a review from BenjaminPelletier January 30, 2023 20:49

barroco added 2 commits January 30, 2023 22:00

[terraform-aws-kubernetes] Remove unused data

1bcab61

[terraform-aws-kubernetes] Disable useless verification stage for man…

6d44621

…ual dns configuration

barroco marked this pull request as ready for review January 30, 2023 22:16

BenjaminPelletier reviewed Jan 31, 2023

View reviewed changes

barroco assigned marcadamsge Jan 31, 2023

barroco unassigned marcadamsge Jan 31, 2023

barroco requested a review from marcadamsge January 31, 2023 08:27

barroco added 3 commits January 31, 2023 10:23

Address PR comments

695e252

Address PR comments

cfa8777

Indent

2123736

barroco requested review from BenjaminPelletier and hrishiballal January 31, 2023 09:40

hrishiballal suggested changes Jan 31, 2023

View reviewed changes

[terraform] Fix variables generator to bring back global variables

e5509a9

barroco requested a review from hrishiballal January 31, 2023 17:39

BenjaminPelletier approved these changes Jan 31, 2023

View reviewed changes

Address PR comment

e884a79

marcadamsge reviewed Feb 2, 2023

View reviewed changes

hrishiballal suggested changes Feb 2, 2023

View reviewed changes

hrishiballal reviewed Feb 6, 2023

View reviewed changes

barroco added 7 commits February 8, 2023 14:20

Add README to /deploy

1588bc5

Update authorization description

20a3b28

Improve DNS documentation

aba1ddb

Fix tk delete

2802f93

Fix static name to prevent conflicts with multiple deployments

c1a0eee

Add as default tags the cluster name

6c0bbdf

small refactoring

c1f6fe4

marcadamsge reviewed Feb 16, 2023

View reviewed changes

marcadamsge approved these changes Feb 16, 2023

View reviewed changes

barroco merged commit a039aec into interuss:master Feb 16, 2023

barroco deleted the 874-terraform-aws branch February 16, 2023 09:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[terraform] #874: terraform module for AWS #942

[terraform] #874: terraform module for AWS #942

barroco commented Jan 30, 2023 •

edited

Loading

barroco commented Jan 30, 2023 •

edited

Loading

BenjaminPelletier left a comment

BenjaminPelletier Jan 31, 2023

barroco Jan 31, 2023

BenjaminPelletier Jan 31, 2023

barroco Jan 31, 2023

hrishiballal Jan 31, 2023

barroco Jan 31, 2023 •

edited

Loading

BenjaminPelletier left a comment

marcadamsge Feb 2, 2023

barroco Feb 8, 2023

marcadamsge Feb 2, 2023

barroco Feb 8, 2023

marcadamsge Feb 2, 2023

barroco Feb 8, 2023

marcadamsge Feb 2, 2023

barroco Feb 8, 2023

hrishiballal Feb 2, 2023

barroco Feb 8, 2023

hrishiballal Feb 2, 2023 •

edited

Loading

hrishiballal commented Feb 3, 2023

hrishiballal Feb 6, 2023

hrishiballal Feb 6, 2023

hrishiballal Feb 6, 2023

barroco Feb 8, 2023

marcadamsge left a comment

	if using the provided us-demo.pem, use the path /public-certs/us-demo.pem. Note that your .pem file should built
	if using the provided us-demo.pem, use the path /public-certs/us-demo.pem. Note that your .pem file should be built

		@@ -0,0 +1,27 @@
		# This file is an example, please adapt it to your configuration.
		# See TFVARS.md for full set of variables and related descriptions.

	# See TFVARS.md for full set of variables and related descriptions.
	# See TFVARS.md for the full set of variables and related descriptions.

[terraform] #874: terraform module for AWS #942

[terraform] #874: terraform module for AWS #942

Conversation

barroco commented Jan 30, 2023 • edited Loading

barroco commented Jan 30, 2023 • edited Loading

BenjaminPelletier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

barroco Jan 31, 2023 • edited Loading

Choose a reason for hiding this comment

BenjaminPelletier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hrishiballal Feb 2, 2023 • edited Loading

Choose a reason for hiding this comment

hrishiballal commented Feb 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcadamsge left a comment

Choose a reason for hiding this comment

barroco commented Jan 30, 2023 •

edited

Loading

barroco commented Jan 30, 2023 •

edited

Loading

barroco Jan 31, 2023 •

edited

Loading

hrishiballal Feb 2, 2023 •

edited

Loading