Not working for windows ECS

[benorgil]

This is not working for windows. The error I get seemed to be related to the way you are adding an EBS volume?

Trying this:

data "aws_ami" "win_ecs" {
  owners = ["amazon"]
  most_recent = true
  filter {
    name = "name"
    values = ["Windows_Server-2016-English-Full-ECS_Optimized*"]
  }
}

module "ecs-cluster" {
  source    = "github.com/terraform-community-modules/tf_aws_ecs"
  name      = "mycluster"
  subnet_id = ["subnet-1235434","subnet-123345"]
  vpc_id    = "vpc-123345"
  security_group_ids = ["sg-1233w45"]
  key_name = "mykey"
  region = "eu-central-1"
  ami = "${data.aws_ami.win_ecs.id}"

  servers   = 2
  instance_type = "t2.medium"
  docker_storage_size = 27
}

And I get this error:

* aws_autoscaling_group.ecs: "asg-ecs-.......": Waiting up to 10m0s: Need at least 2 healthy instances in ASG, have 0. Most recent activity: {
  ActivityId: "0c858809-d3a8-c8d7-a931-c43cbe369e02",
  AutoScalingGroupName: "asg-ecs-......",
  Cause: "At 2018-03-07T21:53:58Z an instance was started in response to a difference between desired and actual capacity, increasing the capacity from 0 to 2.",
  Description: "Launching a new EC2 instance.  Status Reason: The device 'xvdcz' is used in more than one block-device mapping. Launching EC2 instance failed.",
  Details: "{\"Subnet ID\":\"subnet-1232342\",\"Availability Zone\":\"eu-central-1a\"}",
  EndTime: 2018-03-07 21:53:59 +0000 UTC,
  Progress: 100,
  StartTime: 2018-03-07 21:53:59.346 +0000 UTC,
  StatusCode: "Failed",
  StatusMessage: "The device 'xvdcz' is used in more than one block-device mapping. Launching EC2 instance failed."

[tfhartmann]

Thanks for opening this issue!

Hmm yes, this module is built under the assumption that the EBS volume will be mounted at /dev/xvdcz ala this doc... https://docs.aws.amazon.com/AmazonECS/latest/developerguide/ecs-ami-storage-config.html I hadn't considered using the Windows AMI with this module! I'm in no way opposed to getting this working, but I suspect there are a number of other changes that would need to be accounted for as well, for example the Userdata template https://github.com/terraform-community-modules/tf_aws_ecs/blob/master/templates/user_data.tpl

[benorgil]

Is there an easy way to just pass in my own template to the module? or will this module need to be modified?

Do you think its just the userdata script that will need to change or other stuff too? Adding a template for windows seems pretty doable.

This looks promising: https://github.com/aws/amazon-ecs-agent/blob/master/misc/windows-deploy/user-data.ps1

[tfhartmann]

Yes, you can pass in a custom template using the user_data parameter. https://github.com/terraform-community-modules/tf_aws_ecs/blob/master/variables.tf#L135

[benorgil]

Before I spend more time with this, let me ask if you would object to just having a windows conditional? There's going to only be 2 ECS platforms for the foreseeable future i expect.

What do you think about doing something like this:

locals {
  linux_ami = "amzn-ami-${var.ami_version}-amazon-ecs-optimized"
  windows_ami = "Windows_Server-2016-English-Full-ECS_Optimized-${var.ami_version}"
}

data "aws_ami" "ecs_ami" {
  most_recent = true

  filter {
    name   = "owner-alias"
    values = ["amazon"]
  }

  filter {
    name   = "name"
    values = "${var.ecs_platform == "windows" ? local.windows_ami : local.linux_ami}"
  }
}

[tfhartmann]

Sorry for the response lag 😄 so here's kind of my thoughts in no particular order.

First off thanks you so much for opening #32 thats awesome, I love love love the idea of potentially adding this as a feature, I am concerned however about my (along with @hakamadare ) ability to support it, as Windows support isn't something we have a lot of cycles for, and don't have any Windows infra to test against at the moment. That doesn't mean we shouldn't do it, I just want to put it out there as a concern.

Basically, my concerns fall into two categories, first, being able to continue to support Windows as a first class citizen, and feature parity. My second concern is likely a result of the first.

Regarding feature parity, if we add windows as a default platform to this module, I'd want to make sure that the "default" behavior matched the Linux behavior in as many ways as possible, so for example supporting the docker_storage_size , {dockerhub_token} and ${dockerhub_email} parameters.

Think I think we should do for sure - add a ebs_block_device_name parameter for the ebs_block_device block, so that it can be changed to /dev/sda2, /dev/xvdcz, or /dev/xvdcy or whatever the user of the module wants, that makes a ton of sense to me.

I can think of a couple of ways we could support other platforms

• Add a ebs_block_device_name parameter, and leave it up to the user to over-ride the required values outside of the module, so basically giving the end user all the knobs, and dials to configure different platforms by making the module flexible enough to handle it
• Adding windows as an default configuration and committing to keeping the required configs and scripts updated

** for both of those example, I think we should absolutely add to the README and examples an example of how windows would work.

Anyway, @benorgil @hakamadare @antonbabenko ( and anyone else who has thoughts) let me know what you think, I think having this work on windows would be super awesome!

[benorgil]

Cool thanks for taking the time to look at this. Hmm so if the ebs_block_device_name was parameterized I could override that with "/dev/sda2" and can override the userdata script and ami and just do that until windows can be added as a default.

I haven't done a prod deploy of ECS on Windows yet so I'm not sure what other quirks it might have. Ill keep modifying my fork with any updates.

What do you think about using locals{} like i did and using a ternary to check if its windows or linux?

[tfhartmann]

What do you think about using locals{} like i did and using a ternary to check if its windows or linux?

I really like the idea of using more locals in this module, I think there is a bunch of stuff in this code that could benefit from using locals!