From 1b5b03a7ad3d58cb288e32fd253c3cb0f3c77e79 Mon Sep 17 00:00:00 2001
From: Magicloud <1886157+Magicloud@users.noreply.github.com>
Date: Fri, 3 Apr 2020 08:20:02 +0800
Subject: [PATCH 1/8] Support multiple data volumes in single-node-asg module.

Tested in examples/single-node-asg-tester

The change in all three modules is compatible with old code that defines just one data volume.
---
 examples/single-node-asg-tester/main.tf       | 51 ++++++++++++++
 .../instance_id.tpl                           | 10 +++
 .../init-snippet-attach-ebs-volume/main.tf    | 66 ++++---------------
 .../snippet.tpl                               | 37 ++++-------
 .../variables.tf                              | 46 +++++++++++++
 modules/persistent-ebs/data.tf                |  6 +-
 modules/persistent-ebs/locals.tf              | 11 ++++
 modules/persistent-ebs/main.tf                | 16 +++--
 modules/persistent-ebs/outputs.tf             |  4 +-
 modules/persistent-ebs/variables.tf           | 50 ++++----------
 modules/single-node-asg/main.tf               | 38 +++++++----
 modules/single-node-asg/outputs.tf            |  4 +-
 modules/single-node-asg/variables.tf          | 45 ++-----------
 13 files changed, 203 insertions(+), 181 deletions(-)
 create mode 100644 examples/single-node-asg-tester/main.tf
 create mode 100644 modules/init-snippet-attach-ebs-volume/instance_id.tpl
 create mode 100644 modules/init-snippet-attach-ebs-volume/variables.tf
 create mode 100644 modules/persistent-ebs/locals.tf

diff --git a/examples/single-node-asg-tester/main.tf b/examples/single-node-asg-tester/main.tf
new file mode 100644
index 00000000..3ecfa62f
--- /dev/null
+++ b/examples/single-node-asg-tester/main.tf
@@ -0,0 +1,51 @@
+provider "aws" {
+  region = "ap-northeast-1"
+}
+
+locals {
+  cidr                 = "192.168.0.0/16"
+  private_subnet_cidrs = ["192.168.100.0/24", "192.168.101.0/24"]
+  public_subnet_cidrs  = ["192.168.0.0/24", "192.168.1.0/24"]
+}
+
+data "aws_vpc" "vpc" {
+  cidr_block = local.cidr
+}
+
+data "aws_subnet" "public" {
+  count = length(local.public_subnet_cidrs)
+  cidr_block = local.public_subnet_cidrs[count.index]
+}
+
+module "ubuntu" {
+  source = "fpco/foundation/aws//modules/ami-ubuntu"
+}
+
+resource "aws_security_group" "ssh" {
+  vpc_id = data.aws_vpc.vpc.id
+  ingress {
+    from_port   = 22
+    to_port     = 22
+    protocol    = "tcp"
+    cidr_blocks = ["0.0.0.0/0"]
+  }
+  egress {
+    from_port   = 0
+    to_port     = 0
+    protocol    = "-1"
+    cidr_blocks = ["0.0.0.0/0"]
+  }
+}
+
+module "tester" {
+  source             = "../../modules/single-node-asg"
+  name_prefix        = "ebs"
+  name_suffix        = "test"
+  key_name           = "tokyo"
+  ami                = module.ubuntu.id
+  instance_type      = "t2.micro"
+  subnet_id          = data.aws_subnet.public[0].id
+  security_group_ids = [aws_security_group.ssh.id]
+  region             = "ap-northeast-1"
+  data_volumes       = [{ name = "a", device = "/dev/xvdm", size = 50 }, { name = "b", device = "/dev/xvdn" }]
+}
diff --git a/modules/init-snippet-attach-ebs-volume/instance_id.tpl b/modules/init-snippet-attach-ebs-volume/instance_id.tpl
new file mode 100644
index 00000000..0e3e8663
--- /dev/null
+++ b/modules/init-snippet-attach-ebs-volume/instance_id.tpl
@@ -0,0 +1,10 @@
+if which wget; then
+  INSTANCE_ID="$(wget -O- http://169.254.169.254/latest/meta-data/instance-id)"
+elif which curl; then
+  INSTANCE_ID="$(curl http://169.254.169.254/latest/meta-data/instance-id)"
+fi
+
+if [ "x$${INSTANCE_ID}" == "x" ]; then
+  echo 'There is no wget or curl tool installed. Hence bootstrap cannot get instance ID.'
+  exit 1
+fi
diff --git a/modules/init-snippet-attach-ebs-volume/main.tf b/modules/init-snippet-attach-ebs-volume/main.tf
index 71d16726..c381691d 100644
--- a/modules/init-snippet-attach-ebs-volume/main.tf
+++ b/modules/init-snippet-attach-ebs-volume/main.tf
@@ -9,70 +9,32 @@
  *
  */
 
