hashicorp/terraform-aws-nomad

Convert aws_launch_configuration to aws_launch_template

fideloper opened this issue · 6 comments

fideloper commented

Hello!

Quick question:

Launch Templates are preferred by AWS over Launch Configurations.

I was wondering if converting the aws_launch_configuration to aws_launch_template would be a welcome PR or if there was a reason to continue with aws_launch_configuration that I'm not aware of?

Some benefits:

  1. Template support additional options ("latest features") such as metadata_options
  2. We may not require the create_before_destroy = true calls to prevent cyclical dependency errors
fideloper commented

Here's a start:

I've marked 3 TODO items in here that would need to be adjusted still

resource "aws_launch_template" "launch_template" {
    name_prefix   = "${var.cluster_name}-"
    image_id      = var.ami_id
    instance_type = var.instance_type
    user_data     = base64encode(data.template_file.user_data_server.rendered)

    # TODO: convert spot_price to instance_market_options{}

    iam_instance_profile {
        name = aws_iam_instance_profile.instance_profile.name
    }

    key_name = var.ssh_key_name

    placement {
        tenancy = var.tenancy
    }

    network_interfaces {
        associate_public_ip_address = var.associate_public_ip_address

        # TODO: New variable? false by default, but that will consume available private IP addresses
        #              as auto scaling brings servers up / takes servers down
        delete_on_termination = true

        # Security groups moved here
        security_groups = concat(
            [aws_security_group.lc_security_group.id],
            var.additional_security_group_ids,
        )
    }

    ebs_optimized = var.root_volume_ebs_optimized

    # Root device follows AMI settings
    # but you can over-ride here if you know the device_name
    block_device_mappings {
        device_name = "/dev/sda1"   # TODO: Need to know the root device name
        ebs {
            delete_on_termination = var.root_volume_delete_on_termination
            volume_type = var.root_volume_type
            volume_size = var.root_volume_size
        }
    }

    # Add additional EBS volumes
    dynamic "block_device_mappings" {
        for_each = var.ebs_block_devices
        content {
            device_name = block_device_mappings.value["device_name"]
            ebs {
                volume_size             = block_device_mappings.value["volume_size"]
                snapshot_id             = lookup(block_device_mappings.value, "snapshot_id", null)
                iops                    = lookup(block_device_mappings.value, "iops", null)
                encrypted               = lookup(block_device_mappings.value, "encrypted", null)
                delete_on_termination   = lookup(block_device_mappings.value, "delete_on_termination", null)
            }
        }
    }

    # Important note: whenever using a launch configuration with an auto scaling group, you must set
    # create_before_destroy = true. However, as soon as you set create_before_destroy = true in one resource, you must
    # also set it in every resource that it depends on, or you'll get an error about cyclic dependencies (especially when
    # removing resources). For more info, see:
    #
    # https://www.terraform.io/docs/providers/aws/r/launch_configuration.html
    # https://terraform.io/docs/configuration/resources.html
    lifecycle {
        create_before_destroy = true
    }
}

This can then allow us to add additional options such as:

 metadata_options {
    http_endpoint              = "enabled" # Make sure it's on
    http_tokens                 = "optional" # Don't force the use of tokens unless you want to
    http_put_response_hop_limit = 2 # Increase to 2 if calling meta data service within container networks
  }
brikis98 commented

I think this would be a good improvement, but how can we do it while allowing users with existing clusters to transition without downtime?

fideloper commented

Hi!

Hmmm yep, that could be a hard one.

I THINK, for an existing infrastructure, terraform would delete the ASG's reference to the launch config, and add in the launch template. I don't believe this would kill instances within an ASG (correct me if you know otherwise!). Perhaps that would destroy the asg instead of update in place?

Since AWS is putting support behind templates over configurations, there may be a time where the bandaid needs to be ripped, in which case I suppose one thing to do is bump the release up a major version and document the change (since the project is at 0.6.*, maybe that's asking a lot!)

brikis98 commented

I'm not exactly sure what'll happen. Would you be up for trying it and seeing what the result is?

fideloper commented
gthieleb commented

Can you provide a PR and perhaps containing a basic test module? I appreciate to test the behavior.