From 9ef9501e654a23faf4b4787db2f17748168db53c Mon Sep 17 00:00:00 2001 From: Clint Date: Wed, 12 Apr 2017 14:19:38 -0500 Subject: [PATCH] provider/aws: Fix EMR Bootstrap Action Ordering (#13580) * provider/aws: Add failing test for EMR Bootstrap Actions * aws_emr_cluster: Fix bootstrap action parameter ordering * provider/aws: Fix EMR Bootstrap arguments * provider/aws: Args needs to be ForceNew, because we can't update them --- .../providers/aws/resource_aws_emr_cluster.go | 32 +- .../aws/resource_aws_emr_cluster_test.go | 407 +++++++++++++++++- helper/acctest/random.go | 7 + 3 files changed, 430 insertions(+), 16 deletions(-) diff --git a/builtin/providers/aws/resource_aws_emr_cluster.go b/builtin/providers/aws/resource_aws_emr_cluster.go index 9217d0ed7..62b138505 100644 --- a/builtin/providers/aws/resource_aws_emr_cluster.go +++ b/builtin/providers/aws/resource_aws_emr_cluster.go @@ -138,10 +138,10 @@ func resourceAwsEMRCluster() *schema.Resource { Required: true, }, "args": { - Type: schema.TypeSet, + Type: schema.TypeList, Optional: true, + ForceNew: true, Elem: &schema.Schema{Type: schema.TypeString}, - Set: schema.HashString, }, }, }, @@ -381,6 +381,18 @@ func resourceAwsEMRClusterRead(d *schema.ResourceData, meta interface{}) error { if err := d.Set("ec2_attributes", flattenEc2Attributes(cluster.Ec2InstanceAttributes)); err != nil { log.Printf("[ERR] Error setting EMR Ec2 Attributes: %s", err) } + + respBootstraps, err := emrconn.ListBootstrapActions(&emr.ListBootstrapActionsInput{ + ClusterId: cluster.Id, + }) + if err != nil { + log.Printf("[WARN] Error listing bootstrap actions: %s", err) + } + + if err := d.Set("bootstrap_action", flattenBootstrapArguments(respBootstraps.BootstrapActions)); err != nil { + log.Printf("[WARN] Error setting Bootstrap Actions: %s", err) + } + return nil } @@ -589,6 +601,20 @@ func flattenEc2Attributes(ia *emr.Ec2InstanceAttributes) []map[string]interface{ return result } +func flattenBootstrapArguments(actions []*emr.Command) []map[string]interface{} { + result := make([]map[string]interface{}, 0) + + for _, b := range actions { + attrs := make(map[string]interface{}) + attrs["name"] = *b.Name + attrs["path"] = *b.ScriptPath + attrs["args"] = flattenStringList(b.Args) + result = append(result, attrs) + } + + return result +} + func loadGroups(d *schema.ResourceData, meta interface{}) ([]*emr.InstanceGroup, error) { emrconn := meta.(*AWSClient).emrconn reqGrps := &emr.ListInstanceGroupsInput{ @@ -699,7 +725,7 @@ func expandBootstrapActions(bootstrapActions []interface{}) []*emr.BootstrapActi actionAttributes := raw.(map[string]interface{}) actionName := actionAttributes["name"].(string) actionPath := actionAttributes["path"].(string) - actionArgs := actionAttributes["args"].(*schema.Set).List() + actionArgs := actionAttributes["args"].([]interface{}) action := &emr.BootstrapActionConfig{ Name: aws.String(actionName), diff --git a/builtin/providers/aws/resource_aws_emr_cluster_test.go b/builtin/providers/aws/resource_aws_emr_cluster_test.go index 2760e8e76..688c86f3f 100644 --- a/builtin/providers/aws/resource_aws_emr_cluster_test.go +++ b/builtin/providers/aws/resource_aws_emr_cluster_test.go @@ -3,6 +3,7 @@ package aws import ( "fmt" "log" + "reflect" "testing" "github.com/aws/aws-sdk-go/aws" @@ -14,7 +15,7 @@ import ( ) func TestAccAWSEMRCluster_basic(t *testing.T) { - var jobFlow emr.RunJobFlowOutput + var cluster emr.Cluster r := acctest.RandInt() resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -23,14 +24,51 @@ func TestAccAWSEMRCluster_basic(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccAWSEmrClusterConfig(r), - Check: testAccCheckAWSEmrClusterExists("aws_emr_cluster.tf-test-cluster", &jobFlow), + Check: testAccCheckAWSEmrClusterExists("aws_emr_cluster.tf-test-cluster", &cluster), + }, + }, + }) +} + +func TestAccAWSEMRCluster_bootstrap_ordering(t *testing.T) { + var cluster emr.Cluster + rName := acctest.RandomWithPrefix("tf-emr-bootstrap") + argsInts := []string{ + "1", + "2", + "3", + "4", + "5", + "6", + "7", + "8", + "9", + "10", + } + + argsStrings := []string{ + "instance.isMaster=true", + "echo running on master node", + } + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSEmrDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSEmrClusterConfig_bootstrap(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSEmrClusterExists("aws_emr_cluster.test", &cluster), + testAccCheck_bootstrap_order(&cluster, argsInts, argsStrings), + ), }, }, }) } func TestAccAWSEMRCluster_terminationProtected(t *testing.T) { - var jobFlow emr.RunJobFlowOutput + var cluster emr.Cluster r := acctest.RandInt() resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -40,7 +78,7 @@ func TestAccAWSEMRCluster_terminationProtected(t *testing.T) { { Config: testAccAWSEmrClusterConfig(r), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSEmrClusterExists("aws_emr_cluster.tf-test-cluster", &jobFlow), + testAccCheckAWSEmrClusterExists("aws_emr_cluster.tf-test-cluster", &cluster), resource.TestCheckResourceAttr( "aws_emr_cluster.tf-test-cluster", "termination_protection", "false"), ), @@ -48,7 +86,7 @@ func TestAccAWSEMRCluster_terminationProtected(t *testing.T) { { Config: testAccAWSEmrClusterConfigTerminationPolicyUpdated(r), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSEmrClusterExists("aws_emr_cluster.tf-test-cluster", &jobFlow), + testAccCheckAWSEmrClusterExists("aws_emr_cluster.tf-test-cluster", &cluster), resource.TestCheckResourceAttr( "aws_emr_cluster.tf-test-cluster", "termination_protection", "true"), ), @@ -57,7 +95,7 @@ func TestAccAWSEMRCluster_terminationProtected(t *testing.T) { //Need to turn off termination_protection to allow the job to be deleted Config: testAccAWSEmrClusterConfig(r), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSEmrClusterExists("aws_emr_cluster.tf-test-cluster", &jobFlow), + testAccCheckAWSEmrClusterExists("aws_emr_cluster.tf-test-cluster", &cluster), ), }, }, @@ -65,7 +103,7 @@ func TestAccAWSEMRCluster_terminationProtected(t *testing.T) { } func TestAccAWSEMRCluster_visibleToAllUsers(t *testing.T) { - var jobFlow emr.RunJobFlowOutput + var cluster emr.Cluster r := acctest.RandInt() resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -75,7 +113,7 @@ func TestAccAWSEMRCluster_visibleToAllUsers(t *testing.T) { { Config: testAccAWSEmrClusterConfig(r), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSEmrClusterExists("aws_emr_cluster.tf-test-cluster", &jobFlow), + testAccCheckAWSEmrClusterExists("aws_emr_cluster.tf-test-cluster", &cluster), resource.TestCheckResourceAttr( "aws_emr_cluster.tf-test-cluster", "visible_to_all_users", "true"), ), @@ -83,7 +121,7 @@ func TestAccAWSEMRCluster_visibleToAllUsers(t *testing.T) { { Config: testAccAWSEmrClusterConfigVisibleToAllUsersUpdated(r), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSEmrClusterExists("aws_emr_cluster.tf-test-cluster", &jobFlow), + testAccCheckAWSEmrClusterExists("aws_emr_cluster.tf-test-cluster", &cluster), resource.TestCheckResourceAttr( "aws_emr_cluster.tf-test-cluster", "visible_to_all_users", "false"), ), @@ -93,7 +131,7 @@ func TestAccAWSEMRCluster_visibleToAllUsers(t *testing.T) { } func TestAccAWSEMRCluster_tags(t *testing.T) { - var jobFlow emr.RunJobFlowOutput + var cluster emr.Cluster r := acctest.RandInt() resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -103,7 +141,7 @@ func TestAccAWSEMRCluster_tags(t *testing.T) { { Config: testAccAWSEmrClusterConfig(r), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSEmrClusterExists("aws_emr_cluster.tf-test-cluster", &jobFlow), + testAccCheckAWSEmrClusterExists("aws_emr_cluster.tf-test-cluster", &cluster), resource.TestCheckResourceAttr("aws_emr_cluster.tf-test-cluster", "tags.%", "4"), resource.TestCheckResourceAttr( "aws_emr_cluster.tf-test-cluster", "tags.role", "rolename"), @@ -117,7 +155,7 @@ func TestAccAWSEMRCluster_tags(t *testing.T) { { Config: testAccAWSEmrClusterConfigUpdatedTags(r), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSEmrClusterExists("aws_emr_cluster.tf-test-cluster", &jobFlow), + testAccCheckAWSEmrClusterExists("aws_emr_cluster.tf-test-cluster", &cluster), resource.TestCheckResourceAttr("aws_emr_cluster.tf-test-cluster", "tags.%", "3"), resource.TestCheckResourceAttr( "aws_emr_cluster.tf-test-cluster", "tags.dns_zone", "new_zone"), @@ -131,6 +169,45 @@ func TestAccAWSEMRCluster_tags(t *testing.T) { }) } +func testAccCheck_bootstrap_order(cluster *emr.Cluster, argsInts, argsStrings []string) resource.TestCheckFunc { + return func(s *terraform.State) error { + + emrconn := testAccProvider.Meta().(*AWSClient).emrconn + req := emr.ListBootstrapActionsInput{ + ClusterId: cluster.Id, + } + + resp, err := emrconn.ListBootstrapActions(&req) + if err != nil { + return fmt.Errorf("[ERR] Error listing boostrap actions in test: %s", err) + } + + // make sure we actually checked something + var ran bool + for _, ba := range resp.BootstrapActions { + // assume name matches the config + rArgs := aws.StringValueSlice(ba.Args) + if *ba.Name == "test" { + ran = true + if !reflect.DeepEqual(argsInts, rArgs) { + return fmt.Errorf("Error matching Bootstrap args:\n\texpected: %#v\n\tgot: %#v", argsInts, rArgs) + } + } else if *ba.Name == "runif" { + ran = true + if !reflect.DeepEqual(argsStrings, rArgs) { + return fmt.Errorf("Error matching Bootstrap args:\n\texpected: %#v\n\tgot: %#v", argsStrings, rArgs) + } + } + } + + if !ran { + return fmt.Errorf("Expected to compare bootstrap actions, but no checks were ran") + } + + return nil + } +} + func testAccCheckAWSEmrDestroy(s *terraform.State) error { conn := testAccProvider.Meta().(*AWSClient).emrconn @@ -163,7 +240,7 @@ func testAccCheckAWSEmrDestroy(s *terraform.State) error { return nil } -func testAccCheckAWSEmrClusterExists(n string, v *emr.RunJobFlowOutput) resource.TestCheckFunc { +func testAccCheckAWSEmrClusterExists(n string, v *emr.Cluster) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { @@ -185,6 +262,8 @@ func testAccCheckAWSEmrClusterExists(n string, v *emr.RunJobFlowOutput) resource return fmt.Errorf("EMR cluser not found") } + *v = *describe.Cluster + if describe.Cluster != nil && *describe.Cluster.Status.State != "WAITING" { return fmt.Errorf("EMR cluser is not up yet") @@ -194,6 +273,308 @@ func testAccCheckAWSEmrClusterExists(n string, v *emr.RunJobFlowOutput) resource } } +func testAccAWSEmrClusterConfig_bootstrap(r string) string { + return fmt.Sprintf(` +resource "aws_emr_cluster" "test" { + count = 1 + name = "%s" + release_label = "emr-5.0.0" + applications = ["Hadoop", "Hive"] + log_uri = "s3n://terraform/testlog/" + master_instance_type = "m4.large" + core_instance_type = "m1.small" + core_instance_count = 1 + service_role = "${aws_iam_role.iam_emr_default_role.arn}" + + depends_on = ["aws_main_route_table_association.a"] + + ec2_attributes { + subnet_id = "${aws_subnet.main.id}" + + emr_managed_master_security_group = "${aws_security_group.allow_all.id}" + emr_managed_slave_security_group = "${aws_security_group.allow_all.id}" + instance_profile = "${aws_iam_instance_profile.emr_profile.arn}" + } + + bootstrap_action { + path = "s3://elasticmapreduce/bootstrap-actions/run-if" + name = "runif" + args = ["instance.isMaster=true", "echo running on master node"] + } + + bootstrap_action = [ + { + path = "s3://${aws_s3_bucket.tester.bucket}/testscript.sh" + name = "test" + + args = ["1", + "2", + "3", + "4", + "5", + "6", + "7", + "8", + "9", + "10", + ] + }, + ] +} + +resource "aws_iam_instance_profile" "emr_profile" { + name = "%s_profile" + role = "${aws_iam_role.iam_emr_profile_role.name}" +} + +resource "aws_iam_role" "iam_emr_default_role" { + name = "%s_default_role" + + assume_role_policy = <