From e4dc2a48f440e9e31241d090e16d83bd56ee6f74 Mon Sep 17 00:00:00 2001 From: Roberth Kulbin Date: Wed, 17 Jun 2020 10:15:22 +0100 Subject: [PATCH 01/12] resource/aws_autoscaling_group: add instance_refresh block --- aws/autoscaling_tags.go | 38 +- aws/resource_aws_autoscaling_group.go | 152 +++++++- aws/resource_aws_autoscaling_group_test.go | 361 ++++++++++++++++++ .../docs/r/autoscaling_group.html.markdown | 60 ++- 4 files changed, 598 insertions(+), 13 deletions(-) diff --git a/aws/autoscaling_tags.go b/aws/autoscaling_tags.go index c0bbbe134d8..bf60e5c0905 100644 --- a/aws/autoscaling_tags.go +++ b/aws/autoscaling_tags.go @@ -51,8 +51,12 @@ func autoscalingTagToHash(v interface{}) int { } // setTags is a helper to set the tags for a resource. It expects the -// tags field to be named "tag" -func setAutoscalingTags(conn *autoscaling.AutoScaling, d *schema.ResourceData) error { +// tags field to be named "tag". +// +// When the return value requiresPropagation is true, instances of the +// ASG should be refreshed in order for the changed or removed tags to +// fully take effect. +func setAutoscalingTags(conn *autoscaling.AutoScaling, d *schema.ResourceData) (requiresPropagation bool, err error) { resourceID := d.Get("name").(string) var createTags, removeTags []*autoscaling.Tag @@ -63,17 +67,17 @@ func setAutoscalingTags(conn *autoscaling.AutoScaling, d *schema.ResourceData) e old, err := autoscalingTagsFromMap(o, resourceID) if err != nil { - return err + return false, err } new, err := autoscalingTagsFromMap(n, resourceID) if err != nil { - return err + return false, err } c, r, err := diffAutoscalingTags(old, new, resourceID) if err != nil { - return err + return false, err } createTags = append(createTags, c...) @@ -82,17 +86,17 @@ func setAutoscalingTags(conn *autoscaling.AutoScaling, d *schema.ResourceData) e oraw, nraw = d.GetChange("tags") old, err = autoscalingTagsFromList(oraw.(*schema.Set).List(), resourceID) if err != nil { - return err + return false, err } new, err = autoscalingTagsFromList(nraw.(*schema.Set).List(), resourceID) if err != nil { - return err + return false, err } c, r, err = diffAutoscalingTags(old, new, resourceID) if err != nil { - return err + return false, err } createTags = append(createTags, c...) @@ -108,7 +112,13 @@ func setAutoscalingTags(conn *autoscaling.AutoScaling, d *schema.ResourceData) e } if _, err := conn.DeleteTags(&remove); err != nil { - return err + return false, err + } + + for _, tag := range removeTags { + if aws.BoolValue(tag.PropagateAtLaunch) { + requiresPropagation = true + } } } @@ -120,11 +130,17 @@ func setAutoscalingTags(conn *autoscaling.AutoScaling, d *schema.ResourceData) e } if _, err := conn.CreateOrUpdateTags(&create); err != nil { - return err + return false, err + } + + for _, tag := range createTags { + if aws.BoolValue(tag.PropagateAtLaunch) { + requiresPropagation = true + } } } - return nil + return requiresPropagation, nil } // diffTags takes our tags locally and the ones remotely and returns diff --git a/aws/resource_aws_autoscaling_group.go b/aws/resource_aws_autoscaling_group.go index 0349c4a0305..d648609a397 100644 --- a/aws/resource_aws_autoscaling_group.go +++ b/aws/resource_aws_autoscaling_group.go @@ -442,6 +442,35 @@ func resourceAwsAutoscalingGroup() *schema.Resource { Optional: true, Computed: true, }, + + "instance_refresh": { + Type: schema.TypeList, + MaxItems: 1, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "instance_warmup_seconds": { + Type: schema.TypeInt, + Optional: true, + Default: -1, // default to health_check_grace_period + ValidateFunc: validation.IntAtLeast(-1), + }, + "min_healthy_percentage": { + Type: schema.TypeInt, + Optional: true, + Default: 90, + ValidateFunc: validation.IntBetween(0, 100), + }, + "strategy": { + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.StringInSlice( + []string{autoscaling.RefreshStrategyRolling}, + false), + }, + }, + }, + }, }, CustomizeDiff: customdiff.Sequence( @@ -894,6 +923,7 @@ func waitUntilAutoscalingGroupLoadBalancerTargetGroupsAdded(conn *autoscaling.Au func resourceAwsAutoscalingGroupUpdate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).autoscalingconn shouldWaitForCapacity := false + shouldRefreshInstances := false opts := autoscaling.UpdateAutoScalingGroupInput{ AutoScalingGroupName: aws.String(d.Id()), @@ -914,16 +944,21 @@ func resourceAwsAutoscalingGroupUpdate(d *schema.ResourceData, meta interface{}) if v, ok := d.GetOk("launch_configuration"); ok { opts.LaunchConfigurationName = aws.String(v.(string)) } + + shouldRefreshInstances = true } if d.HasChange("launch_template") { if v, ok := d.GetOk("launch_template"); ok && len(v.([]interface{})) > 0 { opts.LaunchTemplate, _ = expandLaunchTemplateSpecification(v.([]interface{})) } + + shouldRefreshInstances = true } if d.HasChange("mixed_instances_policy") { opts.MixedInstancesPolicy = expandAutoScalingMixedInstancesPolicy(d.Get("mixed_instances_policy").([]interface{})) + shouldRefreshInstances = true } if d.HasChange("min_size") { @@ -950,16 +985,20 @@ func resourceAwsAutoscalingGroupUpdate(d *schema.ResourceData, meta interface{}) if d.HasChange("vpc_zone_identifier") { opts.VPCZoneIdentifier = expandVpcZoneIdentifiers(d.Get("vpc_zone_identifier").(*schema.Set).List()) + shouldRefreshInstances = true } if d.HasChange("availability_zones") { if v, ok := d.GetOk("availability_zones"); ok && v.(*schema.Set).Len() > 0 { opts.AvailabilityZones = expandStringList(v.(*schema.Set).List()) } + + shouldRefreshInstances = true } if d.HasChange("placement_group") { opts.PlacementGroup = aws.String(d.Get("placement_group").(string)) + shouldRefreshInstances = true } if d.HasChange("termination_policies") { @@ -977,8 +1016,11 @@ func resourceAwsAutoscalingGroupUpdate(d *schema.ResourceData, meta interface{}) opts.ServiceLinkedRoleARN = aws.String(d.Get("service_linked_role_arn").(string)) } - if err := setAutoscalingTags(conn, d); err != nil { + switch requiresPropagation, err := setAutoscalingTags(conn, d); { + case err != nil: return err + case requiresPropagation: + shouldRefreshInstances = true } log.Printf("[DEBUG] AutoScaling Group update configuration: %#v", opts) @@ -1144,6 +1186,12 @@ func resourceAwsAutoscalingGroupUpdate(d *schema.ResourceData, meta interface{}) } } + if shouldRefreshInstances { + if err := startAutoscalingInstanceRefresh(d, conn); err != nil { + return fmt.Errorf("failed to start instance refresh of asg %s: %s", d.Id(), err) + } + } + return resourceAwsAutoscalingGroupRead(d, meta) } @@ -1756,3 +1804,105 @@ func waitUntilAutoscalingGroupLoadBalancersRemoved(conn *autoscaling.AutoScaling return nil } + +// startAutoscalingInstanceRefresh starts a new Instance Refresh in this +// Auto-Scaling Group. If there is already an active refresh, it is cancelled. +func startAutoscalingInstanceRefresh(d *schema.ResourceData, conn *autoscaling.AutoScaling) error { + asgName := d.Id() + input := autoscaling.StartInstanceRefreshInput{ + AutoScalingGroupName: aws.String(asgName), + Preferences: &autoscaling.RefreshPreferences{}, + Strategy: nil, + } + + if block, ok := d.Get("instance_refresh").([]interface{}); ok && len(block) > 0 { + m := block[0].(map[string]interface{}) + + if warmup := m["instance_warmup_seconds"].(int); warmup > -1 { + // -1 would mean defaulting to using the group's health_check_grace_period + input.Preferences.InstanceWarmup = aws.Int64(int64(warmup)) + } + + // validated by schema + input.Preferences.MinHealthyPercentage = aws.Int64(int64(m["min_healthy_percentage"].(int))) + input.Strategy = aws.String(m["strategy"].(string)) + } else { + log.Printf("[DEBUG] Instance refresh not enabled in ASG %s", asgName) + return nil + } + + log.Printf("[DEBUG] Cancelling active refresh in ASG %s, if any...", asgName) + + if err := cancelAutoscalingInstanceRefresh(d, conn); err != nil { + // todo: add comment about subsequent ASG updates not picking up the refresh? + return fmt.Errorf("failed to cancel previous refresh: %s", err) + } + + log.Printf("[DEBUG] Starting instance refresh in ASG %s...", asgName) + + instanceRefreshId := "" + switch output, err := conn.StartInstanceRefresh(&input); { + case err != nil: + return err + default: + instanceRefreshId = aws.StringValue(output.InstanceRefreshId) + } + + log.Printf("[INFO] Started instance refresh %s in ASG %s", instanceRefreshId, asgName) + + return nil +} + +// cancelAutoscalingInstanceRefresh cancels the currently active Instance +// Refresh of this Auto-Scaling Group, and waits until the refresh reaches a +// terminal state (usually Cancelled). If there is no active refresh, the +// function short-circuits without error. +func cancelAutoscalingInstanceRefresh(d *schema.ResourceData, conn *autoscaling.AutoScaling) error { + asgName := d.Id() + input := autoscaling.CancelInstanceRefreshInput{ + AutoScalingGroupName: aws.String(asgName), + } + + _, err := conn.CancelInstanceRefresh(&input) + switch { + case isAWSErr(err, autoscaling.ErrCodeActiveInstanceRefreshNotFoundFault, ""): + log.Printf("[DEBUG] No active Instance Refresh in ASG %s", asgName) + return nil + case err != nil: + return err + } + + err = resource.Retry(5*time.Minute, func() *resource.RetryError { + input := autoscaling.DescribeInstanceRefreshesInput{ + AutoScalingGroupName: aws.String(asgName), + MaxRecords: aws.Int64(1), + } + + output, err := conn.DescribeInstanceRefreshes(&input) + switch { + case err != nil: + return resource.NonRetryableError(err) + case len(output.InstanceRefreshes) != 1: + return nil + } + + switch status := aws.StringValue(output.InstanceRefreshes[0].Status); status { + case + autoscaling.InstanceRefreshStatusCancelled, + autoscaling.InstanceRefreshStatusFailed, + autoscaling.InstanceRefreshStatusSuccessful: + + return nil + default: + return resource.RetryableError(fmt.Errorf("refresh status %s is not terminal", status)) + } + }) + + if isResourceTimeoutError(err) { + return fmt.Errorf("timed out before the previous refresh reached a terminal state") + } + + log.Printf("[INFO] Cancelled active instance refresh in ASG %s", asgName) + + return nil +} diff --git a/aws/resource_aws_autoscaling_group_test.go b/aws/resource_aws_autoscaling_group_test.go index 4a9fe50e543..c4710fdcf18 100644 --- a/aws/resource_aws_autoscaling_group_test.go +++ b/aws/resource_aws_autoscaling_group_test.go @@ -99,6 +99,7 @@ func TestAccAWSAutoScalingGroup_basic(t *testing.T) { testAccCheckAWSAutoScalingGroupAttributes(&group, randName), testAccMatchResourceAttrRegionalARN("aws_autoscaling_group.bar", "arn", "autoscaling", regexp.MustCompile(`autoScalingGroup:.+`)), tfawsresource.TestCheckTypeSetElemAttr("aws_autoscaling_group.bar", "availability_zones.*", "us-west-2a"), + testAccCheckAutoscalingLatestInstanceRefreshState(&group, 0, 0, nil), resource.TestCheckResourceAttr("aws_autoscaling_group.bar", "default_cooldown", "300"), resource.TestCheckResourceAttr("aws_autoscaling_group.bar", "desired_capacity", "4"), resource.TestCheckResourceAttr("aws_autoscaling_group.bar", "enabled_metrics.#", "0"), @@ -126,6 +127,7 @@ func TestAccAWSAutoScalingGroup_basic(t *testing.T) { resource.TestCheckResourceAttr("aws_autoscaling_group.bar", "termination_policies.1", "ClosestToNextInstanceHour"), resource.TestCheckResourceAttr("aws_autoscaling_group.bar", "vpc_zone_identifier.#", "0"), resource.TestCheckResourceAttr("aws_autoscaling_group.bar", "max_instance_lifetime", "0"), + resource.TestCheckNoResourceAttr("aws_autoscaling_group.bar", "instance_refresh.#"), ), }, { @@ -147,6 +149,7 @@ func TestAccAWSAutoScalingGroup_basic(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckAWSAutoScalingGroupExists("aws_autoscaling_group.bar", &group), testAccCheckAWSLaunchConfigurationExists("aws_launch_configuration.new", &lc), + testAccCheckAutoscalingLatestInstanceRefreshState(&group, 0, 0, nil), resource.TestCheckResourceAttr( "aws_autoscaling_group.bar", "desired_capacity", "5"), resource.TestCheckResourceAttr( @@ -4250,3 +4253,361 @@ resource "aws_autoscaling_group" "test" { } `, rName) } + +func TestAccAWSAutoScalingGroup_InstanceRefresh_Enabled(t *testing.T) { + var group autoscaling.Group + resourceName := "aws_autoscaling_group.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSAutoScalingGroupDestroy, + Steps: []resource.TestStep{ + { + // check that an instance refresh isn't started by a new asg + Config: testAccAwsAutoScalingGroup_InstanceRefresh_Enabled("Alpha", true, 1), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSAutoScalingGroupExists(resourceName, &group), + resource.TestCheckResourceAttr(resourceName, "min_size", "1"), + resource.TestCheckResourceAttr(resourceName, "max_size", "2"), + resource.TestCheckResourceAttr(resourceName, "desired_capacity", "1"), + testAccCheckAutoscalingLatestInstanceRefreshState(&group, 0, 0, nil), + resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.instance_warmup_seconds", "-1"), + resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.min_healthy_percentage", "90"), + resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.strategy", "Rolling"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "force_delete", + "wait_for_capacity_timeout", + "instance_refresh", + }, + }, + { + // check that changing asg size doesn't trigger a refresh + Config: testAccAwsAutoScalingGroup_InstanceRefresh_Enabled("Alpha", false, 2), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSAutoScalingGroupExists(resourceName, &group), + resource.TestCheckResourceAttr(resourceName, "min_size", "2"), + resource.TestCheckResourceAttr(resourceName, "max_size", "4"), + resource.TestCheckResourceAttr(resourceName, "desired_capacity", "2"), + testAccCheckAutoscalingLatestInstanceRefreshState(&group, 0, 0, nil), + ), + }, + { + // check that changing propagated tags triggers a refresh + Config: testAccAwsAutoScalingGroup_InstanceRefresh_Enabled("Bravo", false, 1), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSAutoScalingGroupExists(resourceName, &group), + resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.instance_warmup_seconds", "10"), + resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.min_healthy_percentage", "50"), + resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.strategy", "Rolling"), + testAccCheckAutoscalingLatestInstanceRefreshState(&group, 1, 0, []string{ + autoscaling.InstanceRefreshStatusPending, + autoscaling.InstanceRefreshStatusInProgress, + }), + ), + }, + { + // check that an active refresh is cancelled in favour of a new one + Config: testAccAwsAutoScalingGroup_InstanceRefresh_Enabled("Charlie", false, 1), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSAutoScalingGroupExists(resourceName, &group), + testAccCheckAutoscalingLatestInstanceRefreshState(&group, 2, 1, []string{ + autoscaling.InstanceRefreshStatusCancelled, + }), + testAccCheckAutoscalingLatestInstanceRefreshState(&group, 2, 0, []string{ + autoscaling.InstanceRefreshStatusPending, + autoscaling.InstanceRefreshStatusInProgress, + }), + ), + }, + }, + }) +} + +func testAccAwsAutoScalingGroup_InstanceRefresh_Enabled( + tagValue string, + defaults bool, + sizeFactor int, +) string { + preference := `` + if !defaults { + preference = ` + min_healthy_percentage = 50 + instance_warmup_seconds = 10` + } + + return fmt.Sprintf(` +locals { + size_factor = %[3]d +} + +data "aws_ami" "test" { + most_recent = true + owners = ["amazon"] + + filter { + name = "name" + values = ["amzn-ami-hvm-*-x86_64-gp2"] + } +} + +data "aws_availability_zones" "current" { + state = "available" + + filter { + name = "opt-in-status" + values = ["opt-in-not-required"] + } +} + +resource "aws_launch_configuration" "test" { + image_id = "${data.aws_ami.test.id}" + instance_type = "t3.nano" +} + +resource "aws_autoscaling_group" "test" { + availability_zones = [data.aws_availability_zones.current.names[0]] + max_size = 2 * local.size_factor + min_size = 1 * local.size_factor + desired_capacity = 1 * local.size_factor + launch_configuration = "${aws_launch_configuration.test.name}" + + tag { + key = "Test" + value = %[1]q + propagate_at_launch = true + } + + instance_refresh { + strategy = "Rolling" +%[2]s + } +} +`, + tagValue, + preference, + sizeFactor) +} + +func TestAccAWSAutoScalingGroup_InstanceRefresh_Triggers(t *testing.T) { + // note: propagated tags have been implicitly checked + // by TestAccAWSAutoScalingGroup_InstanceRefresh_Enabled + + matrix := []struct { + AvailabilityZoneCount int + SubnetCount int + InstanceType string + UseLaunchConfiguration bool + UseLaunchTemplate bool + UseMixedInstancesPolicy bool + UsePlacementGroup bool + ExpectRefreshCount int + }{ + {2, 0, "t3.nano", true, false, false, false, 0}, // create asg + {1, 0, "t3.nano", true, false, false, false, 1}, // drop 1 subnet + {0, 2, "t3.nano", true, false, false, false, 2}, // add 2 vpcs, drop subnets + {0, 1, "t3.nano", true, false, false, false, 3}, // drop 1 vpc + {0, 1, "t3.nano", false, true, false, false, 4}, // drop launch config, add template + {0, 1, "t3.micro", false, true, false, false, 5}, // update template + {0, 1, "t3.micro", false, false, true, false, 6}, // drop template, add mixed policy + {0, 1, "t3.nano", false, false, true, false, 7}, // update mixed policy + {0, 1, "t3.nano", false, false, true, true, 8}, // use placement group + } + + var group autoscaling.Group + resourceName := "aws_autoscaling_group.test" + placementGroupName := fmt.Sprintf("tf-test-%s", acctest.RandString(8)) + + steps := make([]resource.TestStep, len(matrix)) + for i, test := range matrix { + steps[i] = resource.TestStep{ + Config: testAccAwsAutoScalingGroup_InstanceRefresh_Triggers( + test.AvailabilityZoneCount, + test.SubnetCount, + test.InstanceType, + test.UseLaunchConfiguration, + test.UseLaunchTemplate, + test.UseMixedInstancesPolicy, + test.UsePlacementGroup, + placementGroupName, + ), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSAutoScalingGroupExists(resourceName, &group), + testAccCheckAutoscalingLatestInstanceRefreshState(&group, test.ExpectRefreshCount, 0, nil), + ), + } + } + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSAutoScalingGroupDestroy, + Steps: steps, + }) +} + +func testAccAwsAutoScalingGroup_InstanceRefresh_Triggers( + availabilityZoneCount int, + subnetCount int, + instanceType string, + useLaunchConfiguration bool, + useLaunchTemplate bool, + useMixedInstancesPolicy bool, + usePlacementGroup bool, + placementGroupName string, +) string { + return fmt.Sprintf(` +locals { + availability_zone_count = %[1]d + subnet_count = %[2]d + instance_type = %[3]q + use_launch_configuration = %[4]v + use_launch_template = %[5]v + use_mixed_instances_policy = %[6]v + use_placement_group = %[7]v + placement_group_name = %[8]q +} + +data "aws_ami" "test" { + most_recent = true + owners = ["amazon"] + + filter { + name = "name" + values = ["amzn-ami-hvm-*-x86_64-gp2"] + } +} + +data "aws_availability_zones" "current" { + state = "available" + + filter { + name = "opt-in-status" + values = ["opt-in-not-required"] + } +} + +resource "aws_default_subnet" "current" { + count = length(data.aws_availability_zones.current.names) + availability_zone = data.aws_availability_zones.current.names[count.index] +} + +resource "aws_launch_configuration" "test" { + image_id = "${data.aws_ami.test.id}" + instance_type = local.instance_type +} + +resource "aws_launch_template" "test" { + image_id = data.aws_ami.test.image_id + instance_type = local.instance_type +} + +resource "aws_placement_group" "test" { + name = local.placement_group_name + strategy = "cluster" +} + +resource "aws_autoscaling_group" "test" { + availability_zones = local.availability_zone_count > 0 ? slice(data.aws_availability_zones.current.names, 0, local.availability_zone_count) : null + max_size = 1 + min_size = 1 + desired_capacity = 1 + launch_configuration = local.use_launch_configuration ? aws_launch_configuration.test.name : null + vpc_zone_identifier = local.subnet_count > 0 ? slice(aws_default_subnet.current.*.id, 0, local.subnet_count) : null + placement_group = local.use_placement_group ? aws_placement_group.test.name : null + + dynamic "launch_template" { + for_each = local.use_launch_template ? [1] : [] + content { + id = aws_launch_template.test.id + version = aws_launch_template.test.latest_version + } + } + + dynamic "mixed_instances_policy" { + for_each = local.use_mixed_instances_policy ? [1] : [] + content { + launch_template { + launch_template_specification { + launch_template_id = aws_launch_template.test.id + version = aws_launch_template.test.latest_version + } + } + } + } + + instance_refresh { + strategy = "Rolling" + } +} +`, + availabilityZoneCount, + subnetCount, + instanceType, + useLaunchConfiguration, + useLaunchTemplate, + useMixedInstancesPolicy, + usePlacementGroup, + placementGroupName, + ) +} + +// testAccCheckAutoscalingLatestInstanceRefreshState checks the Instance Refreshes +// of an Auto-Scaling Group. +// +// Use length to set the number of refreshes (of any state) that are expected. +// +// Use the offset parameter to choose the instance refresh to check. Offset 0 +// is the latest refresh, with negative offsets yielding successive elements. +// When length is 0, this argument has no effect. +// +// When length is greater than 0 and acceptedStatuses is non-nil, expect the +// refresh at the given offset to have one of the given accepted statuses. +func testAccCheckAutoscalingLatestInstanceRefreshState( + group *autoscaling.Group, + length int, + offset int, + acceptedStatuses []string, +) resource.TestCheckFunc { + return func(state *terraform.State) error { + conn := testAccProvider.Meta().(*AWSClient).autoscalingconn + name := aws.StringValue(group.AutoScalingGroupName) + input := autoscaling.DescribeInstanceRefreshesInput{ + AutoScalingGroupName: aws.String(name), + } + + output, err := conn.DescribeInstanceRefreshes(&input) + switch { + case err != nil: + return err + case len(output.InstanceRefreshes) != length: + return fmt.Errorf("expected %d instance refreshes, but found %d", length, len(output.InstanceRefreshes)) + } + + switch { + case length == 0: + return nil + case len(acceptedStatuses) == 0: + return nil + } + + status := aws.StringValue(output.InstanceRefreshes[offset].Status) + for _, acceptedStatus := range acceptedStatuses { + if status == acceptedStatus { + return nil + } + } + + return fmt.Errorf( + "expected status of refresh at offset %d to be one of %s, got %s", + offset, + strings.Join(acceptedStatuses, " or "), + status) + } +} diff --git a/website/docs/r/autoscaling_group.html.markdown b/website/docs/r/autoscaling_group.html.markdown index a2c582ad2f9..fd679932a54 100644 --- a/website/docs/r/autoscaling_group.html.markdown +++ b/website/docs/r/autoscaling_group.html.markdown @@ -123,7 +123,7 @@ resource "aws_autoscaling_group" "example" { } ``` -## Interpolated tags +### Interpolated tags ```hcl variable "extra_tags" { @@ -158,6 +158,42 @@ resource "aws_autoscaling_group" "bar" { } ``` +### Automatically refresh all instances after the group is updated + +```hcl +data "aws_ami" "example" { + most_recent = true + owners = ["amazon"] + + filter { + name = "name" + values = ["amzn-ami-hvm-*-x86_64-gp2"] + } +} + +resource "aws_launch_template" "example" { + image_id = "${data.aws_ami.example.id}" + instance_type = "t3.nano" +} + +resource "aws_autoscaling_group" "example" { + availability_zones = ["us-east-1a"] + desired_capacity = 1 + max_size = 2 + min_size = 1 + + launch_template { + id = "${aws_launch_template.example.id}" + version = "${aws_launch_template.example.latest_version}" + } + + instance_refresh { + strategy = "Rolling" + min_healthy_percentage = 50 + } +} +``` + ## Argument Reference The following arguments are supported: @@ -221,6 +257,9 @@ Note that if you suspend either the `Launch` or `Terminate` process types, it ca during scale in events. * `service_linked_role_arn` (Optional) The ARN of the service-linked role that the ASG will use to call other AWS services * `max_instance_lifetime` (Optional) The maximum amount of time, in seconds, that an instance can be in service, values must be either equal to 0 or between 604800 and 31536000 seconds. +* `instance_refresh` - (Optional) If this block is configured, start an + [Instance Refresh](https://docs.aws.amazon.com/autoscaling/ec2/userguide/asg-instance-refresh.html) + when this autoscaling group is updated. Defined below. ### launch_template @@ -288,6 +327,25 @@ This allows the construction of dynamic lists of tags which is not possible usin ~> **NOTE:** Other AWS APIs may automatically add special tags to their associated Auto Scaling Group for management purposes, such as ECS Capacity Providers adding the `AmazonECSManaged` tag. To ignore the removal of these automatic tags, see the [`ignore_tags` provider configuration](https://www.terraform.io/docs/providers/aws/index.html#ignore_tags) or the [`ignore_changes` lifecycle argument for Terraform resources](https://www.terraform.io/docs/configuration/resources.html#ignore_changes). +### instance_refresh + +This configuration block supports the following: + +* `instance_warmup_seconds` - (Optional) The number of seconds until a newly launched + instance is configured and ready to use. Default behavior (set with `-1` or `null`) + is to match the autoscaling group's health check grace period. +* `min_healthy_percentage` - (Optional) The amount of capacity in the Auto Scaling group + that must remain healthy during an instance refresh to allow the operation to continue, + as a percentage of the desired capacity of the Auto Scaling group. Defaults to `90`. +* `strategy` - (Required) The strategy to use for instance refresh. The only allowed + value is `"Rolling"`. See [StartInstanceRefresh Action](https://docs.aws.amazon.com/autoscaling/ec2/APIReference/API_StartInstanceRefresh.html#API_StartInstanceRefresh_RequestParameters) for more information. + +~> **NOTE:** A refresh is only started when any of the following autoscaling group properties change: `launch_configuration`, `launch_template`, `mixed_instances_policy`, `vpc_zone_identifier`, `availability_zones`, `placement_group`, or any `tag` or `tags` configured to propagate at launch. + +~> **NOTE:** Autoscaling groups support up to one active instance refresh at a time. When this resource is updated, any existing refresh is cancelled. + +~> **NOTE:** Depending on health check settings and group size, an instance refresh may take a long time or fail. This resource does not wait for the instance refresh to complete. + ## Attributes Reference In addition to all arguments above, the following attributes are exported: From 908c553eb96ea387d16f0f32abbe75dc34a41c10 Mon Sep 17 00:00:00 2001 From: Roberth Kulbin Date: Fri, 26 Jun 2020 13:55:21 +0100 Subject: [PATCH 02/12] resource/aws_autoscaling_group: fix typos in test comments --- aws/resource_aws_autoscaling_group_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/aws/resource_aws_autoscaling_group_test.go b/aws/resource_aws_autoscaling_group_test.go index c4710fdcf18..d3be987dd33 100644 --- a/aws/resource_aws_autoscaling_group_test.go +++ b/aws/resource_aws_autoscaling_group_test.go @@ -4409,10 +4409,10 @@ func TestAccAWSAutoScalingGroup_InstanceRefresh_Triggers(t *testing.T) { UsePlacementGroup bool ExpectRefreshCount int }{ - {2, 0, "t3.nano", true, false, false, false, 0}, // create asg - {1, 0, "t3.nano", true, false, false, false, 1}, // drop 1 subnet - {0, 2, "t3.nano", true, false, false, false, 2}, // add 2 vpcs, drop subnets - {0, 1, "t3.nano", true, false, false, false, 3}, // drop 1 vpc + {2, 0, "t3.nano", true, false, false, false, 0}, // create asg with 2 az-s + {1, 0, "t3.nano", true, false, false, false, 1}, // drop 1 az + {0, 2, "t3.nano", true, false, false, false, 2}, // add 2 subnets, drop az-s + {0, 1, "t3.nano", true, false, false, false, 3}, // drop 1 subnet {0, 1, "t3.nano", false, true, false, false, 4}, // drop launch config, add template {0, 1, "t3.micro", false, true, false, false, 5}, // update template {0, 1, "t3.micro", false, false, true, false, 6}, // drop template, add mixed policy From e9afdc81bf71c6e444a9e2c79d99fb6acdbb6160 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Wed, 25 Nov 2020 17:20:44 -0800 Subject: [PATCH 03/12] Reformats tests --- aws/resource_aws_autoscaling_group_test.go | 429 ++++++++++----------- 1 file changed, 207 insertions(+), 222 deletions(-) diff --git a/aws/resource_aws_autoscaling_group_test.go b/aws/resource_aws_autoscaling_group_test.go index 87b7d3aa6ac..aa7e2f54efd 100644 --- a/aws/resource_aws_autoscaling_group_test.go +++ b/aws/resource_aws_autoscaling_group_test.go @@ -553,8 +553,7 @@ func TestAccAWSAutoScalingGroup_withPlacementGroup(t *testing.T) { Config: testAccAWSAutoScalingGroupConfig_withPlacementGroup(randName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSAutoScalingGroupExists("aws_autoscaling_group.bar", &group), - resource.TestCheckResourceAttr( - "aws_autoscaling_group.bar", "placement_group", randName), + resource.TestCheckResourceAttr("aws_autoscaling_group.bar", "placement_group", randName), ), }, { @@ -996,6 +995,153 @@ func TestAccAWSAutoScalingGroup_ALB_TargetGroups_ELBCapacity(t *testing.T) { }) } +func TestAccAWSAutoScalingGroup_InstanceRefresh_Enabled(t *testing.T) { + var group autoscaling.Group + resourceName := "aws_autoscaling_group.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSAutoScalingGroupDestroy, + Steps: []resource.TestStep{ + { + // check that an instance refresh isn't started by a new asg + Config: testAccAwsAutoScalingGroup_InstanceRefresh_Enabled("Alpha", true, 1), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSAutoScalingGroupExists(resourceName, &group), + resource.TestCheckResourceAttr(resourceName, "min_size", "1"), + resource.TestCheckResourceAttr(resourceName, "max_size", "2"), + resource.TestCheckResourceAttr(resourceName, "desired_capacity", "1"), + testAccCheckAutoscalingLatestInstanceRefreshState(&group, 0, 0, nil), + resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.instance_warmup_seconds", "-1"), + resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.min_healthy_percentage", "90"), + resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.strategy", "Rolling"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "force_delete", + "wait_for_capacity_timeout", + "instance_refresh", + }, + }, + { + // check that changing asg size doesn't trigger a refresh + Config: testAccAwsAutoScalingGroup_InstanceRefresh_Enabled("Alpha", false, 2), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSAutoScalingGroupExists(resourceName, &group), + resource.TestCheckResourceAttr(resourceName, "min_size", "2"), + resource.TestCheckResourceAttr(resourceName, "max_size", "4"), + resource.TestCheckResourceAttr(resourceName, "desired_capacity", "2"), + testAccCheckAutoscalingLatestInstanceRefreshState(&group, 0, 0, nil), + ), + }, + { + // check that changing tags doesn't trigger a refresh + Config: testAccAwsAutoScalingGroup_InstanceRefresh_Enabled("Bravo", false, 1), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSAutoScalingGroupExists(resourceName, &group), + resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.instance_warmup_seconds", "10"), + resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.min_healthy_percentage", "50"), + resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.strategy", "Rolling"), + testAccCheckAutoscalingLatestInstanceRefreshState(&group, 0, 0, nil), + ), + }, + // TODO: check that an active refresh is cancelled in favour of a new one + }, + }) +} + +func TestAccAWSAutoScalingGroup_InstanceRefresh_Triggers(t *testing.T) { + matrix := []struct { + AvailabilityZoneCount int + SubnetCount int + InstanceType string + UseLaunchConfiguration bool + UseLaunchTemplate bool + UseMixedInstancesPolicy bool + UsePlacementGroup bool + ExpectRefreshCount int + }{ + {2, 0, "t3.nano", true, false, false, false, 0}, // create asg with 2 az-s + {1, 0, "t3.nano", true, false, false, false, 1}, // drop 1 az + {0, 2, "t3.nano", true, false, false, false, 2}, // add 2 subnets, drop az-s + {0, 1, "t3.nano", true, false, false, false, 3}, // drop 1 subnet + {0, 1, "t3.nano", false, true, false, false, 4}, // drop launch config, add template + {0, 1, "t3.micro", false, true, false, false, 5}, // update template + {0, 1, "t3.micro", false, false, true, false, 6}, // drop template, add mixed policy + {0, 1, "t3.nano", false, false, true, false, 7}, // update mixed policy + {0, 1, "t3.nano", false, false, true, true, 8}, // use placement group + } + + var group autoscaling.Group + resourceName := "aws_autoscaling_group.test" + placementGroupName := fmt.Sprintf("tf-test-%s", acctest.RandString(8)) + + steps := make([]resource.TestStep, len(matrix)) + for i, test := range matrix { + steps[i] = resource.TestStep{ + Config: testAccAwsAutoScalingGroup_InstanceRefresh_Triggers( + test.AvailabilityZoneCount, + test.SubnetCount, + test.InstanceType, + test.UseLaunchConfiguration, + test.UseLaunchTemplate, + test.UseMixedInstancesPolicy, + test.UsePlacementGroup, + placementGroupName, + ), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSAutoScalingGroupExists(resourceName, &group), + testAccCheckAutoscalingLatestInstanceRefreshState(&group, test.ExpectRefreshCount, 0, nil), + ), + } + } + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSAutoScalingGroupDestroy, + Steps: steps, + }) +} + +func testAccCheckAWSAutoScalingGroupExists(n string, group *autoscaling.Group) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[n] + if !ok { + return fmt.Errorf("Not found: %s", n) + } + + if rs.Primary.ID == "" { + return fmt.Errorf("No AutoScaling Group ID is set") + } + + conn := testAccProvider.Meta().(*AWSClient).autoscalingconn + + describeGroups, err := conn.DescribeAutoScalingGroups( + &autoscaling.DescribeAutoScalingGroupsInput{ + AutoScalingGroupNames: []*string{aws.String(rs.Primary.ID)}, + }) + + if err != nil { + return err + } + + if len(describeGroups.AutoScalingGroups) != 1 || + *describeGroups.AutoScalingGroups[0].AutoScalingGroupName != rs.Primary.ID { + return fmt.Errorf("AutoScaling Group not found") + } + + *group = *describeGroups.AutoScalingGroups[0] + + return nil + } +} + func testAccCheckAWSAutoScalingGroupDestroy(s *terraform.State) error { conn := testAccProvider.Meta().(*AWSClient).autoscalingconn @@ -1089,39 +1235,6 @@ func testAccCheckAWSAutoScalingGroupAttributesLoadBalancer(group *autoscaling.Gr } } -func testAccCheckAWSAutoScalingGroupExists(n string, group *autoscaling.Group) resource.TestCheckFunc { - return func(s *terraform.State) error { - rs, ok := s.RootModule().Resources[n] - if !ok { - return fmt.Errorf("Not found: %s", n) - } - - if rs.Primary.ID == "" { - return fmt.Errorf("No AutoScaling Group ID is set") - } - - conn := testAccProvider.Meta().(*AWSClient).autoscalingconn - - describeGroups, err := conn.DescribeAutoScalingGroups( - &autoscaling.DescribeAutoScalingGroupsInput{ - AutoScalingGroupNames: []*string{aws.String(rs.Primary.ID)}, - }) - - if err != nil { - return err - } - - if len(describeGroups.AutoScalingGroups) != 1 || - *describeGroups.AutoScalingGroups[0].AutoScalingGroupName != rs.Primary.ID { - return fmt.Errorf("AutoScaling Group not found") - } - - *group = *describeGroups.AutoScalingGroups[0] - - return nil - } -} - func testLaunchConfigurationName(n string, lc *autoscaling.LaunchConfiguration) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] @@ -4156,66 +4269,6 @@ resource "aws_autoscaling_group" "test" { `, rName) } -func TestAccAWSAutoScalingGroup_InstanceRefresh_Enabled(t *testing.T) { - var group autoscaling.Group - resourceName := "aws_autoscaling_group.test" - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccCheckAWSAutoScalingGroupDestroy, - Steps: []resource.TestStep{ - { - // check that an instance refresh isn't started by a new asg - Config: testAccAwsAutoScalingGroup_InstanceRefresh_Enabled("Alpha", true, 1), - Check: resource.ComposeTestCheckFunc( - testAccCheckAWSAutoScalingGroupExists(resourceName, &group), - resource.TestCheckResourceAttr(resourceName, "min_size", "1"), - resource.TestCheckResourceAttr(resourceName, "max_size", "2"), - resource.TestCheckResourceAttr(resourceName, "desired_capacity", "1"), - testAccCheckAutoscalingLatestInstanceRefreshState(&group, 0, 0, nil), - resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.instance_warmup_seconds", "-1"), - resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.min_healthy_percentage", "90"), - resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.strategy", "Rolling"), - ), - }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{ - "force_delete", - "wait_for_capacity_timeout", - "instance_refresh", - }, - }, - { - // check that changing asg size doesn't trigger a refresh - Config: testAccAwsAutoScalingGroup_InstanceRefresh_Enabled("Alpha", false, 2), - Check: resource.ComposeTestCheckFunc( - testAccCheckAWSAutoScalingGroupExists(resourceName, &group), - resource.TestCheckResourceAttr(resourceName, "min_size", "2"), - resource.TestCheckResourceAttr(resourceName, "max_size", "4"), - resource.TestCheckResourceAttr(resourceName, "desired_capacity", "2"), - testAccCheckAutoscalingLatestInstanceRefreshState(&group, 0, 0, nil), - ), - }, - { - // check that changing propagated tags doesn't trigger a refresh - Config: testAccAwsAutoScalingGroup_InstanceRefresh_Enabled("Bravo", false, 1), - Check: resource.ComposeTestCheckFunc( - testAccCheckAWSAutoScalingGroupExists(resourceName, &group), - resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.instance_warmup_seconds", "10"), - resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.min_healthy_percentage", "50"), - resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.strategy", "Rolling"), - testAccCheckAutoscalingLatestInstanceRefreshState(&group, 0, 0, nil), - ), - }, - // TODO: check that an active refresh is cancelled in favour of a new one - }, - }) -} - func testAccAwsAutoScalingGroup_InstanceRefresh_Enabled( tagValue string, defaults bool, @@ -4229,6 +4282,25 @@ func testAccAwsAutoScalingGroup_InstanceRefresh_Enabled( } return fmt.Sprintf(` +resource "aws_autoscaling_group" "test" { + availability_zones = [data.aws_availability_zones.current.names[0]] + max_size = 2 * local.size_factor + min_size = 1 * local.size_factor + desired_capacity = 1 * local.size_factor + launch_configuration = aws_launch_configuration.test.name + + tag { + key = "Test" + value = %[1]q + propagate_at_launch = true + } + + instance_refresh { + strategy = "Rolling" +%[2]s + } +} + locals { size_factor = %[3]d } @@ -4256,86 +4328,7 @@ resource "aws_launch_configuration" "test" { image_id = data.aws_ami.test.id instance_type = "t3.nano" } - -resource "aws_autoscaling_group" "test" { - availability_zones = [data.aws_availability_zones.current.names[0]] - max_size = 2 * local.size_factor - min_size = 1 * local.size_factor - desired_capacity = 1 * local.size_factor - launch_configuration = aws_launch_configuration.test.name - - tag { - key = "Test" - value = %[1]q - propagate_at_launch = true - } - - instance_refresh { - strategy = "Rolling" -%[2]s - } -} -`, - tagValue, - preference, - sizeFactor) -} - -func TestAccAWSAutoScalingGroup_InstanceRefresh_Triggers(t *testing.T) { - // note: propagated tags have been implicitly checked - // by TestAccAWSAutoScalingGroup_InstanceRefresh_Enabled - - matrix := []struct { - AvailabilityZoneCount int - SubnetCount int - InstanceType string - UseLaunchConfiguration bool - UseLaunchTemplate bool - UseMixedInstancesPolicy bool - UsePlacementGroup bool - ExpectRefreshCount int - }{ - {2, 0, "t3.nano", true, false, false, false, 0}, // create asg with 2 az-s - {1, 0, "t3.nano", true, false, false, false, 1}, // drop 1 az - {0, 2, "t3.nano", true, false, false, false, 2}, // add 2 subnets, drop az-s - {0, 1, "t3.nano", true, false, false, false, 3}, // drop 1 subnet - {0, 1, "t3.nano", false, true, false, false, 4}, // drop launch config, add template - {0, 1, "t3.micro", false, true, false, false, 5}, // update template - {0, 1, "t3.micro", false, false, true, false, 6}, // drop template, add mixed policy - {0, 1, "t3.nano", false, false, true, false, 7}, // update mixed policy - {0, 1, "t3.nano", false, false, true, true, 8}, // use placement group - } - - var group autoscaling.Group - resourceName := "aws_autoscaling_group.test" - placementGroupName := fmt.Sprintf("tf-test-%s", acctest.RandString(8)) - - steps := make([]resource.TestStep, len(matrix)) - for i, test := range matrix { - steps[i] = resource.TestStep{ - Config: testAccAwsAutoScalingGroup_InstanceRefresh_Triggers( - test.AvailabilityZoneCount, - test.SubnetCount, - test.InstanceType, - test.UseLaunchConfiguration, - test.UseLaunchTemplate, - test.UseMixedInstancesPolicy, - test.UsePlacementGroup, - placementGroupName, - ), - Check: resource.ComposeTestCheckFunc( - testAccCheckAWSAutoScalingGroupExists(resourceName, &group), - testAccCheckAutoscalingLatestInstanceRefreshState(&group, test.ExpectRefreshCount, 0, nil), - ), - } - } - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccCheckAWSAutoScalingGroupDestroy, - Steps: steps, - }) +`, tagValue, preference, sizeFactor) } func testAccAwsAutoScalingGroup_InstanceRefresh_Triggers( @@ -4349,6 +4342,40 @@ func testAccAwsAutoScalingGroup_InstanceRefresh_Triggers( placementGroupName string, ) string { return fmt.Sprintf(` +resource "aws_autoscaling_group" "test" { + availability_zones = local.availability_zone_count > 0 ? slice(data.aws_availability_zones.current.names, 0, local.availability_zone_count) : null + max_size = 1 + min_size = 1 + desired_capacity = 1 + launch_configuration = local.use_launch_configuration ? aws_launch_configuration.test.name : null + vpc_zone_identifier = local.subnet_count > 0 ? slice(aws_subnet.test.*.id, 0, local.subnet_count) : null + placement_group = local.use_placement_group ? aws_placement_group.test.name : null + + dynamic "launch_template" { + for_each = local.use_launch_template ? [1] : [] + content { + id = aws_launch_template.test.id + version = aws_launch_template.test.latest_version + } + } + + dynamic "mixed_instances_policy" { + for_each = local.use_mixed_instances_policy ? [1] : [] + content { + launch_template { + launch_template_specification { + launch_template_id = aws_launch_template.test.id + version = aws_launch_template.test.latest_version + } + } + } + } + + instance_refresh { + strategy = "Rolling" + } +} + locals { availability_zone_count = %[1]d subnet_count = %[2]d @@ -4379,6 +4406,10 @@ data "aws_availability_zones" "current" { } } +resource "aws_vpc" "test" { + cidr_block = "10.0.0.0/16" +} + resource "aws_subnet" "test" { count = length(data.aws_availability_zones.current.names) availability_zone = data.aws_availability_zones.current.names[count.index] @@ -4400,53 +4431,7 @@ resource "aws_placement_group" "test" { name = local.placement_group_name strategy = "cluster" } - -resource "aws_autoscaling_group" "test" { - availability_zones = local.availability_zone_count > 0 ? slice(data.aws_availability_zones.current.names, 0, local.availability_zone_count) : null - max_size = 1 - min_size = 1 - desired_capacity = 1 - launch_configuration = local.use_launch_configuration ? aws_launch_configuration.test.name : null - vpc_zone_identifier = local.subnet_count > 0 ? slice(aws_subnet.test.*.id, 0, local.subnet_count) : null - placement_group = local.use_placement_group ? aws_placement_group.test.name : null - - dynamic "launch_template" { - for_each = local.use_launch_template ? [1] : [] - content { - id = aws_launch_template.test.id - version = aws_launch_template.test.latest_version - } - } - - dynamic "mixed_instances_policy" { - for_each = local.use_mixed_instances_policy ? [1] : [] - content { - launch_template { - launch_template_specification { - launch_template_id = aws_launch_template.test.id - version = aws_launch_template.test.latest_version - } - } - } - } - - instance_refresh { - strategy = "Rolling" - } -} - -resource "aws_vpc" "test" { - cidr_block = "10.0.0.0/16" -} -`, availabilityZoneCount, - subnetCount, - instanceType, - useLaunchConfiguration, - useLaunchTemplate, - useMixedInstancesPolicy, - usePlacementGroup, - placementGroupName, - ) +`, availabilityZoneCount, subnetCount, instanceType, useLaunchConfiguration, useLaunchTemplate, useMixedInstancesPolicy, usePlacementGroup, placementGroupName) } // testAccCheckAutoscalingLatestInstanceRefreshState checks the Instance Refreshes From 468a499f215c146518b3463f82db9fd9973e5994 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 26 Nov 2020 17:54:40 -0800 Subject: [PATCH 04/12] Fixes instance refresh error --- .../service/autoscaling/waiter/status.go | 28 +++++ .../service/autoscaling/waiter/waiter.go | 50 +++++++++ aws/resource_aws_autoscaling_group.go | 101 ++++++++---------- 3 files changed, 121 insertions(+), 58 deletions(-) create mode 100644 aws/internal/service/autoscaling/waiter/status.go create mode 100644 aws/internal/service/autoscaling/waiter/waiter.go diff --git a/aws/internal/service/autoscaling/waiter/status.go b/aws/internal/service/autoscaling/waiter/status.go new file mode 100644 index 00000000000..a7bd32d56e0 --- /dev/null +++ b/aws/internal/service/autoscaling/waiter/status.go @@ -0,0 +1,28 @@ +package waiter + +import ( + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/autoscaling" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" +) + +func InstanceRefreshStatus(conn *autoscaling.AutoScaling, asgName, instanceRefreshId string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + input := autoscaling.DescribeInstanceRefreshesInput{ + AutoScalingGroupName: aws.String(asgName), + InstanceRefreshIds: []*string{aws.String(instanceRefreshId)}, + } + output, err := conn.DescribeInstanceRefreshes(&input) + if err != nil { + return nil, "", err + } + + if output == nil || len(output.InstanceRefreshes) == 0 || output.InstanceRefreshes[0] == nil { + return nil, "", nil + } + + instanceRefresh := output.InstanceRefreshes[0] + + return instanceRefresh, aws.StringValue(instanceRefresh.Status), nil + } +} diff --git a/aws/internal/service/autoscaling/waiter/waiter.go b/aws/internal/service/autoscaling/waiter/waiter.go new file mode 100644 index 00000000000..b4bf99543d8 --- /dev/null +++ b/aws/internal/service/autoscaling/waiter/waiter.go @@ -0,0 +1,50 @@ +package waiter + +import ( + "time" + + "github.com/aws/aws-sdk-go/service/autoscaling" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" +) + +const ( + // Maximum amount of time to wait for an InstanceRefresh to be Successful + InstanceRefreshSuccessfulTimeout = 5 * time.Minute + + // Maximum amount of time to wait for an InstanceRefresh to be Cancelled + InstanceRefreshCancelledTimeout = 5 * time.Minute +) + +func InstanceRefreshSuccessful(conn *autoscaling.AutoScaling, asgName, instanceRefreshId string) (*autoscaling.InstanceRefresh, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{autoscaling.InstanceRefreshStatusPending, autoscaling.InstanceRefreshStatusInProgress}, + Target: []string{autoscaling.InstanceRefreshStatusSuccessful}, + Refresh: InstanceRefreshStatus(conn, asgName, instanceRefreshId), + Timeout: InstanceRefreshSuccessfulTimeout, + } + + outputRaw, err := stateConf.WaitForState() + + if v, ok := outputRaw.(*autoscaling.InstanceRefresh); ok { + return v, err + } + + return nil, err +} + +func InstanceRefreshCancelled(conn *autoscaling.AutoScaling, asgName, instanceRefreshId string) (*autoscaling.InstanceRefresh, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{autoscaling.InstanceRefreshStatusPending, autoscaling.InstanceRefreshStatusInProgress, autoscaling.InstanceRefreshStatusCancelling}, + Target: []string{autoscaling.InstanceRefreshStatusCancelled}, + Refresh: InstanceRefreshStatus(conn, asgName, instanceRefreshId), + Timeout: InstanceRefreshCancelledTimeout, + } + + outputRaw, err := stateConf.WaitForState() + + if v, ok := outputRaw.(*autoscaling.InstanceRefresh); ok { + return v, err + } + + return nil, err +} diff --git a/aws/resource_aws_autoscaling_group.go b/aws/resource_aws_autoscaling_group.go index 856e5463349..186b329072d 100644 --- a/aws/resource_aws_autoscaling_group.go +++ b/aws/resource_aws_autoscaling_group.go @@ -15,12 +15,14 @@ import ( "github.com/aws/aws-sdk-go/service/autoscaling" "github.com/aws/aws-sdk-go/service/elb" "github.com/aws/aws-sdk-go/service/elbv2" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/terraform-providers/terraform-provider-aws/aws/internal/hashcode" "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/autoscaling/waiter" ) const ( @@ -492,11 +494,9 @@ func resourceAwsAutoscalingGroup() *schema.Resource { ValidateFunc: validation.IntBetween(0, 100), }, "strategy": { - Type: schema.TypeString, - Required: true, - ValidateFunc: validation.StringInSlice( - []string{autoscaling.RefreshStrategyRolling}, - false), + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.StringInSlice(autoscaling.RefreshStrategy_Values(), false), }, }, }, @@ -854,6 +854,7 @@ func resourceAwsAutoscalingGroupRead(d *schema.ResourceData, meta interface{}) e return nil } +// TODO: make this a waiter function func waitUntilAutoscalingGroupLoadBalancerTargetGroupsRemoved(conn *autoscaling.AutoScaling, asgName string) error { input := &autoscaling.DescribeLoadBalancerTargetGroupsInput{ AutoScalingGroupName: aws.String(asgName), @@ -861,6 +862,7 @@ func waitUntilAutoscalingGroupLoadBalancerTargetGroupsRemoved(conn *autoscaling. var tgRemoving bool for { + // TODO: generate Pages function output, err := conn.DescribeLoadBalancerTargetGroups(input) if err != nil { @@ -890,6 +892,7 @@ func waitUntilAutoscalingGroupLoadBalancerTargetGroupsRemoved(conn *autoscaling. return nil } +// TODO: make this a waiter function func waitUntilAutoscalingGroupLoadBalancerTargetGroupsAdded(conn *autoscaling.AutoScaling, asgName string) error { input := &autoscaling.DescribeLoadBalancerTargetGroupsInput{ AutoScalingGroupName: aws.String(asgName), @@ -897,6 +900,7 @@ func waitUntilAutoscalingGroupLoadBalancerTargetGroupsAdded(conn *autoscaling.Au var tgAdding bool for { + // TODO: generate Pages function output, err := conn.DescribeLoadBalancerTargetGroups(input) if err != nil { @@ -962,7 +966,7 @@ func resourceAwsAutoscalingGroupUpdate(d *schema.ResourceData, meta interface{}) if d.HasChange("mixed_instances_policy") { opts.MixedInstancesPolicy = expandAutoScalingMixedInstancesPolicy(d.Get("mixed_instances_policy").([]interface{})) - // TODO: probably not + // TODO: optional trigger shouldRefreshInstances = true } @@ -991,7 +995,7 @@ func resourceAwsAutoscalingGroupUpdate(d *schema.ResourceData, meta interface{}) // TODO: this probably needs a wait for capacity if d.HasChange("vpc_zone_identifier") { opts.VPCZoneIdentifier = expandVpcZoneIdentifiers(d.Get("vpc_zone_identifier").(*schema.Set).List()) - // TODO: probably not + // TODO: no shouldRefreshInstances = true } @@ -1000,14 +1004,14 @@ func resourceAwsAutoscalingGroupUpdate(d *schema.ResourceData, meta interface{}) if v, ok := d.GetOk("availability_zones"); ok && v.(*schema.Set).Len() > 0 { opts.AvailabilityZones = expandStringList(v.(*schema.Set).List()) } - // TODO: probably not + // TODO: no shouldRefreshInstances = true } // TODO: does this need a wait for capacity? if d.HasChange("placement_group") { opts.PlacementGroup = aws.String(d.Get("placement_group").(string)) - // TODO: probably not + // TODO: optional trigger shouldRefreshInstances = true } @@ -1293,6 +1297,8 @@ func resourceAwsAutoscalingGroupDelete(d *schema.ResourceData, meta interface{}) return nil } +// TODO: make this a finder +// TODO: this should return a NotFoundError if not found func getAwsAutoscalingGroup(asgName string, conn *autoscaling.AutoScaling) (*autoscaling.Group, error) { describeOpts := autoscaling.DescribeAutoScalingGroupsInput{ AutoScalingGroupNames: []*string{aws.String(asgName)}, @@ -1753,6 +1759,7 @@ func flattenAutoScalingMixedInstancesPolicy(mixedInstancesPolicy *autoscaling.Mi return []interface{}{m} } +// TODO: make this a waiter function func waitUntilAutoscalingGroupLoadBalancersAdded(conn *autoscaling.AutoScaling, asgName string) error { input := &autoscaling.DescribeLoadBalancersInput{ AutoScalingGroupName: aws.String(asgName), @@ -1789,6 +1796,7 @@ func waitUntilAutoscalingGroupLoadBalancersAdded(conn *autoscaling.AutoScaling, return nil } +// TODO: make this a waiter function func waitUntilAutoscalingGroupLoadBalancersRemoved(conn *autoscaling.AutoScaling, asgName string) error { input := &autoscaling.DescribeLoadBalancersInput{ AutoScalingGroupName: aws.String(asgName), @@ -1796,6 +1804,7 @@ func waitUntilAutoscalingGroupLoadBalancersRemoved(conn *autoscaling.AutoScaling var lbRemoving bool for { + // TODO: generate Pages function output, err := conn.DescribeLoadBalancers(input) if err != nil { @@ -1853,76 +1862,52 @@ func startAutoscalingInstanceRefresh(d *schema.ResourceData, conn *autoscaling.A log.Printf("[DEBUG] Cancelling active refresh in ASG %s, if any...", asgName) - if err := cancelAutoscalingInstanceRefresh(d, conn); err != nil { + if err := cancelAutoscalingInstanceRefresh(conn, asgName); err != nil { // todo: add comment about subsequent ASG updates not picking up the refresh? - return fmt.Errorf("failed to cancel previous refresh: %s", err) + return fmt.Errorf("failed to cancel previous refresh: %w", err) } log.Printf("[DEBUG] Starting instance refresh in ASG %s...", asgName) - instanceRefreshId := "" - switch output, err := conn.StartInstanceRefresh(&input); { - case err != nil: + output, err := conn.StartInstanceRefresh(&input) + if err != nil { return err - default: - instanceRefreshId = aws.StringValue(output.InstanceRefreshId) } + instanceRefreshID := aws.StringValue(output.InstanceRefreshId) - log.Printf("[INFO] Started instance refresh %s in ASG %s", instanceRefreshId, asgName) + log.Printf("[INFO] Started instance refresh %s in ASG %s", instanceRefreshID, asgName) return nil } -// cancelAutoscalingInstanceRefresh cancels the currently active Instance -// Refresh of this Auto-Scaling Group, and waits until the refresh reaches a -// terminal state (usually Cancelled). If there is no active refresh, the -// function short-circuits without error. -func cancelAutoscalingInstanceRefresh(d *schema.ResourceData, conn *autoscaling.AutoScaling) error { - asgName := d.Id() +// cancelAutoscalingInstanceRefresh cancels the currently active Instance Refresh +// of this Auto-Scaling Group, if any, and waits until the refresh is Cancelled. +func cancelAutoscalingInstanceRefresh(conn *autoscaling.AutoScaling, asgName string) error { input := autoscaling.CancelInstanceRefreshInput{ AutoScalingGroupName: aws.String(asgName), } - - _, err := conn.CancelInstanceRefresh(&input) - switch { - case isAWSErr(err, autoscaling.ErrCodeActiveInstanceRefreshNotFoundFault, ""): - log.Printf("[DEBUG] No active Instance Refresh in ASG %s", asgName) + log.Printf("[DEBUG] Attempting to cancel Instance Refresh on ASG (%s): %s", asgName, input) + output, err := conn.CancelInstanceRefresh(&input) + if tfawserr.ErrCodeEquals(err, autoscaling.ErrCodeActiveInstanceRefreshNotFoundFault) { + log.Printf("[DEBUG] No active Instance Refresh on ASG (%s)", asgName) return nil - case err != nil: - return err + } + if err != nil { + return fmt.Errorf("error cancelling Instance Refresh on ASG (%s): %w", asgName, err) + } + if output == nil { + return fmt.Errorf("error cancelling Instance Refresh on ASG (%s): empty result", asgName) } - err = resource.Retry(5*time.Minute, func() *resource.RetryError { - input := autoscaling.DescribeInstanceRefreshesInput{ - AutoScalingGroupName: aws.String(asgName), - MaxRecords: aws.Int64(1), - } - - output, err := conn.DescribeInstanceRefreshes(&input) - switch { - case err != nil: - return resource.NonRetryableError(err) - case len(output.InstanceRefreshes) != 1: - return nil - } - - switch status := aws.StringValue(output.InstanceRefreshes[0].Status); status { - case - autoscaling.InstanceRefreshStatusCancelled, - autoscaling.InstanceRefreshStatusFailed, - autoscaling.InstanceRefreshStatusSuccessful: + instanceRefreshID := aws.StringValue(output.InstanceRefreshId) - return nil - default: - return resource.RetryableError(fmt.Errorf("refresh status %s is not terminal", status)) - } - }) - - if isResourceTimeoutError(err) { - return fmt.Errorf("timed out before the previous refresh reached a terminal state") + log.Printf("[DEBUG] Waiting for cancellation of Instance Refresh (%s) on ASG (%s)", instanceRefreshID, asgName) + _, err = waiter.InstanceRefreshCancelled(conn, asgName, instanceRefreshID) + if err != nil { + return fmt.Errorf("error waiting for cancellation of Instance Refresh (%s) on ASG (%s): %w", instanceRefreshID, asgName, err) } - log.Printf("[INFO] Cancelled active instance refresh in ASG %s", asgName) + log.Printf("[INFO] Cancelled Instance Refresh (%s) on ASG (%s)", instanceRefreshID, asgName) return nil } From 0614a00f0b6d940692565f87028d619602cf26ed Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Mon, 30 Nov 2020 13:57:16 -0800 Subject: [PATCH 05/12] Updates instance_refresh structure to match API --- aws/resource_aws_autoscaling_group.go | 97 +++++++----- aws/resource_aws_autoscaling_group_test.go | 148 +++++++++++++++--- .../docs/r/autoscaling_group.html.markdown | 58 +++---- 3 files changed, 212 insertions(+), 91 deletions(-) diff --git a/aws/resource_aws_autoscaling_group.go b/aws/resource_aws_autoscaling_group.go index 186b329072d..0bff7005276 100644 --- a/aws/resource_aws_autoscaling_group.go +++ b/aws/resource_aws_autoscaling_group.go @@ -481,23 +481,32 @@ func resourceAwsAutoscalingGroup() *schema.Resource { Optional: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ - "instance_warmup_seconds": { - Type: schema.TypeInt, - Optional: true, - Default: -1, // default to health_check_grace_period - ValidateFunc: validation.IntAtLeast(-1), - }, - "min_healthy_percentage": { - Type: schema.TypeInt, - Optional: true, - Default: 90, - ValidateFunc: validation.IntBetween(0, 100), - }, "strategy": { Type: schema.TypeString, Required: true, ValidateFunc: validation.StringInSlice(autoscaling.RefreshStrategy_Values(), false), }, + "preferences": { + Type: schema.TypeList, + MaxItems: 1, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "instance_warmup": { + Type: schema.TypeInt, + Optional: true, + Default: -1, // default to health_check_grace_period + ValidateFunc: validation.IntAtLeast(-1), + }, + "min_healthy_percentage": { + Type: schema.TypeInt, + Optional: true, + Default: 90, + ValidateFunc: validation.IntBetween(0, 100), + }, + }, + }, + }, }, }, }, @@ -740,6 +749,7 @@ func resourceAwsAutoscalingGroupCreate(d *schema.ResourceData, meta interface{}) return resourceAwsAutoscalingGroupRead(d, meta) } +// TODO: wrap all top-level error returns func resourceAwsAutoscalingGroupRead(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).autoscalingconn ignoreTagsConfig := meta.(*AWSClient).IgnoreTagsConfig @@ -1008,7 +1018,6 @@ func resourceAwsAutoscalingGroupUpdate(d *schema.ResourceData, meta interface{}) shouldRefreshInstances = true } - // TODO: does this need a wait for capacity? if d.HasChange("placement_group") { opts.PlacementGroup = aws.String(d.Get("placement_group").(string)) // TODO: optional trigger @@ -1210,9 +1219,9 @@ func resourceAwsAutoscalingGroupUpdate(d *schema.ResourceData, meta interface{}) } } - if shouldRefreshInstances { - if err := startAutoscalingInstanceRefresh(d, conn); err != nil { - return fmt.Errorf("failed to start instance refresh of asg %s: %s", d.Id(), err) + if instanceRefreshRaw, ok := d.GetOk("instance_refresh"); ok && shouldRefreshInstances { + if err := autoScalingGroupRefreshInstances(conn, d.Id(), instanceRefreshRaw.([]interface{})); err != nil { + return fmt.Errorf("failed to start instance refresh of asg %s: %w", d.Id(), err) } } @@ -1834,32 +1843,46 @@ func waitUntilAutoscalingGroupLoadBalancersRemoved(conn *autoscaling.AutoScaling return nil } -// startAutoscalingInstanceRefresh starts a new Instance Refresh in this -// Auto-Scaling Group. If there is already an active refresh, it is cancelled. -func startAutoscalingInstanceRefresh(d *schema.ResourceData, conn *autoscaling.AutoScaling) error { - asgName := d.Id() - input := autoscaling.StartInstanceRefreshInput{ - AutoScalingGroupName: aws.String(asgName), - Preferences: &autoscaling.RefreshPreferences{}, - Strategy: nil, +// TODO: rename +func expandAutoScalingGroupInstanceRefresh(asgName string, l []interface{}) *autoscaling.StartInstanceRefreshInput { + if len(l) == 0 || l[0] == nil { + return nil } - if block, ok := d.Get("instance_refresh").([]interface{}); ok && len(block) > 0 { - m := block[0].(map[string]interface{}) + m := l[0].(map[string]interface{}) - if warmup := m["instance_warmup_seconds"].(int); warmup > -1 { - // -1 would mean defaulting to using the group's health_check_grace_period - input.Preferences.InstanceWarmup = aws.Int64(int64(warmup)) - } + return &autoscaling.StartInstanceRefreshInput{ + AutoScalingGroupName: aws.String(asgName), + Strategy: aws.String(m["strategy"].(string)), + Preferences: expandAutoScalingGroupInstanceRefreshPreferences(m["preferences"].([]interface{})), + } +} - // validated by schema - input.Preferences.MinHealthyPercentage = aws.Int64(int64(m["min_healthy_percentage"].(int))) - input.Strategy = aws.String(m["strategy"].(string)) - } else { - log.Printf("[DEBUG] Instance refresh not enabled in ASG %s", asgName) +func expandAutoScalingGroupInstanceRefreshPreferences(l []interface{}) *autoscaling.RefreshPreferences { + if len(l) == 0 || l[0] == nil { return nil } + m := l[0].(map[string]interface{}) + + refreshPreferences := &autoscaling.RefreshPreferences{} + + if v, ok := m["instance_warmup"]; ok { + refreshPreferences.InstanceWarmup = aws.Int64(int64(v.(int))) + } + + if v, ok := m["min_healthy_percentage"]; ok { + refreshPreferences.MinHealthyPercentage = aws.Int64(int64(v.(int))) + } + + return refreshPreferences +} + +// autoScalingGroupRefreshInstances starts a new Instance Refresh in this +// Auto Scaling Group. If there is already an active refresh, it is cancelled. +func autoScalingGroupRefreshInstances(conn *autoscaling.AutoScaling, asgName string, d []interface{}) error { + input := expandAutoScalingGroupInstanceRefresh(asgName, d) + log.Printf("[DEBUG] Cancelling active refresh in ASG %s, if any...", asgName) if err := cancelAutoscalingInstanceRefresh(conn, asgName); err != nil { @@ -1869,7 +1892,7 @@ func startAutoscalingInstanceRefresh(d *schema.ResourceData, conn *autoscaling.A log.Printf("[DEBUG] Starting instance refresh in ASG %s...", asgName) - output, err := conn.StartInstanceRefresh(&input) + output, err := conn.StartInstanceRefresh(input) if err != nil { return err } @@ -1900,8 +1923,8 @@ func cancelAutoscalingInstanceRefresh(conn *autoscaling.AutoScaling, asgName str } instanceRefreshID := aws.StringValue(output.InstanceRefreshId) + log.Printf("[INFO] Requested cancellation of Instance Refresh (%s) on ASG (%s)", instanceRefreshID, asgName) - log.Printf("[DEBUG] Waiting for cancellation of Instance Refresh (%s) on ASG (%s)", instanceRefreshID, asgName) _, err = waiter.InstanceRefreshCancelled(conn, asgName, instanceRefreshID) if err != nil { return fmt.Errorf("error waiting for cancellation of Instance Refresh (%s) on ASG (%s): %w", instanceRefreshID, asgName, err) diff --git a/aws/resource_aws_autoscaling_group_test.go b/aws/resource_aws_autoscaling_group_test.go index aa7e2f54efd..d29c98502a1 100644 --- a/aws/resource_aws_autoscaling_group_test.go +++ b/aws/resource_aws_autoscaling_group_test.go @@ -1006,16 +1006,16 @@ func TestAccAWSAutoScalingGroup_InstanceRefresh_Enabled(t *testing.T) { Steps: []resource.TestStep{ { // check that an instance refresh isn't started by a new asg - Config: testAccAwsAutoScalingGroup_InstanceRefresh_Enabled("Alpha", true, 1), + Config: testAccAwsAutoScalingGroupConfig_InstanceRefresh_Basic("Alpha", 1), Check: resource.ComposeTestCheckFunc( testAccCheckAWSAutoScalingGroupExists(resourceName, &group), resource.TestCheckResourceAttr(resourceName, "min_size", "1"), resource.TestCheckResourceAttr(resourceName, "max_size", "2"), resource.TestCheckResourceAttr(resourceName, "desired_capacity", "1"), testAccCheckAutoscalingLatestInstanceRefreshState(&group, 0, 0, nil), - resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.instance_warmup_seconds", "-1"), - resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.min_healthy_percentage", "90"), + resource.TestCheckResourceAttr(resourceName, "instance_refresh.#", "1"), resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.strategy", "Rolling"), + resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.preferences.#", "0"), ), }, { @@ -1030,31 +1030,41 @@ func TestAccAWSAutoScalingGroup_InstanceRefresh_Enabled(t *testing.T) { }, { // check that changing asg size doesn't trigger a refresh - Config: testAccAwsAutoScalingGroup_InstanceRefresh_Enabled("Alpha", false, 2), + Config: testAccAwsAutoScalingGroupConfig_InstanceRefresh_Enabled("Alpha", 2), Check: resource.ComposeTestCheckFunc( testAccCheckAWSAutoScalingGroupExists(resourceName, &group), resource.TestCheckResourceAttr(resourceName, "min_size", "2"), resource.TestCheckResourceAttr(resourceName, "max_size", "4"), resource.TestCheckResourceAttr(resourceName, "desired_capacity", "2"), testAccCheckAutoscalingLatestInstanceRefreshState(&group, 0, 0, nil), + resource.TestCheckResourceAttr(resourceName, "instance_refresh.#", "1"), + resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.strategy", "Rolling"), + resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.preferences.#", "1"), + resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.preferences.0.instance_warmup", "10"), + resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.preferences.0.min_healthy_percentage", "50"), ), }, { // check that changing tags doesn't trigger a refresh - Config: testAccAwsAutoScalingGroup_InstanceRefresh_Enabled("Bravo", false, 1), + Config: testAccAwsAutoScalingGroupConfig_InstanceRefresh_Enabled("Bravo", 1), Check: resource.ComposeTestCheckFunc( testAccCheckAWSAutoScalingGroupExists(resourceName, &group), - resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.instance_warmup_seconds", "10"), - resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.min_healthy_percentage", "50"), - resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.strategy", "Rolling"), testAccCheckAutoscalingLatestInstanceRefreshState(&group, 0, 0, nil), ), }, - // TODO: check that an active refresh is cancelled in favour of a new one + { + Config: testAccAwsAutoScalingGroupConfig_InstanceRefresh_Disabled("Bravo", 1), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSAutoScalingGroupExists(resourceName, &group), + resource.TestCheckNoResourceAttr(resourceName, "instance_refresh.#"), + ), + }, }, }) } +// TODO: check that an active refresh is cancelled in favour of a new one + func TestAccAWSAutoScalingGroup_InstanceRefresh_Triggers(t *testing.T) { matrix := []struct { AvailabilityZoneCount int @@ -1084,7 +1094,7 @@ func TestAccAWSAutoScalingGroup_InstanceRefresh_Triggers(t *testing.T) { steps := make([]resource.TestStep, len(matrix)) for i, test := range matrix { steps[i] = resource.TestStep{ - Config: testAccAwsAutoScalingGroup_InstanceRefresh_Triggers( + Config: testAccAwsAutoScalingGroupConfig_InstanceRefresh_Triggers( test.AvailabilityZoneCount, test.SubnetCount, test.InstanceType, @@ -4269,18 +4279,57 @@ resource "aws_autoscaling_group" "test" { `, rName) } -func testAccAwsAutoScalingGroup_InstanceRefresh_Enabled( - tagValue string, - defaults bool, - sizeFactor int, -) string { - preference := `` - if !defaults { - preference = ` - min_healthy_percentage = 50 - instance_warmup_seconds = 10` - } +func testAccAwsAutoScalingGroupConfig_InstanceRefresh_Basic(tagValue string, sizeFactor int) string { + return fmt.Sprintf(` +resource "aws_autoscaling_group" "test" { + availability_zones = [data.aws_availability_zones.current.names[0]] + max_size = 2 * local.size_factor + min_size = 1 * local.size_factor + desired_capacity = 1 * local.size_factor + launch_configuration = aws_launch_configuration.test.name + + tag { + key = "Test" + value = %[1]q + propagate_at_launch = true + } + + instance_refresh { + strategy = "Rolling" + } +} + +locals { + size_factor = %[2]d +} + +data "aws_ami" "test" { + most_recent = true + owners = ["amazon"] + + filter { + name = "name" + values = ["amzn-ami-hvm-*-x86_64-gp2"] + } +} + +data "aws_availability_zones" "current" { + state = "available" + + filter { + name = "opt-in-status" + values = ["opt-in-not-required"] + } +} +resource "aws_launch_configuration" "test" { + image_id = data.aws_ami.test.id + instance_type = "t3.nano" +} +`, tagValue, sizeFactor) +} + +func testAccAwsAutoScalingGroupConfig_InstanceRefresh_Enabled(tagValue string, sizeFactor int) string { return fmt.Sprintf(` resource "aws_autoscaling_group" "test" { availability_zones = [data.aws_availability_zones.current.names[0]] @@ -4297,12 +4346,61 @@ resource "aws_autoscaling_group" "test" { instance_refresh { strategy = "Rolling" -%[2]s + preferences { + instance_warmup = 10 + min_healthy_percentage = 50 + } + } +} + +locals { + size_factor = %[2]d +} + +data "aws_ami" "test" { + most_recent = true + owners = ["amazon"] + + filter { + name = "name" + values = ["amzn-ami-hvm-*-x86_64-gp2"] + } +} + +data "aws_availability_zones" "current" { + state = "available" + + filter { + name = "opt-in-status" + values = ["opt-in-not-required"] + } +} + +resource "aws_launch_configuration" "test" { + image_id = data.aws_ami.test.id + instance_type = "t3.nano" +} +`, tagValue, sizeFactor) +} + +func testAccAwsAutoScalingGroupConfig_InstanceRefresh_Disabled(tagValue string, sizeFactor int) string { + return fmt.Sprintf(` +resource "aws_autoscaling_group" "test" { + availability_zones = [data.aws_availability_zones.current.names[0]] + max_size = 2 * local.size_factor + min_size = 1 * local.size_factor + desired_capacity = 1 * local.size_factor + launch_configuration = aws_launch_configuration.test.name + + tag { + key = "Test" + value = %[1]q + propagate_at_launch = true } } locals { - size_factor = %[3]d + size_factor = %[2]d } data "aws_ami" "test" { @@ -4328,10 +4426,10 @@ resource "aws_launch_configuration" "test" { image_id = data.aws_ami.test.id instance_type = "t3.nano" } -`, tagValue, preference, sizeFactor) +`, tagValue, sizeFactor) } -func testAccAwsAutoScalingGroup_InstanceRefresh_Triggers( +func testAccAwsAutoScalingGroupConfig_InstanceRefresh_Triggers( availabilityZoneCount int, subnetCount int, instanceType string, diff --git a/website/docs/r/autoscaling_group.html.markdown b/website/docs/r/autoscaling_group.html.markdown index 73caf88010c..5ad6477eb9b 100644 --- a/website/docs/r/autoscaling_group.html.markdown +++ b/website/docs/r/autoscaling_group.html.markdown @@ -3,7 +3,7 @@ subcategory: "Autoscaling" layout: "aws" page_title: "AWS: aws_autoscaling_group" description: |- - Provides an AutoScaling Group resource. + Provides an Auto Scaling Group resource. --- # Resource: aws_autoscaling_group @@ -205,8 +205,10 @@ resource "aws_autoscaling_group" "example" { } instance_refresh { - strategy = "Rolling" - min_healthy_percentage = 50 + strategy = "Rolling" + preferences { + min_healthy_percentage = 50 + } } } ``` @@ -215,11 +217,11 @@ resource "aws_autoscaling_group" "example" { The following arguments are supported: -* `name` - (Optional) The name of the auto scaling group. By default generated by Terraform. +* `name` - (Optional) The name of the Auto Scaling Group. By default generated by Terraform. * `name_prefix` - (Optional) Creates a unique name beginning with the specified prefix. Conflicts with `name`. -* `max_size` - (Required) The maximum size of the auto scale group. -* `min_size` - (Required) The minimum size of the auto scale group. +* `max_size` - (Required) The maximum size of the Auto Scaling Group. +* `min_size` - (Required) The minimum size of the Auto Scaling Group. (See also [Waiting for Capacity](#waiting-for-capacity) below.) * `availability_zones` - (Optional) A list of one or more availability zones for the group. Used for EC2-Classic and default subnets when not specified with `vpc_zone_identifier` argument. Conflicts with `vpc_zone_identifier`. * `default_cooldown` - (Optional) The amount of time, in seconds, after a scaling activity completes before another scaling activity can start. @@ -228,18 +230,18 @@ The following arguments are supported: * `mixed_instances_policy` (Optional) Configuration block containing settings to define launch targets for Auto Scaling groups. Defined below. * `initial_lifecycle_hook` - (Optional) One or more [Lifecycle Hooks](http://docs.aws.amazon.com/autoscaling/latest/userguide/lifecycle-hooks.html) - to attach to the autoscaling group **before** instances are launched. The + to attach to the Auto Scaling Group **before** instances are launched. The syntax is exactly the same as the separate [`aws_autoscaling_lifecycle_hook`](/docs/providers/aws/r/autoscaling_lifecycle_hook.html) resource, without the `autoscaling_group_name` attribute. Please note that this will only work when creating - a new autoscaling group. For all other use-cases, please use `aws_autoscaling_lifecycle_hook` resource. + a new Auto Scaling Group. For all other use-cases, please use `aws_autoscaling_lifecycle_hook` resource. * `health_check_grace_period` - (Optional, Default: 300) Time (in seconds) after instance comes into service before checking health. * `health_check_type` - (Optional) "EC2" or "ELB". Controls how health checking is done. * `desired_capacity` - (Optional) The number of Amazon EC2 instances that should be running in the group. (See also [Waiting for Capacity](#waiting-for-capacity) below.) -* `force_delete` - (Optional) Allows deleting the autoscaling group without waiting - for all instances in the pool to terminate. You can force an autoscaling group to delete +* `force_delete` - (Optional) Allows deleting the Auto Scaling Group without waiting + for all instances in the pool to terminate. You can force an Auto Scaling Group to delete even if it's in the process of scaling a resource. Normally, Terraform drains all the instances before deleting the group. This bypasses that behavior and potentially leaves resources dangling. @@ -247,9 +249,9 @@ The following arguments are supported: group names. Only valid for classic load balancers. For ALBs, use `target_group_arns` instead. * `vpc_zone_identifier` (Optional) A list of subnet IDs to launch resources in. Subnets automatically determine which availability zones the group will reside. Conflicts with `availability_zones`. * `target_group_arns` (Optional) A set of `aws_alb_target_group` ARNs, for use with Application or Network Load Balancing. -* `termination_policies` (Optional) A list of policies to decide how the instances in the auto scale group should be terminated. The allowed values are `OldestInstance`, `NewestInstance`, `OldestLaunchConfiguration`, `ClosestToNextInstanceHour`, `OldestLaunchTemplate`, `AllocationStrategy`, `Default`. -* `suspended_processes` - (Optional) A list of processes to suspend for the AutoScaling Group. The allowed values are `Launch`, `Terminate`, `HealthCheck`, `ReplaceUnhealthy`, `AZRebalance`, `AlarmNotification`, `ScheduledActions`, `AddToLoadBalancer`. -Note that if you suspend either the `Launch` or `Terminate` process types, it can prevent your autoscaling group from functioning properly. +* `termination_policies` (Optional) A list of policies to decide how the instances in the Auto Scaling Group should be terminated. The allowed values are `OldestInstance`, `NewestInstance`, `OldestLaunchConfiguration`, `ClosestToNextInstanceHour`, `OldestLaunchTemplate`, `AllocationStrategy`, `Default`. +* `suspended_processes` - (Optional) A list of processes to suspend for the Auto Scaling Group. The allowed values are `Launch`, `Terminate`, `HealthCheck`, `ReplaceUnhealthy`, `AZRebalance`, `AlarmNotification`, `ScheduledActions`, `AddToLoadBalancer`. +Note that if you suspend either the `Launch` or `Terminate` process types, it can prevent your Auto Scaling Group from functioning properly. * `tag` (Optional) Configuration block(s) containing resource tags. Conflicts with `tags`. Documented below. * `tags` (Optional) Set of maps containing resource tags. Conflicts with `tag`. Documented below. * `placement_group` (Optional) The name of the placement group into which you'll launch your instances, if any. @@ -261,22 +263,22 @@ Note that if you suspend either the `Launch` or `Terminate` process types, it ca for Capacity](#waiting-for-capacity) below.) Setting this to "0" causes Terraform to skip all Capacity Waiting behavior. * `min_elb_capacity` - (Optional) Setting this causes Terraform to wait for - this number of instances from this autoscaling group to show up healthy in the + this number of instances from this Auto Scaling Group to show up healthy in the ELB only on creation. Updates will not wait on ELB instance number changes. (See also [Waiting for Capacity](#waiting-for-capacity) below.) * `wait_for_elb_capacity` - (Optional) Setting this will cause Terraform to wait - for exactly this number of healthy instances from this autoscaling group in + for exactly this number of healthy instances from this Auto Scaling Group in all attached load balancers on both create and update operations. (Takes precedence over `min_elb_capacity` behavior.) (See also [Waiting for Capacity](#waiting-for-capacity) below.) * `protect_from_scale_in` (Optional) Allows setting instance protection. The - autoscaling group will not select instances with this setting for termination + Auto Scaling Group will not select instances with this setting for termination during scale in events. * `service_linked_role_arn` (Optional) The ARN of the service-linked role that the ASG will use to call other AWS services * `max_instance_lifetime` (Optional) The maximum amount of time, in seconds, that an instance can be in service, values must be either equal to 0 or between 604800 and 31536000 seconds. * `instance_refresh` - (Optional) If this block is configured, start an [Instance Refresh](https://docs.aws.amazon.com/autoscaling/ec2/userguide/asg-instance-refresh.html) - when this autoscaling group is updated. Defined below. + when this Auto Scaling Group is updated. Defined [below](#instance_refresh). ### launch_template @@ -348,18 +350,16 @@ This allows the construction of dynamic lists of tags which is not possible usin This configuration block supports the following: -* `instance_warmup_seconds` - (Optional) The number of seconds until a newly launched - instance is configured and ready to use. Default behavior (set with `-1` or `null`) - is to match the autoscaling group's health check grace period. -* `min_healthy_percentage` - (Optional) The amount of capacity in the Auto Scaling group +* `strategy` - (Required) The strategy to use for instance refresh. The only allowed value is `Rolling`. See [StartInstanceRefresh Action](https://docs.aws.amazon.com/autoscaling/ec2/APIReference/API_StartInstanceRefresh.html#API_StartInstanceRefresh_RequestParameters) for more information. +* `preferences` - (Optional) Override default parameters for Instance Refresh. + * `instance_warmup_seconds` - (Optional) The number of seconds until a newly launched instance is configured and ready to use. Default behavior (set with `-1` or `null`) is to match the Auto Scaling Group's health check grace period. + * `min_healthy_percentage` - (Optional) The amount of capacity in the Auto Scaling group that must remain healthy during an instance refresh to allow the operation to continue, as a percentage of the desired capacity of the Auto Scaling group. Defaults to `90`. -* `strategy` - (Required) The strategy to use for instance refresh. The only allowed - value is `"Rolling"`. See [StartInstanceRefresh Action](https://docs.aws.amazon.com/autoscaling/ec2/APIReference/API_StartInstanceRefresh.html#API_StartInstanceRefresh_RequestParameters) for more information. -~> **NOTE:** A refresh is only started when any of the following autoscaling group properties change: `launch_configuration`, `launch_template`, `mixed_instances_policy`, `vpc_zone_identifier`, `availability_zones`, `placement_group`, or any `tag` or `tags` configured to propagate at launch. +~> **NOTE:** A refresh is only started when any of the following Auto Scaling Group properties change: `launch_configuration`, `launch_template`, `mixed_instances_policy`, `vpc_zone_identifier`, `availability_zones`, `placement_group`, or any `tag` or `tags` configured to propagate at launch. -~> **NOTE:** Autoscaling groups support up to one active instance refresh at a time. When this resource is updated, any existing refresh is cancelled. +~> **NOTE:** Auto Scaling Groups support up to one active instance refresh at a time. When this resource is updated, any existing refresh is cancelled. ~> **NOTE:** Depending on health check settings and group size, an instance refresh may take a long time or fail. This resource does not wait for the instance refresh to complete. @@ -367,8 +367,8 @@ This configuration block supports the following: In addition to all arguments above, the following attributes are exported: -* `id` - The autoscaling group id. -* `arn` - The ARN for this AutoScaling Group +* `id` - The Auto Scaling Group id. +* `arn` - The ARN for this Auto Scaling Group * `availability_zones` - The availability zones of the autoscale group. * `min_size` - The minimum size of the autoscale group * `max_size` - The maximum size of the autoscale group @@ -387,7 +387,7 @@ the `initial_lifecycle_hook` attribute from this resource, or via the separate [`aws_autoscaling_lifecycle_hook`](/docs/providers/aws/r/autoscaling_lifecycle_hook.html) resource. `initial_lifecycle_hook` exists here because any lifecycle hooks added with `aws_autoscaling_lifecycle_hook` will not be added until the -autoscaling group has been created, and depending on your +Auto Scaling Group has been created, and depending on your [capacity](#waiting-for-capacity) settings, after the initial instances have been launched, creating unintended behavior. If you need hooks to run on all instances, add them with `initial_lifecycle_hook` here, but take @@ -466,7 +466,7 @@ for more information. ## Import -AutoScaling Groups can be imported using the `name`, e.g. +Auto Scaling Groups can be imported using the `name`, e.g. ``` $ terraform import aws_autoscaling_group.web web-asg From 130fc3ebef3ebac71bc5491d2fba665ab26f64c8 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Tue, 1 Dec 2020 17:56:02 -0800 Subject: [PATCH 06/12] Uses the TypeStringNullableInt pattern for instance_warmup --- aws/internal/nullable/int.go | 78 +++++++++ aws/internal/nullable/int_test.go | 100 ++++++++++++ aws/internal/nullable/testing.go | 45 ++++++ .../service/autoscaling/waiter/waiter.go | 2 +- aws/resource_aws_autoscaling_group.go | 25 +-- aws/resource_aws_autoscaling_group_test.go | 148 +++++++++++++++++- go.mod | 2 +- go.sum | 30 ---- .../docs/r/autoscaling_group.html.markdown | 6 +- 9 files changed, 382 insertions(+), 54 deletions(-) create mode 100644 aws/internal/nullable/int.go create mode 100644 aws/internal/nullable/int_test.go create mode 100644 aws/internal/nullable/testing.go diff --git a/aws/internal/nullable/int.go b/aws/internal/nullable/int.go new file mode 100644 index 00000000000..53981603485 --- /dev/null +++ b/aws/internal/nullable/int.go @@ -0,0 +1,78 @@ +package nullable + +import ( + "fmt" + "strconv" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" +) + +const ( + TypeNullableInt = schema.TypeString +) + +type Int string + +func (i Int) IsNull() bool { + return i == "" +} + +func (i Int) Value() (int64, bool, error) { + if i.IsNull() { + return 0, true, nil + } + + value, err := strconv.ParseInt(string(i), 10, 64) + if err != nil { + return 0, false, err + } + return value, false, nil +} + +// ValidateTypeStringNullableInt provides custom error messaging for TypeString ints +// Some arguments require an int value or unspecified, empty field. +func ValidateTypeStringNullableInt(v interface{}, k string) (ws []string, es []error) { + value, ok := v.(string) + if !ok { + es = append(es, fmt.Errorf("expected type of %s to be string", k)) + return + } + + if value == "" { + return + } + + if _, err := strconv.ParseInt(value, 10, 64); err != nil { + es = append(es, fmt.Errorf("%s: cannot parse '%s' as int: %w", k, value, err)) + } + + return +} + +// ValidateTypeStringNullableIntAtLeast provides custom error messaging for TypeString ints +// Some arguments require an int value or unspecified, empty field. +func ValidateTypeStringNullableIntAtLeast(min int) schema.SchemaValidateFunc { + return func(i interface{}, k string) (ws []string, es []error) { + value, ok := i.(string) + if !ok { + es = append(es, fmt.Errorf("expected type of %s to be string", k)) + return + } + + if value == "" { + return + } + + v, err := strconv.ParseInt(value, 10, 64) + if err != nil { + es = append(es, fmt.Errorf("%s: cannot parse '%s' as int: %w", k, value, err)) + return + } + + if v < int64(min) { + es = append(es, fmt.Errorf("expected %s to be at least (%d), got %d", k, min, v)) + } + + return + } +} diff --git a/aws/internal/nullable/int_test.go b/aws/internal/nullable/int_test.go new file mode 100644 index 00000000000..cc1c61e554e --- /dev/null +++ b/aws/internal/nullable/int_test.go @@ -0,0 +1,100 @@ +package nullable + +import ( + "errors" + "regexp" + "strconv" + "testing" +) + +func TestNullableInt(t *testing.T) { + cases := []struct { + val string + expectedNull bool + expectedValue int64 + expectedErr error + }{ + { + val: "1", + expectedNull: false, + expectedValue: 1, + }, + { + val: "", + expectedNull: true, + expectedValue: 0, + }, + { + val: "A", + expectedNull: false, + expectedValue: 0, + expectedErr: strconv.ErrSyntax, + }, + } + + for i, tc := range cases { + v := Int(tc.val) + + if null := v.IsNull(); null != tc.expectedNull { + t.Fatalf("expected test case %d IsNull to return %t, got %t", i, null, tc.expectedNull) + } + + value, null, err := v.Value() + if value != tc.expectedValue { + t.Fatalf("expected test case %d Value to be %d, got %d", i, tc.expectedValue, value) + } + if null != tc.expectedNull { + t.Fatalf("expected test case %d Value null flag to be %t, got %t", i, tc.expectedNull, null) + } + if tc.expectedErr == nil && err != nil { + t.Fatalf("expected test case %d to succeed, got error %s", i, err) + } + if tc.expectedErr != nil { + if !errors.Is(err, tc.expectedErr) { + t.Fatalf("expected test case %d to have error matching \"%s\", got %s", i, tc.expectedErr, err) + } + } + } +} + +func TestValidationInt(t *testing.T) { + runTestCases(t, []testCase{ + { + val: "1", + f: ValidateTypeStringNullableInt, + }, + { + val: "A", + f: ValidateTypeStringNullableInt, + expectedErr: regexp.MustCompile(`[\w]+: cannot parse 'A' as int: .*`), + }, + { + val: 1, + f: ValidateTypeStringNullableInt, + expectedErr: regexp.MustCompile(`expected type of [\w]+ to be string`), + }, + }) +} + +func TestValidationIntAtLeast(t *testing.T) { + runTestCases(t, []testCase{ + { + val: "1", + f: ValidateTypeStringNullableIntAtLeast(1), + }, + { + val: "1", + f: ValidateTypeStringNullableIntAtLeast(0), + }, + { + val: "1", + f: ValidateTypeStringNullableIntAtLeast(2), + expectedErr: regexp.MustCompile(`expected [\w]+ to be at least \(2\), got 1`), + }, + { + val: 1, + f: ValidateTypeStringNullableIntAtLeast(2), + expectedErr: regexp.MustCompile(`expected type of [\w]+ to be string`), + }, + }) +} diff --git a/aws/internal/nullable/testing.go b/aws/internal/nullable/testing.go new file mode 100644 index 00000000000..9921ac5375f --- /dev/null +++ b/aws/internal/nullable/testing.go @@ -0,0 +1,45 @@ +package nullable + +import ( + "regexp" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + testing "github.com/mitchellh/go-testing-interface" +) + +type testCase struct { + val interface{} + f schema.SchemaValidateFunc + expectedErr *regexp.Regexp +} + +func runTestCases(t testing.T, cases []testCase) { + t.Helper() + + matchErr := func(errs []error, r *regexp.Regexp) bool { + // err must match one provided + for _, err := range errs { + if r.MatchString(err.Error()) { + return true + } + } + + return false + } + + for i, tc := range cases { + _, errs := tc.f(tc.val, "test_property") + + if len(errs) == 0 && tc.expectedErr == nil { + continue + } + + if len(errs) != 0 && tc.expectedErr == nil { + t.Fatalf("expected test case %d to produce no errors, got %v", i, errs) + } + + if !matchErr(errs, tc.expectedErr) { + t.Fatalf("expected test case %d to produce error matching \"%s\", got %v", i, tc.expectedErr, errs) + } + } +} diff --git a/aws/internal/service/autoscaling/waiter/waiter.go b/aws/internal/service/autoscaling/waiter/waiter.go index b4bf99543d8..f278c3f859a 100644 --- a/aws/internal/service/autoscaling/waiter/waiter.go +++ b/aws/internal/service/autoscaling/waiter/waiter.go @@ -12,7 +12,7 @@ const ( InstanceRefreshSuccessfulTimeout = 5 * time.Minute // Maximum amount of time to wait for an InstanceRefresh to be Cancelled - InstanceRefreshCancelledTimeout = 5 * time.Minute + InstanceRefreshCancelledTimeout = 10 * time.Minute ) func InstanceRefreshSuccessful(conn *autoscaling.AutoScaling, asgName, instanceRefreshId string) (*autoscaling.InstanceRefresh, error) { diff --git a/aws/resource_aws_autoscaling_group.go b/aws/resource_aws_autoscaling_group.go index 0bff7005276..6d54e890ca8 100644 --- a/aws/resource_aws_autoscaling_group.go +++ b/aws/resource_aws_autoscaling_group.go @@ -22,6 +22,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/terraform-providers/terraform-provider-aws/aws/internal/hashcode" "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/nullable" "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/autoscaling/waiter" ) @@ -493,10 +494,9 @@ func resourceAwsAutoscalingGroup() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "instance_warmup": { - Type: schema.TypeInt, + Type: nullable.TypeNullableInt, Optional: true, - Default: -1, // default to health_check_grace_period - ValidateFunc: validation.IntAtLeast(-1), + ValidateFunc: nullable.ValidateTypeStringNullableIntAtLeast(0), }, "min_healthy_percentage": { Type: schema.TypeInt, @@ -1843,8 +1843,7 @@ func waitUntilAutoscalingGroupLoadBalancersRemoved(conn *autoscaling.AutoScaling return nil } -// TODO: rename -func expandAutoScalingGroupInstanceRefresh(asgName string, l []interface{}) *autoscaling.StartInstanceRefreshInput { +func createAutoScalingGroupInstanceRefreshInput(asgName string, l []interface{}) *autoscaling.StartInstanceRefreshInput { if len(l) == 0 || l[0] == nil { return nil } @@ -1868,7 +1867,10 @@ func expandAutoScalingGroupInstanceRefreshPreferences(l []interface{}) *autoscal refreshPreferences := &autoscaling.RefreshPreferences{} if v, ok := m["instance_warmup"]; ok { - refreshPreferences.InstanceWarmup = aws.Int64(int64(v.(int))) + i := nullable.Int(v.(string)) + if v, null, _ := i.Value(); !null { + refreshPreferences.InstanceWarmup = aws.Int64(v) + } } if v, ok := m["min_healthy_percentage"]; ok { @@ -1880,25 +1882,24 @@ func expandAutoScalingGroupInstanceRefreshPreferences(l []interface{}) *autoscal // autoScalingGroupRefreshInstances starts a new Instance Refresh in this // Auto Scaling Group. If there is already an active refresh, it is cancelled. -func autoScalingGroupRefreshInstances(conn *autoscaling.AutoScaling, asgName string, d []interface{}) error { - input := expandAutoScalingGroupInstanceRefresh(asgName, d) +func autoScalingGroupRefreshInstances(conn *autoscaling.AutoScaling, asgName string, refreshConfig []interface{}) error { - log.Printf("[DEBUG] Cancelling active refresh in ASG %s, if any...", asgName) + log.Printf("[DEBUG] Cancelling active Instance Refresh in ASG %s, if any...", asgName) if err := cancelAutoscalingInstanceRefresh(conn, asgName); err != nil { // todo: add comment about subsequent ASG updates not picking up the refresh? return fmt.Errorf("failed to cancel previous refresh: %w", err) } - log.Printf("[DEBUG] Starting instance refresh in ASG %s...", asgName) - + input := createAutoScalingGroupInstanceRefreshInput(asgName, refreshConfig) + log.Printf("[DEBUG] Starting Instance Refresh on ASG (%s): %s", asgName, input) output, err := conn.StartInstanceRefresh(input) if err != nil { return err } instanceRefreshID := aws.StringValue(output.InstanceRefreshId) - log.Printf("[INFO] Started instance refresh %s in ASG %s", instanceRefreshID, asgName) + log.Printf("[INFO] Started Instance Refresh (%s) on ASG (%s)", instanceRefreshID, asgName) return nil } diff --git a/aws/resource_aws_autoscaling_group_test.go b/aws/resource_aws_autoscaling_group_test.go index d29c98502a1..441c9ae8de8 100644 --- a/aws/resource_aws_autoscaling_group_test.go +++ b/aws/resource_aws_autoscaling_group_test.go @@ -149,12 +149,9 @@ func TestAccAWSAutoScalingGroup_basic(t *testing.T) { testAccCheckAWSAutoScalingGroupExists("aws_autoscaling_group.bar", &group), testAccCheckAWSLaunchConfigurationExists("aws_launch_configuration.new", &lc), testAccCheckAutoscalingLatestInstanceRefreshState(&group, 0, 0, nil), - resource.TestCheckResourceAttr( - "aws_autoscaling_group.bar", "desired_capacity", "5"), - resource.TestCheckResourceAttr( - "aws_autoscaling_group.bar", "termination_policies.0", "ClosestToNextInstanceHour"), - resource.TestCheckResourceAttr( - "aws_autoscaling_group.bar", "protect_from_scale_in", "true"), + resource.TestCheckResourceAttr("aws_autoscaling_group.bar", "desired_capacity", "5"), + resource.TestCheckResourceAttr("aws_autoscaling_group.bar", "termination_policies.0", "ClosestToNextInstanceHour"), + resource.TestCheckResourceAttr("aws_autoscaling_group.bar", "protect_from_scale_in", "true"), testLaunchConfigurationName("aws_autoscaling_group.bar", &lc), testAccCheckAutoscalingTags(&group.Tags, "FromTags1Changed", map[string]interface{}{ "value": "value1changed", @@ -4585,3 +4582,142 @@ func testAccCheckAutoscalingLatestInstanceRefreshState( status) } } + +func TestCreateAutoScalingGroupInstanceRefreshInput(t *testing.T) { + const asgName = "test-asg" + testCases := []struct { + name string + input []interface{} + expected *autoscaling.StartInstanceRefreshInput + }{ + { + name: "empty list", + input: []interface{}{}, + expected: nil, + }, + { + name: "nil", + input: []interface{}{nil}, + expected: nil, + }, + { + name: "defaults", + input: []interface{}{map[string]interface{}{ + "strategy": "Rolling", + "preferences": []interface{}{}, + }}, + expected: &autoscaling.StartInstanceRefreshInput{ + AutoScalingGroupName: aws.String(asgName), + Strategy: aws.String("Rolling"), + Preferences: nil, + }, + }, + { + name: "instance_warmup only", + input: []interface{}{map[string]interface{}{ + "strategy": "Rolling", + "preferences": []interface{}{ + map[string]interface{}{ + "instance_warmup": "60", + }, + }, + }}, + expected: &autoscaling.StartInstanceRefreshInput{ + AutoScalingGroupName: aws.String(asgName), + Strategy: aws.String("Rolling"), + Preferences: &autoscaling.RefreshPreferences{ + InstanceWarmup: aws.Int64(60), + MinHealthyPercentage: nil, + }, + }, + }, + { + name: "instance_warmup zero", + input: []interface{}{map[string]interface{}{ + "strategy": "Rolling", + "preferences": []interface{}{ + map[string]interface{}{ + "instance_warmup": "0", + }, + }, + }}, + expected: &autoscaling.StartInstanceRefreshInput{ + AutoScalingGroupName: aws.String(asgName), + Strategy: aws.String("Rolling"), + Preferences: &autoscaling.RefreshPreferences{ + InstanceWarmup: aws.Int64(0), + MinHealthyPercentage: nil, + }, + }, + }, + { + name: "instance_warmup empty string", + input: []interface{}{map[string]interface{}{ + "strategy": "Rolling", + "preferences": []interface{}{ + map[string]interface{}{ + "instance_warmup": "", + "min_healthy_percentage": 80, + }, + }, + }}, + expected: &autoscaling.StartInstanceRefreshInput{ + AutoScalingGroupName: aws.String(asgName), + Strategy: aws.String("Rolling"), + Preferences: &autoscaling.RefreshPreferences{ + InstanceWarmup: nil, + MinHealthyPercentage: aws.Int64(80), + }, + }, + }, + { + name: "min_healthy_percentage only", + input: []interface{}{map[string]interface{}{ + "strategy": "Rolling", + "preferences": []interface{}{ + map[string]interface{}{ + "min_healthy_percentage": 80, + }, + }, + }}, + expected: &autoscaling.StartInstanceRefreshInput{ + AutoScalingGroupName: aws.String(asgName), + Strategy: aws.String("Rolling"), + Preferences: &autoscaling.RefreshPreferences{ + InstanceWarmup: nil, + MinHealthyPercentage: aws.Int64(80), + }, + }, + }, + { + name: "preferences", + input: []interface{}{map[string]interface{}{ + "strategy": "Rolling", + "preferences": []interface{}{ + map[string]interface{}{ + "instance_warmup": "60", + "min_healthy_percentage": 80, + }, + }, + }}, + expected: &autoscaling.StartInstanceRefreshInput{ + AutoScalingGroupName: aws.String(asgName), + Strategy: aws.String("Rolling"), + Preferences: &autoscaling.RefreshPreferences{ + InstanceWarmup: aws.Int64(60), + MinHealthyPercentage: aws.Int64(80), + }, + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + got := createAutoScalingGroupInstanceRefreshInput(asgName, testCase.input) + + if !reflect.DeepEqual(got, testCase.expected) { + t.Errorf("got %s, expected %s", got, testCase.expected) + } + }) + } +} diff --git a/go.mod b/go.mod index 7f1a06c8ae7..afb8bb7b154 100644 --- a/go.mod +++ b/go.mod @@ -11,13 +11,13 @@ require ( github.com/hashicorp/go-hclog v0.10.0 // indirect github.com/hashicorp/go-multierror v1.1.0 github.com/hashicorp/go-version v1.2.1 - github.com/hashicorp/terraform-plugin-sdk v1.16.0 github.com/hashicorp/terraform-plugin-sdk/v2 v2.2.0 github.com/jen20/awspolicyequivalence v1.1.0 github.com/keybase/go-crypto v0.0.0-20161004153544-93f5b35093ba github.com/mattn/go-colorable v0.1.7 // indirect github.com/mitchellh/copystructure v1.0.0 github.com/mitchellh/go-homedir v1.1.0 + github.com/mitchellh/go-testing-interface v1.0.4 github.com/pquerna/otp v1.3.0 github.com/stretchr/testify v1.6.1 // indirect gopkg.in/yaml.v2 v2.3.0 diff --git a/go.sum b/go.sum index be1108232cd..546e2fbba57 100644 --- a/go.sum +++ b/go.sum @@ -59,8 +59,6 @@ github.com/apparentlymart/go-textseg v1.0.0 h1:rRmlIsPEEhUTIKQb7T++Nz/A5Q6C9IuX2 github.com/apparentlymart/go-textseg v1.0.0/go.mod h1:z96Txxhf3xSFMPmb5X/1W05FF/Nj9VFpLOpjS5yuumk= github.com/apparentlymart/go-textseg/v12 v12.0.0/go.mod h1:S/4uRK2UtaQttw1GenVJEynmyUenKwP++x/+DdGV/Ec= github.com/armon/go-radix v0.0.0-20180808171621-7fddfc383310/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8= -github.com/armon/go-radix v1.0.0 h1:F4z6KzEeeQIMeLFa97iZU6vupzoecKdU5TX24SNppXI= -github.com/armon/go-radix v1.0.0/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs= github.com/aws/aws-sdk-go v1.15.78/go.mod h1:E3/ieXAlvM0XWO57iftYVDLLvQ824smPP3ATZkfNZeM= @@ -164,8 +162,6 @@ github.com/google/pprof v0.0.0-20200229191704-1ebb73c60ed3/go.mod h1:ZgVRPoUq/hf github.com/google/pprof v0.0.0-20200430221834-fc25d7d30c6d/go.mod h1:ZgVRPoUq/hfqzAqh7sHMqb3I9Rq5C59dIz2SbBwJ4eM= github.com/google/pprof v0.0.0-20200708004538-1a94d8640e99/go.mod h1:ZgVRPoUq/hfqzAqh7sHMqb3I9Rq5C59dIz2SbBwJ4eM= github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI= -github.com/google/uuid v1.1.1 h1:Gkbcsh/GbpXz7lPftLA3P6TYMwjCLYm83jiFQZF/3gY= -github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+vpHVxEJEs9eg= github.com/googleapis/gax-go/v2 v2.0.5 h1:sjZBwGj9Jlw33ImPtvFviGYvseOtDM7hkSKB7+Tv3SM= github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk= @@ -181,7 +177,6 @@ github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtng github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320 h1:1/D3zfFHttUKaCaGKZ/dR2roBXv0vKbSCnssIldfQdI= github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320/go.mod h1:EiZBMaudVLy8fmjf9Npq1dq9RalhveqZG5w/yz3mHWs= github.com/hashicorp/go-getter v1.4.0/go.mod h1:7qxyCd8rBfcShwsvxgIguu4KbS3l8bUCwg2Umn7RjeY= -github.com/hashicorp/go-getter v1.4.2-0.20200106182914-9813cbd4eb02/go.mod h1:7qxyCd8rBfcShwsvxgIguu4KbS3l8bUCwg2Umn7RjeY= github.com/hashicorp/go-getter v1.5.0 h1:ciWJaeZWSMbc5OiLMpKp40MKFPqO44i0h3uyfXPBkkk= github.com/hashicorp/go-getter v1.5.0/go.mod h1:a7z7NPPfNQpJWcn4rSWFtdrSldqLdLPEF3d8nFMsSLM= github.com/hashicorp/go-hclog v0.0.0-20180709165350-ff2cf002a8dd/go.mod h1:9bjs9uLqI8l75knNv3lV1kA55veR+WUPSiKIWcQHudI= @@ -199,32 +194,22 @@ github.com/hashicorp/go-uuid v1.0.0/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/b github.com/hashicorp/go-uuid v1.0.1 h1:fv1ep09latC32wFoVwnqcnKJGnMSdBanPczbHAYm1BE= github.com/hashicorp/go-uuid v1.0.1/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= github.com/hashicorp/go-version v1.1.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= -github.com/hashicorp/go-version v1.2.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= github.com/hashicorp/go-version v1.2.1 h1:zEfKbn2+PDgroKdiOzqiE8rsmLqU2uwi5PB5pBJ3TkI= github.com/hashicorp/go-version v1.2.1/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= -github.com/hashicorp/hcl v0.0.0-20170504190234-a4b07c25de5f h1:UdxlrJz4JOnY8W+DbLISwf2B8WXEolNRA8BGCwI9jws= -github.com/hashicorp/hcl v0.0.0-20170504190234-a4b07c25de5f/go.mod h1:oZtUIOe8dh44I2q6ScRibXws4Ajl+d+nod3AaR9vL5w= -github.com/hashicorp/hcl/v2 v2.0.0/go.mod h1:oVVDG71tEinNGYCxinCYadcmKU9bglqW9pV3txagJ90= github.com/hashicorp/hcl/v2 v2.3.0 h1:iRly8YaMwTBAKhn1Ybk7VSdzbnopghktCD031P8ggUE= github.com/hashicorp/hcl/v2 v2.3.0/go.mod h1:d+FwDBbOLvpAM3Z6J7gPj/VoAGkNe/gm352ZhjJ/Zv8= github.com/hashicorp/logutils v1.0.0 h1:dLEQVugN8vlakKOUE3ihGLTZJRB4j+M2cdTm/ORI65Y= github.com/hashicorp/logutils v1.0.0/go.mod h1:QIAnNjmIWmVIIkWDTG1z5v++HQmx9WQRO+LraFDTW64= -github.com/hashicorp/terraform-config-inspect v0.0.0-20191115094559-17f92b0546e8/go.mod h1:p+ivJws3dpqbp1iP84+npOyAmTTOLMgCzrXd3GSdn/A= github.com/hashicorp/terraform-exec v0.10.0 h1:3nh/1e3u9gYRUQGOKWp/8wPR7ABlL2F14sZMZBrp+dM= github.com/hashicorp/terraform-exec v0.10.0/go.mod h1:tOT8j1J8rP05bZBGWXfMyU3HkLi1LWyqL3Bzsc3CJjo= github.com/hashicorp/terraform-json v0.5.0 h1:7TV3/F3y7QVSuN4r9BEXqnWqrAyeOtON8f0wvREtyzs= github.com/hashicorp/terraform-json v0.5.0/go.mod h1:eAbqb4w0pSlRmdvl8fOyHAi/+8jnkVYN28gJkSJrLhU= github.com/hashicorp/terraform-plugin-go v0.1.0 h1:kyXZ0nkHxiRev/q18N40IbRRk4AV0zE/MDJkDM3u8dY= github.com/hashicorp/terraform-plugin-go v0.1.0/go.mod h1:10V6F3taeDWVAoLlkmArKttR3IULlRWFAGtQIQTIDr4= -github.com/hashicorp/terraform-plugin-sdk v1.16.0 h1:NrkXMRjHErUPPTHQkZ6JIn6bByiJzGnlJzH1rVdNEuE= -github.com/hashicorp/terraform-plugin-sdk v1.16.0/go.mod h1:5sVxrwW6/xzFhZyql+Q9zXCUEJaGWcBIxBbZFLpVXOI= github.com/hashicorp/terraform-plugin-sdk/v2 v2.2.0 h1:2m4uKA97R8ijHGLwhHdpSJyI8Op1FpS/ozpoF21jK7s= github.com/hashicorp/terraform-plugin-sdk/v2 v2.2.0/go.mod h1:+12dJQebYjuU/yiq94iZUPuC66abfRBrXdpVJia3ojk= -github.com/hashicorp/terraform-plugin-test/v2 v2.1.2/go.mod h1:jerO5mrd+jVNALy8aiq+VZOg/CR8T2T1QR3jd6JKGOI= -github.com/hashicorp/terraform-svchost v0.0.0-20191011084731-65d371908596 h1:hjyO2JsNZUKT1ym+FAdlBEkGPevazYsmVgIMw7dVELg= -github.com/hashicorp/terraform-svchost v0.0.0-20191011084731-65d371908596/go.mod h1:kNDNcF7sN4DocDLBkQYz73HGKwN1ANB1blq4lIYLYvg= github.com/hashicorp/yamux v0.0.0-20180604194846-3520598351bb/go.mod h1:+NfK9FKeTrX5uv1uIXGdwYDTeHna2qgaIlx54MXqjAM= github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d h1:kJCB4vdITiW1eC1vq2e6IsrXKrZit1bv/TDYFGMp4BQ= github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d/go.mod h1:+NfK9FKeTrX5uv1uIXGdwYDTeHna2qgaIlx54MXqjAM= @@ -264,13 +249,11 @@ github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348/go.mod h1:B69LE github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc= github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU= -github.com/mattn/go-colorable v0.1.1/go.mod h1:FuOcm+DKB9mbwrcAfNl7/TZVBZ6rcnceauSikq3lYCQ= github.com/mattn/go-colorable v0.1.4/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE= github.com/mattn/go-colorable v0.1.7 h1:bQGKb3vps/j0E9GfJQ03JyhRuxsvdAanXlT9BTw3mdw= github.com/mattn/go-colorable v0.1.7/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= github.com/mattn/go-isatty v0.0.3/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNxMWT7Zi4= github.com/mattn/go-isatty v0.0.4/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNxMWT7Zi4= -github.com/mattn/go-isatty v0.0.5/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s= github.com/mattn/go-isatty v0.0.8/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s= github.com/mattn/go-isatty v0.0.10/go.mod h1:qgIWMr58cqv1PHHyhnkY9lrL7etaEgOFcMEpPG5Rm84= github.com/mattn/go-isatty v0.0.11/go.mod h1:PhnuNfih5lzO57/f3n+odYbM4JtupLOxQOAqxQCu2WE= @@ -279,7 +262,6 @@ github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Ky github.com/mattn/go-runewidth v0.0.4/go.mod h1:LwmH8dsx7+W8Uxz3IHJYH5QSwggIsqBzpuz5H//U1FU= github.com/mitchellh/cli v1.1.1 h1:J64v/xD7Clql+JVKSvkYojLOXu1ibnY9ZjGLwSt/89w= github.com/mitchellh/cli v1.1.1/go.mod h1:xcISNoH86gajksDmfB23e/pu+B+GeFRMYmoHXxx3xhI= -github.com/mitchellh/colorstring v0.0.0-20190213212951-d06e56a500db/go.mod h1:l0dey0ia/Uv7NcFFVbCLtqEBQbrT4OCwCSKTEv6enCw= github.com/mitchellh/copystructure v1.0.0 h1:Laisrj+bAB6b/yJwB5Bt3ITZhGJdqmxquMKeZ+mmkFQ= github.com/mitchellh/copystructure v1.0.0/go.mod h1:SNtv71yrdKgLRyLFxmLdkAbkKEFWgYaq1OVrnRcwhnw= github.com/mitchellh/go-homedir v1.0.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= @@ -311,8 +293,6 @@ github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/posener/complete v1.1.1/go.mod h1:em0nMJCgc9GFtwrmVmEMR/ZL6WyhyjMBndrE9hABlRI= -github.com/posener/complete v1.2.1 h1:LrvDIY//XNo65Lq84G/akBuMGlawHvGBABv8f/ZN6DI= -github.com/posener/complete v1.2.1/go.mod h1:6gapUrK/U1TAN7ciCoNRIdVC5sbdBTUh1DKN0g6uH7E= github.com/pquerna/otp v1.3.0 h1:oJV/SkzR33anKXwQU3Of42rL4wbrffP4uvUf1SvS5Xs= github.com/pquerna/otp v1.3.0/go.mod h1:dkJfzwRKNiegxyNb54X/3fLwhCynbMspSyWKnvi1AEg= github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= @@ -320,10 +300,7 @@ github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFR github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo= github.com/sergi/go-diff v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0= github.com/sergi/go-diff v1.1.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM= -github.com/spf13/afero v1.2.2 h1:5jhuqJyZCZf2JRofRvN/nIFgIWNzPa3/Vz8mYylgbWc= -github.com/spf13/afero v1.2.2/go.mod h1:9ZxEEn6pIJ8Rxe320qSDBk6AsU0r9pR7Q4OcevTdifk= github.com/spf13/pflag v1.0.2/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= -github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= @@ -332,11 +309,9 @@ github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5 github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/ulikunitz/xz v0.5.5/go.mod h1:2bypXElzHzzJZwzH67Y6wb67pO62Rzfn7BSiF4ABRW8= -github.com/ulikunitz/xz v0.5.7/go.mod h1:nbz6k7qbPmH4IRqmfOplQw/tblSgqTqBwxkY0oWt/14= github.com/ulikunitz/xz v0.5.8 h1:ERv8V6GKqVi23rgu5cj9pVfVzJbOqAY2Ntl88O6c2nQ= github.com/ulikunitz/xz v0.5.8/go.mod h1:nbz6k7qbPmH4IRqmfOplQw/tblSgqTqBwxkY0oWt/14= github.com/vmihailenco/msgpack v3.3.3+incompatible/go.mod h1:fy3FlTQTDXWkZ7Bh6AcGMlsjHatGryHQYUTf1ShIgkk= -github.com/vmihailenco/msgpack v4.0.1+incompatible/go.mod h1:fy3FlTQTDXWkZ7Bh6AcGMlsjHatGryHQYUTf1ShIgkk= github.com/vmihailenco/msgpack v4.0.4+incompatible h1:dSLoQfGFAo3F6OoNhwUmLwVgaUXK79GlxNBwueZn0xI= github.com/vmihailenco/msgpack v4.0.4+incompatible/go.mod h1:fy3FlTQTDXWkZ7Bh6AcGMlsjHatGryHQYUTf1ShIgkk= github.com/xanzy/ssh-agent v0.2.1 h1:TCbipTQL2JiiCprBWx9frJ2eJlCYT00NmctrHxVAr70= @@ -344,13 +319,9 @@ github.com/xanzy/ssh-agent v0.2.1/go.mod h1:mLlQY/MoOhWBj+gOGMQkOeiEvkx+8pJSI+0B github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= -github.com/zclconf/go-cty v1.0.0/go.mod h1:xnAOWiHeOqg2nWS62VtQ7pbOu17FtxJNW8RLEih+O3s= -github.com/zclconf/go-cty v1.1.0/go.mod h1:xnAOWiHeOqg2nWS62VtQ7pbOu17FtxJNW8RLEih+O3s= github.com/zclconf/go-cty v1.2.0/go.mod h1:hOPWgoHbaTUnI5k4D2ld+GRpFJSCe6bCM7m1q/N4PQ8= github.com/zclconf/go-cty v1.2.1 h1:vGMsygfmeCl4Xb6OA5U5XVAaQZ69FvoG7X2jUtQujb8= github.com/zclconf/go-cty v1.2.1/go.mod h1:hOPWgoHbaTUnI5k4D2ld+GRpFJSCe6bCM7m1q/N4PQ8= -github.com/zclconf/go-cty-yaml v1.0.1 h1:up11wlgAaDvlAGENcFDnZgkn0qUJurso7k6EpURKNF8= -github.com/zclconf/go-cty-yaml v1.0.1/go.mod h1:IP3Ylp0wQpYm50IHK8OZWKMu6sPJIUgKa8XhiVHura0= go.opencensus.io v0.21.0/go.mod h1:mSImk1erAIZhrmZN+AvHh14ztQfjbGwt4TtuofqLduU= go.opencensus.io v0.22.0/go.mod h1:+kGneAE2xo2IficOXnaByMWTGM9T73dGwxeWcUqIpI8= go.opencensus.io v0.22.2/go.mod h1:yxeiOL68Rb0Xd1ddK5vPZ/oVn4vY4Ynel7k9FzqtOIw= @@ -412,7 +383,6 @@ golang.org/x/net v0.0.0-20190603091049-60506f45cf65/go.mod h1:HSz+uSET+XFnRR8LxR golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20190628185345-da137c7871d7/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20190724013045-ca1201d0de80/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= -golang.org/x/net v0.0.0-20191009170851-d66e71096ffb/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20191209160850-c0dbc17a3553/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200114155413-6afb5195e5aa/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200202094626-16171245cfb2/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= diff --git a/website/docs/r/autoscaling_group.html.markdown b/website/docs/r/autoscaling_group.html.markdown index 5ad6477eb9b..901a0471ac7 100644 --- a/website/docs/r/autoscaling_group.html.markdown +++ b/website/docs/r/autoscaling_group.html.markdown @@ -352,10 +352,8 @@ This configuration block supports the following: * `strategy` - (Required) The strategy to use for instance refresh. The only allowed value is `Rolling`. See [StartInstanceRefresh Action](https://docs.aws.amazon.com/autoscaling/ec2/APIReference/API_StartInstanceRefresh.html#API_StartInstanceRefresh_RequestParameters) for more information. * `preferences` - (Optional) Override default parameters for Instance Refresh. - * `instance_warmup_seconds` - (Optional) The number of seconds until a newly launched instance is configured and ready to use. Default behavior (set with `-1` or `null`) is to match the Auto Scaling Group's health check grace period. - * `min_healthy_percentage` - (Optional) The amount of capacity in the Auto Scaling group - that must remain healthy during an instance refresh to allow the operation to continue, - as a percentage of the desired capacity of the Auto Scaling group. Defaults to `90`. + * `instance_warmup_seconds` - (Optional) The number of seconds until a newly launched instance is configured and ready to use. Default behavior is to use the Auto Scaling Group's health check grace period. + * `min_healthy_percentage` - (Optional) The amount of capacity in the Auto Scaling group that must remain healthy during an instance refresh to allow the operation to continue, as a percentage of the desired capacity of the Auto Scaling group. Defaults to `90`. ~> **NOTE:** A refresh is only started when any of the following Auto Scaling Group properties change: `launch_configuration`, `launch_template`, `mixed_instances_policy`, `vpc_zone_identifier`, `availability_zones`, `placement_group`, or any `tag` or `tags` configured to propagate at launch. From 09ab3277f2f94c3121a50dba42486abdee101373 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 4 Dec 2020 15:09:28 -0800 Subject: [PATCH 07/12] Moves nullable to experimental --- aws/internal/{ => experimental}/nullable/int.go | 0 aws/internal/{ => experimental}/nullable/int_test.go | 0 aws/internal/{ => experimental}/nullable/testing.go | 0 aws/resource_aws_autoscaling_group.go | 5 ++--- 4 files changed, 2 insertions(+), 3 deletions(-) rename aws/internal/{ => experimental}/nullable/int.go (100%) rename aws/internal/{ => experimental}/nullable/int_test.go (100%) rename aws/internal/{ => experimental}/nullable/testing.go (100%) diff --git a/aws/internal/nullable/int.go b/aws/internal/experimental/nullable/int.go similarity index 100% rename from aws/internal/nullable/int.go rename to aws/internal/experimental/nullable/int.go diff --git a/aws/internal/nullable/int_test.go b/aws/internal/experimental/nullable/int_test.go similarity index 100% rename from aws/internal/nullable/int_test.go rename to aws/internal/experimental/nullable/int_test.go diff --git a/aws/internal/nullable/testing.go b/aws/internal/experimental/nullable/testing.go similarity index 100% rename from aws/internal/nullable/testing.go rename to aws/internal/experimental/nullable/testing.go diff --git a/aws/resource_aws_autoscaling_group.go b/aws/resource_aws_autoscaling_group.go index 6d54e890ca8..b420d9fc8b7 100644 --- a/aws/resource_aws_autoscaling_group.go +++ b/aws/resource_aws_autoscaling_group.go @@ -20,9 +20,9 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/experimental/nullable" "github.com/terraform-providers/terraform-provider-aws/aws/internal/hashcode" "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" - "github.com/terraform-providers/terraform-provider-aws/aws/internal/nullable" "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/autoscaling/waiter" ) @@ -1867,8 +1867,7 @@ func expandAutoScalingGroupInstanceRefreshPreferences(l []interface{}) *autoscal refreshPreferences := &autoscaling.RefreshPreferences{} if v, ok := m["instance_warmup"]; ok { - i := nullable.Int(v.(string)) - if v, null, _ := i.Value(); !null { + if v, null, _ := nullable.Int(v.(string)).Value(); !null { refreshPreferences.InstanceWarmup = aws.Int64(v) } } From 7abd257ef10dcc7f1a08c50dec847e2048e5e42d Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 4 Dec 2020 15:15:20 -0800 Subject: [PATCH 08/12] Updates naming to "Auto Scaling Group" --- aws/resource_aws_autoscaling_group.go | 98 +++++++++---------- aws/resource_aws_autoscaling_group_test.go | 14 +-- aws/resource_aws_autoscaling_group_waiting.go | 6 +- 3 files changed, 59 insertions(+), 59 deletions(-) diff --git a/aws/resource_aws_autoscaling_group.go b/aws/resource_aws_autoscaling_group.go index b420d9fc8b7..4b1d1c3c2db 100644 --- a/aws/resource_aws_autoscaling_group.go +++ b/aws/resource_aws_autoscaling_group.go @@ -617,7 +617,7 @@ func resourceAwsAutoscalingGroupCreate(d *schema.ResourceData, meta interface{}) launchTemplateValue, launchTemplateOk := d.GetOk("launch_template") if createOpts.MixedInstancesPolicy == nil && !launchConfigurationOk && !launchTemplateOk { - return fmt.Errorf("One of `launch_configuration`, `launch_template`, or `mixed_instances_policy` must be set for an autoscaling group") + return fmt.Errorf("One of `launch_configuration`, `launch_template`, or `mixed_instances_policy` must be set for an Auto Scaling Group") } if launchConfigurationOk { @@ -688,7 +688,7 @@ func resourceAwsAutoscalingGroupCreate(d *schema.ResourceData, meta interface{}) createOpts.MaxInstanceLifetime = aws.Int64(int64(v.(int))) } - log.Printf("[DEBUG] AutoScaling Group create configuration: %#v", createOpts) + log.Printf("[DEBUG] Auto Scaling Group create configuration: %#v", createOpts) // Retry for IAM eventual consistency err := resource.Retry(1*time.Minute, func() *resource.RetryError { @@ -709,11 +709,11 @@ func resourceAwsAutoscalingGroupCreate(d *schema.ResourceData, meta interface{}) _, err = conn.CreateAutoScalingGroup(&createOpts) } if err != nil { - return fmt.Errorf("Error creating AutoScaling Group: %s", err) + return fmt.Errorf("Error creating Auto Scaling Group: %s", err) } d.SetId(d.Get("name").(string)) - log.Printf("[INFO] AutoScaling Group ID: %s", d.Id()) + log.Printf("[INFO] Auto Scaling Group ID: %s", d.Id()) if twoPhases { for _, hook := range generatePutLifecycleHookInputs(asgName, initialLifecycleHooks) { @@ -724,7 +724,7 @@ func resourceAwsAutoscalingGroupCreate(d *schema.ResourceData, meta interface{}) _, err = conn.UpdateAutoScalingGroup(&updateOpts) if err != nil { - return fmt.Errorf("Error setting AutoScaling Group initial capacity: %s", err) + return fmt.Errorf("Error setting Auto Scaling Group initial capacity: %s", err) } } @@ -759,7 +759,7 @@ func resourceAwsAutoscalingGroupRead(d *schema.ResourceData, meta interface{}) e return err } if g == nil { - log.Printf("[WARN] Autoscaling Group (%s) not found, removing from state", d.Id()) + log.Printf("[WARN] Auto Scaling Group (%s) not found, removing from state", d.Id()) d.SetId("") return nil } @@ -1056,10 +1056,10 @@ func resourceAwsAutoscalingGroupUpdate(d *schema.ResourceData, meta interface{}) } } - log.Printf("[DEBUG] AutoScaling Group update configuration: %#v", opts) + log.Printf("[DEBUG] Auto Scaling Group update configuration: %#v", opts) _, err := conn.UpdateAutoScalingGroup(&opts) if err != nil { - return fmt.Errorf("Error updating Autoscaling group: %s", err) + return fmt.Errorf("Error updating Auto Scaling Group: %s", err) } if d.HasChange("load_balancers") { @@ -1095,11 +1095,11 @@ func resourceAwsAutoscalingGroupUpdate(d *schema.ResourceData, meta interface{}) }) if err != nil { - return fmt.Errorf("error detaching AutoScaling Group (%s) Load Balancers: %s", d.Id(), err) + return fmt.Errorf("error detaching Auto Scaling Group (%s) Load Balancers: %s", d.Id(), err) } if err := waitUntilAutoscalingGroupLoadBalancersRemoved(conn, d.Id()); err != nil { - return fmt.Errorf("error describing AutoScaling Group (%s) Load Balancers being removed: %s", d.Id(), err) + return fmt.Errorf("error describing Auto Scaling Group (%s) Load Balancers being removed: %s", d.Id(), err) } } } @@ -1122,11 +1122,11 @@ func resourceAwsAutoscalingGroupUpdate(d *schema.ResourceData, meta interface{}) }) if err != nil { - return fmt.Errorf("error attaching AutoScaling Group (%s) Load Balancers: %s", d.Id(), err) + return fmt.Errorf("error attaching Auto Scaling Group (%s) Load Balancers: %s", d.Id(), err) } if err := waitUntilAutoscalingGroupLoadBalancersAdded(conn, d.Id()); err != nil { - return fmt.Errorf("error describing AutoScaling Group (%s) Load Balancers being added: %s", d.Id(), err) + return fmt.Errorf("error describing Auto Scaling Group (%s) Load Balancers being added: %s", d.Id(), err) } } } @@ -1164,11 +1164,11 @@ func resourceAwsAutoscalingGroupUpdate(d *schema.ResourceData, meta interface{}) TargetGroupARNs: batch, }) if err != nil { - return fmt.Errorf("Error updating Load Balancers Target Groups for AutoScaling Group (%s), error: %s", d.Id(), err) + return fmt.Errorf("Error updating Load Balancers Target Groups for Auto Scaling Group (%s), error: %s", d.Id(), err) } if err := waitUntilAutoscalingGroupLoadBalancerTargetGroupsRemoved(conn, d.Id()); err != nil { - return fmt.Errorf("error describing AutoScaling Group (%s) Load Balancer Target Groups being removed: %s", d.Id(), err) + return fmt.Errorf("error describing Auto Scaling Group (%s) Load Balancer Target Groups being removed: %s", d.Id(), err) } } @@ -1191,11 +1191,11 @@ func resourceAwsAutoscalingGroupUpdate(d *schema.ResourceData, meta interface{}) }) if err != nil { - return fmt.Errorf("Error updating Load Balancers Target Groups for AutoScaling Group (%s), error: %s", d.Id(), err) + return fmt.Errorf("Error updating Load Balancers Target Groups for Auto Scaling Group (%s), error: %s", d.Id(), err) } if err := waitUntilAutoscalingGroupLoadBalancerTargetGroupsAdded(conn, d.Id()); err != nil { - return fmt.Errorf("error describing AutoScaling Group (%s) Load Balancer Target Groups being added: %s", d.Id(), err) + return fmt.Errorf("error describing Auto Scaling Group (%s) Load Balancer Target Groups being added: %s", d.Id(), err) } } } @@ -1203,25 +1203,25 @@ func resourceAwsAutoscalingGroupUpdate(d *schema.ResourceData, meta interface{}) if shouldWaitForCapacity { if err := waitForASGCapacity(d, meta, capacitySatisfiedUpdate); err != nil { - return fmt.Errorf("Error waiting for AutoScaling Group Capacity: %s", err) + return fmt.Errorf("Error waiting for Auto Scaling Group Capacity: %s", err) } } if d.HasChange("enabled_metrics") { if err := updateASGMetricsCollection(d, conn); err != nil { - return fmt.Errorf("Error updating AutoScaling Group Metrics collection: %s", err) + return fmt.Errorf("Error updating Auto Scaling Group Metrics collection: %s", err) } } if d.HasChange("suspended_processes") { if err := updateASGSuspendedProcesses(d, conn); err != nil { - return fmt.Errorf("Error updating AutoScaling Group Suspended Processes: %s", err) + return fmt.Errorf("Error updating Auto Scaling Group Suspended Processes: %s", err) } } if instanceRefreshRaw, ok := d.GetOk("instance_refresh"); ok && shouldRefreshInstances { if err := autoScalingGroupRefreshInstances(conn, d.Id(), instanceRefreshRaw.([]interface{})); err != nil { - return fmt.Errorf("failed to start instance refresh of asg %s: %w", d.Id(), err) + return fmt.Errorf("failed to start instance refresh of Auto Scaling Group %s: %w", d.Id(), err) } } @@ -1231,7 +1231,7 @@ func resourceAwsAutoscalingGroupUpdate(d *schema.ResourceData, meta interface{}) func resourceAwsAutoscalingGroupDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).autoscalingconn - // Read the autoscaling group first. If it doesn't exist, we're done. + // Read the Auto Scaling Group first. If it doesn't exist, we're done. // We need the group in order to check if there are instances attached. // If so, we need to remove those first. g, err := getAwsAutoscalingGroup(d.Id(), conn) @@ -1239,7 +1239,7 @@ func resourceAwsAutoscalingGroupDelete(d *schema.ResourceData, meta interface{}) return err } if g == nil { - log.Printf("[WARN] Autoscaling Group (%s) not found, removing from state", d.Id()) + log.Printf("[WARN] Auto Scaling Group (%s) not found, removing from state", d.Id()) return nil } if len(g.Instances) > 0 || *g.DesiredCapacity > 0 { @@ -1248,7 +1248,7 @@ func resourceAwsAutoscalingGroupDelete(d *schema.ResourceData, meta interface{}) } } - log.Printf("[DEBUG] AutoScaling Group destroy: %v", d.Id()) + log.Printf("[DEBUG] Auto Scaling Group destroy: %v", d.Id()) deleteopts := autoscaling.DeleteAutoScalingGroupInput{ AutoScalingGroupName: aws.String(d.Id()), ForceDelete: aws.Bool(d.Get("force_delete").(bool)), @@ -1282,7 +1282,7 @@ func resourceAwsAutoscalingGroupDelete(d *schema.ResourceData, meta interface{}) } } if err != nil { - return fmt.Errorf("Error deleting autoscaling group: %s", err) + return fmt.Errorf("Error deleting Auto Scaling Group: %s", err) } var group *autoscaling.Group @@ -1301,7 +1301,7 @@ func resourceAwsAutoscalingGroupDelete(d *schema.ResourceData, meta interface{}) } } if err != nil { - return fmt.Errorf("Error deleting autoscaling group: %s", err) + return fmt.Errorf("Error deleting Auto Scaling Group: %s", err) } return nil } @@ -1313,7 +1313,7 @@ func getAwsAutoscalingGroup(asgName string, conn *autoscaling.AutoScaling) (*aut AutoScalingGroupNames: []*string{aws.String(asgName)}, } - log.Printf("[DEBUG] AutoScaling Group describe configuration: %#v", describeOpts) + log.Printf("[DEBUG] Auto Scaling Group describe configuration: %#v", describeOpts) describeGroups, err := conn.DescribeAutoScalingGroups(&describeOpts) if err != nil { autoscalingerr, ok := err.(awserr.Error) @@ -1321,10 +1321,10 @@ func getAwsAutoscalingGroup(asgName string, conn *autoscaling.AutoScaling) (*aut return nil, nil } - return nil, fmt.Errorf("Error retrieving AutoScaling groups: %s", err) + return nil, fmt.Errorf("Error retrieving Auto Scaling Groups: %s", err) } - // Search for the autoscaling group + // Search for the Auto Scaling Group for idx, asc := range describeGroups.AutoScalingGroups { if *asc.AutoScalingGroupName == asgName { return describeGroups.AutoScalingGroups[idx], nil @@ -1338,12 +1338,12 @@ func resourceAwsAutoscalingGroupDrain(d *schema.ResourceData, meta interface{}) conn := meta.(*AWSClient).autoscalingconn if d.Get("force_delete").(bool) { - log.Printf("[DEBUG] Skipping ASG drain, force_delete was set.") + log.Printf("[DEBUG] Skipping Auto Scaling Group drain, force_delete was set.") return nil } // First, set the capacity to zero so the group will drain - log.Printf("[DEBUG] Reducing autoscaling group capacity to zero") + log.Printf("[DEBUG] Reducing Auto Scaling Group capacity to zero") opts := autoscaling.UpdateAutoScalingGroupInput{ AutoScalingGroupName: aws.String(d.Id()), DesiredCapacity: aws.Int64(0), @@ -1363,7 +1363,7 @@ func resourceAwsAutoscalingGroupDrain(d *schema.ResourceData, meta interface{}) return resource.NonRetryableError(err) } if g == nil { - log.Printf("[WARN] Autoscaling Group (%s) not found, removing from state", d.Id()) + log.Printf("[WARN] Auto Scaling Group (%s) not found, removing from state", d.Id()) d.SetId("") return nil } @@ -1378,14 +1378,14 @@ func resourceAwsAutoscalingGroupDrain(d *schema.ResourceData, meta interface{}) if isResourceTimeoutError(err) { g, err = getAwsAutoscalingGroup(d.Id(), conn) if err != nil { - return fmt.Errorf("Error getting autoscaling group info when draining: %s", err) + return fmt.Errorf("Error getting Auto Scaling Group info when draining: %s", err) } if g != nil && len(g.Instances) > 0 { return fmt.Errorf("Group still has %d instances", len(g.Instances)) } } if err != nil { - return fmt.Errorf("Error draining autoscaling group: %s", err) + return fmt.Errorf("Error draining Auto Scaling Group: %s", err) } return nil } @@ -1407,7 +1407,7 @@ func enableASGMetricsCollection(d *schema.ResourceData, conn *autoscaling.AutoSc Metrics: expandStringList(d.Get("enabled_metrics").(*schema.Set).List()), } - log.Printf("[INFO] Enabling metrics collection for the ASG: %s", d.Id()) + log.Printf("[INFO] Enabling metrics collection for the Auto Scaling Group: %s", d.Id()) _, metricsErr := conn.EnableMetricsCollection(props) return metricsErr @@ -1434,7 +1434,7 @@ func updateASGSuspendedProcesses(d *schema.ResourceData, conn *autoscaling.AutoS _, err := conn.ResumeProcesses(props) if err != nil { - return fmt.Errorf("Error Resuming Processes for ASG %q: %s", d.Id(), err) + return fmt.Errorf("Error Resuming Processes for Auto Scaling Group %q: %s", d.Id(), err) } } @@ -1447,7 +1447,7 @@ func updateASGSuspendedProcesses(d *schema.ResourceData, conn *autoscaling.AutoS _, err := conn.SuspendProcesses(props) if err != nil { - return fmt.Errorf("Error Suspending Processes for ASG %q: %s", d.Id(), err) + return fmt.Errorf("Error Suspending Processes for Auto Scaling Group %q: %s", d.Id(), err) } } @@ -1477,7 +1477,7 @@ func updateASGMetricsCollection(d *schema.ResourceData, conn *autoscaling.AutoSc _, err := conn.DisableMetricsCollection(props) if err != nil { - return fmt.Errorf("Failure to Disable metrics collection types for ASG %s: %s", d.Id(), err) + return fmt.Errorf("Failure to Disable metrics collection types for Auto Scaling Group %s: %s", d.Id(), err) } } @@ -1491,7 +1491,7 @@ func updateASGMetricsCollection(d *schema.ResourceData, conn *autoscaling.AutoSc _, err := conn.EnableMetricsCollection(props) if err != nil { - return fmt.Errorf("Failure to Enable metrics collection types for ASG %s: %s", d.Id(), err) + return fmt.Errorf("Failure to Enable metrics collection types for Auto Scaling Group %s: %s", d.Id(), err) } } @@ -1883,22 +1883,22 @@ func expandAutoScalingGroupInstanceRefreshPreferences(l []interface{}) *autoscal // Auto Scaling Group. If there is already an active refresh, it is cancelled. func autoScalingGroupRefreshInstances(conn *autoscaling.AutoScaling, asgName string, refreshConfig []interface{}) error { - log.Printf("[DEBUG] Cancelling active Instance Refresh in ASG %s, if any...", asgName) + log.Printf("[DEBUG] Cancelling active Instance Refresh in Auto Scaling Group %s, if any...", asgName) if err := cancelAutoscalingInstanceRefresh(conn, asgName); err != nil { - // todo: add comment about subsequent ASG updates not picking up the refresh? + // todo: add comment about subsequent Auto Scaling Group updates not picking up the refresh? return fmt.Errorf("failed to cancel previous refresh: %w", err) } input := createAutoScalingGroupInstanceRefreshInput(asgName, refreshConfig) - log.Printf("[DEBUG] Starting Instance Refresh on ASG (%s): %s", asgName, input) + log.Printf("[DEBUG] Starting Instance Refresh on Auto Scaling Group (%s): %s", asgName, input) output, err := conn.StartInstanceRefresh(input) if err != nil { return err } instanceRefreshID := aws.StringValue(output.InstanceRefreshId) - log.Printf("[INFO] Started Instance Refresh (%s) on ASG (%s)", instanceRefreshID, asgName) + log.Printf("[INFO] Started Instance Refresh (%s) on Auto Scaling Group (%s)", instanceRefreshID, asgName) return nil } @@ -1909,28 +1909,28 @@ func cancelAutoscalingInstanceRefresh(conn *autoscaling.AutoScaling, asgName str input := autoscaling.CancelInstanceRefreshInput{ AutoScalingGroupName: aws.String(asgName), } - log.Printf("[DEBUG] Attempting to cancel Instance Refresh on ASG (%s): %s", asgName, input) + log.Printf("[DEBUG] Attempting to cancel Instance Refresh on Auto Scaling Group (%s): %s", asgName, input) output, err := conn.CancelInstanceRefresh(&input) if tfawserr.ErrCodeEquals(err, autoscaling.ErrCodeActiveInstanceRefreshNotFoundFault) { - log.Printf("[DEBUG] No active Instance Refresh on ASG (%s)", asgName) + log.Printf("[DEBUG] No active Instance Refresh on Auto Scaling Group (%s)", asgName) return nil } if err != nil { - return fmt.Errorf("error cancelling Instance Refresh on ASG (%s): %w", asgName, err) + return fmt.Errorf("error cancelling Instance Refresh on Auto Scaling Group (%s): %w", asgName, err) } if output == nil { - return fmt.Errorf("error cancelling Instance Refresh on ASG (%s): empty result", asgName) + return fmt.Errorf("error cancelling Instance Refresh on Auto Scaling Group (%s): empty result", asgName) } instanceRefreshID := aws.StringValue(output.InstanceRefreshId) - log.Printf("[INFO] Requested cancellation of Instance Refresh (%s) on ASG (%s)", instanceRefreshID, asgName) + log.Printf("[INFO] Requested cancellation of Instance Refresh (%s) on Auto Scaling Group (%s)", instanceRefreshID, asgName) _, err = waiter.InstanceRefreshCancelled(conn, asgName, instanceRefreshID) if err != nil { - return fmt.Errorf("error waiting for cancellation of Instance Refresh (%s) on ASG (%s): %w", instanceRefreshID, asgName, err) + return fmt.Errorf("error waiting for cancellation of Instance Refresh (%s) on Auto Scaling Group (%s): %w", instanceRefreshID, asgName, err) } - log.Printf("[INFO] Cancelled Instance Refresh (%s) on ASG (%s)", instanceRefreshID, asgName) + log.Printf("[INFO] Cancelled Instance Refresh (%s) on Auto Scaling Group (%s)", instanceRefreshID, asgName) return nil } diff --git a/aws/resource_aws_autoscaling_group_test.go b/aws/resource_aws_autoscaling_group_test.go index 441c9ae8de8..f86eb1ec17c 100644 --- a/aws/resource_aws_autoscaling_group_test.go +++ b/aws/resource_aws_autoscaling_group_test.go @@ -37,14 +37,14 @@ func testSweepAutoscalingGroups(region string) error { resp, err := conn.DescribeAutoScalingGroups(&autoscaling.DescribeAutoScalingGroupsInput{}) if err != nil { if testSweepSkipSweepError(err) { - log.Printf("[WARN] Skipping AutoScaling Group sweep for %s: %s", region, err) + log.Printf("[WARN] Skipping Auto Scaling Group sweep for %s: %s", region, err) return nil } - return fmt.Errorf("Error retrieving AutoScaling Groups in Sweeper: %s", err) + return fmt.Errorf("Error retrieving Auto Scaling Groups in Sweeper: %s", err) } if len(resp.AutoScalingGroups) == 0 { - log.Print("[DEBUG] No aws autoscaling groups to sweep") + log.Print("[DEBUG] No Auto Scaling Groups to sweep") return nil } @@ -1124,7 +1124,7 @@ func testAccCheckAWSAutoScalingGroupExists(n string, group *autoscaling.Group) r } if rs.Primary.ID == "" { - return fmt.Errorf("No AutoScaling Group ID is set") + return fmt.Errorf("No Auto Scaling Group ID is set") } conn := testAccProvider.Meta().(*AWSClient).autoscalingconn @@ -1140,7 +1140,7 @@ func testAccCheckAWSAutoScalingGroupExists(n string, group *autoscaling.Group) r if len(describeGroups.AutoScalingGroups) != 1 || *describeGroups.AutoScalingGroups[0].AutoScalingGroupName != rs.Primary.ID { - return fmt.Errorf("AutoScaling Group not found") + return fmt.Errorf("Auto Scaling Group not found") } *group = *describeGroups.AutoScalingGroups[0] @@ -1166,7 +1166,7 @@ func testAccCheckAWSAutoScalingGroupDestroy(s *terraform.State) error { if err == nil { if len(describeGroups.AutoScalingGroups) != 0 && *describeGroups.AutoScalingGroups[0].AutoScalingGroupName == rs.Primary.ID { - return fmt.Errorf("AutoScaling Group still exists") + return fmt.Errorf("Auto Scaling Group still exists") } } @@ -1186,7 +1186,7 @@ func testAccCheckAWSAutoScalingGroupDestroy(s *terraform.State) error { func testAccCheckAWSAutoScalingGroupAttributes(group *autoscaling.Group, name string) resource.TestCheckFunc { return func(s *terraform.State) error { if *group.AutoScalingGroupName != name { - return fmt.Errorf("Bad Autoscaling Group name, expected (%s), got (%s)", name, *group.AutoScalingGroupName) + return fmt.Errorf("Bad Auto Scaling Group name, expected (%s), got (%s)", name, *group.AutoScalingGroupName) } if *group.MaxSize != 5 { diff --git a/aws/resource_aws_autoscaling_group_waiting.go b/aws/resource_aws_autoscaling_group_waiting.go index 1e3279c3d3d..4a432d149a1 100644 --- a/aws/resource_aws_autoscaling_group_waiting.go +++ b/aws/resource_aws_autoscaling_group_waiting.go @@ -41,7 +41,7 @@ func waitForASGCapacity( return resource.NonRetryableError(err) } if g == nil { - log.Printf("[WARN] Autoscaling Group (%s) not found, removing from state", d.Id()) + log.Printf("[WARN] Auto Scaling Group (%s) not found, removing from state", d.Id()) d.SetId("") return nil } @@ -57,11 +57,11 @@ func waitForASGCapacity( g, err := getAwsAutoscalingGroup(d.Id(), meta.(*AWSClient).autoscalingconn) if err != nil { - return fmt.Errorf("Error getting autoscaling group info: %s", err) + return fmt.Errorf("Error getting Auto Scaling Group info: %s", err) } if g == nil { - log.Printf("[WARN] Autoscaling Group (%s) not found, removing from state", d.Id()) + log.Printf("[WARN] Auto Scaling Group (%s) not found, removing from state", d.Id()) d.SetId("") return nil } From 707d2e456b92fe16b7d8dc272707578b794c184d Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 4 Dec 2020 16:07:24 -0800 Subject: [PATCH 09/12] Only try to cancel Instance Refresh when we get an "InstanceRefreshInProgress" error --- .../service/autoscaling/waiter/waiter.go | 25 ++-------- aws/resource_aws_autoscaling_group.go | 48 ++++++++----------- 2 files changed, 26 insertions(+), 47 deletions(-) diff --git a/aws/internal/service/autoscaling/waiter/waiter.go b/aws/internal/service/autoscaling/waiter/waiter.go index f278c3f859a..b8985880b55 100644 --- a/aws/internal/service/autoscaling/waiter/waiter.go +++ b/aws/internal/service/autoscaling/waiter/waiter.go @@ -8,30 +8,15 @@ import ( ) const ( - // Maximum amount of time to wait for an InstanceRefresh to be Successful - InstanceRefreshSuccessfulTimeout = 5 * time.Minute + // Maximum amount of time to wait for an InstanceRefresh to be started + // Must be at least as long as InstanceRefreshCancelledTimeout, since we try to cancel any + // existing Instance Refreshes when starting. + InstanceRefreshStartedTimeout = InstanceRefreshCancelledTimeout - // Maximum amount of time to wait for an InstanceRefresh to be Cancelled + // Maximum amount of time to wait for an Instance Refresh to be Cancelled InstanceRefreshCancelledTimeout = 10 * time.Minute ) -func InstanceRefreshSuccessful(conn *autoscaling.AutoScaling, asgName, instanceRefreshId string) (*autoscaling.InstanceRefresh, error) { - stateConf := &resource.StateChangeConf{ - Pending: []string{autoscaling.InstanceRefreshStatusPending, autoscaling.InstanceRefreshStatusInProgress}, - Target: []string{autoscaling.InstanceRefreshStatusSuccessful}, - Refresh: InstanceRefreshStatus(conn, asgName, instanceRefreshId), - Timeout: InstanceRefreshSuccessfulTimeout, - } - - outputRaw, err := stateConf.WaitForState() - - if v, ok := outputRaw.(*autoscaling.InstanceRefresh); ok { - return v, err - } - - return nil, err -} - func InstanceRefreshCancelled(conn *autoscaling.AutoScaling, asgName, instanceRefreshId string) (*autoscaling.InstanceRefresh, error) { stateConf := &resource.StateChangeConf{ Pending: []string{autoscaling.InstanceRefreshStatusPending, autoscaling.InstanceRefreshStatusInProgress, autoscaling.InstanceRefreshStatusCancelling}, diff --git a/aws/resource_aws_autoscaling_group.go b/aws/resource_aws_autoscaling_group.go index 4b1d1c3c2db..4430c0dec7e 100644 --- a/aws/resource_aws_autoscaling_group.go +++ b/aws/resource_aws_autoscaling_group.go @@ -1776,6 +1776,7 @@ func waitUntilAutoscalingGroupLoadBalancersAdded(conn *autoscaling.AutoScaling, var lbAdding bool for { + // TODO: generate Pages function output, err := conn.DescribeLoadBalancers(input) if err != nil { @@ -1879,40 +1880,38 @@ func expandAutoScalingGroupInstanceRefreshPreferences(l []interface{}) *autoscal return refreshPreferences } -// autoScalingGroupRefreshInstances starts a new Instance Refresh in this -// Auto Scaling Group. If there is already an active refresh, it is cancelled. func autoScalingGroupRefreshInstances(conn *autoscaling.AutoScaling, asgName string, refreshConfig []interface{}) error { - - log.Printf("[DEBUG] Cancelling active Instance Refresh in Auto Scaling Group %s, if any...", asgName) - - if err := cancelAutoscalingInstanceRefresh(conn, asgName); err != nil { - // todo: add comment about subsequent Auto Scaling Group updates not picking up the refresh? - return fmt.Errorf("failed to cancel previous refresh: %w", err) - } - input := createAutoScalingGroupInstanceRefreshInput(asgName, refreshConfig) - log.Printf("[DEBUG] Starting Instance Refresh on Auto Scaling Group (%s): %s", asgName, input) - output, err := conn.StartInstanceRefresh(input) + err := resource.Retry(waiter.InstanceRefreshStartedTimeout, func() *resource.RetryError { + _, err := conn.StartInstanceRefresh(input) + if tfawserr.ErrCodeEquals(err, autoscaling.ErrCodeInstanceRefreshInProgressFault) { + cancelErr := cancelAutoscalingInstanceRefresh(conn, asgName) + if cancelErr != nil { + return resource.NonRetryableError(cancelErr) + } + return resource.RetryableError(err) + } + if err != nil { + return resource.NonRetryableError(err) + } + return nil + }) + if isResourceTimeoutError(err) { + _, err = conn.StartInstanceRefresh(input) + } if err != nil { - return err + return fmt.Errorf("error starting Instance Refresh: %w", err) } - instanceRefreshID := aws.StringValue(output.InstanceRefreshId) - - log.Printf("[INFO] Started Instance Refresh (%s) on Auto Scaling Group (%s)", instanceRefreshID, asgName) return nil } -// cancelAutoscalingInstanceRefresh cancels the currently active Instance Refresh -// of this Auto-Scaling Group, if any, and waits until the refresh is Cancelled. func cancelAutoscalingInstanceRefresh(conn *autoscaling.AutoScaling, asgName string) error { input := autoscaling.CancelInstanceRefreshInput{ AutoScalingGroupName: aws.String(asgName), } - log.Printf("[DEBUG] Attempting to cancel Instance Refresh on Auto Scaling Group (%s): %s", asgName, input) output, err := conn.CancelInstanceRefresh(&input) if tfawserr.ErrCodeEquals(err, autoscaling.ErrCodeActiveInstanceRefreshNotFoundFault) { - log.Printf("[DEBUG] No active Instance Refresh on Auto Scaling Group (%s)", asgName) return nil } if err != nil { @@ -1922,15 +1921,10 @@ func cancelAutoscalingInstanceRefresh(conn *autoscaling.AutoScaling, asgName str return fmt.Errorf("error cancelling Instance Refresh on Auto Scaling Group (%s): empty result", asgName) } - instanceRefreshID := aws.StringValue(output.InstanceRefreshId) - log.Printf("[INFO] Requested cancellation of Instance Refresh (%s) on Auto Scaling Group (%s)", instanceRefreshID, asgName) - - _, err = waiter.InstanceRefreshCancelled(conn, asgName, instanceRefreshID) + _, err = waiter.InstanceRefreshCancelled(conn, asgName, aws.StringValue(output.InstanceRefreshId)) if err != nil { - return fmt.Errorf("error waiting for cancellation of Instance Refresh (%s) on Auto Scaling Group (%s): %w", instanceRefreshID, asgName, err) + return fmt.Errorf("error waiting for cancellation of Instance Refresh (%s) on Auto Scaling Group (%s): %w", aws.StringValue(output.InstanceRefreshId), asgName, err) } - log.Printf("[INFO] Cancelled Instance Refresh (%s) on Auto Scaling Group (%s)", instanceRefreshID, asgName) - return nil } From 2bf871a3a52665e609f8f40c5ec3ffd7124eff51 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Tue, 8 Dec 2020 11:29:32 -0800 Subject: [PATCH 10/12] Removes non-default refresh triggers and adds testing for cancelling in-progress refreshes --- .../service/autoscaling/waiter/waiter.go | 2 +- aws/resource_aws_autoscaling_group.go | 31 +- aws/resource_aws_autoscaling_group_test.go | 264 ++++++++++-------- .../docs/r/autoscaling_group.html.markdown | 2 +- 4 files changed, 156 insertions(+), 143 deletions(-) diff --git a/aws/internal/service/autoscaling/waiter/waiter.go b/aws/internal/service/autoscaling/waiter/waiter.go index b8985880b55..54e11e736c7 100644 --- a/aws/internal/service/autoscaling/waiter/waiter.go +++ b/aws/internal/service/autoscaling/waiter/waiter.go @@ -14,7 +14,7 @@ const ( InstanceRefreshStartedTimeout = InstanceRefreshCancelledTimeout // Maximum amount of time to wait for an Instance Refresh to be Cancelled - InstanceRefreshCancelledTimeout = 10 * time.Minute + InstanceRefreshCancelledTimeout = 15 * time.Minute ) func InstanceRefreshCancelled(conn *autoscaling.AutoScaling, asgName, instanceRefreshId string) (*autoscaling.InstanceRefresh, error) { diff --git a/aws/resource_aws_autoscaling_group.go b/aws/resource_aws_autoscaling_group.go index 4430c0dec7e..0def21ee46c 100644 --- a/aws/resource_aws_autoscaling_group.go +++ b/aws/resource_aws_autoscaling_group.go @@ -864,7 +864,6 @@ func resourceAwsAutoscalingGroupRead(d *schema.ResourceData, meta interface{}) e return nil } -// TODO: make this a waiter function func waitUntilAutoscalingGroupLoadBalancerTargetGroupsRemoved(conn *autoscaling.AutoScaling, asgName string) error { input := &autoscaling.DescribeLoadBalancerTargetGroupsInput{ AutoScalingGroupName: aws.String(asgName), @@ -872,7 +871,6 @@ func waitUntilAutoscalingGroupLoadBalancerTargetGroupsRemoved(conn *autoscaling. var tgRemoving bool for { - // TODO: generate Pages function output, err := conn.DescribeLoadBalancerTargetGroups(input) if err != nil { @@ -902,7 +900,6 @@ func waitUntilAutoscalingGroupLoadBalancerTargetGroupsRemoved(conn *autoscaling. return nil } -// TODO: make this a waiter function func waitUntilAutoscalingGroupLoadBalancerTargetGroupsAdded(conn *autoscaling.AutoScaling, asgName string) error { input := &autoscaling.DescribeLoadBalancerTargetGroupsInput{ AutoScalingGroupName: aws.String(asgName), @@ -910,7 +907,6 @@ func waitUntilAutoscalingGroupLoadBalancerTargetGroupsAdded(conn *autoscaling.Au var tgAdding bool for { - // TODO: generate Pages function output, err := conn.DescribeLoadBalancerTargetGroups(input) if err != nil { @@ -976,7 +972,6 @@ func resourceAwsAutoscalingGroupUpdate(d *schema.ResourceData, meta interface{}) if d.HasChange("mixed_instances_policy") { opts.MixedInstancesPolicy = expandAutoScalingMixedInstancesPolicy(d.Get("mixed_instances_policy").([]interface{})) - // TODO: optional trigger shouldRefreshInstances = true } @@ -1002,26 +997,18 @@ func resourceAwsAutoscalingGroupUpdate(d *schema.ResourceData, meta interface{}) opts.HealthCheckType = aws.String(d.Get("health_check_type").(string)) } - // TODO: this probably needs a wait for capacity if d.HasChange("vpc_zone_identifier") { opts.VPCZoneIdentifier = expandVpcZoneIdentifiers(d.Get("vpc_zone_identifier").(*schema.Set).List()) - // TODO: no - shouldRefreshInstances = true } - // TODO: this probably needs a wait for capacity if d.HasChange("availability_zones") { if v, ok := d.GetOk("availability_zones"); ok && v.(*schema.Set).Len() > 0 { opts.AvailabilityZones = expandStringList(v.(*schema.Set).List()) } - // TODO: no - shouldRefreshInstances = true } if d.HasChange("placement_group") { opts.PlacementGroup = aws.String(d.Get("placement_group").(string)) - // TODO: optional trigger - shouldRefreshInstances = true } if d.HasChange("termination_policies") { @@ -1201,9 +1188,15 @@ func resourceAwsAutoscalingGroupUpdate(d *schema.ResourceData, meta interface{}) } } + if instanceRefreshRaw, ok := d.GetOk("instance_refresh"); ok && shouldRefreshInstances { + if err := autoScalingGroupRefreshInstances(conn, d.Id(), instanceRefreshRaw.([]interface{})); err != nil { + return fmt.Errorf("failed to start instance refresh of Auto Scaling Group %s: %w", d.Id(), err) + } + } + if shouldWaitForCapacity { if err := waitForASGCapacity(d, meta, capacitySatisfiedUpdate); err != nil { - return fmt.Errorf("Error waiting for Auto Scaling Group Capacity: %s", err) + return fmt.Errorf("error waiting for Auto Scaling Group Capacity: %w", err) } } @@ -1219,12 +1212,6 @@ func resourceAwsAutoscalingGroupUpdate(d *schema.ResourceData, meta interface{}) } } - if instanceRefreshRaw, ok := d.GetOk("instance_refresh"); ok && shouldRefreshInstances { - if err := autoScalingGroupRefreshInstances(conn, d.Id(), instanceRefreshRaw.([]interface{})); err != nil { - return fmt.Errorf("failed to start instance refresh of Auto Scaling Group %s: %w", d.Id(), err) - } - } - return resourceAwsAutoscalingGroupRead(d, meta) } @@ -1768,7 +1755,6 @@ func flattenAutoScalingMixedInstancesPolicy(mixedInstancesPolicy *autoscaling.Mi return []interface{}{m} } -// TODO: make this a waiter function func waitUntilAutoscalingGroupLoadBalancersAdded(conn *autoscaling.AutoScaling, asgName string) error { input := &autoscaling.DescribeLoadBalancersInput{ AutoScalingGroupName: aws.String(asgName), @@ -1776,7 +1762,6 @@ func waitUntilAutoscalingGroupLoadBalancersAdded(conn *autoscaling.AutoScaling, var lbAdding bool for { - // TODO: generate Pages function output, err := conn.DescribeLoadBalancers(input) if err != nil { @@ -1806,7 +1791,6 @@ func waitUntilAutoscalingGroupLoadBalancersAdded(conn *autoscaling.AutoScaling, return nil } -// TODO: make this a waiter function func waitUntilAutoscalingGroupLoadBalancersRemoved(conn *autoscaling.AutoScaling, asgName string) error { input := &autoscaling.DescribeLoadBalancersInput{ AutoScalingGroupName: aws.String(asgName), @@ -1814,7 +1798,6 @@ func waitUntilAutoscalingGroupLoadBalancersRemoved(conn *autoscaling.AutoScaling var lbRemoving bool for { - // TODO: generate Pages function output, err := conn.DescribeLoadBalancers(input) if err != nil { diff --git a/aws/resource_aws_autoscaling_group_test.go b/aws/resource_aws_autoscaling_group_test.go index f86eb1ec17c..2c1efaef165 100644 --- a/aws/resource_aws_autoscaling_group_test.go +++ b/aws/resource_aws_autoscaling_group_test.go @@ -98,7 +98,7 @@ func TestAccAWSAutoScalingGroup_basic(t *testing.T) { testAccCheckAWSAutoScalingGroupAttributes(&group, randName), testAccMatchResourceAttrRegionalARN("aws_autoscaling_group.bar", "arn", "autoscaling", regexp.MustCompile(`autoScalingGroup:.+`)), resource.TestCheckTypeSetElemAttrPair("aws_autoscaling_group.bar", "availability_zones.*", "data.aws_availability_zones.available", "names.0"), - testAccCheckAutoscalingLatestInstanceRefreshState(&group, 0, 0, nil), + testAccCheckAutoScalingInstanceRefreshCount(&group, 0), resource.TestCheckResourceAttr("aws_autoscaling_group.bar", "default_cooldown", "300"), resource.TestCheckResourceAttr("aws_autoscaling_group.bar", "desired_capacity", "4"), resource.TestCheckResourceAttr("aws_autoscaling_group.bar", "enabled_metrics.#", "0"), @@ -148,7 +148,7 @@ func TestAccAWSAutoScalingGroup_basic(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckAWSAutoScalingGroupExists("aws_autoscaling_group.bar", &group), testAccCheckAWSLaunchConfigurationExists("aws_launch_configuration.new", &lc), - testAccCheckAutoscalingLatestInstanceRefreshState(&group, 0, 0, nil), + testAccCheckAutoScalingInstanceRefreshCount(&group, 0), resource.TestCheckResourceAttr("aws_autoscaling_group.bar", "desired_capacity", "5"), resource.TestCheckResourceAttr("aws_autoscaling_group.bar", "termination_policies.0", "ClosestToNextInstanceHour"), resource.TestCheckResourceAttr("aws_autoscaling_group.bar", "protect_from_scale_in", "true"), @@ -992,7 +992,7 @@ func TestAccAWSAutoScalingGroup_ALB_TargetGroups_ELBCapacity(t *testing.T) { }) } -func TestAccAWSAutoScalingGroup_InstanceRefresh_Enabled(t *testing.T) { +func TestAccAWSAutoScalingGroup_InstanceRefresh_Basic(t *testing.T) { var group autoscaling.Group resourceName := "aws_autoscaling_group.test" @@ -1002,14 +1002,9 @@ func TestAccAWSAutoScalingGroup_InstanceRefresh_Enabled(t *testing.T) { CheckDestroy: testAccCheckAWSAutoScalingGroupDestroy, Steps: []resource.TestStep{ { - // check that an instance refresh isn't started by a new asg - Config: testAccAwsAutoScalingGroupConfig_InstanceRefresh_Basic("Alpha", 1), + Config: testAccAwsAutoScalingGroupConfig_InstanceRefresh_Basic(), Check: resource.ComposeTestCheckFunc( testAccCheckAWSAutoScalingGroupExists(resourceName, &group), - resource.TestCheckResourceAttr(resourceName, "min_size", "1"), - resource.TestCheckResourceAttr(resourceName, "max_size", "2"), - resource.TestCheckResourceAttr(resourceName, "desired_capacity", "1"), - testAccCheckAutoscalingLatestInstanceRefreshState(&group, 0, 0, nil), resource.TestCheckResourceAttr(resourceName, "instance_refresh.#", "1"), resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.strategy", "Rolling"), resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.preferences.#", "0"), @@ -1026,14 +1021,9 @@ func TestAccAWSAutoScalingGroup_InstanceRefresh_Enabled(t *testing.T) { }, }, { - // check that changing asg size doesn't trigger a refresh - Config: testAccAwsAutoScalingGroupConfig_InstanceRefresh_Enabled("Alpha", 2), + Config: testAccAwsAutoScalingGroupConfig_InstanceRefresh_Full(), Check: resource.ComposeTestCheckFunc( testAccCheckAWSAutoScalingGroupExists(resourceName, &group), - resource.TestCheckResourceAttr(resourceName, "min_size", "2"), - resource.TestCheckResourceAttr(resourceName, "max_size", "4"), - resource.TestCheckResourceAttr(resourceName, "desired_capacity", "2"), - testAccCheckAutoscalingLatestInstanceRefreshState(&group, 0, 0, nil), resource.TestCheckResourceAttr(resourceName, "instance_refresh.#", "1"), resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.strategy", "Rolling"), resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.preferences.#", "1"), @@ -1042,46 +1032,78 @@ func TestAccAWSAutoScalingGroup_InstanceRefresh_Enabled(t *testing.T) { ), }, { - // check that changing tags doesn't trigger a refresh - Config: testAccAwsAutoScalingGroupConfig_InstanceRefresh_Enabled("Bravo", 1), + Config: testAccAwsAutoScalingGroupConfig_InstanceRefresh_Disabled(), Check: resource.ComposeTestCheckFunc( testAccCheckAWSAutoScalingGroupExists(resourceName, &group), - testAccCheckAutoscalingLatestInstanceRefreshState(&group, 0, 0, nil), + resource.TestCheckNoResourceAttr(resourceName, "instance_refresh.#"), ), }, + }, + }) +} + +func TestAccAWSAutoScalingGroup_InstanceRefresh_Start(t *testing.T) { + var group autoscaling.Group + resourceName := "aws_autoscaling_group.test" + launchConfigurationName := "aws_launch_configuration.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSAutoScalingGroupDestroy, + Steps: []resource.TestStep{ { - Config: testAccAwsAutoScalingGroupConfig_InstanceRefresh_Disabled("Bravo", 1), + Config: testAccAwsAutoScalingGroupConfig_InstanceRefresh_Start("one"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSAutoScalingGroupExists(resourceName, &group), - resource.TestCheckNoResourceAttr(resourceName, "instance_refresh.#"), + resource.TestCheckResourceAttrPair(resourceName, "launch_configuration", launchConfigurationName, "name"), + testAccCheckAutoScalingInstanceRefreshCount(&group, 0), + ), + }, + { + Config: testAccAwsAutoScalingGroupConfig_InstanceRefresh_Start("two"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSAutoScalingGroupExists(resourceName, &group), + resource.TestCheckResourceAttrPair(resourceName, "launch_configuration", launchConfigurationName, "name"), + testAccCheckAutoScalingInstanceRefreshCount(&group, 1), + testAccCheckAutoScalingInstanceRefreshStatus(&group, 0, autoscaling.InstanceRefreshStatusPending, autoscaling.InstanceRefreshStatusInProgress), + ), + }, + { + Config: testAccAwsAutoScalingGroupConfig_InstanceRefresh_Start("three"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSAutoScalingGroupExists(resourceName, &group), + resource.TestCheckResourceAttrPair(resourceName, "launch_configuration", launchConfigurationName, "name"), + testAccCheckAutoScalingInstanceRefreshCount(&group, 2), + testAccCheckAutoScalingInstanceRefreshStatus(&group, 0, autoscaling.InstanceRefreshStatusPending, autoscaling.InstanceRefreshStatusInProgress), + testAccCheckAutoScalingInstanceRefreshStatus(&group, 1, autoscaling.InstanceRefreshStatusCancelled), ), }, }, }) } -// TODO: check that an active refresh is cancelled in favour of a new one - func TestAccAWSAutoScalingGroup_InstanceRefresh_Triggers(t *testing.T) { matrix := []struct { AvailabilityZoneCount int SubnetCount int InstanceType string + Capacity int UseLaunchConfiguration bool UseLaunchTemplate bool UseMixedInstancesPolicy bool UsePlacementGroup bool ExpectRefreshCount int }{ - {2, 0, "t3.nano", true, false, false, false, 0}, // create asg with 2 az-s - {1, 0, "t3.nano", true, false, false, false, 1}, // drop 1 az - {0, 2, "t3.nano", true, false, false, false, 2}, // add 2 subnets, drop az-s - {0, 1, "t3.nano", true, false, false, false, 3}, // drop 1 subnet - {0, 1, "t3.nano", false, true, false, false, 4}, // drop launch config, add template - {0, 1, "t3.micro", false, true, false, false, 5}, // update template - {0, 1, "t3.micro", false, false, true, false, 6}, // drop template, add mixed policy - {0, 1, "t3.nano", false, false, true, false, 7}, // update mixed policy - {0, 1, "t3.nano", false, false, true, true, 8}, // use placement group + {2, 0, "t3.nano", 1, true, false, false, false, 0}, // create asg with 2 az-s + {1, 0, "t3.nano", 1, true, false, false, false, 0}, // drop 1 az + {0, 2, "t3.nano", 4, true, false, false, false, 0}, // add 2 subnets, drop az-s + {0, 1, "t3.nano", 1, true, false, false, false, 0}, // drop 1 subnet + {0, 1, "t3.nano", 1, false, true, false, false, 1}, // drop launch config, add template + {0, 1, "t3.micro", 1, false, true, false, false, 2}, // update template + {0, 1, "t3.micro", 1, false, false, true, false, 3}, // drop template, add mixed policy + {0, 1, "t3.nano", 1, false, false, true, false, 4}, // update mixed policy + {0, 1, "t3.nano", 1, false, false, true, true, 4}, // use placement group } var group autoscaling.Group @@ -1095,6 +1117,7 @@ func TestAccAWSAutoScalingGroup_InstanceRefresh_Triggers(t *testing.T) { test.AvailabilityZoneCount, test.SubnetCount, test.InstanceType, + test.Capacity, test.UseLaunchConfiguration, test.UseLaunchTemplate, test.UseMixedInstancesPolicy, @@ -1103,7 +1126,7 @@ func TestAccAWSAutoScalingGroup_InstanceRefresh_Triggers(t *testing.T) { ), Check: resource.ComposeTestCheckFunc( testAccCheckAWSAutoScalingGroupExists(resourceName, &group), - testAccCheckAutoscalingLatestInstanceRefreshState(&group, test.ExpectRefreshCount, 0, nil), + testAccCheckAutoScalingInstanceRefreshCount(&group, test.ExpectRefreshCount), ), } } @@ -4276,30 +4299,20 @@ resource "aws_autoscaling_group" "test" { `, rName) } -func testAccAwsAutoScalingGroupConfig_InstanceRefresh_Basic(tagValue string, sizeFactor int) string { +func testAccAwsAutoScalingGroupConfig_InstanceRefresh_Basic() string { return fmt.Sprintf(` resource "aws_autoscaling_group" "test" { availability_zones = [data.aws_availability_zones.current.names[0]] - max_size = 2 * local.size_factor - min_size = 1 * local.size_factor - desired_capacity = 1 * local.size_factor + max_size = 2 + min_size = 1 + desired_capacity = 1 launch_configuration = aws_launch_configuration.test.name - tag { - key = "Test" - value = %[1]q - propagate_at_launch = true - } - instance_refresh { strategy = "Rolling" } } -locals { - size_factor = %[2]d -} - data "aws_ami" "test" { most_recent = true owners = ["amazon"] @@ -4323,24 +4336,18 @@ resource "aws_launch_configuration" "test" { image_id = data.aws_ami.test.id instance_type = "t3.nano" } -`, tagValue, sizeFactor) +`) } -func testAccAwsAutoScalingGroupConfig_InstanceRefresh_Enabled(tagValue string, sizeFactor int) string { +func testAccAwsAutoScalingGroupConfig_InstanceRefresh_Full() string { return fmt.Sprintf(` resource "aws_autoscaling_group" "test" { availability_zones = [data.aws_availability_zones.current.names[0]] - max_size = 2 * local.size_factor - min_size = 1 * local.size_factor - desired_capacity = 1 * local.size_factor + max_size = 2 + min_size = 1 + desired_capacity = 1 launch_configuration = aws_launch_configuration.test.name - tag { - key = "Test" - value = %[1]q - propagate_at_launch = true - } - instance_refresh { strategy = "Rolling" preferences { @@ -4350,10 +4357,6 @@ resource "aws_autoscaling_group" "test" { } } -locals { - size_factor = %[2]d -} - data "aws_ami" "test" { most_recent = true owners = ["amazon"] @@ -4377,27 +4380,57 @@ resource "aws_launch_configuration" "test" { image_id = data.aws_ami.test.id instance_type = "t3.nano" } -`, tagValue, sizeFactor) +`) } -func testAccAwsAutoScalingGroupConfig_InstanceRefresh_Disabled(tagValue string, sizeFactor int) string { +func testAccAwsAutoScalingGroupConfig_InstanceRefresh_Disabled() string { return fmt.Sprintf(` resource "aws_autoscaling_group" "test" { availability_zones = [data.aws_availability_zones.current.names[0]] - max_size = 2 * local.size_factor - min_size = 1 * local.size_factor - desired_capacity = 1 * local.size_factor + max_size = 2 + min_size = 1 + desired_capacity = 1 launch_configuration = aws_launch_configuration.test.name +} - tag { - key = "Test" - value = %[1]q - propagate_at_launch = true +data "aws_ami" "test" { + most_recent = true + owners = ["amazon"] + + filter { + name = "name" + values = ["amzn-ami-hvm-*-x86_64-gp2"] } } -locals { - size_factor = %[2]d +data "aws_availability_zones" "current" { + state = "available" + + filter { + name = "opt-in-status" + values = ["opt-in-not-required"] + } +} + +resource "aws_launch_configuration" "test" { + image_id = data.aws_ami.test.id + instance_type = "t3.nano" +} +`) +} + +func testAccAwsAutoScalingGroupConfig_InstanceRefresh_Start(launchConfigurationName string) string { + return fmt.Sprintf(` +resource "aws_autoscaling_group" "test" { + availability_zones = [data.aws_availability_zones.current.names[0]] + max_size = 2 + min_size = 1 + desired_capacity = 1 + launch_configuration = aws_launch_configuration.test.name + + instance_refresh { + strategy = "Rolling" + } } data "aws_ami" "test" { @@ -4420,16 +4453,22 @@ data "aws_availability_zones" "current" { } resource "aws_launch_configuration" "test" { + name_prefix = %[1]q image_id = data.aws_ami.test.id instance_type = "t3.nano" + + lifecycle { + create_before_destroy = true + } } -`, tagValue, sizeFactor) +`, launchConfigurationName) } func testAccAwsAutoScalingGroupConfig_InstanceRefresh_Triggers( availabilityZoneCount int, subnetCount int, instanceType string, + capacity int, useLaunchConfiguration bool, useLaunchTemplate bool, useMixedInstancesPolicy bool, @@ -4439,9 +4478,9 @@ func testAccAwsAutoScalingGroupConfig_InstanceRefresh_Triggers( return fmt.Sprintf(` resource "aws_autoscaling_group" "test" { availability_zones = local.availability_zone_count > 0 ? slice(data.aws_availability_zones.current.names, 0, local.availability_zone_count) : null - max_size = 1 min_size = 1 - desired_capacity = 1 + max_size = %[9]d + desired_capacity = %[9]d launch_configuration = local.use_launch_configuration ? aws_launch_configuration.test.name : null vpc_zone_identifier = local.subnet_count > 0 ? slice(aws_subnet.test.*.id, 0, local.subnet_count) : null placement_group = local.use_placement_group ? aws_placement_group.test.name : null @@ -4526,60 +4565,51 @@ resource "aws_placement_group" "test" { name = local.placement_group_name strategy = "cluster" } -`, availabilityZoneCount, subnetCount, instanceType, useLaunchConfiguration, useLaunchTemplate, useMixedInstancesPolicy, usePlacementGroup, placementGroupName) -} - -// testAccCheckAutoscalingLatestInstanceRefreshState checks the Instance Refreshes -// of an Auto-Scaling Group. -// -// Use length to set the number of refreshes (of any state) that are expected. -// -// Use the offset parameter to choose the instance refresh to check. Offset 0 -// is the latest refresh, with negative offsets yielding successive elements. -// When length is 0, this argument has no effect. -// -// When length is greater than 0 and acceptedStatuses is non-nil, expect the -// refresh at the given offset to have one of the given accepted statuses. -func testAccCheckAutoscalingLatestInstanceRefreshState( - group *autoscaling.Group, - length int, - offset int, - acceptedStatuses []string, -) resource.TestCheckFunc { +`, availabilityZoneCount, subnetCount, instanceType, useLaunchConfiguration, useLaunchTemplate, useMixedInstancesPolicy, usePlacementGroup, placementGroupName, capacity) +} + +func testAccCheckAutoScalingInstanceRefreshCount(group *autoscaling.Group, expected int) resource.TestCheckFunc { return func(state *terraform.State) error { conn := testAccProvider.Meta().(*AWSClient).autoscalingconn - name := aws.StringValue(group.AutoScalingGroupName) + input := autoscaling.DescribeInstanceRefreshesInput{ - AutoScalingGroupName: aws.String(name), + AutoScalingGroupName: group.AutoScalingGroupName, + } + resp, err := conn.DescribeInstanceRefreshes(&input) + if err != nil { + return fmt.Errorf("error describing Auto Scaling Group (%s) Instance Refreshes: %w", aws.StringValue(group.AutoScalingGroupName), err) } - output, err := conn.DescribeInstanceRefreshes(&input) - switch { - case err != nil: - return err - case len(output.InstanceRefreshes) != length: - return fmt.Errorf("expected %d instance refreshes, but found %d", length, len(output.InstanceRefreshes)) + if len(resp.InstanceRefreshes) != expected { + return fmt.Errorf("expected %d Instance Refreshes, got %d", expected, len(resp.InstanceRefreshes)) + } + return nil + } +} + +func testAccCheckAutoScalingInstanceRefreshStatus(group *autoscaling.Group, offset int, expected ...string) resource.TestCheckFunc { + return func(state *terraform.State) error { + conn := testAccProvider.Meta().(*AWSClient).autoscalingconn + + input := autoscaling.DescribeInstanceRefreshesInput{ + AutoScalingGroupName: group.AutoScalingGroupName, + } + resp, err := conn.DescribeInstanceRefreshes(&input) + if err != nil { + return fmt.Errorf("error describing Auto Scaling Group (%s) Instance Refreshes: %w", aws.StringValue(group.AutoScalingGroupName), err) } - switch { - case length == 0: - return nil - case len(acceptedStatuses) == 0: - return nil + if len(resp.InstanceRefreshes) < offset { + return fmt.Errorf("expected at least %d Instance Refreshes, got %d", offset+1, len(resp.InstanceRefreshes)) } - status := aws.StringValue(output.InstanceRefreshes[offset].Status) - for _, acceptedStatus := range acceptedStatuses { - if status == acceptedStatus { + actual := aws.StringValue(resp.InstanceRefreshes[offset].Status) + for _, s := range expected { + if actual == s { return nil } } - - return fmt.Errorf( - "expected status of refresh at offset %d to be one of %s, got %s", - offset, - strings.Join(acceptedStatuses, " or "), - status) + return fmt.Errorf("expected Instance Refresh at index %d to be in %q, got %q", offset, expected, actual) } } diff --git a/website/docs/r/autoscaling_group.html.markdown b/website/docs/r/autoscaling_group.html.markdown index 901a0471ac7..7d8b1372765 100644 --- a/website/docs/r/autoscaling_group.html.markdown +++ b/website/docs/r/autoscaling_group.html.markdown @@ -355,7 +355,7 @@ This configuration block supports the following: * `instance_warmup_seconds` - (Optional) The number of seconds until a newly launched instance is configured and ready to use. Default behavior is to use the Auto Scaling Group's health check grace period. * `min_healthy_percentage` - (Optional) The amount of capacity in the Auto Scaling group that must remain healthy during an instance refresh to allow the operation to continue, as a percentage of the desired capacity of the Auto Scaling group. Defaults to `90`. -~> **NOTE:** A refresh is only started when any of the following Auto Scaling Group properties change: `launch_configuration`, `launch_template`, `mixed_instances_policy`, `vpc_zone_identifier`, `availability_zones`, `placement_group`, or any `tag` or `tags` configured to propagate at launch. +~> **NOTE:** A refresh is only started when any of the following Auto Scaling Group properties change: `launch_configuration`, `launch_template`, `mixed_instances_policy`. ~> **NOTE:** Auto Scaling Groups support up to one active instance refresh at a time. When this resource is updated, any existing refresh is cancelled. From 3720d6624f35cdfb1fcde600130ecdcad5d4bf2c Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Tue, 8 Dec 2020 16:25:36 -0800 Subject: [PATCH 11/12] Adds triggers parameter to instance_refresh --- aws/resource_aws_autoscaling_group.go | 34 +++- aws/resource_aws_autoscaling_group_test.go | 151 ++++-------------- .../docs/r/autoscaling_group.html.markdown | 43 ++--- 3 files changed, 87 insertions(+), 141 deletions(-) diff --git a/aws/resource_aws_autoscaling_group.go b/aws/resource_aws_autoscaling_group.go index 0def21ee46c..28ec2208eca 100644 --- a/aws/resource_aws_autoscaling_group.go +++ b/aws/resource_aws_autoscaling_group.go @@ -507,6 +507,12 @@ func resourceAwsAutoscalingGroup() *schema.Resource { }, }, }, + "triggers": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + Set: schema.HashString, + }, }, }, }, @@ -1188,9 +1194,29 @@ func resourceAwsAutoscalingGroupUpdate(d *schema.ResourceData, meta interface{}) } } - if instanceRefreshRaw, ok := d.GetOk("instance_refresh"); ok && shouldRefreshInstances { - if err := autoScalingGroupRefreshInstances(conn, d.Id(), instanceRefreshRaw.([]interface{})); err != nil { - return fmt.Errorf("failed to start instance refresh of Auto Scaling Group %s: %w", d.Id(), err) + if instanceRefreshRaw, ok := d.GetOk("instance_refresh"); ok { + instanceRefresh := instanceRefreshRaw.([]interface{}) + if !shouldRefreshInstances { + if len(instanceRefresh) > 0 && instanceRefresh[0] != nil { + m := instanceRefresh[0].(map[string]interface{}) + attrsSet := m["triggers"].(*schema.Set) + attrs := attrsSet.List() + strs := make([]string, len(attrs)) + for i, a := range attrs { + strs[i] = a.(string) + } + if attrsSet.Contains("tag") && !attrsSet.Contains("tags") { + strs = append(strs, "tags") + } else if !attrsSet.Contains("tag") && attrsSet.Contains("tags") { + strs = append(strs, "tag") + } + shouldRefreshInstances = d.HasChanges(strs...) + } + } + if shouldRefreshInstances { + if err := autoScalingGroupRefreshInstances(conn, d.Id(), instanceRefresh); err != nil { + return fmt.Errorf("failed to start instance refresh of Auto Scaling Group %s: %w", d.Id(), err) + } } } @@ -1341,7 +1367,7 @@ func resourceAwsAutoscalingGroupDrain(d *schema.ResourceData, meta interface{}) return fmt.Errorf("Error setting capacity to zero to drain: %s", err) } - // Next, wait for the autoscale group to drain + // Next, wait for the Auto Scaling Group to drain log.Printf("[DEBUG] Waiting for group to have zero instances") var g *autoscaling.Group err := resource.Retry(d.Timeout(schema.TimeoutDelete), func() *resource.RetryError { diff --git a/aws/resource_aws_autoscaling_group_test.go b/aws/resource_aws_autoscaling_group_test.go index 2c1efaef165..f1366a6b0d3 100644 --- a/aws/resource_aws_autoscaling_group_test.go +++ b/aws/resource_aws_autoscaling_group_test.go @@ -1084,58 +1084,34 @@ func TestAccAWSAutoScalingGroup_InstanceRefresh_Start(t *testing.T) { } func TestAccAWSAutoScalingGroup_InstanceRefresh_Triggers(t *testing.T) { - matrix := []struct { - AvailabilityZoneCount int - SubnetCount int - InstanceType string - Capacity int - UseLaunchConfiguration bool - UseLaunchTemplate bool - UseMixedInstancesPolicy bool - UsePlacementGroup bool - ExpectRefreshCount int - }{ - {2, 0, "t3.nano", 1, true, false, false, false, 0}, // create asg with 2 az-s - {1, 0, "t3.nano", 1, true, false, false, false, 0}, // drop 1 az - {0, 2, "t3.nano", 4, true, false, false, false, 0}, // add 2 subnets, drop az-s - {0, 1, "t3.nano", 1, true, false, false, false, 0}, // drop 1 subnet - {0, 1, "t3.nano", 1, false, true, false, false, 1}, // drop launch config, add template - {0, 1, "t3.micro", 1, false, true, false, false, 2}, // update template - {0, 1, "t3.micro", 1, false, false, true, false, 3}, // drop template, add mixed policy - {0, 1, "t3.nano", 1, false, false, true, false, 4}, // update mixed policy - {0, 1, "t3.nano", 1, false, false, true, true, 4}, // use placement group - } - var group autoscaling.Group resourceName := "aws_autoscaling_group.test" - placementGroupName := fmt.Sprintf("tf-test-%s", acctest.RandString(8)) - - steps := make([]resource.TestStep, len(matrix)) - for i, test := range matrix { - steps[i] = resource.TestStep{ - Config: testAccAwsAutoScalingGroupConfig_InstanceRefresh_Triggers( - test.AvailabilityZoneCount, - test.SubnetCount, - test.InstanceType, - test.Capacity, - test.UseLaunchConfiguration, - test.UseLaunchTemplate, - test.UseMixedInstancesPolicy, - test.UsePlacementGroup, - placementGroupName, - ), - Check: resource.ComposeTestCheckFunc( - testAccCheckAWSAutoScalingGroupExists(resourceName, &group), - testAccCheckAutoScalingInstanceRefreshCount(&group, test.ExpectRefreshCount), - ), - } - } resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckAWSAutoScalingGroupDestroy, - Steps: steps, + Steps: []resource.TestStep{ + { + Config: testAccAwsAutoScalingGroupConfig_InstanceRefresh_Basic(), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSAutoScalingGroupExists(resourceName, &group), + resource.TestCheckResourceAttr(resourceName, "instance_refresh.#", "1"), + resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.strategy", "Rolling"), + resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.preferences.#", "0"), + ), + }, + { + Config: testAccAwsAutoScalingGroupConfig_InstanceRefresh_Triggers(), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSAutoScalingGroupExists(resourceName, &group), + resource.TestCheckResourceAttr(resourceName, "instance_refresh.0.triggers.#", "1"), + resource.TestCheckTypeSetElemAttr(resourceName, "instance_refresh.0.triggers.*", "tags"), + testAccCheckAutoScalingInstanceRefreshCount(&group, 1), + testAccCheckAutoScalingInstanceRefreshStatus(&group, 0, autoscaling.InstanceRefreshStatusPending, autoscaling.InstanceRefreshStatusInProgress), + ), + }, + }, }) } @@ -4464,61 +4440,25 @@ resource "aws_launch_configuration" "test" { `, launchConfigurationName) } -func testAccAwsAutoScalingGroupConfig_InstanceRefresh_Triggers( - availabilityZoneCount int, - subnetCount int, - instanceType string, - capacity int, - useLaunchConfiguration bool, - useLaunchTemplate bool, - useMixedInstancesPolicy bool, - usePlacementGroup bool, - placementGroupName string, -) string { +func testAccAwsAutoScalingGroupConfig_InstanceRefresh_Triggers() string { return fmt.Sprintf(` resource "aws_autoscaling_group" "test" { - availability_zones = local.availability_zone_count > 0 ? slice(data.aws_availability_zones.current.names, 0, local.availability_zone_count) : null + availability_zones = [data.aws_availability_zones.current.names[0]] + max_size = 2 min_size = 1 - max_size = %[9]d - desired_capacity = %[9]d - launch_configuration = local.use_launch_configuration ? aws_launch_configuration.test.name : null - vpc_zone_identifier = local.subnet_count > 0 ? slice(aws_subnet.test.*.id, 0, local.subnet_count) : null - placement_group = local.use_placement_group ? aws_placement_group.test.name : null - - dynamic "launch_template" { - for_each = local.use_launch_template ? [1] : [] - content { - id = aws_launch_template.test.id - version = aws_launch_template.test.latest_version - } - } - - dynamic "mixed_instances_policy" { - for_each = local.use_mixed_instances_policy ? [1] : [] - content { - launch_template { - launch_template_specification { - launch_template_id = aws_launch_template.test.id - version = aws_launch_template.test.latest_version - } - } - } - } + desired_capacity = 1 + launch_configuration = aws_launch_configuration.test.name instance_refresh { - strategy = "Rolling" + strategy = "Rolling" + triggers = ["tags"] } -} -locals { - availability_zone_count = %[1]d - subnet_count = %[2]d - instance_type = %[3]q - use_launch_configuration = %[4]t - use_launch_template = %[5]t - use_mixed_instances_policy = %[6]t - use_placement_group = %[7]t - placement_group_name = %[8]q + tag { + key = "Key" + value = "Value" + propagate_at_launch = true + } } data "aws_ami" "test" { @@ -4540,32 +4480,11 @@ data "aws_availability_zones" "current" { } } -resource "aws_vpc" "test" { - cidr_block = "10.0.0.0/16" -} - -resource "aws_subnet" "test" { - count = length(data.aws_availability_zones.current.names) - availability_zone = data.aws_availability_zones.current.names[count.index] - cidr_block = cidrsubnet(aws_vpc.test.cidr_block, 2, count.index) - vpc_id = aws_vpc.test.id -} - resource "aws_launch_configuration" "test" { image_id = data.aws_ami.test.id - instance_type = local.instance_type -} - -resource "aws_launch_template" "test" { - image_id = data.aws_ami.test.image_id - instance_type = local.instance_type -} - -resource "aws_placement_group" "test" { - name = local.placement_group_name - strategy = "cluster" + instance_type = "t3.nano" } -`, availabilityZoneCount, subnetCount, instanceType, useLaunchConfiguration, useLaunchTemplate, useMixedInstancesPolicy, usePlacementGroup, placementGroupName, capacity) +`) } func testAccCheckAutoScalingInstanceRefreshCount(group *autoscaling.Group, expected int) resource.TestCheckFunc { diff --git a/website/docs/r/autoscaling_group.html.markdown b/website/docs/r/autoscaling_group.html.markdown index 7d8b1372765..ffcb4d6b0c9 100644 --- a/website/docs/r/autoscaling_group.html.markdown +++ b/website/docs/r/autoscaling_group.html.markdown @@ -178,21 +178,6 @@ resource "aws_autoscaling_group" "bar" { ### Automatically refresh all instances after the group is updated ```hcl -data "aws_ami" "example" { - most_recent = true - owners = ["amazon"] - - filter { - name = "name" - values = ["amzn-ami-hvm-*-x86_64-gp2"] - } -} - -resource "aws_launch_template" "example" { - image_id = data.aws_ami.example.id - instance_type = "t3.nano" -} - resource "aws_autoscaling_group" "example" { availability_zones = ["us-east-1a"] desired_capacity = 1 @@ -211,6 +196,21 @@ resource "aws_autoscaling_group" "example" { } } } + +data "aws_ami" "example" { + most_recent = true + owners = ["amazon"] + + filter { + name = "name" + values = ["amzn-ami-hvm-*-x86_64-gp2"] + } +} + +resource "aws_launch_template" "example" { + image_id = data.aws_ami.example.id + instance_type = "t3.nano" +} ``` ## Argument Reference @@ -354,8 +354,9 @@ This configuration block supports the following: * `preferences` - (Optional) Override default parameters for Instance Refresh. * `instance_warmup_seconds` - (Optional) The number of seconds until a newly launched instance is configured and ready to use. Default behavior is to use the Auto Scaling Group's health check grace period. * `min_healthy_percentage` - (Optional) The amount of capacity in the Auto Scaling group that must remain healthy during an instance refresh to allow the operation to continue, as a percentage of the desired capacity of the Auto Scaling group. Defaults to `90`. +* `triggers` - (Optional) Set of additional property names that will trigger an Instance Refresh. A refresh will always be triggered by a change in any of `launch_configuration`, `launch_template`, or `mixed_instances_policy`. -~> **NOTE:** A refresh is only started when any of the following Auto Scaling Group properties change: `launch_configuration`, `launch_template`, `mixed_instances_policy`. +~> **NOTE:** A refresh is started when any of the following Auto Scaling Group properties change: `launch_configuration`, `launch_template`, `mixed_instances_policy`. Additional properties can be specified in the `triggers` property of `instance_refresh`. ~> **NOTE:** Auto Scaling Groups support up to one active instance refresh at a time. When this resource is updated, any existing refresh is cancelled. @@ -367,15 +368,15 @@ In addition to all arguments above, the following attributes are exported: * `id` - The Auto Scaling Group id. * `arn` - The ARN for this Auto Scaling Group -* `availability_zones` - The availability zones of the autoscale group. -* `min_size` - The minimum size of the autoscale group -* `max_size` - The maximum size of the autoscale group +* `availability_zones` - The availability zones of the Auto Scaling Group. +* `min_size` - The minimum size of the Auto Scaling Group +* `max_size` - The maximum size of the Auto Scaling Group * `default_cooldown` - Time between a scaling activity and the succeeding scaling activity. -* `name` - The name of the autoscale group +* `name` - The name of the Auto Scaling Group * `health_check_grace_period` - Time after instance comes into service before checking health. * `health_check_type` - "EC2" or "ELB". Controls how health checking is done. * `desired_capacity` -The number of Amazon EC2 instances that should be running in the group. -* `launch_configuration` - The launch configuration of the autoscale group +* `launch_configuration` - The launch configuration of the Auto Scaling Group * `vpc_zone_identifier` (Optional) - The VPC zone identifier ~> **NOTE:** When using `ELB` as the `health_check_type`, `health_check_grace_period` is required. From 1529c2a167abf0545e246a0deff050dbc3eefb10 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Wed, 9 Dec 2020 11:57:50 -0800 Subject: [PATCH 12/12] Adds validation for field names --- .../service/autoscaling/waiter/waiter.go | 13 +++++-- aws/resource_aws_autoscaling_group.go | 35 ++++++++++++++++++- aws/resource_aws_autoscaling_group_test.go | 10 +++--- go.mod | 1 + .../docs/r/autoscaling_group.html.markdown | 7 ++++ 5 files changed, 58 insertions(+), 8 deletions(-) diff --git a/aws/internal/service/autoscaling/waiter/waiter.go b/aws/internal/service/autoscaling/waiter/waiter.go index 54e11e736c7..00aed168296 100644 --- a/aws/internal/service/autoscaling/waiter/waiter.go +++ b/aws/internal/service/autoscaling/waiter/waiter.go @@ -19,8 +19,17 @@ const ( func InstanceRefreshCancelled(conn *autoscaling.AutoScaling, asgName, instanceRefreshId string) (*autoscaling.InstanceRefresh, error) { stateConf := &resource.StateChangeConf{ - Pending: []string{autoscaling.InstanceRefreshStatusPending, autoscaling.InstanceRefreshStatusInProgress, autoscaling.InstanceRefreshStatusCancelling}, - Target: []string{autoscaling.InstanceRefreshStatusCancelled}, + Pending: []string{ + autoscaling.InstanceRefreshStatusPending, + autoscaling.InstanceRefreshStatusInProgress, + autoscaling.InstanceRefreshStatusCancelling, + }, + Target: []string{ + autoscaling.InstanceRefreshStatusCancelled, + // Failed and Successful are also acceptable end-states + autoscaling.InstanceRefreshStatusFailed, + autoscaling.InstanceRefreshStatusSuccessful, + }, Refresh: InstanceRefreshStatus(conn, asgName, instanceRefreshId), Timeout: InstanceRefreshCancelledTimeout, } diff --git a/aws/resource_aws_autoscaling_group.go b/aws/resource_aws_autoscaling_group.go index 28ec2208eca..4efe81f95ba 100644 --- a/aws/resource_aws_autoscaling_group.go +++ b/aws/resource_aws_autoscaling_group.go @@ -16,6 +16,8 @@ import ( "github.com/aws/aws-sdk-go/service/elb" "github.com/aws/aws-sdk-go/service/elbv2" "github.com/hashicorp/aws-sdk-go-base/tfawserr" + "github.com/hashicorp/go-cty/cty" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -510,8 +512,11 @@ func resourceAwsAutoscalingGroup() *schema.Resource { "triggers": { Type: schema.TypeSet, Optional: true, - Elem: &schema.Schema{Type: schema.TypeString}, Set: schema.HashString, + Elem: &schema.Schema{ + Type: schema.TypeString, + ValidateDiagFunc: validateAutoScalingGroupInstanceRefreshTriggerFields, + }, }, }, }, @@ -1937,3 +1942,31 @@ func cancelAutoscalingInstanceRefresh(conn *autoscaling.AutoScaling, asgName str return nil } + +func validateAutoScalingGroupInstanceRefreshTriggerFields(i interface{}, path cty.Path) diag.Diagnostics { + v, ok := i.(string) + if !ok { + return diag.Errorf("expected type to be string") + } + + if v == "launch_configuration" || v == "launch_template" || v == "mixed_instances_policy" { + return diag.Diagnostics{ + diag.Diagnostic{ + Severity: diag.Warning, + Summary: fmt.Sprintf("'%s' always triggers an instance refresh and can be removed", v), + }, + } + } + + schema := resourceAwsAutoscalingGroup().Schema + for attr, attrSchema := range schema { + if v == attr { + if attrSchema.Computed && !attrSchema.Optional { + return diag.Errorf("'%s' is a read-only parameter and cannot be used to trigger an instance refresh", v) + } + return nil + } + } + + return diag.Errorf("'%s' is not a recognized parameter name for aws_autoscaling_group", v) +} diff --git a/aws/resource_aws_autoscaling_group_test.go b/aws/resource_aws_autoscaling_group_test.go index f1366a6b0d3..b4565529be2 100644 --- a/aws/resource_aws_autoscaling_group_test.go +++ b/aws/resource_aws_autoscaling_group_test.go @@ -4450,14 +4450,14 @@ resource "aws_autoscaling_group" "test" { launch_configuration = aws_launch_configuration.test.name instance_refresh { - strategy = "Rolling" - triggers = ["tags"] + strategy = "Rolling" + triggers = ["tags"] } tag { - key = "Key" - value = "Value" - propagate_at_launch = true + key = "Key" + value = "Value" + propagate_at_launch = true } } diff --git a/go.mod b/go.mod index afb8bb7b154..b92f51c7c45 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/fatih/color v1.9.0 // indirect github.com/hashicorp/aws-sdk-go-base v0.7.0 github.com/hashicorp/go-cleanhttp v0.5.1 + github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320 github.com/hashicorp/go-hclog v0.10.0 // indirect github.com/hashicorp/go-multierror v1.1.0 github.com/hashicorp/go-version v1.2.1 diff --git a/website/docs/r/autoscaling_group.html.markdown b/website/docs/r/autoscaling_group.html.markdown index ffcb4d6b0c9..1ea072bff5a 100644 --- a/website/docs/r/autoscaling_group.html.markdown +++ b/website/docs/r/autoscaling_group.html.markdown @@ -189,11 +189,18 @@ resource "aws_autoscaling_group" "example" { version = aws_launch_template.example.latest_version } + tag { + key = "Key" + value = "Value" + propagate_at_launch = true + } + instance_refresh { strategy = "Rolling" preferences { min_healthy_percentage = 50 } + triggers = ["tag"] } }