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

[RE-270][RE-271] Add Support for GovCloud #175

Merged
merged 4 commits into from Jan 13, 2020
Merged

[RE-270][RE-271] Add Support for GovCloud #175

merged 4 commits into from Jan 13, 2020

Conversation

@MikeyYeahYeah
Copy link
Contributor

MikeyYeahYeah commented Jan 10, 2020

  • Add GovCloud Support
    • Create a new flag to toggle GovCloud support. The flag will adjust the ARN Partitions (aws/aws-gov-cloud) within the IAM policy.
  • Update VM Service AWS Instance Types
    • GovCloud does not support t2 or m4 insatnce types. Updated the default values to t3 and m5 types.
Michael Marquez added 2 commits Jan 10, 2020
Create a new flag to toggle GovCloud support.  The flag will adjust the ARN Partitions (aws/aws-gov-cloud) within the IAM policy.
GovCloud does not support t2 or m4 insatnce types.  Updated the default values to t3 and m5 types.
Copy link
Contributor

MadVikingGod left a comment

Minor: there are still references to r3 and m4 in the terraform.tfvars.template.

Other then that LGTM

| services_instance_type | Instance type for the centralized services box. We recommend a m4 instance | m4.2xlarge |
| builder_instance_type | Instance type for the 1.0 builder machines. We recommend a r3 instance | r3.2xlarge |
| services_instance_type | Instance type for the centralized services box. We recommend a m4 instance | m5.2xlarge |
| builder_instance_type | Instance type for the 1.0 builder machines. We recommend a r3 instance | r5.2xlarge |

This comment has been minimized.

Copy link
@MadVikingGod

MadVikingGod Jan 10, 2020

Contributor

Minor: the recommended doesn't match what we are defaulting to.

This comment has been minimized.

Copy link
@MikeyYeahYeah

MikeyYeahYeah Jan 13, 2020

Author Contributor

Pushed some additional commits to address this. Let me know if the changes resolve the issue please.

Michael Marquez added 2 commits Jan 13, 2020
Michael Marquez
Pick slightly less expensive instance sizes
@MikeyYeahYeah MikeyYeahYeah merged commit 4c27bc4 into master Jan 13, 2020
2 checks passed
2 checks passed
ci/circleci: terraform Your tests passed on CircleCI!
Details
ci/circleci: validate_amis Your tests passed on CircleCI!
Details
@MikeyYeahYeah MikeyYeahYeah deleted the add_govcloud branch Jan 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.