Skip to content

Commit

Permalink
resource/aws_batch_job_definition: Prevent extraneous differences wit…
Browse files Browse the repository at this point in the history 
…h container properties missing environment, mount point, ulimits, and volumes configuration

Reference: #11998
Reference: #11488

Now that this resource is properly refreshing the `container_properties` attribute into the Terraform state, additional cases have been reported where the API canonicalizes the response with empty arrays. Here we account for `environment`, `mountPoints`, `ulimits`, and `volumes`.

Previous output from unit testing (before code update):

```
2020/02/11 12:37:58 [DEBUG] Canonical Batch Container Properties JSON are not equal.
First: {"command":["start.py","Ref::S3bucket","Ref::S3key"],"image":"example:image","jobRoleArn":"arn:aws:iam::123456789012:role/example","memory":2048,"vcpus":8}
Second: {"command":["start.py","Ref::S3bucket","Ref::S3key"],"environment":[],"image":"example:image","jobRoleArn":"arn:aws:iam::123456789012:role/example","memory":2048,"mountPoints":[],"ulimits":[],"vcpus":8,"volumes":[]}
--- FAIL: TestEquivalentBatchContainerPropertiesJSON (0.00s)
    --- FAIL: TestEquivalentBatchContainerPropertiesJSON/empty_environment,_mountPoints,_ulimits,_and_volumes (0.00s)
        container_properties_test.go:226: got false, expected true
```

Previous output from acceptance testing (before code update):

```
--- FAIL: TestAccAWSBatchJobDefinition_basic (13.16s)
    testing.go:640: Step 0 error: After applying this step, the plan was not empty:

        DIFF:

        DESTROY/CREATE: aws_batch_job_definition.test
          arn:                  "arn:aws:batch:us-west-2:--OMITTED--:job-definition/tf-acc-test-118932874341187373:1" => "<computed>"
          container_properties: "{\"command\":[\"echo\",\"test\"],\"environment\":[],\"image\":\"busybox\",\"memory\":128,\"mountPoints\":[],\"resourceRequirements\":[],\"ulimits\":[],\"vcpus\":1,\"volumes\":[]}" => "{\"command\":[\"echo\",\"test\"],\"image\":\"busybox\",\"memory\":128,\"vcpus\":1}" (forces new resource)
          id:                   "arn:aws:batch:us-west-2:--OMITTED--:job-definition/tf-acc-test-118932874341187373:1" => "<computed>"
          name:                 "tf-acc-test-118932874341187373" => "tf-acc-test-118932874341187373"
          retry_strategy.#:     "0" => "0"
          revision:             "1" => "<computed>"
          timeout.#:            "0" => "0"
          type:                 "container" => "container"
```

Output from acceptance testing:

```
--- PASS: TestAccAWSBatchJobDefinition_basic (16.80s)
--- PASS: TestAccAWSBatchJobDefinition_ContainerProperties_Advanced (16.84s)
--- PASS: TestAccAWSBatchJobDefinition_updateForcesNewResource (27.13s)
```
  • Loading branch information
@bflad
bflad committed Feb 11, 2020
1 parent d084902 commit 0711498
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 22 deletions.
16 changes: 16 additions & 0 deletions aws/internal/service/batch/equivalency/container_properties.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,26 @@ func (cp *containerProperties) Reduce() error {
return aws.StringValue(cp.Environment[i].Name) < aws.StringValue(cp.Environment[j].Name)
})

if len(cp.Environment) == 0 {
cp.Environment = nil
}

if len(cp.MountPoints) == 0 {
cp.MountPoints = nil
}

if len(cp.ResourceRequirements) == 0 {
cp.ResourceRequirements = nil
}

if len(cp.Ulimits) == 0 {
cp.Ulimits = nil
}

if len(cp.Volumes) == 0 {
cp.Volumes = nil
}

return nil
}

