diff --git a/.changelog/48422.txt b/.changelog/48422.txt new file mode 100644 index 000000000000..3ef364c28d6a --- /dev/null +++ b/.changelog/48422.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_ecs_daemon: Fix inconsistent final plan errors when updating `capacity_provider_arns` +``` diff --git a/internal/service/ecs/daemon.go b/internal/service/ecs/daemon.go index d62ce004aab7..886b9d900c7e 100644 --- a/internal/service/ecs/daemon.go +++ b/internal/service/ecs/daemon.go @@ -6,6 +6,7 @@ package ecs import ( "context" "fmt" + "slices" "strings" "time" @@ -218,20 +219,16 @@ func (r *daemonResource) Create(ctx context.Context, request resource.CreateRequ // Save ARN to state so Terraform can track the resource if the waiter times out. response.State.SetAttribute(ctx, path.Root(names.AttrARN), output.DaemonArn) - createTimeout := r.CreateTimeout(ctx, plan.Timeouts) - if err := waitDaemonActive(ctx, conn, plan.DaemonArn.ValueString(), createTimeout); err != nil { - response.Diagnostics.AddError(fmt.Sprintf("waiting for ECS Daemon (%s) create", plan.DaemonArn.ValueString()), err.Error()) + capacityProviderARNs, diags := daemonCapacityProviderARNsFromSet(ctx, plan.CapacityProviderArns) + response.Diagnostics.Append(diags...) + if response.Diagnostics.HasError() { return } - outputFind, err := findDaemonByARN(ctx, conn, plan.DaemonArn.ValueString()) - if retry.NotFound(err) { - response.Diagnostics.Append(fwdiag.NewResourceNotFoundWarningDiagnostic(err)) - response.State.RemoveResource(ctx) - return - } + createTimeout := r.CreateTimeout(ctx, plan.Timeouts) + outputFind, err := waitDaemonActiveCurrentRevision(ctx, conn, plan.DaemonArn.ValueString(), plan.DaemonTaskDefinitionArn.ValueString(), capacityProviderARNs, createTimeout) if err != nil { - response.Diagnostics.AddError(fmt.Sprintf("reading ECS Daemon (%s)", plan.DaemonArn.ValueString()), err.Error()) + response.Diagnostics.AddError(fmt.Sprintf("waiting for ECS Daemon (%s) create", plan.DaemonArn.ValueString()), err.Error()) return } @@ -303,6 +300,8 @@ func (r *daemonResource) Update(ctx context.Context, request resource.UpdateRequ return } + var output *awstypes.DaemonDetail + if diff.HasChanges() { var input ecs.UpdateDaemonInput response.Diagnostics.Append(fwflex.Expand(ctx, plan, &input)...) @@ -316,17 +315,27 @@ func (r *daemonResource) Update(ctx context.Context, request resource.UpdateRequ return } + capacityProviderARNs, diags := daemonCapacityProviderARNsFromSet(ctx, plan.CapacityProviderArns) + response.Diagnostics.Append(diags...) + if response.Diagnostics.HasError() { + return + } + updateTimeout := r.UpdateTimeout(ctx, plan.Timeouts) - if err := waitDaemonActive(ctx, conn, plan.DaemonArn.ValueString(), updateTimeout); err != nil { + output, err = waitDaemonActiveCurrentRevision(ctx, conn, plan.DaemonArn.ValueString(), plan.DaemonTaskDefinitionArn.ValueString(), capacityProviderARNs, updateTimeout) + if err != nil { response.Diagnostics.AddError(fmt.Sprintf("waiting for ECS Daemon (%s) update", plan.DaemonArn.ValueString()), err.Error()) return } } - output, err := findDaemonByARN(ctx, conn, plan.DaemonArn.ValueString()) - if err != nil { - response.Diagnostics.AddError(fmt.Sprintf("reading ECS Daemon (%s)", plan.DaemonArn.ValueString()), err.Error()) - return + if output == nil { + var err error + output, err = findDaemonByARN(ctx, conn, plan.DaemonArn.ValueString()) + if err != nil { + response.Diagnostics.AddError(fmt.Sprintf("reading ECS Daemon (%s)", plan.DaemonArn.ValueString()), err.Error()) + return + } } response.Diagnostics.Append(fwflex.Flatten(ctx, output, &plan)...) @@ -387,6 +396,38 @@ func daemonNameFromARN(arnStr string) types.String { return types.StringNull() } +func daemonCapacityProviderARNsFromSet(ctx context.Context, set fwtypes.SetOfString) ([]string, diag.Diagnostics) { + var arns []string + diags := set.ElementsAs(ctx, &arns, false) + + return arns, diags +} + +func daemonRevisionDetailCapacityProviderARNs(revisionDetail awstypes.DaemonRevisionDetail) []string { + arns := make([]string, 0, len(revisionDetail.CapacityProviders)) + + for _, cp := range revisionDetail.CapacityProviders { + if cp.Arn != nil { + arns = append(arns, aws.ToString(cp.Arn)) + } + } + + return arns +} + +func daemonCapacityProviderARNsEqual(a, b []string) bool { + if len(a) != len(b) { + return false + } + + a = slices.Clone(a) + b = slices.Clone(b) + slices.Sort(a) + slices.Sort(b) + + return slices.Equal(a, b) +} + // flattenDaemonRevision populates task definition ARN and capacity // provider ARNs from a DaemonRevision and DaemonRevisionDetail. DaemonTaskDefinitionArn is only // set when the model's value is null (e.g., during import) to avoid overwriting @@ -397,13 +438,7 @@ func flattenDaemonRevision(ctx context.Context, revision *awstypes.DaemonRevisio } if len(revisionDetail.CapacityProviders) > 0 { - cpArns := make([]string, 0, len(revisionDetail.CapacityProviders)) - for _, cp := range revisionDetail.CapacityProviders { - if cp.Arn != nil { - cpArns = append(cpArns, aws.ToString(cp.Arn)) - } - } - model.CapacityProviderArns = fwflex.FlattenFrameworkStringValueSetOfString(ctx, cpArns) + model.CapacityProviderArns = fwflex.FlattenFrameworkStringValueSetOfString(ctx, daemonRevisionDetailCapacityProviderARNs(revisionDetail)) } } @@ -496,6 +531,13 @@ func findDaemonRevisionByARN(ctx context.Context, conn *ecs.Client, arn string) output, err := conn.DescribeDaemonRevisions(ctx, input) + if errs.IsA[*awstypes.DaemonNotFoundException](err) { + return nil, &sdkretry.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + if err != nil { return nil, err } @@ -503,6 +545,63 @@ func findDaemonRevisionByARN(ctx context.Context, conn *ecs.Client, arn string) return tfresource.AssertSingleValueResult(output.DaemonRevisions) } +func waitDaemonActiveCurrentRevision(ctx context.Context, conn *ecs.Client, arn, daemonTaskDefinitionARN string, capacityProviderARNs []string, timeout time.Duration) (*awstypes.DaemonDetail, error) { + var output *awstypes.DaemonDetail + + err := tfresource.Retry(ctx, timeout, func(ctx context.Context) *tfresource.RetryError { + daemon, err := findDaemonByARN(ctx, conn, arn) + + if retry.NotFound(err) { + return tfresource.RetryableError(err) + } + + if err != nil { + return tfresource.NonRetryableError(err) + } + + output = daemon + + if daemon.Status != awstypes.DaemonStatusActive { + return tfresource.RetryableError(fmt.Errorf("status is %s", daemon.Status)) + } + + if len(daemon.CurrentRevisions) == 0 || daemon.CurrentRevisions[0].Arn == nil { + return tfresource.RetryableError(fmt.Errorf("current revision is not available")) + } + + revisionARN := aws.ToString(daemon.CurrentRevisions[0].Arn) + revision, err := findDaemonRevisionByARN(ctx, conn, revisionARN) + + if retry.NotFound(err) { + return tfresource.RetryableError(err) + } + + if err != nil { + return tfresource.NonRetryableError(err) + } + + if got, want := aws.ToString(revision.DaemonTaskDefinitionArn), daemonTaskDefinitionARN; got != want { + return tfresource.RetryableError(fmt.Errorf("daemon task definition ARN is %s, want %s", got, want)) + } + + if got, want := daemonRevisionDetailCapacityProviderARNs(daemon.CurrentRevisions[0]), capacityProviderARNs; !daemonCapacityProviderARNsEqual(got, want) { + return tfresource.RetryableError(fmt.Errorf("capacity provider ARNs are %v, want %v", got, want)) + } + + return nil + }) + + if err != nil { + return nil, err + } + + if output == nil { + return nil, tfresource.NewEmptyResultError() + } + + return output, nil +} + func statusDaemon(ctx context.Context, conn *ecs.Client, arn string) sdkretry.StateRefreshFunc { return func() (any, string, error) { output, err := findDaemonByARN(ctx, conn, arn) @@ -519,19 +618,6 @@ func statusDaemon(ctx context.Context, conn *ecs.Client, arn string) sdkretry.St } } -func waitDaemonActive(ctx context.Context, conn *ecs.Client, arn string, timeout time.Duration) error { - stateConf := &sdkretry.StateChangeConf{ - Pending: []string{}, - Target: enum.Slice(awstypes.DaemonStatusActive), - Refresh: statusDaemon(ctx, conn, arn), - Timeout: timeout, - } - - _, err := stateConf.WaitForStateContext(ctx) - - return err -} - func waitDaemonDeleted(ctx context.Context, conn *ecs.Client, arn string, timeout time.Duration) error { stateConf := &sdkretry.StateChangeConf{ Pending: enum.Slice(awstypes.DaemonStatusActive, awstypes.DaemonStatusDeleteInProgress), diff --git a/internal/service/ecs/daemon_test.go b/internal/service/ecs/daemon_test.go index 9651bae88cf2..5dbba0e92a31 100644 --- a/internal/service/ecs/daemon_test.go +++ b/internal/service/ecs/daemon_test.go @@ -72,6 +72,66 @@ func TestDaemonNameFromARN(t *testing.T) { } } +func TestDaemonCapacityProviderARNsEqual(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + a []string + b []string + expected bool + }{ + { + name: "same order", + a: []string{"one", "two"}, + b: []string{"one", "two"}, + expected: true, + }, + { + name: "different order", + a: []string{"one", "two"}, + b: []string{"two", "one"}, + expected: true, + }, + { + name: "missing", + a: []string{"one", "two"}, + b: []string{"one"}, + expected: false, + }, + { + name: "extra", + a: []string{"one"}, + b: []string{"one", "two"}, + expected: false, + }, + { + name: "empty", + a: []string{}, + b: []string{}, + expected: true, + }, + { + name: "nil and empty", + a: nil, + b: []string{}, + expected: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got := tfecs.DaemonCapacityProviderARNsEqual(tc.a, tc.b) + + if got != tc.expected { + t.Errorf("got %t, expected %t", got, tc.expected) + } + }) + } +} + func TestAccECSDaemon_basic(t *testing.T) { ctx := acctest.Context(t) rName := acctest.RandomWithPrefix(t, acctest.ResourcePrefix) @@ -112,6 +172,44 @@ func TestAccECSDaemon_basic(t *testing.T) { }) } +func TestAccECSDaemon_updateCapacityProviderARNs(t *testing.T) { + ctx := acctest.Context(t) + rName := acctest.RandomWithPrefix(t, acctest.ResourcePrefix) + resourceName := "aws_ecs_daemon.test" + + acctest.ParallelTest(ctx, t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.ECSServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckDaemonDestroy(ctx, t), + Steps: []resource.TestStep{ + { + Config: testAccDaemonConfig_basic(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckDaemonExists(ctx, t, resourceName), + resource.TestCheckResourceAttr(resourceName, "capacity_provider_arns.#", "1"), + resource.TestCheckTypeSetElemAttrPair(resourceName, "capacity_provider_arns.*", "aws_ecs_capacity_provider.test", names.AttrARN), + ), + }, + { + Config: testAccDaemonConfig_updateCapacityProviderARNs(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckDaemonExists(ctx, t, resourceName), + resource.TestCheckResourceAttr(resourceName, "capacity_provider_arns.#", "3"), + resource.TestCheckTypeSetElemAttrPair(resourceName, "capacity_provider_arns.*", "aws_ecs_capacity_provider.test", names.AttrARN), + resource.TestCheckTypeSetElemAttrPair(resourceName, "capacity_provider_arns.*", "aws_ecs_capacity_provider.test2", names.AttrARN), + resource.TestCheckTypeSetElemAttrPair(resourceName, "capacity_provider_arns.*", "aws_ecs_capacity_provider.test3", names.AttrARN), + ), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate), + }, + }, + }, + }, + }) +} + func TestAccECSDaemon_disappears(t *testing.T) { ctx := acctest.Context(t) rName := acctest.RandomWithPrefix(t, acctest.ResourcePrefix) @@ -649,6 +747,57 @@ resource "aws_ecs_daemon" "test" { `, rName)) } +func testAccDaemonConfig_updateCapacityProviderARNs(rName string) string { + return acctest.ConfigCompose(testAccDaemonConfig_base(rName), fmt.Sprintf(` +resource "aws_ecs_capacity_provider" "test2" { + name = "%[1]s-2" + cluster = aws_ecs_cluster.test.name + + managed_instances_provider { + infrastructure_role_arn = aws_iam_role.infra.arn + + instance_launch_template { + ec2_instance_profile_arn = aws_iam_instance_profile.test.arn + + network_configuration { + subnets = [aws_subnet.test[0].id] + security_groups = [aws_security_group.test.id] + } + } + } +} + +resource "aws_ecs_capacity_provider" "test3" { + name = "%[1]s-3" + cluster = aws_ecs_cluster.test.name + + managed_instances_provider { + infrastructure_role_arn = aws_iam_role.infra.arn + + instance_launch_template { + ec2_instance_profile_arn = aws_iam_instance_profile.test.arn + + network_configuration { + subnets = [aws_subnet.test[0].id] + security_groups = [aws_security_group.test.id] + } + } + } +} + +resource "aws_ecs_daemon" "test" { + name = %[1]q + cluster_arn = aws_ecs_cluster.test.arn + daemon_task_definition_arn = aws_ecs_daemon_task_definition.test.arn + capacity_provider_arns = [ + aws_ecs_capacity_provider.test.arn, + aws_ecs_capacity_provider.test2.arn, + aws_ecs_capacity_provider.test3.arn, + ] +} +`, rName)) +} + func testAccDaemonConfig_deploymentConfiguration(rName string, drainPercent float64, bakeTime int) string { return acctest.ConfigCompose(testAccDaemonConfig_base(rName), fmt.Sprintf(` resource "aws_ecs_daemon" "test" { diff --git a/internal/service/ecs/exports_test.go b/internal/service/ecs/exports_test.go index b0eea8217ca2..e8b939b45197 100644 --- a/internal/service/ecs/exports_test.go +++ b/internal/service/ecs/exports_test.go @@ -18,6 +18,7 @@ var ( ResourceTaskSet = resourceTaskSet ClusterNameFromARN = clusterNameFromARN + DaemonCapacityProviderARNsEqual = daemonCapacityProviderARNsEqual DaemonNameFromARN = daemonNameFromARN FindCapacityProviderByARN = findCapacityProviderByARN FindClusterByNameOrARN = findClusterByNameOrARN