Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

autoscaling/group: Handle eventual consistency #40088

Merged
merged 11 commits into from
Nov 11, 2024
11 changes: 11 additions & 0 deletions .changelog/40088.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
```release-note:bug
resource/aws_autoscaling_group: Handle eventual consistency issues that occur when using a `launch_template` that is updated causing `ValidationError: You must use a valid fully-formed launch template.`
```

```release-note:bug
resource/aws_iam_instance_profile: Handle eventual consistency issues that occur when this resource is updated and has dependents
```

```release-note:bug
resource/aws_launch_template: Handle eventual consistency issues that occur when this resource is updated and has dependents
```
5 changes: 5 additions & 0 deletions internal/service/autoscaling/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,8 @@ var (
errCodeResourceInUseFault = (*awstypes.ResourceInUseFault)(nil).ErrorCode()
errCodeScalingActivityInProgressFault = (*awstypes.ScalingActivityInProgressFault)(nil).ErrorCode()
)

const (
errCodeOperationError = "operation error"
errCodeUpdateASG = "UpdateAutoScalingGroup"
)
6 changes: 5 additions & 1 deletion internal/service/autoscaling/group.go
Original file line number Diff line number Diff line change
Expand Up @@ -1481,7 +1481,11 @@ func resourceGroupUpdate(ctx context.Context, d *schema.ResourceData, meta inter
input.VPCZoneIdentifier = expandVPCZoneIdentifiers(d.Get("vpc_zone_identifier").(*schema.Set).List())
}

_, err := conn.UpdateAutoScalingGroup(ctx, input)
_, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, d.Timeout(schema.TimeoutUpdate),
func() (interface{}, error) {
return conn.UpdateAutoScalingGroup(ctx, input)
},
errCodeOperationError, errCodeUpdateASG, errCodeValidationError)

if err != nil {
return sdkdiag.AppendErrorf(diags, "updating Auto Scaling Group (%s): %s", d.Id(), err)
Expand Down
114 changes: 112 additions & 2 deletions internal/service/autoscaling/group_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1914,6 +1914,36 @@ func TestAccAutoScalingGroup_launchTempPartitionNum(t *testing.T) {
})
}

// TestAccAutoScalingGroup_launchTemplateIAMInstanceProfile is an eventually consistent test that fails
// without the update retry, or waiters in the IAM instance profile and launch template resources.
func TestAccAutoScalingGroup_launchTemplateIAMInstanceProfile(t *testing.T) {
ctx := acctest.Context(t)
var group awstypes.AutoScalingGroup
resourceName := "aws_autoscaling_group.test"
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, names.AutoScalingServiceID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: testAccCheckGroupDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccGroupConfig_launchTemplateIAMInstanceProfile(rName, rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckGroupExists(ctx, resourceName, &group),
),
},
{
Config: testAccGroupConfig_launchTemplateIAMInstanceProfile(rName, sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)),
Check: resource.ComposeTestCheckFunc(
testAccCheckGroupExists(ctx, resourceName, &group),
),
},
},
})
}