Expand Down
27 changes: 27 additions & 0 deletions aws/internal/service/batch/equivalency/container_properties_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,33 @@ func TestEquivalentBatchContainerPropertiesJSON(t *testing.T) {
}
]
}
`,
ExpectEquivalent: true,
},
{
Name: "empty environment, mountPoints, ulimits, and volumes",
ApiJson: `
{
"image": "example:image",
"vcpus": 8,
"memory": 2048,
"command": ["start.py", "Ref::S3bucket", "Ref::S3key"],
"jobRoleArn": "arn:aws:iam::123456789012:role/example",
"volumes": [],
"environment": [],
"mountPoints": [],
"ulimits": [],
"resourceRequirements": []
}
`,
ConfigurationJson: `
{
"command": ["start.py", "Ref::S3bucket", "Ref::S3key"],
"image": "example:image",
"memory": 2048,
"vcpus": 8,
"jobRoleArn": "arn:aws:iam::123456789012:role/example"
}
`,
ExpectEquivalent: true,
},
Expand Down
81 changes: 59 additions & 22 deletions aws/resource_aws_batch_job_definition_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ package aws

import (
"fmt"
"strings"
"testing"

"reflect"
"testing"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/batch"
Expand All @@ -15,6 +13,31 @@ import (
)

func TestAccAWSBatchJobDefinition_basic(t *testing.T) {
var jd batch.JobDefinition
rName := acctest.RandomWithPrefix("tf-acc-test")
resourceName := "aws_batch_job_definition.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSBatch(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckBatchJobDefinitionDestroy,
Steps: []resource.TestStep{
{
Config: testAccBatchJobDefinitionConfigName(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckBatchJobDefinitionExists(resourceName, &jd),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func TestAccAWSBatchJobDefinition_ContainerProperties_Advanced(t *testing.T) {
var jd batch.JobDefinition
compare := batch.JobDefinition{
Parameters: map[string]*string{
Expand Down Expand Up @@ -50,16 +73,16 @@ func TestAccAWSBatchJobDefinition_basic(t *testing.T) {
},
},
}
ri := acctest.RandInt()
config := fmt.Sprintf(testAccBatchJobDefinitionBaseConfig, ri)
rName := acctest.RandomWithPrefix("tf-acc-test")
resourceName := "aws_batch_job_definition.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSBatch(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckBatchJobDefinitionDestroy,
Steps: []resource.TestStep{
{
Config: config,
Config: testAccBatchJobDefinitionConfigContainerPropertiesAdvanced(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckBatchJobDefinitionExists(resourceName, &jd),
testAccCheckBatchJobDefinitionAttributes(&jd, &compare),
Expand All @@ -75,26 +98,24 @@ func TestAccAWSBatchJobDefinition_basic(t *testing.T) {
}

func TestAccAWSBatchJobDefinition_updateForcesNewResource(t *testing.T) {
var before batch.JobDefinition
var after batch.JobDefinition
ri := acctest.RandInt()
config := fmt.Sprintf(testAccBatchJobDefinitionBaseConfig, ri)
updateConfig := fmt.Sprintf(testAccBatchJobDefinitionUpdateConfig, ri)
var before, after batch.JobDefinition
rName := acctest.RandomWithPrefix("tf-acc-test")
resourceName := "aws_batch_job_definition.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSBatch(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckBatchJobDefinitionDestroy,
Steps: []resource.TestStep{
{
Config: config,
Config: testAccBatchJobDefinitionConfigContainerPropertiesAdvanced(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckBatchJobDefinitionExists(resourceName, &before),
testAccCheckBatchJobDefinitionAttributes(&before, nil),
),
},
{
Config: updateConfig,
Config: testAccBatchJobDefinitionConfigContainerPropertiesAdvancedUpdate(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckBatchJobDefinitionExists(resourceName, &after),
testAccCheckJobDefinitionRecreated(t, &before, &after),
Expand Down Expand Up @@ -137,9 +158,6 @@ func testAccCheckBatchJobDefinitionExists(n string, jd *batch.JobDefinition) res

func testAccCheckBatchJobDefinitionAttributes(jd *batch.JobDefinition, compare *batch.JobDefinition) resource.TestCheckFunc {
return func(s *terraform.State) error {
if !strings.HasPrefix(*jd.JobDefinitionName, "tf_acctest_batch_job_definition") {
return fmt.Errorf("Bad Job Definition name: %s", *jd.JobDefinitionName)
}
for _, rs := range s.RootModule().Resources {
if rs.Type != "aws_batch_job_definition" {
continue
Expand Down Expand Up @@ -190,9 +208,10 @@ func testAccCheckBatchJobDefinitionDestroy(s *terraform.State) error {
return nil
}

const testAccBatchJobDefinitionBaseConfig = `
func testAccBatchJobDefinitionConfigContainerPropertiesAdvanced(rName string) string {
return fmt.Sprintf(`
resource "aws_batch_job_definition" "test" {
name = "tf_acctest_batch_job_definition_%[1]d"
name = %[1]q
type = "container"
parameters = {
param1 = "val1"
Expand Down Expand Up @@ -238,11 +257,13 @@ resource "aws_batch_job_definition" "test" {
}
CONTAINER_PROPERTIES
}
`
`, rName)
}

const testAccBatchJobDefinitionUpdateConfig = `
func testAccBatchJobDefinitionConfigContainerPropertiesAdvancedUpdate(rName string) string {
return fmt.Sprintf(`
resource "aws_batch_job_definition" "test" {
name = "tf_acctest_batch_job_definition_%[1]d"
name = %[1]q
type = "container"
container_properties = <<CONTAINER_PROPERTIES
{
Expand Down Expand Up @@ -278,4 +299,20 @@ resource "aws_batch_job_definition" "test" {
}
CONTAINER_PROPERTIES
}
`
`, rName)
}

func testAccBatchJobDefinitionConfigName(rName string) string {
return fmt.Sprintf(`
resource "aws_batch_job_definition" "test" {
container_properties = jsonencode({
command = ["echo", "test"]
image = "busybox"
memory = 128
vcpus = 1
})
name = %[1]q
type = "container"
}
`, rName)
}

0 comments on commit 0711498

Please sign in to comment.