From 039108b24809bacbbea378be5431a69a49f4c7f0 Mon Sep 17 00:00:00 2001 From: Daniel Quackenbush <25692880+danquack@users.noreply.github.com> Date: Tue, 19 Dec 2023 18:51:46 -0500 Subject: [PATCH 01/12] Batch datasource Co-authored-by: Drew Mullen --- internal/service/batch/findv2.go | 54 +++++++++++++++++++ .../batch/job_definition_data_source_test.go | 5 ++ 2 files changed, 59 insertions(+) create mode 100644 internal/service/batch/findv2.go diff --git a/internal/service/batch/findv2.go b/internal/service/batch/findv2.go new file mode 100644 index 00000000000..812ef300cd6 --- /dev/null +++ b/internal/service/batch/findv2.go @@ -0,0 +1,54 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package batch + +import ( + "context" + + "github.com/aws/aws-sdk-go-v2/service/batch" + "github.com/aws/aws-sdk-go-v2/service/batch/types" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" +) + +func FindJobDefinitionV2ByARN(ctx context.Context, conn *batch.Client, arn string) (*types.JobDefinition, error) { + input := &batch.DescribeJobDefinitionsInput{ + JobDefinitions: []string{arn}, + } + + out, err := conn.DescribeJobDefinitions(ctx, input) + + if err != nil { + return nil, err + } + + if out == nil || len(out.JobDefinitions) == 0 { + return nil, tfresource.NewEmptyResultError(input) + } + + if count := len(out.JobDefinitions); count > 1 { + return nil, tfresource.NewTooManyResultsError(count, input) + } + + return &out.JobDefinitions[0], nil +} + +func ListJobDefinitionsV2ByNameWithStatus(ctx context.Context, conn *batch.Client, input *batch.DescribeJobDefinitionsInput) ([]types.JobDefinition, error) { + var out []types.JobDefinition + + pages := batch.NewDescribeJobDefinitionsPaginator(conn, input) + for pages.HasMorePages() { + page, err := pages.NextPage(ctx) + + if err != nil { + return nil, err + } + out = append(out, page.JobDefinitions...) + } + + if out == nil || len(out) == 0 { + return nil, tfresource.NewEmptyResultError(input) + } + + return out, nil +} diff --git a/internal/service/batch/job_definition_data_source_test.go b/internal/service/batch/job_definition_data_source_test.go index 0a1d43efb12..075cc503c74 100644 --- a/internal/service/batch/job_definition_data_source_test.go +++ b/internal/service/batch/job_definition_data_source_test.go @@ -7,6 +7,7 @@ import ( "fmt" "testing" + "github.com/YakDriver/regexache" sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-provider-aws/internal/acctest" @@ -65,6 +66,10 @@ func TestAccBatchJobDefinitionDataSource_basicARN(t *testing.T) { Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr(dataSourceName, "retry_strategy.0.attempts", "10"), resource.TestCheckResourceAttr(dataSourceName, "revision", "1"), + testAccCheckJobDefinitionV2Exists(ctx, dataSourceName, &jd), + resource.TestCheckResourceAttr(dataSourceName, "revision", "1"), + resource.TestCheckResourceAttr(dataSourceName, "retry_strategy.attempts", "10"), + acctest.MatchResourceAttrRegionalARN(dataSourceName, "arn", "batch", regexache.MustCompile(fmt.Sprintf(`job-definition/%s:\d+`, rName))), ), }, { From 7aa8047e75eca8848ed3f61dfd458ac5a5588457 Mon Sep 17 00:00:00 2001 From: Drew Mullen Date: Tue, 9 Jan 2024 14:17:05 -0500 Subject: [PATCH 02/12] initial remove of ForceNew --- internal/service/batch/job_definition.go | 138 +++++++++- internal/service/batch/job_definition_test.go | 248 +++++++++++++++++- .../docs/r/batch_job_definition.html.markdown | 1 + 3 files changed, 361 insertions(+), 26 deletions(-) diff --git a/internal/service/batch/job_definition.go b/internal/service/batch/job_definition.go index 34d64193a15..fbade0c5c9b 100644 --- a/internal/service/batch/job_definition.go +++ b/internal/service/batch/job_definition.go @@ -57,7 +57,6 @@ func ResourceJobDefinition() *schema.Resource { "container_properties": { Type: schema.TypeString, Optional: true, - ForceNew: true, ConflictsWith: []string{"eks_properties", "node_properties"}, StateFunc: func(v interface{}) string { json, _ := structure.NormalizeJsonString(v) @@ -71,6 +70,12 @@ func ResourceJobDefinition() *schema.Resource { ValidateFunc: validJobContainerProperties, }, + "deregister_on_new_revision": { + Type: schema.TypeBool, + Default: true, + Optional: true, + }, + "name": { Type: schema.TypeString, Required: true, @@ -81,7 +86,6 @@ func ResourceJobDefinition() *schema.Resource { "node_properties": { Type: schema.TypeString, Optional: true, - ForceNew: true, ConflictsWith: []string{"container_properties", "eks_properties"}, StateFunc: func(v interface{}) string { json, _ := structure.NormalizeJsonString(v) @@ -323,14 +327,12 @@ func ResourceJobDefinition() *schema.Resource { "parameters": { Type: schema.TypeMap, Optional: true, - ForceNew: true, Elem: &schema.Schema{Type: schema.TypeString}, }, "platform_capabilities": { Type: schema.TypeSet, Optional: true, - ForceNew: true, Elem: &schema.Schema{ Type: schema.TypeString, ValidateFunc: validation.StringInSlice(batch.PlatformCapability_Values(), false), @@ -340,14 +342,12 @@ func ResourceJobDefinition() *schema.Resource { "propagate_tags": { Type: schema.TypeBool, Optional: true, - ForceNew: true, Default: false, }, "retry_strategy": { Type: schema.TypeList, Optional: true, - ForceNew: true, MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ @@ -416,7 +416,6 @@ func ResourceJobDefinition() *schema.Resource { "scheduling_priority": { Type: schema.TypeInt, Optional: true, - ForceNew: true, }, names.AttrTags: tftags.TagsSchema(), @@ -442,7 +441,6 @@ func ResourceJobDefinition() *schema.Resource { "type": { Type: schema.TypeString, Required: true, - ForceNew: true, ValidateFunc: validation.StringInSlice([]string{batch.JobDefinitionTypeContainer, batch.JobDefinitionTypeMultinode}, true), }, }, @@ -624,8 +622,95 @@ func resourceJobDefinitionRead(ctx context.Context, d *schema.ResourceData, meta func resourceJobDefinitionUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics + conn := meta.(*conns.AWSClient).BatchConn(ctx) + + if d.HasChangesExcept("tags", "tags_all") { + name := d.Get("name").(string) + input := &batch.RegisterJobDefinitionInput{ + JobDefinitionName: aws.String(name), + Type: aws.String(d.Get("type").(string)), + } + + if v, ok := d.GetOk("container_properties"); ok { + props, err := expandJobContainerProperties(v.(string)) + if err != nil { + return sdkdiag.AppendErrorf(diags, "updating Batch Job Definition (%s): %s", name, err) + } + + if aws.StringValue(input.Type) == batch.JobDefinitionTypeContainer { + removeEmptyEnvironmentVariables(&diags, props.Environment, cty.GetAttrPath("container_properties")) + input.ContainerProperties = props + } + } + + if v, ok := d.GetOk("eks_properties"); ok { + eksProps := v.([]interface{})[0].(map[string]interface{}) + if podProps, ok := eksProps["pod_properties"].([]interface{}); ok && len(podProps) > 0 { + props := expandEKSPodProperties(podProps[0].(map[string]interface{})) + input.EksProperties = &batch.EksProperties{ + PodProperties: props, + } + } + } - // Tags only. + if v, ok := d.GetOk("node_properties"); ok { + props, err := expandJobNodeProperties(v.(string)) + if err != nil { + return sdkdiag.AppendErrorf(diags, "updating Batch Job Definition (%s): %s", name, err) + } + + for _, node := range props.NodeRangeProperties { + removeEmptyEnvironmentVariables(&diags, node.Container.Environment, cty.GetAttrPath("node_properties")) + } + input.NodeProperties = props + } + + if v, ok := d.GetOk("propagate_tags"); ok { + input.PropagateTags = aws.Bool(v.(bool)) + } + + if v, ok := d.GetOk("parameters"); ok { + input.Parameters = expandJobDefinitionParameters(v.(map[string]interface{})) + } + + if v, ok := d.GetOk("platform_capabilities"); ok && v.(*schema.Set).Len() > 0 { + input.PlatformCapabilities = flex.ExpandStringSet(v.(*schema.Set)) + } + + if v, ok := d.GetOk("scheduling_priority"); ok { + input.SchedulingPriority = aws.Int64(int64(v.(int))) + } + + if v, ok := d.GetOk("retry_strategy"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { + input.RetryStrategy = expandRetryStrategy(v.([]interface{})[0].(map[string]interface{})) + } + + if v, ok := d.GetOk("timeout"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { + input.Timeout = expandJobTimeout(v.([]interface{})[0].(map[string]interface{})) + } + + jd, err := conn.RegisterJobDefinitionWithContext(ctx, input) + + if err != nil { + return sdkdiag.AppendErrorf(diags, "updating Batch Job Definition (%s): %s", name, err) + } + + // arn contains revision which is used in the Read call + currentARN := d.Get("arn").(string) + d.SetId(aws.StringValue(jd.JobDefinitionArn)) + d.Set("revision", jd.Revision) + + if v := d.Get("deregister_on_new_revision"); v == true { + log.Printf("[DEBUG] Deleting Previous Batch Job Definition: %s", currentARN) + _, err := conn.DeregisterJobDefinitionWithContext(ctx, &batch.DeregisterJobDefinitionInput{ + JobDefinition: aws.String(currentARN), + }) + + if err != nil { + return sdkdiag.AppendErrorf(diags, "deleting Batch Job Definition (%s): %s", currentARN, err) + } + } + } return append(diags, resourceJobDefinitionRead(ctx, d, meta)...) } @@ -634,13 +719,23 @@ func resourceJobDefinitionDelete(ctx context.Context, d *schema.ResourceData, me var diags diag.Diagnostics conn := meta.(*conns.AWSClient).BatchConn(ctx) - log.Printf("[DEBUG] Deleting Batch Job Definition: %s", d.Id()) - _, err := conn.DeregisterJobDefinitionWithContext(ctx, &batch.DeregisterJobDefinitionInput{ - JobDefinition: aws.String(d.Id()), - }) + name := d.Get("name").(string) + jds, err := ListActiveJobDefinitionByName(ctx, conn, name) if err != nil { - return sdkdiag.AppendErrorf(diags, "deleting Batch Job Definition (%s): %s", d.Id(), err) + return sdkdiag.AppendErrorf(diags, "deleting Batch Job Definitions (%s): %s", name, err) + } + + for i := range jds { + arn := aws.StringValue(jds[i].JobDefinitionArn) + log.Printf("[DEBUG] Deleting Batch Job Definition: %s", arn) + _, err := conn.DeregisterJobDefinitionWithContext(ctx, &batch.DeregisterJobDefinitionInput{ + JobDefinition: aws.String(arn), + }) + + if err != nil { + return sdkdiag.AppendErrorf(diags, "deleting Batch Job Definition (%s): %s", arn, err) + } } return diags @@ -670,6 +765,21 @@ func FindJobDefinitionByARN(ctx context.Context, conn *batch.Batch, arn string) return output, nil } +func ListActiveJobDefinitionByName(ctx context.Context, conn *batch.Batch, name string) ([]*batch.JobDefinition, error) { + input := &batch.DescribeJobDefinitionsInput{ + JobDefinitionName: aws.String(name), + Status: aws.String(JobDefinitionStatusActive), + } + + output, err := conn.DescribeJobDefinitionsWithContext(ctx, input) + + if err != nil { + return nil, err + } + + return output.JobDefinitions, nil +} + func findJobDefinition(ctx context.Context, conn *batch.Batch, input *batch.DescribeJobDefinitionsInput) (*batch.JobDefinition, error) { output, err := conn.DescribeJobDefinitionsWithContext(ctx, input) diff --git a/internal/service/batch/job_definition_test.go b/internal/service/batch/job_definition_test.go index 8c5985d693a..ce529c7243f 100644 --- a/internal/service/batch/job_definition_test.go +++ b/internal/service/batch/job_definition_test.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "reflect" + "strconv" "strings" "testing" @@ -19,8 +20,11 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" tfbatch "github.com/hashicorp/terraform-provider-aws/internal/service/batch" +<<<<<<< HEAD "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/names" +======= +>>>>>>> 1c264545ba (initial remove of ForceNew) ) func TestAccBatchJobDefinition_basic(t *testing.T) { @@ -69,6 +73,100 @@ func TestAccBatchJobDefinition_basic(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "deregister_on_new_revision", + }, + }, + }, + }) +} + +func TestAccBatchJobDefinition_attributes(t *testing.T) { + ctx := acctest.Context(t) + var jd batch.JobDefinition + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_batch_job_definition.test" + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t); testAccPreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, batch.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckJobDefinitionDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccJobDefinitionConfig_attributes(rName, 2, true, 3, 120, false), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckJobDefinitionExists(ctx, resourceName, &jd), + acctest.MatchResourceAttrRegionalARN(resourceName, "arn", "batch", regexache.MustCompile(fmt.Sprintf(`job-definition/%s:\d+`, rName))), + acctest.MatchResourceAttrRegionalARN(resourceName, "arn_prefix", "batch", regexache.MustCompile(fmt.Sprintf(`job-definition/%s`, rName))), + resource.TestCheckResourceAttr(resourceName, "name", rName), + resource.TestCheckResourceAttr(resourceName, "parameters.%", "0"), + resource.TestCheckResourceAttr(resourceName, "platform_capabilities.#", "1"), + resource.TestCheckTypeSetElemAttr(resourceName, "platform_capabilities.*", "EC2"), + resource.TestCheckResourceAttr(resourceName, "propagate_tags", "true"), + resource.TestCheckResourceAttr(resourceName, "retry_strategy.#", "1"), + resource.TestCheckResourceAttr(resourceName, "retry_strategy.0.attempts", "3"), + resource.TestCheckResourceAttr(resourceName, "revision", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + resource.TestCheckResourceAttr(resourceName, "timeout.#", "1"), + resource.TestCheckResourceAttr(resourceName, "timeout.0.attempt_duration_seconds", "120"), + resource.TestCheckResourceAttr(resourceName, "type", "container"), + resource.TestCheckResourceAttr(resourceName, "scheduling_priority", "2"), + ), + }, + { + Config: testAccJobDefinitionConfig_attributes(rName, 2, true, 4, 120, false), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckJobDefinitionExists(ctx, resourceName, &jd), + testAccCheckJobDefinitionPreviousRegistered(ctx, resourceName), + resource.TestCheckResourceAttr(resourceName, "revision", "2"), + ), + }, + { + Config: testAccJobDefinitionConfig_name(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckJobDefinitionExists(ctx, resourceName, &jd), + testAccCheckJobDefinitionPreviousDeregistered(ctx, resourceName), + acctest.MatchResourceAttrRegionalARN(resourceName, "arn", "batch", regexache.MustCompile(fmt.Sprintf(`job-definition/%s:\d+`, rName))), + acctest.MatchResourceAttrRegionalARN(resourceName, "arn_prefix", "batch", regexache.MustCompile(fmt.Sprintf(`job-definition/%s`, rName))), + resource.TestCheckResourceAttr(resourceName, "name", rName), + resource.TestCheckResourceAttr(resourceName, "parameters.%", "0"), + resource.TestCheckResourceAttr(resourceName, "platform_capabilities.#", "0"), + resource.TestCheckResourceAttr(resourceName, "retry_strategy.#", "0"), + resource.TestCheckResourceAttr(resourceName, "revision", "3"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + resource.TestCheckResourceAttr(resourceName, "timeout.#", "0"), + resource.TestCheckResourceAttr(resourceName, "type", "container"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "deregister_on_new_revision", + }, + }, + { + Config: testAccJobDefinitionConfig_attributes(rName, 1, false, 1, 60, true), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckJobDefinitionExists(ctx, resourceName, &jd), + testAccCheckJobDefinitionPreviousDeregistered(ctx, resourceName), + acctest.MatchResourceAttrRegionalARN(resourceName, "arn", "batch", regexache.MustCompile(fmt.Sprintf(`job-definition/%s:\d+`, rName))), + acctest.MatchResourceAttrRegionalARN(resourceName, "arn_prefix", "batch", regexache.MustCompile(fmt.Sprintf(`job-definition/%s`, rName))), + resource.TestCheckResourceAttr(resourceName, "name", rName), + resource.TestCheckResourceAttr(resourceName, "parameters.%", "0"), + resource.TestCheckResourceAttr(resourceName, "platform_capabilities.#", "1"), + resource.TestCheckTypeSetElemAttr(resourceName, "platform_capabilities.*", "EC2"), + resource.TestCheckResourceAttr(resourceName, "propagate_tags", "false"), + resource.TestCheckResourceAttr(resourceName, "retry_strategy.#", "1"), + resource.TestCheckResourceAttr(resourceName, "retry_strategy.0.attempts", "1"), + resource.TestCheckResourceAttr(resourceName, "revision", "4"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + resource.TestCheckResourceAttr(resourceName, "timeout.#", "1"), + resource.TestCheckResourceAttr(resourceName, "timeout.0.attempt_duration_seconds", "60"), + resource.TestCheckResourceAttr(resourceName, "type", "container"), + resource.TestCheckResourceAttr(resourceName, "scheduling_priority", "1"), + ), }, }, }) @@ -143,6 +241,9 @@ func TestAccBatchJobDefinition_PlatformCapabilities_ec2(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "deregister_on_new_revision", + }, }, }, }) @@ -187,6 +288,9 @@ func TestAccBatchJobDefinition_PlatformCapabilitiesFargate_containerPropertiesDe ResourceName: resourceName, ImportState: true, ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "deregister_on_new_revision", + }, }, }, }) @@ -231,6 +335,9 @@ func TestAccBatchJobDefinition_PlatformCapabilities_fargate(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "deregister_on_new_revision", + }, }, }, }) @@ -298,6 +405,7 @@ func TestAccBatchJobDefinition_ContainerProperties_advanced(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, +<<<<<<< HEAD }, }, }) @@ -333,6 +441,11 @@ func TestAccBatchJobDefinition_updateForcesNewResource(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, +======= + ImportStateVerifyIgnore: []string{ + "deregister_on_new_revision", + }, +>>>>>>> 1c264545ba (initial remove of ForceNew) }, }, }) @@ -362,6 +475,9 @@ func TestAccBatchJobDefinition_tags(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "deregister_on_new_revision", + }, }, { Config: testAccJobDefinitionConfig_tags2(rName, "key1", "value1updated", "key2", "value2"), @@ -455,6 +571,9 @@ func TestAccBatchJobDefinition_ContainerProperties_EmptyField(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "deregister_on_new_revision", + }, }, }, }) @@ -529,6 +648,7 @@ func TestAccBatchJobDefinition_NodeProperties_basic(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, +<<<<<<< HEAD }, }, }) @@ -564,6 +684,11 @@ func TestAccBatchJobDefinition_NodePropertiesupdateForcesNewResource(t *testing. ResourceName: resourceName, ImportState: true, ImportStateVerify: true, +======= + ImportStateVerifyIgnore: []string{ + "deregister_on_new_revision", + }, +>>>>>>> 1c264545ba (initial remove of ForceNew) }, }, }) @@ -595,6 +720,9 @@ func TestAccBatchJobDefinition_EKSProperties_basic(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "deregister_on_new_revision", + }, }, }, }) @@ -632,6 +760,9 @@ func TestAccBatchJobDefinition_EKSProperties_update(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "deregister_on_new_revision", + }, }, }, }) @@ -726,6 +857,78 @@ func testAccCheckJobDefinitionExists(ctx context.Context, n string, jd *batch.Jo } } +func testAccCheckJobDefinitionPreviousRegistered(ctx context.Context, n string) 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 Batch Job Queue ID is set") + } + + conn := acctest.Provider.Meta().(*conns.AWSClient).BatchConn(ctx) + + previousARN := parseJobDefinitionPreviousARN(rs.Primary.ID) + + jobDefinition, err := tfbatch.FindJobDefinitionByARN(ctx, conn, previousARN) + + if err != nil { + return err + } + + if aws.StringValue(jobDefinition.Status) != "ACTIVE" { + return fmt.Errorf("Batch Job Definition %s is a previous revision that is not ACTIVE", previousARN) + } + + return nil + } +} + +func testAccCheckJobDefinitionPreviousDeregistered(ctx context.Context, n string) 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 Batch Job Queue ID is set") + } + + conn := acctest.Provider.Meta().(*conns.AWSClient).BatchConn(ctx) + + previousARN := parseJobDefinitionPreviousARN(rs.Primary.ID) + + _, err := tfbatch.FindJobDefinitionByARN(ctx, conn, previousARN) + + // FindJobDefinitionByARN returns an error if the job is INACTIVE (deregistered) + if err != nil { + if err.Error() == "INACTIVE" { + return nil + } + return err + } + + return fmt.Errorf("Batch Job Definition %s is a previous revision that is still ACTIVE", previousARN) + } +} + +func parseJobDefinitionPreviousARN(currentARN string) (previousARN string) { + re := regexache.MustCompile(`job-definition/.*?:(.*)`) + revisionCurrentStr := re.FindStringSubmatch(currentARN)[1] + + revisionCurrent, _ := strconv.Atoi(revisionCurrentStr) + revisionPrevious := revisionCurrent - 1 + + re = regexache.MustCompile(`^(arn:.*:batch:[a-z0-9-]+:[0-9]+:job-definition/[a-z0-9-]+):`) + arnPrefix := re.FindStringSubmatch(currentARN)[1] + previousARN = fmt.Sprintf("%s:%d", arnPrefix, revisionPrevious) + + return previousARN +} + func testAccCheckJobDefinitionAttributes(jd *batch.JobDefinition, compare *batch.JobDefinition) resource.TestCheckFunc { return func(s *terraform.State) error { for _, rs := range s.RootModule().Resources { @@ -757,15 +960,6 @@ func testAccCheckJobDefinitionAttributes(jd *batch.JobDefinition, compare *batch } } -func testAccCheckJobDefinitionRecreated(t *testing.T, before, after *batch.JobDefinition) resource.TestCheckFunc { - return func(s *terraform.State) error { - if aws.Int64Value(before.Revision) == aws.Int64Value(after.Revision) { - t.Fatalf("Expected change of JobDefinition Revisions, but both were %d", aws.Int64Value(before.Revision)) - } - return nil - } -} - func testAccCheckJobDefinitionDestroy(ctx context.Context) resource.TestCheckFunc { return func(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).BatchConn(ctx) @@ -775,9 +969,12 @@ func testAccCheckJobDefinitionDestroy(ctx context.Context) resource.TestCheckFun continue } - _, err := tfbatch.FindJobDefinitionByARN(ctx, conn, rs.Primary.ID) + re := regexache.MustCompile(`job-definition/(.*?):`) + name := re.FindStringSubmatch(rs.Primary.ID)[1] + + jds, err := tfbatch.ListActiveJobDefinitionByName(ctx, conn, name) - if tfresource.NotFound(err) { + if count := len(jds); count == 0 { continue } @@ -785,7 +982,7 @@ func testAccCheckJobDefinitionDestroy(ctx context.Context) resource.TestCheckFun return err } - return fmt.Errorf("Batch Job Definition %s still exists", rs.Primary.ID) + return fmt.Errorf("Batch Job Definition %s has revisions that still exist", name) } return nil } @@ -1449,3 +1646,30 @@ resource "aws_batch_job_definition" "test" { } `, rName, priority) } + +func testAccJobDefinitionConfig_attributes(rName string, sp int, pt bool, rsa int, timeout int, dereg bool) string { + return fmt.Sprintf(` +resource "aws_batch_job_definition" "test" { + name = %[1]q + type = "container" + scheduling_priority = %[2]d + propagate_tags = %[3]t + container_properties = jsonencode({ + command = ["echo", "test"] + image = "busybox" + memory = 128 + vcpus = 1 + }) + retry_strategy { + attempts = %[4]d + } + timeout { + attempt_duration_seconds = %[5]d + } + deregister_on_new_revision = %[6]t + platform_capabilities = [ + "EC2", + ] +} +`, rName, sp, pt, rsa, timeout, dereg) +} diff --git a/website/docs/r/batch_job_definition.html.markdown b/website/docs/r/batch_job_definition.html.markdown index c5204b0c7c0..be69314ffce 100644 --- a/website/docs/r/batch_job_definition.html.markdown +++ b/website/docs/r/batch_job_definition.html.markdown @@ -202,6 +202,7 @@ The following arguments are optional: * `container_properties` - (Optional) A valid [container properties](http://docs.aws.amazon.com/batch/latest/APIReference/API_RegisterJobDefinition.html) provided as a single valid JSON document. This parameter is only valid if the `type` parameter is `container`. +* `deregister_on_new_revision` - (Optional) When updating a job definition a new revision is created. This parameter determines if the previous version is `deregistered` (`INACTIVE`) or left `ACTIVE`. Defaults to `true`. * `node_properties` - (Optional) A valid [node properties](http://docs.aws.amazon.com/batch/latest/APIReference/API_RegisterJobDefinition.html) provided as a single valid JSON document. This parameter is required if the `type` parameter is `multinode`. * `eks_properties` - (Optional) A valid [eks properties](#eks_properties). This parameter is only valid if the `type` parameter is `container`. From 79cd35e88f2f3825b61c36c25a1a4ffdc2dfa92c Mon Sep 17 00:00:00 2001 From: Drew Mullen Date: Thu, 11 Jan 2024 09:43:02 -0500 Subject: [PATCH 03/12] rm ForceNew from a few other attributes --- internal/service/batch/job_definition.go | 7 ------- internal/service/batch/job_definition_test.go | 3 +++ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/internal/service/batch/job_definition.go b/internal/service/batch/job_definition.go index fbade0c5c9b..c0c43ab7035 100644 --- a/internal/service/batch/job_definition.go +++ b/internal/service/batch/job_definition.go @@ -354,7 +354,6 @@ func ResourceJobDefinition() *schema.Resource { "attempts": { Type: schema.TypeInt, Optional: true, - ForceNew: true, ValidateFunc: validation.IntBetween(1, 10), }, "evaluate_on_exit": { @@ -368,7 +367,6 @@ func ResourceJobDefinition() *schema.Resource { "action": { Type: schema.TypeString, Required: true, - ForceNew: true, StateFunc: func(v interface{}) string { return strings.ToLower(v.(string)) }, @@ -377,7 +375,6 @@ func ResourceJobDefinition() *schema.Resource { "on_exit_code": { Type: schema.TypeString, Optional: true, - ForceNew: true, ValidateFunc: validation.All( validation.StringLenBetween(1, 512), validation.StringMatch(regexache.MustCompile(`^[0-9]*\*?$`), "must contain only numbers, and can optionally end with an asterisk"), @@ -386,7 +383,6 @@ func ResourceJobDefinition() *schema.Resource { "on_reason": { Type: schema.TypeString, Optional: true, - ForceNew: true, ValidateFunc: validation.All( validation.StringLenBetween(1, 512), validation.StringMatch(regexache.MustCompile(`^[0-9A-Za-z.:\s]*\*?$`), "must contain letters, numbers, periods, colons, and white space, and can optionally end with an asterisk"), @@ -395,7 +391,6 @@ func ResourceJobDefinition() *schema.Resource { "on_status_reason": { Type: schema.TypeString, Optional: true, - ForceNew: true, ValidateFunc: validation.All( validation.StringLenBetween(1, 512), validation.StringMatch(regexache.MustCompile(`^[0-9A-Za-z.:\s]*\*?$`), "must contain letters, numbers, periods, colons, and white space, and can optionally end with an asterisk"), @@ -424,14 +419,12 @@ func ResourceJobDefinition() *schema.Resource { "timeout": { Type: schema.TypeList, Optional: true, - ForceNew: true, MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "attempt_duration_seconds": { Type: schema.TypeInt, Optional: true, - ForceNew: true, ValidateFunc: validation.IntAtLeast(60), }, }, diff --git a/internal/service/batch/job_definition_test.go b/internal/service/batch/job_definition_test.go index ce529c7243f..efd1cced445 100644 --- a/internal/service/batch/job_definition_test.go +++ b/internal/service/batch/job_definition_test.go @@ -827,6 +827,9 @@ func TestAccBatchJobDefinition_schedulingPriority(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "deregister_on_new_revision", + }, }, }, }) From 3ae3ac1432ae42642c3bde980a368e9df13d22a7 Mon Sep 17 00:00:00 2001 From: Drew Mullen Date: Thu, 11 Jan 2024 16:14:22 -0500 Subject: [PATCH 04/12] add tests for updating node and container properties --- internal/service/batch/job_definition_test.go | 54 ++++++++++++++++++- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/internal/service/batch/job_definition_test.go b/internal/service/batch/job_definition_test.go index efd1cced445..a96cc2a6851 100644 --- a/internal/service/batch/job_definition_test.go +++ b/internal/service/batch/job_definition_test.go @@ -447,6 +447,14 @@ func TestAccBatchJobDefinition_updateForcesNewResource(t *testing.T) { }, >>>>>>> 1c264545ba (initial remove of ForceNew) }, + { + Config: testAccJobDefinitionConfig_containerPropertiesAdvancedUpdate(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckJobDefinitionExists(ctx, resourceName, &jd), + testAccCheckJobDefinitionPreviousDeregistered(ctx, resourceName), + resource.TestCheckResourceAttr(resourceName, "revision", "2"), + ), + }, }, }) } @@ -694,6 +702,48 @@ func TestAccBatchJobDefinition_NodePropertiesupdateForcesNewResource(t *testing. }) } +func TestAccBatchJobDefinition_NodeProperties_advanced(t *testing.T) { + ctx := acctest.Context(t) + var jd batch.JobDefinition + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_batch_job_definition.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t); testAccPreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, batch.EndpointsID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckJobDefinitionDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccJobDefinitionConfig_nodePropertiesAdvanced(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckJobDefinitionExists(ctx, resourceName, &jd), + acctest.MatchResourceAttrRegionalARN(resourceName, "arn", "batch", regexache.MustCompile(fmt.Sprintf(`job-definition/%s:\d+`, rName))), + acctest.CheckResourceAttrEquivalentJSON(resourceName, "node_properties", `{"mainNode":1,"nodeRangeProperties":[{"container":{"command":["ls","-la"],"environment":[{"name":"VARNAME","value":"VARVAL"}],"image":"busybox","memory":512,"mountPoints":[{"containerPath":"/tmp","readOnly":false,"sourceVolume":"tmp"}],"resourceRequirements":[],"secrets":[],"ulimits":[{"hardLimit":1024,"name":"nofile","softLimit":1024}],"vcpus":1,"volumes":[{"host":{"sourcePath":"/tmp"},"name":"tmp"}]},"targetNodes":"0:"},{"container":{"command":["echo","test"],"environment":[],"image":"busybox","memory":128,"mountPoints":[],"resourceRequirements":[],"secrets":[],"ulimits":[],"vcpus":1,"volumes":[]},"targetNodes":"1:"}],"numNodes":4}`), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "deregister_on_new_revision", + }, + }, + { + Config: testAccJobDefinitionConfig_nodePropertiesAdvancedUpdate(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckJobDefinitionExists(ctx, resourceName, &jd), + acctest.MatchResourceAttrRegionalARN(resourceName, "arn", "batch", regexache.MustCompile(fmt.Sprintf(`job-definition/%s:\d+`, rName))), + acctest.CheckResourceAttrEquivalentJSON(resourceName, "node_properties", `{"mainNode":1,"nodeRangeProperties":[{"container":{"command":["ls","-la"],"environment":[],"image":"busybox","memory":512,"mountPoints":[],"resourceRequirements":[],"secrets":[],"ulimits":[],"vcpus":1,"volumes":[]},"targetNodes":"0:"},{"container":{"command":["echo","test"],"environment":[],"image":"busybox","memory":128,"mountPoints":[],"resourceRequirements":[],"secrets":[],"ulimits":[],"vcpus":1,"volumes":[]},"targetNodes":"1:"}],"numNodes":4}`), + testAccCheckJobDefinitionPreviousDeregistered(ctx, resourceName), + resource.TestCheckResourceAttr(resourceName, "revision", "2"), + ), + }, + }, + }) +} + func TestAccBatchJobDefinition_EKSProperties_basic(t *testing.T) { ctx := acctest.Context(t) var jd batch.JobDefinition @@ -1653,8 +1703,8 @@ resource "aws_batch_job_definition" "test" { func testAccJobDefinitionConfig_attributes(rName string, sp int, pt bool, rsa int, timeout int, dereg bool) string { return fmt.Sprintf(` resource "aws_batch_job_definition" "test" { - name = %[1]q - type = "container" + name = %[1]q + type = "container" scheduling_priority = %[2]d propagate_tags = %[3]t container_properties = jsonencode({ From 625de41d4b8c4eed12779fb018c5f596ae6860f8 Mon Sep 17 00:00:00 2001 From: Drew Mullen Date: Fri, 12 Jan 2024 08:32:04 -0500 Subject: [PATCH 05/12] batch changelog --- .changelog/35149.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/35149.txt diff --git a/.changelog/35149.txt b/.changelog/35149.txt new file mode 100644 index 00000000000..d03e8afebf9 --- /dev/null +++ b/.changelog/35149.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/aws_batch_job_definition: Add update functions instead of ForceNew. Add `deregister_on_new_revision` to allow keeping prior versions ACTIVE when a new revision is published. +``` \ No newline at end of file From 967e49061822c1afb90a16f092e7b46ffc6339c3 Mon Sep 17 00:00:00 2001 From: Drew Mullen Date: Tue, 20 Feb 2024 10:02:11 -0500 Subject: [PATCH 06/12] fix issues from rebase --- internal/service/batch/job_definition.go | 2 +- internal/service/batch/job_definition_data_source_test.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/service/batch/job_definition.go b/internal/service/batch/job_definition.go index c0c43ab7035..d482c9ea703 100644 --- a/internal/service/batch/job_definition.go +++ b/internal/service/batch/job_definition.go @@ -761,7 +761,7 @@ func FindJobDefinitionByARN(ctx context.Context, conn *batch.Batch, arn string) func ListActiveJobDefinitionByName(ctx context.Context, conn *batch.Batch, name string) ([]*batch.JobDefinition, error) { input := &batch.DescribeJobDefinitionsInput{ JobDefinitionName: aws.String(name), - Status: aws.String(JobDefinitionStatusActive), + Status: aws.String(jobDefinitionStatusActive), } output, err := conn.DescribeJobDefinitionsWithContext(ctx, input) diff --git a/internal/service/batch/job_definition_data_source_test.go b/internal/service/batch/job_definition_data_source_test.go index 075cc503c74..322c09be82c 100644 --- a/internal/service/batch/job_definition_data_source_test.go +++ b/internal/service/batch/job_definition_data_source_test.go @@ -66,7 +66,6 @@ func TestAccBatchJobDefinitionDataSource_basicARN(t *testing.T) { Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr(dataSourceName, "retry_strategy.0.attempts", "10"), resource.TestCheckResourceAttr(dataSourceName, "revision", "1"), - testAccCheckJobDefinitionV2Exists(ctx, dataSourceName, &jd), resource.TestCheckResourceAttr(dataSourceName, "revision", "1"), resource.TestCheckResourceAttr(dataSourceName, "retry_strategy.attempts", "10"), acctest.MatchResourceAttrRegionalARN(dataSourceName, "arn", "batch", regexache.MustCompile(fmt.Sprintf(`job-definition/%s:\d+`, rName))), From fa2ff1d56bfe2c11fed741628b4e32e3925c77ea Mon Sep 17 00:00:00 2001 From: Drew Mullen Date: Tue, 20 Feb 2024 10:16:38 -0500 Subject: [PATCH 07/12] golint --- internal/service/batch/findv2.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/batch/findv2.go b/internal/service/batch/findv2.go index 812ef300cd6..55314394123 100644 --- a/internal/service/batch/findv2.go +++ b/internal/service/batch/findv2.go @@ -46,7 +46,7 @@ func ListJobDefinitionsV2ByNameWithStatus(ctx context.Context, conn *batch.Clien out = append(out, page.JobDefinitions...) } - if out == nil || len(out) == 0 { + if len(out) == 0 { return nil, tfresource.NewEmptyResultError(input) } From 70eb4d1e62e302ec96679f977b1b5699516c8cd6 Mon Sep 17 00:00:00 2001 From: Drew Mullen Date: Mon, 25 Mar 2024 11:29:16 -0400 Subject: [PATCH 08/12] fix merge conflict --- internal/service/batch/job_definition_test.go | 76 ------------------- 1 file changed, 76 deletions(-) diff --git a/internal/service/batch/job_definition_test.go b/internal/service/batch/job_definition_test.go index a96cc2a6851..5b2e9f13bd6 100644 --- a/internal/service/batch/job_definition_test.go +++ b/internal/service/batch/job_definition_test.go @@ -405,47 +405,9 @@ func TestAccBatchJobDefinition_ContainerProperties_advanced(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, -<<<<<<< HEAD - }, - }, - }) -} - -func TestAccBatchJobDefinition_updateForcesNewResource(t *testing.T) { - ctx := acctest.Context(t) - var before, after batch.JobDefinition - rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - resourceName := "aws_batch_job_definition.test" - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(ctx, t); testAccPreCheck(ctx, t) }, - ErrorCheck: acctest.ErrorCheck(t, names.BatchServiceID), - ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: testAccCheckJobDefinitionDestroy(ctx), - Steps: []resource.TestStep{ - { - Config: testAccJobDefinitionConfig_containerPropertiesAdvanced(rName), - Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckJobDefinitionExists(ctx, resourceName, &before), - testAccCheckJobDefinitionAttributes(&before, nil), - ), - }, - { - Config: testAccJobDefinitionConfig_containerPropertiesAdvancedUpdate(rName), - Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckJobDefinitionExists(ctx, resourceName, &after), - testAccCheckJobDefinitionRecreated(t, &before, &after), - ), - }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, -======= ImportStateVerifyIgnore: []string{ "deregister_on_new_revision", }, ->>>>>>> 1c264545ba (initial remove of ForceNew) }, { Config: testAccJobDefinitionConfig_containerPropertiesAdvancedUpdate(rName), @@ -656,47 +618,9 @@ func TestAccBatchJobDefinition_NodeProperties_basic(t *testing.T) { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, -<<<<<<< HEAD - }, - }, - }) -} - -func TestAccBatchJobDefinition_NodePropertiesupdateForcesNewResource(t *testing.T) { - ctx := acctest.Context(t) - var before, after batch.JobDefinition - rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - resourceName := "aws_batch_job_definition.test" - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(ctx, t); testAccPreCheck(ctx, t) }, - ErrorCheck: acctest.ErrorCheck(t, names.BatchServiceID), - ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: testAccCheckJobDefinitionDestroy(ctx), - Steps: []resource.TestStep{ - { - Config: testAccJobDefinitionConfig_nodePropertiesAdvanced(rName), - Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckJobDefinitionExists(ctx, resourceName, &before), - testAccCheckJobDefinitionAttributes(&before, nil), - ), - }, - { - Config: testAccJobDefinitionConfig_nodePropertiesAdvancedUpdate(rName), - Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckJobDefinitionExists(ctx, resourceName, &after), - testAccCheckJobDefinitionRecreated(t, &before, &after), - ), - }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, -======= ImportStateVerifyIgnore: []string{ "deregister_on_new_revision", }, ->>>>>>> 1c264545ba (initial remove of ForceNew) }, }, }) From 78b8665438c75dea40ed75bd3f397d58e797ef3f Mon Sep 17 00:00:00 2001 From: Drew Mullen Date: Mon, 25 Mar 2024 11:29:51 -0400 Subject: [PATCH 09/12] fix merge conflict --- internal/service/batch/job_definition_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/internal/service/batch/job_definition_test.go b/internal/service/batch/job_definition_test.go index 5b2e9f13bd6..f71c453c4ec 100644 --- a/internal/service/batch/job_definition_test.go +++ b/internal/service/batch/job_definition_test.go @@ -20,11 +20,7 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" tfbatch "github.com/hashicorp/terraform-provider-aws/internal/service/batch" -<<<<<<< HEAD - "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/names" -======= ->>>>>>> 1c264545ba (initial remove of ForceNew) ) func TestAccBatchJobDefinition_basic(t *testing.T) { From 2e1e21aceb5a9d2dd10bae26b9c265931c5f4bbd Mon Sep 17 00:00:00 2001 From: Drew Mullen Date: Mon, 25 Mar 2024 11:48:04 -0400 Subject: [PATCH 10/12] fix merge conflict --- internal/service/batch/job_definition_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/service/batch/job_definition_test.go b/internal/service/batch/job_definition_test.go index f71c453c4ec..16ce8405ae5 100644 --- a/internal/service/batch/job_definition_test.go +++ b/internal/service/batch/job_definition_test.go @@ -84,7 +84,7 @@ func TestAccBatchJobDefinition_attributes(t *testing.T) { resourceName := "aws_batch_job_definition.test" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t); testAccPreCheck(ctx, t) }, - ErrorCheck: acctest.ErrorCheck(t, batch.EndpointsID), + ErrorCheck: acctest.ErrorCheck(t, names.BatchServiceID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, CheckDestroy: testAccCheckJobDefinitionDestroy(ctx), Steps: []resource.TestStep{ @@ -630,7 +630,7 @@ func TestAccBatchJobDefinition_NodeProperties_advanced(t *testing.T) { resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t); testAccPreCheck(ctx, t) }, - ErrorCheck: acctest.ErrorCheck(t, batch.EndpointsID), + ErrorCheck: acctest.ErrorCheck(t, names.BatchServiceID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, CheckDestroy: testAccCheckJobDefinitionDestroy(ctx), Steps: []resource.TestStep{ From 72e56b50dc858014ad426c60291eb9cba0be3795 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Tue, 2 Apr 2024 11:45:00 -0700 Subject: [PATCH 11/12] Reformats expected JSON for readability --- internal/service/batch/job_definition_test.go | 74 ++++++++++++++++++- 1 file changed, 72 insertions(+), 2 deletions(-) diff --git a/internal/service/batch/job_definition_test.go b/internal/service/batch/job_definition_test.go index 16ce8405ae5..41ba924dd96 100644 --- a/internal/service/batch/job_definition_test.go +++ b/internal/service/batch/job_definition_test.go @@ -639,7 +639,42 @@ func TestAccBatchJobDefinition_NodeProperties_advanced(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( testAccCheckJobDefinitionExists(ctx, resourceName, &jd), acctest.MatchResourceAttrRegionalARN(resourceName, "arn", "batch", regexache.MustCompile(fmt.Sprintf(`job-definition/%s:\d+`, rName))), - acctest.CheckResourceAttrEquivalentJSON(resourceName, "node_properties", `{"mainNode":1,"nodeRangeProperties":[{"container":{"command":["ls","-la"],"environment":[{"name":"VARNAME","value":"VARVAL"}],"image":"busybox","memory":512,"mountPoints":[{"containerPath":"/tmp","readOnly":false,"sourceVolume":"tmp"}],"resourceRequirements":[],"secrets":[],"ulimits":[{"hardLimit":1024,"name":"nofile","softLimit":1024}],"vcpus":1,"volumes":[{"host":{"sourcePath":"/tmp"},"name":"tmp"}]},"targetNodes":"0:"},{"container":{"command":["echo","test"],"environment":[],"image":"busybox","memory":128,"mountPoints":[],"resourceRequirements":[],"secrets":[],"ulimits":[],"vcpus":1,"volumes":[]},"targetNodes":"1:"}],"numNodes":4}`), + acctest.CheckResourceAttrEquivalentJSON(resourceName, "node_properties", `{ + "mainNode": 1, + "nodeRangeProperties": [ + { + "container": { + "command": ["ls","-la"], + "environment": [{"name":"VARNAME","value":"VARVAL"}], + "image": "busybox", + "memory": 512, + "mountPoints": [{"containerPath":"/tmp","readOnly":false,"sourceVolume":"tmp"}], + "resourceRequirements": [], + "secrets": [], + "ulimits": [{"hardLimit":1024,"name":"nofile","softLimit":1024}], + "vcpus": 1, + "volumes": [{"host":{"sourcePath":"/tmp"},"name":"tmp"}] + }, + "targetNodes": "0:" + }, + { + "container": { + "command": ["echo","test"], + "environment": [], + "image": "busybox", + "memory": 128, + "mountPoints": [], + "resourceRequirements": [], + "secrets": [], + "ulimits": [], + "vcpus":1, + "volumes": [] + }, + "targetNodes": "1:" + } + ], + "numNodes":4 + }`), ), }, { @@ -655,7 +690,42 @@ func TestAccBatchJobDefinition_NodeProperties_advanced(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( testAccCheckJobDefinitionExists(ctx, resourceName, &jd), acctest.MatchResourceAttrRegionalARN(resourceName, "arn", "batch", regexache.MustCompile(fmt.Sprintf(`job-definition/%s:\d+`, rName))), - acctest.CheckResourceAttrEquivalentJSON(resourceName, "node_properties", `{"mainNode":1,"nodeRangeProperties":[{"container":{"command":["ls","-la"],"environment":[],"image":"busybox","memory":512,"mountPoints":[],"resourceRequirements":[],"secrets":[],"ulimits":[],"vcpus":1,"volumes":[]},"targetNodes":"0:"},{"container":{"command":["echo","test"],"environment":[],"image":"busybox","memory":128,"mountPoints":[],"resourceRequirements":[],"secrets":[],"ulimits":[],"vcpus":1,"volumes":[]},"targetNodes":"1:"}],"numNodes":4}`), + acctest.CheckResourceAttrEquivalentJSON(resourceName, "node_properties", `{ + "mainNode": 1, + "nodeRangeProperties": [ + { + "container": { + "command": ["ls","-la"], + "environment": [], + "image": "busybox", + "memory": 512, + "mountPoints": [], + "resourceRequirements": [], + "secrets": [], + "ulimits": [], + "vcpus": 1, + "volumes": [] + }, + "targetNodes": "0:" + }, + { + "container": { + "command": ["echo","test"], + "environment": [], + "image": "busybox", + "memory": 128, + "mountPoints": [], + "resourceRequirements": [], + "secrets": [], + "ulimits": [], + "vcpus": 1, + "volumes": [] + }, + "targetNodes": "1:" + } + ], + "numNodes":4 + }`), testAccCheckJobDefinitionPreviousDeregistered(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "revision", "2"), ), From a5dd866c7cf812673a38df36f2f1cb0ef6948304 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Tue, 2 Apr 2024 11:47:51 -0700 Subject: [PATCH 12/12] Adds `instanceTypes` parameter to `node_properties` JSON --- internal/service/batch/job_definition_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/service/batch/job_definition_test.go b/internal/service/batch/job_definition_test.go index 41ba924dd96..88c99e4091d 100644 --- a/internal/service/batch/job_definition_test.go +++ b/internal/service/batch/job_definition_test.go @@ -578,6 +578,7 @@ func TestAccBatchJobDefinition_NodeProperties_basic(t *testing.T) { "vcpus": 1, "volumes": [] }, + "instanceTypes": [], "targetNodes": "0:" }, { @@ -593,6 +594,7 @@ func TestAccBatchJobDefinition_NodeProperties_basic(t *testing.T) { "vcpus": 1, "volumes": [] }, + "instanceTypes": [], "targetNodes": "1:" } ], @@ -655,6 +657,7 @@ func TestAccBatchJobDefinition_NodeProperties_advanced(t *testing.T) { "vcpus": 1, "volumes": [{"host":{"sourcePath":"/tmp"},"name":"tmp"}] }, + "instanceTypes": [], "targetNodes": "0:" }, { @@ -670,6 +673,7 @@ func TestAccBatchJobDefinition_NodeProperties_advanced(t *testing.T) { "vcpus":1, "volumes": [] }, + "instanceTypes": [], "targetNodes": "1:" } ], @@ -706,6 +710,7 @@ func TestAccBatchJobDefinition_NodeProperties_advanced(t *testing.T) { "vcpus": 1, "volumes": [] }, + "instanceTypes": [], "targetNodes": "0:" }, { @@ -721,6 +726,7 @@ func TestAccBatchJobDefinition_NodeProperties_advanced(t *testing.T) { "vcpus": 1, "volumes": [] }, + "instanceTypes": [], "targetNodes": "1:" } ],