Skip to content

Commit

Permalink
Merge pull request #40088 from hashicorp/b-launch-template-validation…
Browse files Browse the repository at this point in the history
…error

autoscaling/group: Handle eventual consistency
  • Loading branch information
YakDriver authored Nov 11, 2024
2 parents 0c6d8bc + cbd9084 commit 76f49c9
Show file tree
Hide file tree
Showing 7 changed files with 256 additions and 3 deletions.
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

0 comments on commit 76f49c9

Please sign in to comment.