-# variables used by this snippet of init shellcode
-variable "device_path" {
-  default     = "/dev/xvdf"
-  description = "path, to the device's path in /dev/"
-  type = string
-}
-
-variable "init_prefix" {
-  default     = ""
-  description = "initial init (shellcode) to prefix this snippet with"
-  type = string
-}
-
-variable "init_suffix" {
-  default     = ""
-  description = "init (shellcode) to append to the end of this snippet"
-  type = string
-}
-
-variable "log_level" {
-  default     = "info"
-  description = "default log level verbosity for apps that support it"
-  type = string
-}
-
-variable "log_prefix" {
-  default     = "OPS: "
-  description = "string to prefix log messages with"
-  type = string
-}
-
-variable "region" {
-  description = "AWS region the volume is in"
-  type = string
-}
-
-variable "wait_interval" {
-  default     = "5"
-  description = "time (in seconds) to wait when looping to find the device"
-  type = number
-}
-
-variable "volume_id" {
-  description = "ID of the EBS volume to attach"
-  type = string
-}
-
 # render init script for a cluster using our generic template
 data "template_file" "init_snippet" {
+  count = length(var.volume_ids)
+
   template = file("${path.module}/snippet.tpl")
 
   vars = {
-    device_path   = var.device_path
-    init_prefix   = var.init_prefix
-    init_suffix   = var.init_suffix
+    device_path   = var.device_paths[count.index]
     log_prefix    = var.log_prefix
     log_level     = var.log_level
     region        = var.region
-    volume_id     = var.volume_id
+    volume_id     = var.volume_ids[count.index]
     wait_interval = var.wait_interval
   }
 }
 
+data "template_file" "instance_id" {
+  template = file("${path.module}/instance_id.tpl")
+}
+
 output "init_snippet" {
-  value = data.template_file.init_snippet.rendered
+  value = <<EOF
+${var.init_prefix}
+${data.template_file.instance_id.rendered}
+${join("\n", data.template_file.init_snippet.*.rendered)}
+${var.init_suffix}
+EOF
 }
 
diff --git a/modules/init-snippet-attach-ebs-volume/snippet.tpl b/modules/init-snippet-attach-ebs-volume/snippet.tpl
index 0bf91f1f..8f390f86 100644
--- a/modules/init-snippet-attach-ebs-volume/snippet.tpl
+++ b/modules/init-snippet-attach-ebs-volume/snippet.tpl
@@ -1,34 +1,19 @@
-# start snippet - attach EBS volume
-${init_prefix}
 export AWS_DEFAULT_REGION=${region}
 VOLUME_ID=${volume_id}
-if which wget; then
-  INSTANCE_ID="$(wget -O- http://169.254.169.254/latest/meta-data/instance-id)"
-elif which curl; then
-  INSTANCE_ID="$(curl http://169.254.169.254/latest/meta-data/instance-id)"
-fi
+echo "${log_prefix} will attach $${VOLUME_ID} via the AWS API in ${region}"
+while ! aws ec2 attach-volume                     \
+        --volume-id "$${VOLUME_ID}"     \
+        --instance-id "$${INSTANCE_ID}" \
+        --device '${device_path}'; do
+  echo "Attaching command failed to run. Retrying."
+  sleep '${wait_interval}'
+done
+echo "${log_prefix} $${VOLUME_ID} attached."
+sleep '${wait_interval}' # Wait for device up
 
-if [ "x$${INSTANCE_ID}" == "x" ]; then
-  echo 'OS not functioning'
-else
-  echo "${log_prefix} will attach $${VOLUME_ID} via the AWS API in ${region}"
-  while ! aws ec2 attach-volume                     \
-          --volume-id "$${VOLUME_ID}"     \
-          --instance-id "$${INSTANCE_ID}" \
-          --device '${device_path}'; do
-    echo "Attaching command failed to run. Retrying."
-    sleep '${wait_interval}'
-  done
-  echo "${log_prefix} $${VOLUME_ID} attached."
-  
+if [ ! -e ${device_path} ]; then
   vol_id="$(echo "$${VOLUME_ID}" | tr -d '-')"
