hashicorp / terraform-aws-nomad

A Terraform Module for how to run Nomad on AWS using Terraform and Packer
Apache License 2.0
255 stars 187 forks source link

Convert aws_launch_configuration to aws_launch_template #76

Open fideloper opened 4 years ago

fideloper commented 4 years ago

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 4 years ago

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 4 years ago

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 4 years ago

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 4 years ago

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

fideloper commented 4 years ago

In theory I did this earlier but my cluster isn’t being used in production yet so I wasn’t worried about running jobs and didn’t pay close enough attention.

I may be able to play with this again soon to test that out (I hope! 2 little kids at home full time + a job + side business is making time short for me personally, but I don’t want to push off the work to everyone else for my own feature request :P)

On Sun, Sep 13, 2020 at 06:11 Yevgeniy Brikman notifications@github.com wrote:

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

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/hashicorp/terraform-aws-nomad/issues/76#issuecomment-691657538, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADSDU5MDMXIRIYZHCVWMELSFSSG3ANCNFSM4QUYK2DA .

gthieleb commented 3 years ago

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