func TestAccAutoScalingGroup_Destroy_whenProtectedFromScaleIn(t *testing.T) {
ctx := acctest.Context(t)
var group awstypes.AutoScalingGroup
Expand Down Expand Up @@ -3199,7 +3229,7 @@ func TestAccAutoScalingGroup_MixedInstancesPolicyLaunchTemplateOverride_instance
Steps: []resource.TestStep{
{
Config: testAccGroupConfig_mixedInstancesPolicyLaunchTemplateOverrideInstanceRequirements(rName,
`cpu_manufacturers = ["amazon-web-services"]
`cpu_manufacturers = ["amazon-web-services", "amd"]
memory_mib {
min = 500
}
Expand All @@ -3213,8 +3243,9 @@ func TestAccAutoScalingGroup_MixedInstancesPolicyLaunchTemplateOverride_instance
resource.TestCheckResourceAttr(resourceName, "mixed_instances_policy.0.launch_template.0.override.#", "1"),

resource.TestCheckResourceAttr(resourceName, "mixed_instances_policy.0.launch_template.0.override.0.instance_requirements.#", "1"),
resource.TestCheckResourceAttr(resourceName, "mixed_instances_policy.0.launch_template.0.override.0.instance_requirements.0.cpu_manufacturers.#", "1"),
resource.TestCheckResourceAttr(resourceName, "mixed_instances_policy.0.launch_template.0.override.0.instance_requirements.0.cpu_manufacturers.#", "2"),
resource.TestCheckTypeSetElemAttr(resourceName, "mixed_instances_policy.0.launch_template.0.override.0.instance_requirements.0.cpu_manufacturers.*", "amazon-web-services"),
resource.TestCheckTypeSetElemAttr(resourceName, "mixed_instances_policy.0.launch_template.0.override.0.instance_requirements.0.cpu_manufacturers.*", "amd"),
),
},
testAccGroupImportStep(resourceName),
Expand Down Expand Up @@ -6472,3 +6503,82 @@ resource "aws_autoscaling_group" "test" {
}
`, rName))
}

func testAccGroupConfig_launchTemplateIAMInstanceProfile(rName, instanceProfileName string) string {
return acctest.ConfigCompose(
acctest.ConfigAvailableAZsNoOptInDefaultExclude(),
acctest.ConfigLatestAmazonLinux2HVMEBSX8664AMI(),
fmt.Sprintf(`
resource "aws_iam_role" "test" {
name = %[1]q
assume_role_policy = jsonencode({
Version = "2012-10-17"
Statement = [
{
Action = "sts:AssumeRole"
Principal = {
Service = "ec2.amazonaws.com"
}
Effect = "Allow"
Sid = ""
},
]
})
}

resource "aws_iam_policy" "test" {
name = %[1]q
description = "Policy for EC2 instances"

policy = jsonencode({
Version = "2012-10-17"
Statement = [
{
Effect = "Allow"
Action = [
"s3:ListBucket",
"s3:GetObject"
]
Resource = "*"
}
]
})
}

resource "aws_iam_role_policy_attachment" "test" {
policy_arn = aws_iam_policy.test.arn
role = aws_iam_role.test.name
}

resource "aws_iam_instance_profile" "test" {
name = %[2]q
role = aws_iam_role.test.name
}

resource "aws_launch_template" "test" {
name = %[1]q
image_id = data.aws_ami.amzn2-ami-minimal-hvm-ebs-x86_64.id
instance_type = %[3]q

iam_instance_profile {
arn = aws_iam_instance_profile.test.arn
}

lifecycle {
create_before_destroy = true
}
}

resource "aws_autoscaling_group" "test" {
availability_zones = [data.aws_availability_zones.available.names[0]]
desired_capacity = 2
max_size = 5
min_size = 1

launch_template {
id = aws_launch_template.test.id
version = aws_launch_template.test.latest_version
}
}
`, rName, instanceProfileName, "t3.micro"))
}
51 changes: 51 additions & 0 deletions internal/service/ec2/ec2_launch_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"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/id"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
"github.com/hashicorp/terraform-provider-aws/internal/conns"
Expand Down Expand Up @@ -1018,6 +1019,10 @@ func resourceLaunchTemplateCreate(ctx context.Context, d *schema.ResourceData, m
return sdkdiag.AppendErrorf(diags, "creating EC2 Launch Template (%s): %s", name, err)
}

if err := waitLaunchTemplateReady(ctx, conn, name, true, d.Timeout(schema.TimeoutCreate)); err != nil {
return sdkdiag.AppendErrorf(diags, "waiting for EC2 Launch Template (%s) to be ready: %s", name, err)
}

d.SetId(aws.ToString(output.LaunchTemplate.LaunchTemplateId))

return append(diags, resourceLaunchTemplateRead(ctx, d, meta)...)
Expand Down Expand Up @@ -1151,6 +1156,10 @@ func resourceLaunchTemplateUpdate(ctx context.Context, d *schema.ResourceData, m
}
}

if err := waitLaunchTemplateReady(ctx, conn, d.Id(), false, d.Timeout(schema.TimeoutUpdate)); err != nil {
return sdkdiag.AppendErrorf(diags, "waiting for EC2 Launch Template (%s) to be ready: %s", d.Id(), err)
}

return append(diags, resourceLaunchTemplateRead(ctx, d, meta)...)
}

Expand All @@ -1174,6 +1183,48 @@ func resourceLaunchTemplateDelete(ctx context.Context, d *schema.ResourceData, m
return diags
}

const (
LaunchTemplateFound = "Found"
)

func statusLaunchTemplate(ctx context.Context, conn *ec2.Client, id string, idIsName bool) retry.StateRefreshFunc {
return func() (interface{}, string, error) {
var output *awstypes.LaunchTemplate
var err error
if idIsName {
output, err = findLaunchTemplateByName(ctx, conn, id)
} else {
output, err = findLaunchTemplateByID(ctx, conn, id)
}

if tfresource.NotFound(err) {
return nil, "", nil
}

if err != nil {
return nil, "", err
}

return output, LaunchTemplateFound, nil
}
}

func waitLaunchTemplateReady(ctx context.Context, conn *ec2.Client, id string, idIsName bool, timeout time.Duration) error {
stateConf := &retry.StateChangeConf{
Pending: []string{""},
Target: enum.Slice(LaunchTemplateFound),
Refresh: statusLaunchTemplate(ctx, conn, id, idIsName),
Timeout: timeout,
Delay: 5 * time.Second,
NotFoundChecks: 5,
ContinuousTargetOccurence: 3,
}

_, err := stateConf.WaitForStateContext(ctx)

return err
}

func expandRequestLaunchTemplateData(ctx context.Context, conn *ec2.Client, d *schema.ResourceData) (*awstypes.RequestLaunchTemplateData, error) {
apiObject := &awstypes.RequestLaunchTemplateData{
// Always set at least one field.
Expand Down
21 changes: 21 additions & 0 deletions internal/service/ec2/find.go
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,27 @@ func findLaunchTemplates(ctx context.Context, conn *ec2.Client, input *ec2.Descr
return output, nil
}

func findLaunchTemplateByName(ctx context.Context, conn *ec2.Client, name string) (*awstypes.LaunchTemplate, error) {
input := &ec2.DescribeLaunchTemplatesInput{
LaunchTemplateNames: []string{name},
}

output, err := findLaunchTemplate(ctx, conn, input)

if err != nil {
return nil, err
}

// Eventual consistency check.
if aws.ToString(output.LaunchTemplateName) != name {
return nil, &retry.NotFoundError{
LastRequest: input,
}
}

return output, nil
}

func findLaunchTemplateByID(ctx context.Context, conn *ec2.Client, id string) (*awstypes.LaunchTemplate, error) {
input := &ec2.DescribeLaunchTemplatesInput{
LaunchTemplateIds: []string{id},
Expand Down
51 changes: 51 additions & 0 deletions internal/service/iam/instance_profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"time"

"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/aws/arn"
"github.com/aws/aws-sdk-go-v2/service/iam"
awstypes "github.com/aws/aws-sdk-go-v2/service/iam/types"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
Expand All @@ -18,6 +19,7 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-provider-aws/internal/conns"
"github.com/hashicorp/terraform-provider-aws/internal/create"
"github.com/hashicorp/terraform-provider-aws/internal/enum"
"github.com/hashicorp/terraform-provider-aws/internal/errs"
"github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag"
tftags "github.com/hashicorp/terraform-provider-aws/internal/tags"
Expand Down Expand Up @@ -149,6 +151,10 @@ func resourceInstanceProfileCreate(ctx context.Context, d *schema.ResourceData,
}
}

if err := waitInstanceProfileReady(ctx, conn, name, d.Timeout(schema.TimeoutCreate)); err != nil {
return sdkdiag.AppendErrorf(diags, "waiting for IAM Instance Profile (%s) to be ready: %s", name, err)
}

return append(diags, resourceInstanceProfileRead(ctx, d, meta)...)
}

Expand Down Expand Up @@ -229,6 +235,10 @@ func resourceInstanceProfileUpdate(ctx context.Context, d *schema.ResourceData,
}
}

if err := waitInstanceProfileReady(ctx, conn, d.Id(), d.Timeout(schema.TimeoutCreate)); err != nil {
return sdkdiag.AppendErrorf(diags, "waiting for IAM Instance Profile (%s) to be ready: %s", d.Id(), err)
}

return append(diags, resourceInstanceProfileRead(ctx, d, meta)...)
}

Expand Down Expand Up @@ -334,6 +344,47 @@ func findInstanceProfileByName(ctx context.Context, conn *iam.Client, name strin
return output.InstanceProfile, nil
}

const (
InstanceProfileFound = "Found"
InstanceProfileInvalidARN = "InvalidARN"
)

func statusInstanceProfile(ctx context.Context, conn *iam.Client, name string) retry.StateRefreshFunc {
return func() (interface{}, string, error) {
output, err := findInstanceProfileByName(ctx, conn, name)
if tfresource.NotFound(err) {
return nil, "", nil
}

if err != nil {
return nil, "", err
}

_, err = arn.Parse(aws.ToString(output.Arn))
if err != nil {
return nil, InstanceProfileInvalidARN, nil // lint:ignore nilerr // this is usually a temporary state
}

return output, InstanceProfileFound, nil
}
}

func waitInstanceProfileReady(ctx context.Context, conn *iam.Client, id string, timeout time.Duration) error {
stateConf := &retry.StateChangeConf{
Pending: []string{"", InstanceProfileInvalidARN},
Target: enum.Slice(InstanceProfileFound),
Refresh: statusInstanceProfile(ctx, conn, id),
Timeout: timeout,
Delay: 5 * time.Second,
NotFoundChecks: 5,
ContinuousTargetOccurence: 3,
}

_, err := stateConf.WaitForStateContext(ctx)

return err
}

func instanceProfileTags(ctx context.Context, conn *iam.Client, identifier string) ([]awstypes.Tag, error) {
output, err := conn.ListInstanceProfileTags(ctx, &iam.ListInstanceProfileTagsInput{
InstanceProfileName: aws.String(identifier),
Expand Down
Loading