Protect instance from autoscale in on aws_autoscaling_group

[jwieringa]

Desired functionality: Ability to set Instance Protection to "Protect From Scale In" on an AWS auto scaling group.

I understand this change to be working through manual testing. I'm opening the PR for discussion and feedback.

1. I'm interested if supporting setting this option is compatible with the workflows used in Terraform and
2. if this is compatible I'm interested in feedback on implementation and (lack of) testing.

Please let me know if there are any questions - Thanks!

[jwieringa]

I attempted to use the following:

if d.HasChange("protect_from_scale_in") {
  opts.NewInstancesProtectedFromScaleIn = aws.Bool(d.Get("protect_from_scale_in").(bool))
}

However, I found that the state was never updated. My expectation is that HasChange should be used, not yet clear why I was unable to complete updates when using HasChange.

[stack72]

This is indeed the correct way of updating - otherwise we will be passing an unnecessary parameter to the API each time

I was able to use the same code you had and it updated as expected.

[stack72]

Hi @jwieringa

My apologies for this sliding through the cracks. So I have just tested this with a few changes and it works. Comments inline :)

Paul

[stack72]

In the test:

TestAccAWSAutoScalingGroup_basic we have the following config:

func testAccAWSAutoScalingGroupConfig(name string) string {
    return fmt.Sprintf(`
resource "aws_launch_configuration" "foobar" {
  image_id = "ami-21f78e11"
  instance_type = "t1.micro"
}

resource "aws_placement_group" "test" {
  name = "asg_pg_%s"
  strategy = "cluster"
}

resource "aws_autoscaling_group" "bar" {
  availability_zones = ["us-west-2a"]
  name = "%s"
  max_size = 5
  min_size = 2
  health_check_type = "ELB"
  desired_capacity = 4
  force_delete = true
  termination_policies = ["OldestInstance","ClosestToNextInstanceHour"]

  launch_configuration = "${aws_launch_configuration.foobar.name}"

  tag {
    key = "Foo"
    value = "foo-bar"
    propagate_at_launch = true
  }
}
`, name, name)
}

by default, this will be protect_from_scale_in = false

so we can test that as part of the acceptance test:

resource.TestCheckResourceAttr(
                        "aws_autoscaling_group.bar", "protect_from_scale_in", "false"),

the test then pushes through an update:

func testAccAWSAutoScalingGroupConfigUpdate(name string) string {
    return fmt.Sprintf(`
resource "aws_launch_configuration" "foobar" {
  image_id = "ami-21f78e11"
  instance_type = "t1.micro"
}

resource "aws_launch_configuration" "new" {
  image_id = "ami-21f78e11"
  instance_type = "t1.micro"
}

resource "aws_autoscaling_group" "bar" {
  availability_zones = ["us-west-2a"]
  name = "%s"
  max_size = 5
  min_size = 2
  health_check_grace_period = 300
  health_check_type = "ELB"
  desired_capacity = 5
  force_delete = true
  termination_policies = ["ClosestToNextInstanceHour"]

  launch_configuration = "${aws_launch_configuration.new.name}"

  tag {
    key = "Bar"
    value = "bar-foo"
    propagate_at_launch = true
  }
}
`, name)
}

In this code, we can set protect_from_scale_in = true and Terraform will make that change as expected. We can then test the assertion:

resource.TestCheckResourceAttr(
                        "aws_autoscaling_group.bar", "protect_from_scale_in", "true"),

By making these changes, you will get a test result as follows:

make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSAutoScalingGroup_basic'                                 
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSAutoScalingGroup_basic -timeout 120m
=== RUN   TestAccAWSAutoScalingGroup_basic
--- PASS: TestAccAWSAutoScalingGroup_basic (268.85s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    268.865s

Nice work :)

[jwieringa]

@stack72 Thank you! The detailed explanation on testing is appreciated. Rebased on master. Updated with tests. Removed WIP title.

$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSAutoScalingGroup_basic'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /vendor/)
2016/06/28 18:57:57 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSAutoScalingGroup_basic -timeout 120m
=== RUN   TestAccAWSAutoScalingGroup_basic
--- PASS: TestAccAWSAutoScalingGroup_basic (279.76s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    279.780s

@stack72 Please let me know if there is anything else I can do.

[stack72]

Hi @jwieringa

This looks really good now :) Test results are as expected - nice work! Merging!

make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSAutoScalingGroup_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSAutoScalingGroup_ -timeout 120m
=== RUN   TestAccAWSAutoScalingGroup_importBasic
--- PASS: TestAccAWSAutoScalingGroup_importBasic (172.88s)
=== RUN   TestAccAWSAutoScalingGroup_basic
--- PASS: TestAccAWSAutoScalingGroup_basic (266.92s)
=== RUN   TestAccAWSAutoScalingGroup_autoGeneratedName
--- PASS: TestAccAWSAutoScalingGroup_autoGeneratedName (56.36s)
=== RUN   TestAccAWSAutoScalingGroup_terminationPolicies
--- PASS: TestAccAWSAutoScalingGroup_terminationPolicies (77.63s)
=== RUN   TestAccAWSAutoScalingGroup_tags
--- PASS: TestAccAWSAutoScalingGroup_tags (254.74s)
=== RUN   TestAccAWSAutoScalingGroup_VpcUpdates
--- PASS: TestAccAWSAutoScalingGroup_VpcUpdates (76.22s)
=== RUN   TestAccAWSAutoScalingGroup_WithLoadBalancer
--- PASS: TestAccAWSAutoScalingGroup_WithLoadBalancer (387.78s)
=== RUN   TestAccAWSAutoScalingGroup_withPlacementGroup
--- PASS: TestAccAWSAutoScalingGroup_withPlacementGroup (142.48s)
=== RUN   TestAccAWSAutoScalingGroup_withMetrics
--- PASS: TestAccAWSAutoScalingGroup_withMetrics (43.89s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    1478.927s

Paul

[jwieringa]

@stack72 Thank you!

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.