-  while [ ! -e /dev/disk/by-id/*-Amazon_Elastic_Block_Store_$${vol_id} ]; do
-    sleep '${wait_interval}' 
-  done
-  
   dev_id="$(ls /dev/disk/by-id/*-Amazon_Elastic_Block_Store_$${vol_id} | head -1)"
   dev_name="/dev/$(readlink "$${dev_id}" | tr / '\n' | tail -1)"
   [ "$${dev_name}" == "${device_path}" ] || ln -s "$${dev_name}" "${device_path}"
 fi
-
-${init_suffix}
diff --git a/modules/init-snippet-attach-ebs-volume/variables.tf b/modules/init-snippet-attach-ebs-volume/variables.tf
new file mode 100644
index 00000000..f65c67ec
--- /dev/null
+++ b/modules/init-snippet-attach-ebs-volume/variables.tf
@@ -0,0 +1,46 @@
+variable "device_paths" {
+  type        = list(string)
+  description = "paths, to the device's path in /dev/"
+  default     = []
+}
+
+variable "init_prefix" {
+  default     = ""
+  description = "initial init (shellcode) to prefix this snippet with"
+  type        = string
+}
+
+variable "init_suffix" {
+  default     = ""
+  description = "init (shellcode) to append to the end of this snippet"
+  type        = string
+}
+
+variable "log_level" {
+  default     = "info"
+  description = "default log level verbosity for apps that support it"
+  type        = string
+}
+
+variable "log_prefix" {
+  default     = "OPS: "
+  description = "string to prefix log messages with"
+  type        = string
+}
+
+variable "region" {
+  description = "AWS region the volume is in"
+  type        = string
+}
+
+variable "wait_interval" {
+  default     = "5"
+  description = "time (in seconds) to wait when looping to find the device"
+  type        = number
+}
+
+variable "volume_ids" {
+  description = "IDs of the EBS volumes to attach"
+  type        = list(string)
+  default     = []
+}
diff --git a/modules/persistent-ebs/data.tf b/modules/persistent-ebs/data.tf
index 0050a82a..0911e307 100644
--- a/modules/persistent-ebs/data.tf
+++ b/modules/persistent-ebs/data.tf
@@ -14,9 +14,7 @@ data "aws_iam_policy_document" "attach_ebs_policy_doc" {
       "ec2:DetachVolume",
     ]
 
-    resources = [
-      "arn:${data.aws_partition.current.partition}:ec2:${var.region}:${data.aws_caller_identity.current.account_id}:volume/${aws_ebs_volume.main.id}",
-      "arn:${data.aws_partition.current.partition}:ec2:${var.region}:${data.aws_caller_identity.current.account_id}:instance/*",
-    ]
+    resources = concat(["arn:${data.aws_partition.current.partition}:ec2:${var.region}:${data.aws_caller_identity.current.account_id}:instance/*"],
+    [for x in aws_ebs_volume.main.*.id : "arn:${data.aws_partition.current.partition}:ec2:${var.region}:${data.aws_caller_identity.current.account_id}:volume/${x}"])
   }
 }
diff --git a/modules/persistent-ebs/locals.tf b/modules/persistent-ebs/locals.tf
new file mode 100644
index 00000000..0c658d8e
--- /dev/null
+++ b/modules/persistent-ebs/locals.tf
@@ -0,0 +1,11 @@
+locals {
+  volume_default = {
+    type        = "gp2"
+    iops        = 0
+    size        = 15
+    encrypted   = true
+    kms_key_id  = ""
+    snapshot_id = ""
+  }
+  volumes_default = [for x in var.volumes : merge(local.volume_default, x)]
+}
diff --git a/modules/persistent-ebs/main.tf b/modules/persistent-ebs/main.tf
index 11b2ab6f..04f944af 100644
--- a/modules/persistent-ebs/main.tf
+++ b/modules/persistent-ebs/main.tf
@@ -9,17 +9,19 @@
  */
 
 resource "aws_ebs_volume" "main" {
+  count = length(local.volumes_default)
+
   availability_zone = var.az
-  size              = var.size
-  type              = var.volume_type
-  encrypted         = var.encrypted
-  kms_key_id        = var.kms_key_id
-  snapshot_id       = var.snapshot_id
+  size              = local.volumes_default[count.index].size
+  type              = local.volumes_default[count.index].type
+  encrypted         = local.volumes_default[count.index].encrypted
+  kms_key_id        = local.volumes_default[count.index].kms_key_id
+  snapshot_id       = local.volumes_default[count.index].snapshot_id
 
   # merge Name w/ extra_tags
   tags = merge(
     {
-      "Name" = "${var.name_prefix}-${var.az}"
+      "Name" = "${var.name_prefix}-${var.az}-${local.volumes_default[count.index].name}"
     },
     var.extra_tags,
   )
@@ -27,7 +29,7 @@ resource "aws_ebs_volume" "main" {
 
 # IAM policy that allows attaching this EBS volume to an EC2 instance
 resource "aws_iam_policy" "attach_ebs" {
-  name   = "${var.name_prefix}-attach-ebs-${aws_ebs_volume.main.id}"
+  name   = "${var.name_prefix}-attach-ebs"
   policy = data.aws_iam_policy_document.attach_ebs_policy_doc.json
 }
 
diff --git a/modules/persistent-ebs/outputs.tf b/modules/persistent-ebs/outputs.tf
index 2ad0e844..1ce7fc78 100644
--- a/modules/persistent-ebs/outputs.tf
+++ b/modules/persistent-ebs/outputs.tf
@@ -1,5 +1,5 @@
-output "volume_id" {
-  value       = aws_ebs_volume.main.id
+output "volume_ids" {
+  value       = aws_ebs_volume.main.*.id
   description = "`id` exported from the `aws_ebs_volume`"
 }
 
diff --git a/modules/persistent-ebs/variables.tf b/modules/persistent-ebs/variables.tf
index 1f70f89a..cd7304eb 100644
--- a/modules/persistent-ebs/variables.tf
+++ b/modules/persistent-ebs/variables.tf
@@ -14,42 +14,6 @@ variable "az" {
   type        = string
 }
 
-variable "volume_type" {
-  default     = "gp2"
-  description = "Type of EBS volume to use for the EBS block device"
-  type        = string
-}
-
-variable "size" {
-  default     = "15"
-  description = "Size (in GB) of EBS volume to use for the EBS block device"
-  type        = string
-}
-
-variable "snapshot_id" {
-  default     = ""
-  description = "The ID of the snapshot to base the EBS block device on"
-  type        = string
-}
-
-variable "encrypted" {
-  default     = true
-  description = "Boolean, whether or not to encrypt the EBS block device"
-  type        = string
-}
-
-variable "kms_key_id" {
-  default     = ""
-  description = "ID of the KMS key to use when encyprting the EBS block device"
-  type        = string
-}
-
-variable "iops" {
-  default     = ""
-  description = "The amount of IOPS to provision for the EBS block device"
-  type        = string
-}
-
 variable "extra_tags" {
   default     = {}
   description = "Map with extra tags to be applied to all ebs volumes"
@@ -60,3 +24,17 @@ variable "iam_instance_profile_role_name" {
   description = "The role to attach policy needed by this module."
   type        = string
 }
+
+variable "volumes" {
+  type = list(map(any))
+### Note: what should be contained.
+#    type        = string,
+#    iops        = number,
+#    size        = number,
+#    encrypted   = bool,
+#    kms_key_id  = string,
+#    snapshot_id = string,
+#    name        = string
+  description = "Definition of volumes. `name` is required."
+  default     = []
+}
diff --git a/modules/single-node-asg/main.tf b/modules/single-node-asg/main.tf
index d99b37ae..c81029df 100644
--- a/modules/single-node-asg/main.tf
+++ b/modules/single-node-asg/main.tf
@@ -21,12 +21,20 @@ locals {
   # To give us a short-hand refernce to the AZ
   az = data.aws_subnet.server-subnet.availability_zone
 
-  # The "name_prefix" we provide to the persistent-ebs module
-  data_volume_name_prefix = "${var.name_prefix}-${var.name_suffix}-data"
-
   # The `name_prefix` we provide to the `iam-instance-profile` module
   # The persistent-ebs module appends the AZ to the data node name, other modules don't do that
   name_prefix_with_az = "${var.name_prefix}-${var.name_suffix}-${local.az}"
+
+  data_volume_default = {
+    type        = "gp2"
+    iops        = 0
+    size        = 10
+    encrypted   = true
+    kms_key_id  = ""
+    snapshot_id = ""
+  }
+
+  data_volumes_default = [for x in var.data_volumes : merge(local.data_volume_default, x)]
 }
 
 # Create an IAM Instance profile we can use on EC2, associated with the ASG
@@ -38,15 +46,10 @@ module "instance_profile" {
 # Create a single EBS volume that can be used in a single/specific AZ, for the ASG
 module "service-data" {
   source      = "../persistent-ebs"
-  name_prefix = local.data_volume_name_prefix
+  name_prefix = "${var.name_prefix}-${var.name_suffix}"
   region      = var.region
   az          = local.az
-  size        = var.data_volume_size
-  iops        = var.data_volume_iops
-  volume_type = var.data_volume_type
-  encrypted   = var.data_volume_encrypted
-  kms_key_id  = var.data_volume_kms_key_id
-  snapshot_id = var.data_volume_snapshot_id
+  volumes     = local.data_volumes_default
 
   # EBS module will create an IAM policy and associate with this role
   iam_instance_profile_role_name = module.instance_profile.iam_role_name
@@ -82,8 +85,10 @@ module "server" {
   user_data = <<END_INIT
 #!/bin/bash
 # exec > /tmp/init.log
-# exec 2> /tmp/init-err.log
+exec 2>&1
 # set -x
+apt update
+${module.install-awscli.init_snippet}
 ${var.init_prefix}
 ${module.init-attach-ebs.init_snippet}
 ${var.init_suffix}
@@ -93,7 +98,12 @@ END_INIT
 
 # Render init snippet - boxed module to attach the EBS volume to the node
 module "init-attach-ebs" {
-  source    = "../init-snippet-attach-ebs-volume"
-  region    = var.region
-  volume_id = module.service-data.volume_id
+  source       = "../init-snippet-attach-ebs-volume"
+  region       = var.region
+  volume_ids   = module.service-data.volume_ids
+  device_paths = [for x in local.data_volumes_default : x.device]
+}
+
+module "install-awscli" {
+  source = "../init-snippet-install-awscli"
 }
diff --git a/modules/single-node-asg/outputs.tf b/modules/single-node-asg/outputs.tf
index d2bbbbdc..29b2030c 100644
--- a/modules/single-node-asg/outputs.tf
+++ b/modules/single-node-asg/outputs.tf
@@ -9,6 +9,6 @@ output "asg_iam_role_name" {
 }
 
 output "data_volume_name_tag" {
-  value       = "${local.data_volume_name_prefix}-${local.az}"
-  description = "Name tag value for attached data volume"
+  value       = "${local.name_prefix_with_az}-default"
+  description = "Name tag value for attached data volume. This is for compatible with old code."
 }
diff --git a/modules/single-node-asg/variables.tf b/modules/single-node-asg/variables.tf
index 97de3191..fdfc248a 100644
--- a/modules/single-node-asg/variables.tf
+++ b/modules/single-node-asg/variables.tf
@@ -42,45 +42,9 @@ variable "root_volume_type" {
 }
 
 variable "root_volume_size" {
-  default     = "8"
+  default     = 8
   description = "Size (in GB) of EBS volume to use for the root block device"
-  type        = string
-}
-
-variable "data_volume_type" {
-  default     = "gp2"
-  description = "Type of EBS volume to use for the EBS volume"
-  type        = string
-}
-
-variable "data_volume_size" {
-  default     = "10"
-  description = "Size (in GB) of EBS volume to use for the EBS volume"
-  type        = string
-}
-
-variable "data_volume_encrypted" {
-  default     = true
-  description = "Boolean, whether or not to encrypt the EBS block device"
-  type        = string
-}
-
-variable "data_volume_kms_key_id" {
-  default     = ""
-  description = "ID of the KMS key to use when encyprting the EBS block device"
-  type        = string
-}
-
-variable "data_volume_snapshot_id" {
-  default     = ""
-  description = "The ID of the snapshot to base the EBS block device on"
-  type        = string
-}
-
-variable "data_volume_iops" {
-  default     = ""
-  description = "The amount of IOPS to provision for the EBS block device"
-  type        = string
+  type        = number
 }
 
 variable "init_prefix" {
@@ -121,3 +85,8 @@ variable "load_balancers" {
   description = "The list of load balancers names to pass to the ASG module"
   type        = list(string)
 }
+
+variable "data_volumes" {
+  type = list(map(any))
+  description = "Definition of the data volumes. `name` and `device` are required."
+}

From b98b93b6e973c89cf66bdbe210fdcc646ef01885 Mon Sep 17 00:00:00 2001
From: Magicloud <1886157+Magicloud@users.noreply.github.com>
Date: Fri, 22 May 2020 21:10:01 +0800
Subject: [PATCH 2/8] Example single-node-asg-tester: Add Makefile/README

---
 examples/single-node-asg-tester/Makefile  | 40 +++++++++++++++++++++++
 examples/single-node-asg-tester/README.md |  5 +++
 examples/single-node-asg-tester/main.tf   | 32 ++++++++++--------
 3 files changed, 63 insertions(+), 14 deletions(-)
 create mode 100644 examples/single-node-asg-tester/Makefile
 create mode 100644 examples/single-node-asg-tester/README.md

diff --git a/examples/single-node-asg-tester/Makefile b/examples/single-node-asg-tester/Makefile
new file mode 100644
index 00000000..58f69cd5
--- /dev/null
+++ b/examples/single-node-asg-tester/Makefile
@@ -0,0 +1,40 @@
+.PHONY: init ssh-key apply destroy clean
+
+.DEFAULT_GOAL = help
+
+## Runs terraform get and terraform init for env
+init:
+	@terraform get
+	@terraform init
+
+## Create ssh key
+ssh-key:
+	@ssh-keygen -q -N "" -C "SSH key for vpc-scenario-1 example" -f ./id_rsa
+
+## use 'terraform apply' to apply the setup.
+apply:
+	@terraform apply
+
+## use 'terraform destroy' to remove all resources from AWS
+destroy:
+	@terraform destroy
+
+## rm -rf all files and state
+clean:
+	@rm -f id_rsa
+	@rm -f id_rsa.pub
+	@rm -f terraform.*.backup
+	@rm -f terraform.tfstate
+
+## Show help screen.
+help:
+	@echo "Please use \`make <target>' where <target> is one of\n\n"
+	@awk '/^[a-zA-Z\-\_0-9]+:/ { \
+		helpMessage = match(lastLine, /^## (.*)/); \
+		if (helpMessage) { \
+			helpCommand = substr($$1, 0, index($$1, ":")-1); \
+			helpMessage = substr(lastLine, RSTART + 3, RLENGTH); \
+			printf "%-30s %s\n", helpCommand, helpMessage; \
+		} \
+	} \
+	{ lastLine = $$0 }' $(MAKEFILE_LIST)
diff --git a/examples/single-node-asg-tester/README.md b/examples/single-node-asg-tester/README.md
new file mode 100644
index 00000000..65cff193
--- /dev/null
+++ b/examples/single-node-asg-tester/README.md
@@ -0,0 +1,5 @@
+# single node asg tester
+
+This example shows the basic usage of `single-node-asg` module, especially the multiple EBS attachments.
+
+The module keeps one and only one instance up at all time. And the EBS volumes are reattached when a new instance is up. Hence they are always accessible.
diff --git a/examples/single-node-asg-tester/main.tf b/examples/single-node-asg-tester/main.tf
index 3ecfa62f..f790eb1e 100644
--- a/examples/single-node-asg-tester/main.tf
+++ b/examples/single-node-asg-tester/main.tf
@@ -1,28 +1,32 @@
-provider "aws" {
-  region = "ap-northeast-1"
-}
-
 locals {
   cidr                 = "192.168.0.0/16"
   private_subnet_cidrs = ["192.168.100.0/24", "192.168.101.0/24"]
   public_subnet_cidrs  = ["192.168.0.0/24", "192.168.1.0/24"]
+  region = "ap-northeast-1"
 }
 
-data "aws_vpc" "vpc" {
-  cidr_block = local.cidr
-}
+data "aws_availability_zones" "azs" { }
 
-data "aws_subnet" "public" {
-  count = length(local.public_subnet_cidrs)
-  cidr_block = local.public_subnet_cidrs[count.index]
+module "vpc" {
+  source               = "fpco/foundation/aws//modules/vpc-scenario-2"
+  cidr                 = local.cidr
+  public_subnet_cidrs  = local.public_subnet_cidrs
+  private_subnet_cidrs = local.private_subnet_cidrs
+  azs                  = data.aws_availability_zones.azs.names
+  name_prefix          = "test"
+  region               = local.region
 }
 
 module "ubuntu" {
   source = "fpco/foundation/aws//modules/ami-ubuntu"
 }
 
+resource "aws_key_pair" "main" {
+  public_key = file("./id_rsa.pub")
+}
+
 resource "aws_security_group" "ssh" {
-  vpc_id = data.aws_vpc.vpc.id
+  vpc_id = module.vpc.vpc_id
   ingress {
     from_port   = 22
     to_port     = 22
@@ -41,11 +45,11 @@ module "tester" {
   source             = "../../modules/single-node-asg"
   name_prefix        = "ebs"
   name_suffix        = "test"
-  key_name           = "tokyo"
+  key_name           = aws_key_pair.main.key_name
   ami                = module.ubuntu.id
   instance_type      = "t2.micro"
-  subnet_id          = data.aws_subnet.public[0].id
+  subnet_id          = module.vpc.public_subnet_ids[0]
   security_group_ids = [aws_security_group.ssh.id]
-  region             = "ap-northeast-1"
+  region             = local.region
   data_volumes       = [{ name = "a", device = "/dev/xvdm", size = 50 }, { name = "b", device = "/dev/xvdn" }]
 }

From 71e59ddb2a0d157973a66ddc83dde09533691da0 Mon Sep 17 00:00:00 2001
From: Ketzacoatl <ketzacoatl@users.noreply.github.com>
Date: Wed, 8 Jul 2020 08:14:43 +0100
Subject: [PATCH 3/8] drop name param on alb and alb-default-forward, use tags
 instead

---
 modules/alb-default-forward/main.tf | 1 -
 modules/alb/main.tf                 | 1 -
 2 files changed, 2 deletions(-)

diff --git a/modules/alb-default-forward/main.tf b/modules/alb-default-forward/main.tf
index c6aa32b1..d51957fd 100644
--- a/modules/alb-default-forward/main.tf
+++ b/modules/alb-default-forward/main.tf
@@ -11,7 +11,6 @@ resource "aws_lb_listener" "lb-listener" {
 }
 
 resource "aws_lb_target_group" "lb-tg" {
-  name     = "${var.name_prefix}-tg"
   port     = var.service_port
   protocol = "HTTP"
   vpc_id   = var.vpc_id
diff --git a/modules/alb/main.tf b/modules/alb/main.tf
index 7cbe17e1..5d4d64b8 100644
--- a/modules/alb/main.tf
+++ b/modules/alb/main.tf
@@ -1,5 +1,4 @@
 resource "aws_lb" "alb" {
-  name               = "${var.name_prefix}-alb"
   internal           = var.internal
   load_balancer_type = "application"
   security_groups    = [aws_security_group.alb_sg.id]

From 73516b082fdccf7f07d9e159b5e55a9ad4036a97 Mon Sep 17 00:00:00 2001
From: Ketzacoatl <ketzacoatl@users.noreply.github.com>
Date: Wed, 8 Jul 2020 08:31:12 +0100
Subject: [PATCH 4/8] asg: make name_suffix optional

---
 modules/asg/locals.tf | 7 +++++++
 modules/asg/main.tf   | 4 ++--
 2 files changed, 9 insertions(+), 2 deletions(-)
 create mode 100644 modules/asg/locals.tf

diff --git a/modules/asg/locals.tf b/modules/asg/locals.tf
new file mode 100644
index 00000000..7149a819
--- /dev/null
+++ b/modules/asg/locals.tf
@@ -0,0 +1,7 @@
+locals {
+  name_prefix_with_suffix    = "${var.name_prefix}-${var.name_suffix}"
+  name_prefix_without_suffix = "${var.name_prefix}"
+
+  name_prefix = "${var.name_suffix != "" ? local.name_prefix_without_suffix : local.name_prefix_with_suffix}"
+
+}
diff --git a/modules/asg/main.tf b/modules/asg/main.tf
index 503945c2..e7325369 100644
--- a/modules/asg/main.tf
+++ b/modules/asg/main.tf
@@ -31,7 +31,7 @@ resource "aws_autoscaling_group" "cluster" {
   load_balancers            = var.elb_names
   max_size                  = var.max_nodes
   min_size                  = var.min_nodes
-  name_prefix               = "${var.name_prefix}-${var.name_suffix}"
+  name_prefix               = local.name_prefix
   placement_group           = var.placement_group
   termination_policies      = var.termination_policies
   protect_from_scale_in     = var.protect_from_scale_in
@@ -71,7 +71,7 @@ resource "aws_autoscaling_group" "cluster" {
     [
       {
         "key"                 = "Name"
-        "value"               = "${var.name_prefix}-${var.name_suffix}"
+        "value"               = local.name_prefix
         "propagate_at_launch" = true
       },
     ],

From 1e5157d7220a2ee2eb42d0d0e955b5d57f395ae7 Mon Sep 17 00:00:00 2001
From: Ketzacoatl <ketzacoatl@users.noreply.github.com>
Date: Wed, 8 Jul 2020 15:51:10 +0100
Subject: [PATCH 5/8] single-node-asg: fixup instance-profile module name

---
 modules/single-node-asg/main.tf    | 2 +-
 modules/single-node-asg/outputs.tf | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/modules/single-node-asg/main.tf b/modules/single-node-asg/main.tf
index c81029df..7c7c1f9b 100644
--- a/modules/single-node-asg/main.tf
+++ b/modules/single-node-asg/main.tf
@@ -38,7 +38,7 @@ locals {
 }
 
 # Create an IAM Instance profile we can use on EC2, associated with the ASG
-module "instance_profile" {
+module "instance-profile" {
   source      = "../iam-instance-profile"
   name_prefix = local.name_prefix_with_az
 }
diff --git a/modules/single-node-asg/outputs.tf b/modules/single-node-asg/outputs.tf
index 29b2030c..7c222fd4 100644
--- a/modules/single-node-asg/outputs.tf
+++ b/modules/single-node-asg/outputs.tf
@@ -4,7 +4,7 @@ output "asg_name" {
 }
 
 output "asg_iam_role_name" {
-  value       = module.instance_profile.iam_role_name
+  value       = module.instance-profile.iam_role_name
   description = "`name` exported from the Service Data `aws_iam_role`"
 }
 

From be3222bb1b7fc4058e863a714dcd342c74d6bfd3 Mon Sep 17 00:00:00 2001
From: Ketzacoatl <ketzacoatl@users.noreply.github.com>
Date: Wed, 8 Jul 2020 15:51:23 +0100
Subject: [PATCH 6/8] single-node-asg: make name_suffix optional

---
 modules/single-node-asg/main.tf      | 12 ++++++++----
 modules/single-node-asg/variables.tf |  3 ++-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/modules/single-node-asg/main.tf b/modules/single-node-asg/main.tf
index 7c7c1f9b..9da391a7 100644
--- a/modules/single-node-asg/main.tf
+++ b/modules/single-node-asg/main.tf
@@ -21,9 +21,14 @@ locals {
   # To give us a short-hand refernce to the AZ
   az = data.aws_subnet.server-subnet.availability_zone
 
+  # with or without the suffix, eg, if suffix is empty, skip it and avoid the extra -
+  name_prefix_with_suffix    = "${var.name_prefix}-${var.name_suffix}"
+  name_prefix_without_suffix = "${var.name_prefix}"
+  name_prefix = "${var.name_suffix != "" ? local.name_prefix_without_suffix : local.name_prefix_with_suffix}"
+
   # The `name_prefix` we provide to the `iam-instance-profile` module
   # The persistent-ebs module appends the AZ to the data node name, other modules don't do that
-  name_prefix_with_az = "${var.name_prefix}-${var.name_suffix}-${local.az}"
+  name_prefix_with_az = "${local.name_prefix}-${local.az}"
 
   data_volume_default = {
     type        = "gp2"
@@ -46,7 +51,7 @@ module "instance-profile" {
 # Create a single EBS volume that can be used in a single/specific AZ, for the ASG
 module "service-data" {
   source      = "../persistent-ebs"
-  name_prefix = "${var.name_prefix}-${var.name_suffix}"
+  name_prefix = local.name_prefix_with_az
   region      = var.region
   az          = local.az
   volumes     = local.data_volumes_default
@@ -72,8 +77,7 @@ module "server" {
   public_ip       = var.public_ip
   # the prefix and suffix names are combined in
   # the `asg` module to create the full name
-  name_prefix = var.name_prefix
-  name_suffix = "${var.name_suffix}-${local.az}"
+  name_prefix = local.name_prefix_with_az
 
   root_volume_type   = var.root_volume_type
   root_volume_size   = var.root_volume_size
diff --git a/modules/single-node-asg/variables.tf b/modules/single-node-asg/variables.tf
index fdfc248a..1b534982 100644
--- a/modules/single-node-asg/variables.tf
+++ b/modules/single-node-asg/variables.tf
@@ -25,7 +25,8 @@ variable "placement_group" {
 }
 
 variable "name_suffix" {
-  description = "suffix to include when naming the various resources"
+  description = "suffix to include when naming the various resources, ignored if left as empty string"
+  default     = ""
   type        = string
 }
 

From e59a3e5347972f3adeb6f68c654f8d2821468fd1 Mon Sep 17 00:00:00 2001
From: Ketzacoatl <ketzacoatl@users.noreply.github.com>
Date: Wed, 8 Jul 2020 18:02:41 +0100
Subject: [PATCH 7/8] single-node-asg: fixup instance-profile module name -
 squash me

---
 modules/single-node-asg/main.tf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/modules/single-node-asg/main.tf b/modules/single-node-asg/main.tf
index 9da391a7..fd66779a 100644
--- a/modules/single-node-asg/main.tf
+++ b/modules/single-node-asg/main.tf
@@ -57,7 +57,7 @@ module "service-data" {
   volumes     = local.data_volumes_default
 
   # EBS module will create an IAM policy and associate with this role
-  iam_instance_profile_role_name = module.instance_profile.iam_role_name
+  iam_instance_profile_role_name = module.instance-profile.iam_role_name
 }
 
 # Create an ASG with just 1 EC2 instance

From 335b5afcdc44e3e06e4715d6b1a6770a5ca47e58 Mon Sep 17 00:00:00 2001
From: Ketzacoatl <ketzacoatl@users.noreply.github.com>
Date: Wed, 8 Jul 2020 18:03:42 +0100
Subject: [PATCH 8/8] oops

---
 modules/single-node-asg/main.tf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/modules/single-node-asg/main.tf b/modules/single-node-asg/main.tf
index fd66779a..17946ed1 100644
--- a/modules/single-node-asg/main.tf
+++ b/modules/single-node-asg/main.tf
@@ -68,7 +68,7 @@ module "server" {
   elb_names = var.load_balancers
   key_name  = var.key_name
   # The IAM Instance Profile w/ attach_ebs role
-  iam_profile   = module.instance_profile.iam_profile_id
+  iam_profile   = module.instance-profile.iam_profile_id
   instance_type = var.instance_type
   # 1 EC2 instance <> 1 EBS volume
   max_nodes       = 1