From 4fc98275b00e45501981c449b3d92daeba7ff5d8 Mon Sep 17 00:00:00 2001 From: Stefan Sundin Date: Tue, 8 May 2018 18:26:52 -0700 Subject: [PATCH 01/20] Add Importer to aws_emr_cluster. --- aws/resource_aws_emr_cluster.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/aws/resource_aws_emr_cluster.go b/aws/resource_aws_emr_cluster.go index 06e380b8bbdd..c82c582d9cf3 100644 --- a/aws/resource_aws_emr_cluster.go +++ b/aws/resource_aws_emr_cluster.go @@ -26,6 +26,10 @@ func resourceAwsEMRCluster() *schema.Resource { Read: resourceAwsEMRClusterRead, Update: resourceAwsEMRClusterUpdate, Delete: resourceAwsEMRClusterDelete, + Importer: &schema.ResourceImporter{ + State: schema.ImportStatePassthrough, + }, + Schema: map[string]*schema.Schema{ "name": { Type: schema.TypeString, From 8c548959b7c2170ef2b4f27efa23932f5842d874 Mon Sep 17 00:00:00 2001 From: Stefan Sundin Date: Tue, 8 May 2018 18:27:43 -0700 Subject: [PATCH 02/20] Add import help to aws_emr_cluster documentation. Plus other minor changes. --- website/docs/r/emr_cluster.html.markdown | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/website/docs/r/emr_cluster.html.markdown b/website/docs/r/emr_cluster.html.markdown index fceda7a067cb..83c9826bf763 100644 --- a/website/docs/r/emr_cluster.html.markdown +++ b/website/docs/r/emr_cluster.html.markdown @@ -15,7 +15,7 @@ for more information. ## Example Usage ```hcl -resource "aws_emr_cluster" "emr-test-cluster" { +resource "aws_emr_cluster" "cluster" { name = "emr-test-arn" release_label = "emr-4.6.0" applications = ["Spark"] @@ -37,7 +37,7 @@ EOF emr_managed_slave_security_group = "${aws_security_group.sg.id}" instance_profile = "${aws_iam_instance_profile.emr_profile.arn}" } - + instance_group { instance_role = "CORE" instance_type = "c4.large" @@ -175,7 +175,7 @@ The following arguments are supported: * `name` - (Required) The name of the job flow * `release_label` - (Required) The release label for the Amazon EMR release * `master_instance_type` - (Optional) The EC2 instance type of the master node. Exactly one of `master_instance_type` and `instance_group` must be specified. -* `scale_down_behavior` - (Optional) The way that individual Amazon EC2 instances terminate when an automatic scale-in activity occurs or an `instance group` is resized. +* `scale_down_behavior` - (Optional) The way that individual Amazon EC2 instances terminate when an automatic scale-in activity occurs or an `instance group` is resized. * `additional_info` - (Optional) A JSON string for selecting additional features such as adding proxy information. Note: Currently there is no API to retrieve the value of this argument after EMR cluster creation from provider, therefore Terraform cannot detect drift from the actual EMR cluster if its value is changed outside Terraform. * `service_role` - (Required) IAM role that will be assumed by the Amazon EMR service to access AWS resources * `security_configuration` - (Optional) The security configuration name to attach to the EMR cluster. Only valid for EMR clusters with `release_label` 4.8.0 or greater @@ -343,7 +343,7 @@ provider "aws" { region = "us-west-2" } -resource "aws_emr_cluster" "tf-test-cluster" { +resource "aws_emr_cluster" "cluster" { name = "emr-test-arn" release_label = "emr-4.6.0" applications = ["Spark"] @@ -632,3 +632,11 @@ resource "aws_iam_role_policy" "iam_emr_profile_policy" { EOF } ``` + +## Import + +EMR clusters can be imported using the `id`, e.g. + +``` +$ terraform import aws_emr_cluster.cluster j-123456ABCDEF +``` From 1637bb3e2756f159ac4ae1d41782e9902aedcd47 Mon Sep 17 00:00:00 2001 From: Stefan Sundin Date: Tue, 8 May 2018 18:28:12 -0700 Subject: [PATCH 03/20] Fix typos in aws_emr_security_configuration documentation. --- website/docs/r/emr_security_configuration.html.markdown | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/website/docs/r/emr_security_configuration.html.markdown b/website/docs/r/emr_security_configuration.html.markdown index 856fe93df5a2..c9f4b1292d8c 100644 --- a/website/docs/r/emr_security_configuration.html.markdown +++ b/website/docs/r/emr_security_configuration.html.markdown @@ -1,6 +1,6 @@ --- layout: "aws" -page_title: "AWS: aws_emr_security_configuraiton" +page_title: "AWS: aws_emr_security_configuration" sidebar_current: "docs-aws-resource-emr-security-configuration" description: |- Provides a resource to manage AWS EMR Security Configurations @@ -59,5 +59,5 @@ In addition to all arguments above, the following attributes are exported: EMR Security Configurations can be imported using the `name`, e.g. ``` -$ terraform import aws_emr_security_configuraiton.sc example-sc-name +$ terraform import aws_emr_security_configuration.sc example-sc-name ``` From 6cbb601cfcbfbdb2255fba82a8c9079e617cb9b2 Mon Sep 17 00:00:00 2001 From: Stefan Sundin Date: Wed, 9 May 2018 09:05:07 -0700 Subject: [PATCH 04/20] Add acceptance test for import. --- aws/resource_aws_emr_cluster_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/aws/resource_aws_emr_cluster_test.go b/aws/resource_aws_emr_cluster_test.go index ab60c887da74..389e5d59ccfe 100644 --- a/aws/resource_aws_emr_cluster_test.go +++ b/aws/resource_aws_emr_cluster_test.go @@ -31,6 +31,11 @@ func TestAccAWSEMRCluster_basic(t *testing.T) { resource.TestCheckResourceAttr("aws_emr_cluster.tf-test-cluster", "step.#", "0"), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + }, }, }) } From 2c1c04e8901655cff4a07a6a68996f5d823dcfc8 Mon Sep 17 00:00:00 2001 From: Stefan Sundin Date: Mon, 4 Jun 2018 15:49:15 -0700 Subject: [PATCH 05/20] Make TestAccAWSEMRCluster_basic pass. --- aws/resource_aws_emr_cluster.go | 6 ++++++ aws/resource_aws_emr_cluster_test.go | 7 ++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/aws/resource_aws_emr_cluster.go b/aws/resource_aws_emr_cluster.go index c82c582d9cf3..594135c552bf 100644 --- a/aws/resource_aws_emr_cluster.go +++ b/aws/resource_aws_emr_cluster.go @@ -662,6 +662,11 @@ func resourceAwsEMRClusterRead(d *schema.ResourceData, meta interface{}) error { coreGroup := findGroup(instanceGroups, "CORE") if coreGroup != nil { d.Set("core_instance_type", coreGroup.InstanceType) + d.Set("core_instance_count", coreGroup.RequestedInstanceCount) + } + masterGroup := findGroup(instanceGroups, "MASTER") + if masterGroup != nil { + d.Set("master_instance_type", masterGroup.InstanceType) } if err := d.Set("instance_group", flattenInstanceGroups(instanceGroups)); err != nil { log.Printf("[ERR] Error setting EMR instance groups: %s", err) @@ -680,6 +685,7 @@ func resourceAwsEMRClusterRead(d *schema.ResourceData, meta interface{}) error { d.Set("tags", tagsToMapEMR(cluster.Tags)) d.Set("ebs_root_volume_size", cluster.EbsRootVolumeSize) d.Set("scale_down_behavior", cluster.ScaleDownBehavior) + d.Set("termination_protection", cluster.TerminationProtected) if cluster.CustomAmiId != nil { d.Set("custom_ami_id", cluster.CustomAmiId) diff --git a/aws/resource_aws_emr_cluster_test.go b/aws/resource_aws_emr_cluster_test.go index 389e5d59ccfe..90bdb560152c 100644 --- a/aws/resource_aws_emr_cluster_test.go +++ b/aws/resource_aws_emr_cluster_test.go @@ -32,9 +32,10 @@ func TestAccAWSEMRCluster_basic(t *testing.T) { ), }, { - ResourceName: "aws_emr_cluster.tf-test-cluster", - ImportState: true, - ImportStateVerify: true, + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"configurations", "keep_job_flow_alive_when_no_steps", "core_instance_type", "core_instance_count"}, }, }, }) From 015ae93029866c5c57117773a97c896a37a60361 Mon Sep 17 00:00:00 2001 From: Stefan Sundin Date: Sun, 12 Aug 2018 16:10:14 -0700 Subject: [PATCH 06/20] Remove duplicate code. --- aws/resource_aws_emr_cluster.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/aws/resource_aws_emr_cluster.go b/aws/resource_aws_emr_cluster.go index 594135c552bf..26b9b5bbe0df 100644 --- a/aws/resource_aws_emr_cluster.go +++ b/aws/resource_aws_emr_cluster.go @@ -455,9 +455,6 @@ func resourceAwsEMRClusterCreate(d *schema.ResourceData, meta interface{}) error if v, ok := attributes["subnet_id"]; ok { instanceConfig.Ec2SubnetId = aws.String(v.(string)) } - if v, ok := attributes["subnet_id"]; ok { - instanceConfig.Ec2SubnetId = aws.String(v.(string)) - } if v, ok := attributes["additional_master_security_groups"]; ok { strSlice := strings.Split(v.(string), ",") From ff3c579ab9c090616b7b644053619a48d89a2e68 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Tue, 9 Oct 2018 16:44:27 -0400 Subject: [PATCH 07/20] tests/resource/aws_emr_cluster: Add Import TestStep to all acceptance tests --- aws/resource_aws_emr_cluster_test.go | 182 ++++++++++++++++++++++++++- 1 file changed, 178 insertions(+), 4 deletions(-) diff --git a/aws/resource_aws_emr_cluster_test.go b/aws/resource_aws_emr_cluster_test.go index a299e697107d..c57e5944b29a 100644 --- a/aws/resource_aws_emr_cluster_test.go +++ b/aws/resource_aws_emr_cluster_test.go @@ -32,10 +32,16 @@ func TestAccAWSEMRCluster_basic(t *testing.T) { ), }, { - ResourceName: "aws_emr_cluster.tf-test-cluster", - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"configurations", "keep_job_flow_alive_when_no_steps", "core_instance_type", "core_instance_count"}, + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "configurations", + "core_instance_count", + "core_instance_type", + "kerberos_attributes.0.kdc_admin_password", + "keep_job_flow_alive_when_no_steps", + }, }, }, }) @@ -57,6 +63,18 @@ func TestAccAWSEMRCluster_additionalInfo(t *testing.T) { resource.TestCheckResourceAttr("aws_emr_cluster.tf-test-cluster", "step.#", "0"), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "configurations", + "core_instance_count", + "core_instance_type", + "kerberos_attributes.0.kdc_admin_password", + "keep_job_flow_alive_when_no_steps", + }, + }, }, }) } @@ -77,6 +95,18 @@ func TestAccAWSEMRCluster_configurationsJson(t *testing.T) { regexp.MustCompile("{\"JAVA_HOME\":\"/usr/lib/jvm/java-1.8.0\".+")), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "configurations", + "core_instance_count", + "core_instance_type", + "kerberos_attributes.0.kdc_admin_password", + "keep_job_flow_alive_when_no_steps", + }, + }, }, }) } @@ -97,6 +127,18 @@ func TestAccAWSEMRCluster_instance_group(t *testing.T) { "aws_emr_cluster.tf-test-cluster", "instance_group.#", "2"), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "configurations", + "core_instance_count", + "core_instance_type", + "kerberos_attributes.0.kdc_admin_password", + "keep_job_flow_alive_when_no_steps", + }, + }, }, }) } @@ -117,6 +159,18 @@ func TestAccAWSEMRCluster_instance_group_EBSVolumeType_st1(t *testing.T) { "aws_emr_cluster.tf-test-cluster", "instance_group.#", "2"), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "configurations", + "core_instance_count", + "core_instance_type", + "kerberos_attributes.0.kdc_admin_password", + "keep_job_flow_alive_when_no_steps", + }, + }, }, }) } @@ -140,6 +194,18 @@ func TestAccAWSEMRCluster_Kerberos_ClusterDedicatedKdc(t *testing.T) { resource.TestCheckResourceAttr("aws_emr_cluster.tf-test-cluster", "kerberos_attributes.0.realm", "EC2.INTERNAL"), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "configurations", + "core_instance_count", + "core_instance_type", + "kerberos_attributes.0.kdc_admin_password", + "keep_job_flow_alive_when_no_steps", + }, + }, }, }) } @@ -156,6 +222,18 @@ func TestAccAWSEMRCluster_security_config(t *testing.T) { Config: testAccAWSEmrClusterConfig_SecurityConfiguration(r), Check: testAccCheckAWSEmrClusterExists("aws_emr_cluster.tf-test-cluster", &cluster), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "configurations", + "core_instance_count", + "core_instance_type", + "kerberos_attributes.0.kdc_admin_password", + "keep_job_flow_alive_when_no_steps", + }, + }, }, }) } @@ -183,6 +261,18 @@ func TestAccAWSEMRCluster_Step_Basic(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "step.0.name", "Setup Hadoop Debugging"), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "configurations", + "core_instance_count", + "core_instance_type", + "kerberos_attributes.0.kdc_admin_password", + "keep_job_flow_alive_when_no_steps", + }, + }, }, }) } @@ -214,6 +304,18 @@ func TestAccAWSEMRCluster_Step_Multiple(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "step.1.name", "Spark Step"), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "configurations", + "core_instance_count", + "core_instance_type", + "kerberos_attributes.0.kdc_admin_password", + "keep_job_flow_alive_when_no_steps", + }, + }, }, }) } @@ -279,6 +381,18 @@ func TestAccAWSEMRCluster_terminationProtected(t *testing.T) { "aws_emr_cluster.tf-test-cluster", "termination_protection", "true"), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "configurations", + "core_instance_count", + "core_instance_type", + "kerberos_attributes.0.kdc_admin_password", + "keep_job_flow_alive_when_no_steps", + }, + }, { //Need to turn off termination_protection to allow the job to be deleted Config: testAccAWSEmrClusterConfigTerminationPolicy(r, "false"), @@ -328,6 +442,18 @@ func TestAccAWSEMRCluster_visibleToAllUsers(t *testing.T) { "aws_emr_cluster.tf-test-cluster", "visible_to_all_users", "true"), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "configurations", + "core_instance_count", + "core_instance_type", + "kerberos_attributes.0.kdc_admin_password", + "keep_job_flow_alive_when_no_steps", + }, + }, { Config: testAccAWSEmrClusterConfigVisibleToAllUsersUpdated(r), Check: resource.ComposeTestCheckFunc( @@ -357,6 +483,18 @@ func TestAccAWSEMRCluster_s3Logging(t *testing.T) { resource.TestCheckResourceAttr("aws_emr_cluster.tf-test-cluster", "log_uri", bucketName), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "configurations", + "core_instance_count", + "core_instance_type", + "kerberos_attributes.0.kdc_admin_password", + "keep_job_flow_alive_when_no_steps", + }, + }, }, }) } @@ -396,6 +534,18 @@ func TestAccAWSEMRCluster_tags(t *testing.T) { "aws_emr_cluster.tf-test-cluster", "tags.name", "name-env"), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "configurations", + "core_instance_count", + "core_instance_type", + "kerberos_attributes.0.kdc_admin_password", + "keep_job_flow_alive_when_no_steps", + }, + }, }, }) } @@ -422,6 +572,18 @@ func TestAccAWSEMRCluster_root_volume_size(t *testing.T) { resource.TestCheckResourceAttr("aws_emr_cluster.tf-test-cluster", "ebs_root_volume_size", "48"), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "configurations", + "core_instance_count", + "core_instance_type", + "kerberos_attributes.0.kdc_admin_password", + "keep_job_flow_alive_when_no_steps", + }, + }, }, }) } @@ -441,6 +603,18 @@ func TestAccAWSEMRCluster_custom_ami_id(t *testing.T) { resource.TestCheckResourceAttrSet("aws_emr_cluster.tf-test-cluster", "custom_ami_id"), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "configurations", + "core_instance_count", + "core_instance_type", + "kerberos_attributes.0.kdc_admin_password", + "keep_job_flow_alive_when_no_steps", + }, + }, }, }) } From b359cd42dd2e13295c671a13043f6a0d85b52fd5 Mon Sep 17 00:00:00 2001 From: Matthew Frahry Date: Fri, 16 Nov 2018 11:20:44 -0700 Subject: [PATCH 08/20] Adding import support --- aws/resource_aws_emr_cluster.go | 70 +++++++++++++++++--- aws/resource_aws_emr_cluster_test.go | 98 +++++++++++++++++++++++++++- 2 files changed, 159 insertions(+), 9 deletions(-) diff --git a/aws/resource_aws_emr_cluster.go b/aws/resource_aws_emr_cluster.go index 23a112f06d9d..a22692d642ce 100644 --- a/aws/resource_aws_emr_cluster.go +++ b/aws/resource_aws_emr_cluster.go @@ -65,8 +65,9 @@ func resourceAwsEMRCluster() *schema.Resource { Computed: true, }, "core_instance_count": { - Type: schema.TypeInt, - Optional: true, + Type: schema.TypeInt, + Optional: true, + ValidateFunc: validation.IntAtLeast(1), }, "cluster_state": { Type: schema.TypeString, @@ -433,17 +434,57 @@ func resourceAwsEMRClusterCreate(d *schema.ResourceData, meta interface{}) error TerminationProtected: aws.Bool(terminationProtection), } + /* + if v, ok := d.GetOk("master_instance_type"); ok { + instanceConfig.MasterInstanceType = aws.String(v.(string)) + instanceConfig.SlaveInstanceType = aws.String(v.(string)) + instanceConfig.InstanceCount = aws.Int64(1) + }*/ + if v, ok := d.GetOk("master_instance_type"); ok { - instanceConfig.MasterInstanceType = aws.String(v.(string)) - instanceConfig.SlaveInstanceType = aws.String(v.(string)) + masterInstanceGroupConfig := &emr.InstanceGroupConfig{ + InstanceRole: aws.String("MASTER"), + InstanceType: aws.String(v.(string)), + InstanceCount: aws.Int64(1), + } + instanceConfig.InstanceGroups = append(instanceConfig.InstanceGroups, masterInstanceGroupConfig) } + + var coreInstanceType string if v, ok := d.GetOk("core_instance_type"); ok { - instanceConfig.SlaveInstanceType = aws.String(v.(string)) + coreInstanceType = v.(string) } + var coreInstanceCount int64 if v, ok := d.GetOk("core_instance_count"); ok { - instanceConfig.InstanceCount = aws.Int64(int64(v.(int))) + coreInstanceCount = int64(v.(int)) + } + if (coreInstanceCount == 0 && coreInstanceType != "") || (coreInstanceCount > 0 && coreInstanceType == "") { + return fmt.Errorf("Must specify both `core_instance_count` and `core_instance_type`") + } else if coreInstanceCount > 0 && coreInstanceType != "" { + coreInstanceGroupConfig := &emr.InstanceGroupConfig{ + InstanceCount: aws.Int64(int64(d.Get("core_instance_count").(int))), + InstanceRole: aws.String("CORE"), + InstanceType: aws.String(d.Get("core_instance_type").(string)), + } + instanceConfig.InstanceGroups = append(instanceConfig.InstanceGroups, coreInstanceGroupConfig) } + /* + if v, ok := d.GetOk("core_instance_type"); ok { + instanceConfig.SlaveInstanceType = aws.String(v.(string)) + } + if v, ok := d.GetOk("core_instance_count"); ok { + instanceConfig.InstanceCount = aws.Int64(int64(v.(int))) + }*/ + /* + coreInstanceGroupConfig := &emr.InstanceGroupConfig{ + InstanceCount: aws.Int64(int64(d.Get("core_instance_count").(int))), + InstanceRole: aws.String("CORE"), + InstanceType: aws.String(d.Get("core_instance_type").(string)), + }*/ + + // instanceConfig.InstanceGroups = []*emr.InstanceGroupConfig{masterInstanceGroupConfig, coreInstanceGroupConfig} + var instanceProfile string if a, ok := d.GetOk("ec2_attributes"); ok { ec2Attributes := a.([]interface{}) @@ -602,6 +643,8 @@ func resourceAwsEMRClusterCreate(d *schema.ResourceData, meta interface{}) error } d.SetId(*resp.JobFlowId) + // This value can only be obtained through a deprecated function + d.Set("keep_job_flow_alive_when_no_steps", params.Instances.KeepJobFlowAliveWhenNoSteps) log.Println("[INFO] Waiting for EMR Cluster to be available") @@ -667,7 +710,7 @@ func resourceAwsEMRClusterRead(d *schema.ResourceData, meta interface{}) error { d.Set("core_instance_type", coreGroup.InstanceType) d.Set("core_instance_count", coreGroup.RequestedInstanceCount) } - masterGroup := findGroup(instanceGroups, "MASTER") + masterGroup := findGroupFromRole(instanceGroups, emr.InstanceRoleTypeMaster) if masterGroup != nil { d.Set("master_instance_type", masterGroup.InstanceType) } @@ -1085,7 +1128,9 @@ func flattenInstanceGroups(igs []*emr.InstanceGroup) []map[string]interface{} { attrs["autoscaling_policy"] = "" } - attrs["name"] = *ig.Name + if attrs["name"] != nil { + attrs["name"] = *ig.Name + } result = append(result, attrs) } @@ -1476,3 +1521,12 @@ func resourceAwsEMRClusterStateRefreshFunc(d *schema.ResourceData, meta interfac return resp.Cluster, state, nil } } + +func findGroupFromRole(instanceGroups []*emr.InstanceGroup, role string) *emr.InstanceGroup { + for _, group := range instanceGroups { + if *group.InstanceGroupType == role { + return group + } + } + return nil +} diff --git a/aws/resource_aws_emr_cluster_test.go b/aws/resource_aws_emr_cluster_test.go index 26e0d12eb392..02e34e0141d8 100644 --- a/aws/resource_aws_emr_cluster_test.go +++ b/aws/resource_aws_emr_cluster_test.go @@ -35,7 +35,7 @@ func TestAccAWSEMRCluster_basic(t *testing.T) { ResourceName: "aws_emr_cluster.tf-test-cluster", ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"configurations", "keep_job_flow_alive_when_no_steps", "core_instance_type", "core_instance_count"}, + ImportStateVerifyIgnore: []string{"configurations", "keep_job_flow_alive_when_no_steps"}, }, }, }) @@ -57,6 +57,12 @@ func TestAccAWSEMRCluster_additionalInfo(t *testing.T) { resource.TestCheckResourceAttr("aws_emr_cluster.tf-test-cluster", "step.#", "0"), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"configurations", "keep_job_flow_alive_when_no_steps", "additional_info"}, + }, }, }) } @@ -77,6 +83,12 @@ func TestAccAWSEMRCluster_configurationsJson(t *testing.T) { regexp.MustCompile("{\"JAVA_HOME\":\"/usr/lib/jvm/java-1.8.0\".+")), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"configurations", "keep_job_flow_alive_when_no_steps"}, + }, }, }) } @@ -97,6 +109,12 @@ func TestAccAWSEMRCluster_instance_group(t *testing.T) { "aws_emr_cluster.tf-test-cluster", "instance_group.#", "2"), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"configurations", "keep_job_flow_alive_when_no_steps"}, + }, }, }) } @@ -117,6 +135,12 @@ func TestAccAWSEMRCluster_instance_group_EBSVolumeType_st1(t *testing.T) { "aws_emr_cluster.tf-test-cluster", "instance_group.#", "2"), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"configurations", "keep_job_flow_alive_when_no_steps"}, + }, }, }) } @@ -140,6 +164,12 @@ func TestAccAWSEMRCluster_Kerberos_ClusterDedicatedKdc(t *testing.T) { resource.TestCheckResourceAttr("aws_emr_cluster.tf-test-cluster", "kerberos_attributes.0.realm", "EC2.INTERNAL"), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"configurations", "keep_job_flow_alive_when_no_steps", "kerberos_attributes.0.kdc_admin_password"}, + }, }, }) } @@ -156,6 +186,12 @@ func TestAccAWSEMRCluster_security_config(t *testing.T) { Config: testAccAWSEmrClusterConfig_SecurityConfiguration(r), Check: testAccCheckAWSEmrClusterExists("aws_emr_cluster.tf-test-cluster", &cluster), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"configurations", "keep_job_flow_alive_when_no_steps"}, + }, }, }) } @@ -183,6 +219,12 @@ func TestAccAWSEMRCluster_Step_Basic(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "step.0.name", "Setup Hadoop Debugging"), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"configurations", "keep_job_flow_alive_when_no_steps"}, + }, }, }) } @@ -214,6 +256,12 @@ func TestAccAWSEMRCluster_Step_Multiple(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "step.1.name", "Spark Step"), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"configurations", "keep_job_flow_alive_when_no_steps"}, + }, }, }) } @@ -251,6 +299,12 @@ func TestAccAWSEMRCluster_bootstrap_ordering(t *testing.T) { testAccCheck_bootstrap_order(&cluster, argsInts, argsStrings), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"configurations", "keep_job_flow_alive_when_no_steps"}, + }, }, }) } @@ -288,6 +342,12 @@ func TestAccAWSEMRCluster_terminationProtected(t *testing.T) { "aws_emr_cluster.tf-test-cluster", "termination_protection", "false"), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"configurations", "keep_job_flow_alive_when_no_steps"}, + }, }, }) } @@ -308,6 +368,12 @@ func TestAccAWSEMRCluster_keepJob(t *testing.T) { "aws_emr_cluster.tf-test-cluster", "keep_job_flow_alive_when_no_steps", "false"), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"configurations", "keep_job_flow_alive_when_no_steps"}, + }, }, }) } @@ -336,6 +402,12 @@ func TestAccAWSEMRCluster_visibleToAllUsers(t *testing.T) { "aws_emr_cluster.tf-test-cluster", "visible_to_all_users", "false"), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"configurations", "keep_job_flow_alive_when_no_steps"}, + }, }, }) } @@ -357,6 +429,12 @@ func TestAccAWSEMRCluster_s3Logging(t *testing.T) { resource.TestCheckResourceAttr("aws_emr_cluster.tf-test-cluster", "log_uri", bucketName), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"configurations", "keep_job_flow_alive_when_no_steps"}, + }, }, }) } @@ -396,6 +474,12 @@ func TestAccAWSEMRCluster_tags(t *testing.T) { "aws_emr_cluster.tf-test-cluster", "tags.name", "name-env"), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"configurations", "keep_job_flow_alive_when_no_steps"}, + }, }, }) } @@ -422,6 +506,12 @@ func TestAccAWSEMRCluster_root_volume_size(t *testing.T) { resource.TestCheckResourceAttr("aws_emr_cluster.tf-test-cluster", "ebs_root_volume_size", "48"), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"configurations", "keep_job_flow_alive_when_no_steps"}, + }, }, }) } @@ -441,6 +531,12 @@ func TestAccAWSEMRCluster_custom_ami_id(t *testing.T) { resource.TestCheckResourceAttrSet("aws_emr_cluster.tf-test-cluster", "custom_ami_id"), ), }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"configurations", "keep_job_flow_alive_when_no_steps"}, + }, }, }) } From 063db7f808a23fbadf1d263579d2306b62de70db Mon Sep 17 00:00:00 2001 From: Tracy Holmes Date: Fri, 16 Nov 2018 14:11:12 -0600 Subject: [PATCH 09/20] Fixing tests --- aws/resource_aws_emr_cluster.go | 26 +++----------------------- aws/resource_aws_emr_cluster_test.go | 5 +++-- 2 files changed, 6 insertions(+), 25 deletions(-) diff --git a/aws/resource_aws_emr_cluster.go b/aws/resource_aws_emr_cluster.go index a22692d642ce..bf42f854c32a 100644 --- a/aws/resource_aws_emr_cluster.go +++ b/aws/resource_aws_emr_cluster.go @@ -46,6 +46,7 @@ func resourceAwsEMRCluster() *schema.Resource { Required: false, Optional: true, ForceNew: true, + Computed: true, }, "additional_info": { Type: schema.TypeString, @@ -68,6 +69,7 @@ func resourceAwsEMRCluster() *schema.Resource { Type: schema.TypeInt, Optional: true, ValidateFunc: validation.IntAtLeast(1), + Computed: true, }, "cluster_state": { Type: schema.TypeString, @@ -215,6 +217,7 @@ func resourceAwsEMRCluster() *schema.Resource { "iops": { Type: schema.TypeInt, Optional: true, + Computed: true, }, "size": { Type: schema.TypeInt, @@ -434,13 +437,6 @@ func resourceAwsEMRClusterCreate(d *schema.ResourceData, meta interface{}) error TerminationProtected: aws.Bool(terminationProtection), } - /* - if v, ok := d.GetOk("master_instance_type"); ok { - instanceConfig.MasterInstanceType = aws.String(v.(string)) - instanceConfig.SlaveInstanceType = aws.String(v.(string)) - instanceConfig.InstanceCount = aws.Int64(1) - }*/ - if v, ok := d.GetOk("master_instance_type"); ok { masterInstanceGroupConfig := &emr.InstanceGroupConfig{ InstanceRole: aws.String("MASTER"), @@ -469,22 +465,6 @@ func resourceAwsEMRClusterCreate(d *schema.ResourceData, meta interface{}) error instanceConfig.InstanceGroups = append(instanceConfig.InstanceGroups, coreInstanceGroupConfig) } - /* - if v, ok := d.GetOk("core_instance_type"); ok { - instanceConfig.SlaveInstanceType = aws.String(v.(string)) - } - if v, ok := d.GetOk("core_instance_count"); ok { - instanceConfig.InstanceCount = aws.Int64(int64(v.(int))) - }*/ - /* - coreInstanceGroupConfig := &emr.InstanceGroupConfig{ - InstanceCount: aws.Int64(int64(d.Get("core_instance_count").(int))), - InstanceRole: aws.String("CORE"), - InstanceType: aws.String(d.Get("core_instance_type").(string)), - }*/ - - // instanceConfig.InstanceGroups = []*emr.InstanceGroupConfig{masterInstanceGroupConfig, coreInstanceGroupConfig} - var instanceProfile string if a, ok := d.GetOk("ec2_attributes"); ok { ec2Attributes := a.([]interface{}) diff --git a/aws/resource_aws_emr_cluster_test.go b/aws/resource_aws_emr_cluster_test.go index 77c131a73b2a..e00a065c31ee 100644 --- a/aws/resource_aws_emr_cluster_test.go +++ b/aws/resource_aws_emr_cluster_test.go @@ -268,6 +268,7 @@ func TestAccAWSEMRCluster_Step_Multiple(t *testing.T) { func TestAccAWSEMRCluster_bootstrap_ordering(t *testing.T) { var cluster emr.Cluster + resourceName := "aws_emr_cluster.test" rName := acctest.RandomWithPrefix("tf-emr-bootstrap") argsInts := []string{ "1", @@ -295,12 +296,12 @@ func TestAccAWSEMRCluster_bootstrap_ordering(t *testing.T) { { Config: testAccAWSEmrClusterConfig_bootstrap(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSEmrClusterExists("aws_emr_cluster.test", &cluster), + testAccCheckAWSEmrClusterExists(resourceName, &cluster), testAccCheck_bootstrap_order(&cluster, argsInts, argsStrings), ), }, { - ResourceName: "aws_emr_cluster.tf-test-cluster", + ResourceName: resourceName, ImportState: true, ImportStateVerify: true, ImportStateVerifyIgnore: []string{"configurations", "keep_job_flow_alive_when_no_steps"}, From 4387c3abe27d684e0314ddb13191a4f4c4a368f6 Mon Sep 17 00:00:00 2001 From: Matthew Frahry Date: Tue, 20 Nov 2018 10:41:28 -0700 Subject: [PATCH 10/20] Finalizing tests --- aws/resource_aws_emr_cluster.go | 94 +++++++++++++++++----------- aws/resource_aws_emr_cluster_test.go | 4 +- 2 files changed, 61 insertions(+), 37 deletions(-) diff --git a/aws/resource_aws_emr_cluster.go b/aws/resource_aws_emr_cluster.go index bf42f854c32a..87bd81fba108 100644 --- a/aws/resource_aws_emr_cluster.go +++ b/aws/resource_aws_emr_cluster.go @@ -1,6 +1,7 @@ package aws import ( + "bytes" "log" "encoding/json" @@ -14,6 +15,7 @@ import ( "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/private/protocol/json/jsonutil" "github.com/aws/aws-sdk-go/service/emr" + "github.com/hashicorp/terraform/helper/hashcode" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/helper/structure" @@ -43,7 +45,6 @@ func resourceAwsEMRCluster() *schema.Resource { }, "master_instance_type": { Type: schema.TypeString, - Required: false, Optional: true, ForceNew: true, Computed: true, @@ -201,6 +202,7 @@ func resourceAwsEMRCluster() *schema.Resource { Type: schema.TypeSet, Optional: true, ForceNew: true, + Computed: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "bid_price": { @@ -209,9 +211,10 @@ func resourceAwsEMRCluster() *schema.Resource { Required: false, }, "ebs_config": { - Type: schema.TypeSet, + Type: schema.TypeList, Optional: true, ForceNew: true, + Computed: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "iops": { @@ -246,6 +249,7 @@ func resourceAwsEMRCluster() *schema.Resource { Optional: true, DiffSuppressFunc: suppressEquivalentJsonDiffs, ValidateFunc: validation.ValidateJsonString, + Computed: true, StateFunc: func(v interface{}) string { jsonString, _ := structure.NormalizeJsonString(v) return jsonString @@ -272,6 +276,7 @@ func resourceAwsEMRCluster() *schema.Resource { }, }, }, + Set: resourceAwsEMRClusterHash, }, "bootstrap_action": { Type: schema.TypeSet, @@ -690,12 +695,17 @@ func resourceAwsEMRClusterRead(d *schema.ResourceData, meta interface{}) error { d.Set("core_instance_type", coreGroup.InstanceType) d.Set("core_instance_count", coreGroup.RequestedInstanceCount) } - masterGroup := findGroupFromRole(instanceGroups, emr.InstanceRoleTypeMaster) + masterGroup := findMasterGroup(instanceGroups) if masterGroup != nil { d.Set("master_instance_type", masterGroup.InstanceType) } - if err := d.Set("instance_group", flattenInstanceGroups(instanceGroups)); err != nil { - log.Printf("[ERR] Error setting EMR instance groups: %s", err) + flattenedInstanceGroups, err := flattenInstanceGroups(instanceGroups) + if err != nil { + return fmt.Errorf("error flattening instance groups: %+v", err) + } + // return fmt.Errorf("%+v\n\n\n\n%+v", flattenedInstanceGroups, instanceGroups) + if err := d.Set("instance_group", flattenedInstanceGroups); err != nil { + return fmt.Errorf("[ERR] Error setting EMR instance groups: %s", err) } } @@ -718,13 +728,7 @@ func resourceAwsEMRClusterRead(d *schema.ResourceData, meta interface{}) error { } if err := d.Set("applications", flattenApplications(cluster.Applications)); err != nil { - log.Printf("[ERR] Error setting EMR Applications for cluster (%s): %s", d.Id(), err) - } - - // Configurations is a JSON document. It's built with an expand method but a - // simple string should be returned as JSON - if err := d.Set("configurations", cluster.Configurations); err != nil { - log.Printf("[ERR] Error setting EMR configurations for cluster (%s): %s", d.Id(), err) + return fmt.Errorf("error setting EMR Applications for cluster (%s): %s", d.Id(), err) } if _, ok := d.GetOk("configurations_json"); ok { @@ -738,7 +742,7 @@ 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) + return fmt.Errorf("error setting EMR Ec2 Attributes: %s", err) } if err := d.Set("kerberos_attributes", flattenEmrKerberosAttributes(d, cluster.KerberosAttributes)); err != nil { @@ -749,11 +753,11 @@ func resourceAwsEMRClusterRead(d *schema.ResourceData, meta interface{}) error { ClusterId: cluster.Id, }) if err != nil { - log.Printf("[WARN] Error listing bootstrap actions: %s", err) + return fmt.Errorf("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 fmt.Errorf("error setting Bootstrap Actions: %s", err) } var stepSummaries []*emr.StepSummary @@ -931,7 +935,7 @@ func resourceAwsEMRClusterDelete(d *schema.ResourceData, meta interface{}) error }) if err != nil { - log.Printf("[ERR] Error waiting for EMR Cluster (%s) Instances to drain", d.Id()) + return fmt.Errorf("error waiting for EMR Cluster (%s) Instances to drain", d.Id()) } return nil @@ -1073,7 +1077,7 @@ func flattenEmrStepSummary(stepSummary *emr.StepSummary) map[string]interface{} return m } -func flattenInstanceGroups(igs []*emr.InstanceGroup) []map[string]interface{} { +func flattenInstanceGroups(igs []*emr.InstanceGroup) ([]map[string]interface{}, error) { result := make([]map[string]interface{}, 0) for _, ig := range igs { @@ -1083,27 +1087,38 @@ func flattenInstanceGroups(igs []*emr.InstanceGroup) []map[string]interface{} { } else { attrs["bid_price"] = "" } - ebsConfig := make([]map[string]interface{}, 0) - for _, ebs := range ig.EbsBlockDevices { - ebsAttrs := make(map[string]interface{}) - if ebs.VolumeSpecification.Iops != nil { - ebsAttrs["iops"] = *ebs.VolumeSpecification.Iops - } else { - ebsAttrs["iops"] = "" - } - ebsAttrs["size"] = *ebs.VolumeSpecification.SizeInGB - ebsAttrs["type"] = *ebs.VolumeSpecification.VolumeType - ebsAttrs["volumes_per_instance"] = 1 - ebsConfig = append(ebsConfig, ebsAttrs) + if ig.EbsBlockDevices != nil { + ebsConfig := make([]map[string]interface{}, 0) + for _, ebs := range ig.EbsBlockDevices { + ebsAttrs := make(map[string]interface{}) + if ebs.VolumeSpecification.Iops != nil { + ebsAttrs["iops"] = *ebs.VolumeSpecification.Iops + } + if ebs.VolumeSpecification.SizeInGB != nil { + ebsAttrs["size"] = *ebs.VolumeSpecification.SizeInGB + } + if ebs.VolumeSpecification.VolumeType != nil { + ebsAttrs["type"] = *ebs.VolumeSpecification.VolumeType + } + ebsAttrs["volumes_per_instance"] = 1 + + ebsConfig = append(ebsConfig, ebsAttrs) + } + attrs["ebs_config"] = ebsConfig } - attrs["ebs_config"] = ebsConfig + attrs["instance_count"] = *ig.RequestedInstanceCount attrs["instance_role"] = *ig.InstanceGroupType attrs["instance_type"] = *ig.InstanceType if ig.AutoScalingPolicy != nil { - attrs["autoscaling_policy"] = *ig.AutoScalingPolicy + autoscalingPolicyBytes, err := json.Marshal(ig.AutoScalingPolicy) + autoscalingPolicyString := string(autoscalingPolicyBytes) + if err != nil { + return nil, err + } + attrs["autoscaling_policy"] = autoscalingPolicyString } else { attrs["autoscaling_policy"] = "" } @@ -1114,7 +1129,7 @@ func flattenInstanceGroups(igs []*emr.InstanceGroup) []map[string]interface{} { result = append(result, attrs) } - return result + return result, nil } func flattenBootstrapArguments(actions []*emr.Command) []map[string]interface{} { @@ -1369,7 +1384,8 @@ func applyEbsConfig(configAttributes map[string]interface{}, config *emr.Instanc ebsConfig := &emr.EbsConfiguration{} ebsBlockDeviceConfigs := make([]*emr.EbsBlockDeviceConfig, 0) - for _, rawEbsConfig := range rawEbsConfigs.(*schema.Set).List() { + // for _, rawEbsConfig := range rawEbsConfigs.(*schema.Set).List() { + for _, rawEbsConfig := range rawEbsConfigs.([]interface{}) { rawEbsConfig := rawEbsConfig.(map[string]interface{}) ebsBlockDeviceConfig := &emr.EbsBlockDeviceConfig{ VolumesPerInstance: aws.Int64(int64(rawEbsConfig["volumes_per_instance"].(int))), @@ -1502,11 +1518,19 @@ func resourceAwsEMRClusterStateRefreshFunc(d *schema.ResourceData, meta interfac } } -func findGroupFromRole(instanceGroups []*emr.InstanceGroup, role string) *emr.InstanceGroup { +func findMasterGroup(instanceGroups []*emr.InstanceGroup) *emr.InstanceGroup { for _, group := range instanceGroups { - if *group.InstanceGroupType == role { + if *group.InstanceGroupType == emr.InstanceRoleTypeMaster { return group } } return nil } + +func resourceAwsEMRClusterHash(v interface{}) int { + var buf bytes.Buffer + m := v.(map[string]interface{}) + buf.WriteString(fmt.Sprintf("%s-", m["instance_role"].(string))) + buf.WriteString(fmt.Sprintf("%s", m["name"].(string))) + return hashcode.String(buf.String()) +} diff --git a/aws/resource_aws_emr_cluster_test.go b/aws/resource_aws_emr_cluster_test.go index e00a065c31ee..4b83c077146c 100644 --- a/aws/resource_aws_emr_cluster_test.go +++ b/aws/resource_aws_emr_cluster_test.go @@ -374,7 +374,7 @@ func TestAccAWSEMRCluster_keepJob(t *testing.T) { CheckDestroy: testAccCheckAWSEmrDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSEmrClusterConfig_keepJop(r, "false"), + Config: testAccAWSEmrClusterConfig_keepJob(r, "false"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSEmrClusterExists("aws_emr_cluster.tf-test-cluster", &cluster), resource.TestCheckResourceAttr( @@ -3513,7 +3513,7 @@ resource "aws_iam_role_policy_attachment" "emr-autoscaling-role" { `, r, term) } -func testAccAWSEmrClusterConfig_keepJop(r int, keepJob string) string { +func testAccAWSEmrClusterConfig_keepJob(r int, keepJob string) string { return fmt.Sprintf(` resource "aws_emr_cluster" "tf-test-cluster" { name = "emr-test-%[1]d" From c833636a599601d6f342729678f6f86dca42d3f8 Mon Sep 17 00:00:00 2001 From: Matthew Frahry Date: Tue, 20 Nov 2018 11:02:57 -0700 Subject: [PATCH 11/20] Removing comments --- aws/resource_aws_emr_cluster.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/aws/resource_aws_emr_cluster.go b/aws/resource_aws_emr_cluster.go index 87bd81fba108..3fdb939dd7a3 100644 --- a/aws/resource_aws_emr_cluster.go +++ b/aws/resource_aws_emr_cluster.go @@ -703,7 +703,6 @@ func resourceAwsEMRClusterRead(d *schema.ResourceData, meta interface{}) error { if err != nil { return fmt.Errorf("error flattening instance groups: %+v", err) } - // return fmt.Errorf("%+v\n\n\n\n%+v", flattenedInstanceGroups, instanceGroups) if err := d.Set("instance_group", flattenedInstanceGroups); err != nil { return fmt.Errorf("[ERR] Error setting EMR instance groups: %s", err) } @@ -1384,7 +1383,6 @@ func applyEbsConfig(configAttributes map[string]interface{}, config *emr.Instanc ebsConfig := &emr.EbsConfiguration{} ebsBlockDeviceConfigs := make([]*emr.EbsBlockDeviceConfig, 0) - // for _, rawEbsConfig := range rawEbsConfigs.(*schema.Set).List() { for _, rawEbsConfig := range rawEbsConfigs.([]interface{}) { rawEbsConfig := rawEbsConfig.(map[string]interface{}) ebsBlockDeviceConfig := &emr.EbsBlockDeviceConfig{ From 2fa32bd71178b3b03723bf823f01cdad08d8a53d Mon Sep 17 00:00:00 2001 From: Matthew Frahry Date: Mon, 10 Dec 2018 00:36:52 -0700 Subject: [PATCH 12/20] Adding migration steps to EMR Cluster --- aws/resource_aws_emr_cluster.go | 74 ++-- aws/resource_aws_emr_cluster_migrate.go | 83 ++++ aws/resource_aws_emr_cluster_migrate_test.go | 79 ++++ aws/resource_aws_emr_cluster_test.go | 388 ++++++++++++++++++- 4 files changed, 589 insertions(+), 35 deletions(-) create mode 100644 aws/resource_aws_emr_cluster_migrate.go create mode 100644 aws/resource_aws_emr_cluster_migrate_test.go diff --git a/aws/resource_aws_emr_cluster.go b/aws/resource_aws_emr_cluster.go index 3fdb939dd7a3..5576bac4e373 100644 --- a/aws/resource_aws_emr_cluster.go +++ b/aws/resource_aws_emr_cluster.go @@ -31,6 +31,8 @@ func resourceAwsEMRCluster() *schema.Resource { Importer: &schema.ResourceImporter{ State: schema.ImportStatePassthrough, }, + SchemaVersion: 1, + MigrateState: resourceAwsEMRClusterMigrateState, Schema: map[string]*schema.Schema{ "name": { @@ -214,9 +216,12 @@ func resourceAwsEMRCluster() *schema.Resource { Type: schema.TypeList, Optional: true, ForceNew: true, - Computed: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ + "name": { + Type: schema.TypeString, + Computed: true, + }, "iops": { Type: schema.TypeInt, Optional: true, @@ -249,7 +254,6 @@ func resourceAwsEMRCluster() *schema.Resource { Optional: true, DiffSuppressFunc: suppressEquivalentJsonDiffs, ValidateFunc: validation.ValidateJsonString, - Computed: true, StateFunc: func(v interface{}) string { jsonString, _ := structure.NormalizeJsonString(v) return jsonString @@ -276,7 +280,7 @@ func resourceAwsEMRCluster() *schema.Resource { }, }, }, - Set: resourceAwsEMRClusterHash, + Set: resourceAwsEMRClusterInstanceGroupHash, }, "bootstrap_action": { Type: schema.TypeSet, @@ -1076,37 +1080,20 @@ func flattenEmrStepSummary(stepSummary *emr.StepSummary) map[string]interface{} return m } -func flattenInstanceGroups(igs []*emr.InstanceGroup) ([]map[string]interface{}, error) { - result := make([]map[string]interface{}, 0) +func flattenInstanceGroups(igs []*emr.InstanceGroup) (*schema.Set, error) { + instanceGroupSet := schema.Set{ + F: resourceAwsEMRClusterInstanceGroupHash, + } for _, ig := range igs { - attrs := make(map[string]interface{}) + attrs := map[string]interface{}{} if ig.BidPrice != nil { attrs["bid_price"] = *ig.BidPrice } else { attrs["bid_price"] = "" } - if ig.EbsBlockDevices != nil { - ebsConfig := make([]map[string]interface{}, 0) - for _, ebs := range ig.EbsBlockDevices { - ebsAttrs := make(map[string]interface{}) - if ebs.VolumeSpecification.Iops != nil { - ebsAttrs["iops"] = *ebs.VolumeSpecification.Iops - } - if ebs.VolumeSpecification.SizeInGB != nil { - ebsAttrs["size"] = *ebs.VolumeSpecification.SizeInGB - } - if ebs.VolumeSpecification.VolumeType != nil { - ebsAttrs["type"] = *ebs.VolumeSpecification.VolumeType - } - ebsAttrs["volumes_per_instance"] = 1 - - ebsConfig = append(ebsConfig, ebsAttrs) - } - attrs["ebs_config"] = ebsConfig - } - + attrs["ebs_config"] = flattenEBSConfig(ig.EbsBlockDevices) attrs["instance_count"] = *ig.RequestedInstanceCount attrs["instance_role"] = *ig.InstanceGroupType attrs["instance_type"] = *ig.InstanceType @@ -1115,7 +1102,7 @@ func flattenInstanceGroups(igs []*emr.InstanceGroup) ([]map[string]interface{}, autoscalingPolicyBytes, err := json.Marshal(ig.AutoScalingPolicy) autoscalingPolicyString := string(autoscalingPolicyBytes) if err != nil { - return nil, err + return nil, fmt.Errorf("error parsing EMR Cluster Instance Groups AutoScalingPolicy: %s", err) } attrs["autoscaling_policy"] = autoscalingPolicyString } else { @@ -1125,10 +1112,30 @@ func flattenInstanceGroups(igs []*emr.InstanceGroup) ([]map[string]interface{}, if attrs["name"] != nil { attrs["name"] = *ig.Name } - result = append(result, attrs) + instanceGroupSet.Add(attrs) } - return result, nil + return &instanceGroupSet, nil +} + +func flattenEBSConfig(ebsBlockDevices []*emr.EbsBlockDevice) []map[string]interface{} { + ebsConfig := make([]map[string]interface{}, 0) + for _, ebs := range ebsBlockDevices { + ebsAttrs := make(map[string]interface{}) + if ebs.VolumeSpecification.Iops != nil { + ebsAttrs["iops"] = *ebs.VolumeSpecification.Iops + } + if ebs.VolumeSpecification.SizeInGB != nil { + ebsAttrs["size"] = *ebs.VolumeSpecification.SizeInGB + } + if ebs.VolumeSpecification.VolumeType != nil { + ebsAttrs["type"] = *ebs.VolumeSpecification.VolumeType + } + ebsAttrs["volumes_per_instance"] = 1 + + ebsConfig = append(ebsConfig, ebsAttrs) + } + return ebsConfig } func flattenBootstrapArguments(actions []*emr.Command) []map[string]interface{} { @@ -1525,10 +1532,15 @@ func findMasterGroup(instanceGroups []*emr.InstanceGroup) *emr.InstanceGroup { return nil } -func resourceAwsEMRClusterHash(v interface{}) int { +// EMRCluster always has an instance role of either master, core, or task +// Name is optional for core and master(only group allowed for this type) but needed for task +// since you can have multiple task instance groups. +func resourceAwsEMRClusterInstanceGroupHash(v interface{}) int { var buf bytes.Buffer m := v.(map[string]interface{}) buf.WriteString(fmt.Sprintf("%s-", m["instance_role"].(string))) - buf.WriteString(fmt.Sprintf("%s", m["name"].(string))) + if m["name"] != nil { + buf.WriteString(fmt.Sprintf("%s", m["name"].(string))) + } return hashcode.String(buf.String()) } diff --git a/aws/resource_aws_emr_cluster_migrate.go b/aws/resource_aws_emr_cluster_migrate.go new file mode 100644 index 000000000000..ff74f48337ec --- /dev/null +++ b/aws/resource_aws_emr_cluster_migrate.go @@ -0,0 +1,83 @@ +package aws + +import ( + "fmt" + "log" + "strconv" + "strings" + + "github.com/hashicorp/terraform/terraform" +) + +func resourceAwsEMRClusterMigrateState( + v int, is *terraform.InstanceState, meta interface{}) (*terraform.InstanceState, error) { + switch v { + case 0: + log.Println("[INFO] Found AWS EMR Cluster State v0; migrating to v1") + return migrateEMRClusterStateV0toV1(is) + default: + return is, fmt.Errorf("Unexpected schema version: %d", v) + } +} + +func migrateEMRClusterStateV0toV1(is *terraform.InstanceState) (*terraform.InstanceState, error) { + if is.Empty() || is.Attributes == nil { + log.Println("[DEBUG] Empty EMR Cluster State; nothing to migrate.") + return is, nil + } + + ebsConfigCount := 0 + newEBSConfigKeys := make(map[string]string) + ebsConfigMapHashToInt := make(map[int]int) + keys := make([]string, len(is.Attributes)) + for k := range is.Attributes { + keys = append(keys, k) + } + for _, k := range keys { + if !strings.HasPrefix(k, "instance_group.") { + continue + } + + if !strings.Contains(k, "ebs_config") { + continue + } + + if strings.HasSuffix(k, "ebs_config.#") { + continue + } + + kParts := strings.Split(k, ".") + if len(kParts) != 5 { + return nil, fmt.Errorf("migration error: found `instance_group` key in unexpected format: %s", k) + } + instanceGroupHash, err := strconv.Atoi(kParts[1]) + if err != nil { + return nil, fmt.Errorf("migration error: found `instance_group` hash in unexpected format: %s", kParts[1]) + } + ebsConfigHash, err := strconv.Atoi(kParts[3]) + if err != nil { + return nil, fmt.Errorf("migration error: found `ebs_config` hash in unexpected format: %s", kParts[3]) + } + if _, ok := ebsConfigMapHashToInt[ebsConfigHash]; !ok { + ebsConfigMapHashToInt[ebsConfigHash] = ebsConfigCount + ebsConfigCount++ + } + newKey := fmt.Sprintf("instance_group.%d.ebs_config.%d.%s", instanceGroupHash, ebsConfigMapHashToInt[ebsConfigHash], kParts[4]) + newEBSConfigKeys[newKey] = is.Attributes[k] + delete(is.Attributes, k) + } + + for k, v := range newEBSConfigKeys { + is.Attributes[k] = v + + } + + log.Printf("[DEBUG] EMR Cluster Attributes after migration: %#v", is.Attributes) + + return is, nil +} + +func updateV0ToV1EMRClusterEBSConfig(is *terraform.InstanceState) error { + + return nil +} diff --git a/aws/resource_aws_emr_cluster_migrate_test.go b/aws/resource_aws_emr_cluster_migrate_test.go new file mode 100644 index 000000000000..c0b30c04459e --- /dev/null +++ b/aws/resource_aws_emr_cluster_migrate_test.go @@ -0,0 +1,79 @@ +package aws + +import ( + "testing" + + "github.com/hashicorp/terraform/terraform" +) + +func TestAwsEMRClusterMigrateState(t *testing.T) { + cases := map[string]struct { + StateVersion int + Attributes map[string]string + Expected map[string]string + Meta interface{} + }{ + "change `ebs_config` from set to list": { + StateVersion: 0, + Attributes: map[string]string{ + "instance_group.#": "1", + "instance_group.1693978638.ebs_config.#": "2", + "instance_group.1693978638.ebs_config.1693978638.name": "test1", + "instance_group.1693978638.ebs_config.1693978638.size": "10", + "instance_group.1693978638.ebs_config.112349887.name": "test2", + "instance_group.1693978638.ebs_config.112349887.size": "20", + }, + Expected: map[string]string{ + "instance_group.#": "1", + "instance_group.1693978638.ebs_config.#": "2", + "instance_group.1693978638.ebs_config.0.name": "test1", + "instance_group.1693978638.ebs_config.0.size": "10", + "instance_group.1693978638.ebs_config.1.name": "test2", + "instance_group.1693978638.ebs_config.1.size": "20", + }, + }, + } + for tn, tc := range cases { + is := &terraform.InstanceState{ + ID: "i-abc123", + Attributes: tc.Attributes, + } + is, err := resourceAwsEMRClusterMigrateState( + tc.StateVersion, is, tc.Meta) + + if err != nil { + t.Fatalf("bad: %s, err: %#v", tn, err) + } + + for k, v := range tc.Expected { + if is.Attributes[k] != v { + t.Fatalf( + "bad: %s\n\n expected: %#v -> %#v\n got: %#v -> %#v\n in: %#v", + tn, k, v, k, is.Attributes[k], is.Attributes) + } + } + } +} + +func TestAwsEMRClusterMigrateState_empty(t *testing.T) { + var is *terraform.InstanceState + var meta interface{} + + // should handle nil + is, err := resourceAwsEMRClusterMigrateState(0, is, meta) + + if err != nil { + t.Fatalf("err: %#v", err) + } + if is != nil { + t.Fatalf("expected nil instancestate, got: %#v", is) + } + + // should handle non-nil but empty + is = &terraform.InstanceState{} + is, err = resourceAwsEMRClusterMigrateState(0, is, meta) + + if err != nil { + t.Fatalf("err: %#v", err) + } +} diff --git a/aws/resource_aws_emr_cluster_test.go b/aws/resource_aws_emr_cluster_test.go index 4b83c077146c..45afe953356b 100644 --- a/aws/resource_aws_emr_cluster_test.go +++ b/aws/resource_aws_emr_cluster_test.go @@ -119,6 +119,40 @@ func TestAccAWSEMRCluster_instance_group(t *testing.T) { }) } +func TestAccAWSEMRCluster_instance_group_update(t *testing.T) { + var cluster emr.Cluster + r := acctest.RandInt() + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSEmrDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSEmrClusterConfigInstanceGroups(r), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSEmrClusterExists("aws_emr_cluster.tf-test-cluster", &cluster), + resource.TestCheckResourceAttr( + "aws_emr_cluster.tf-test-cluster", "instance_group.#", "2"), + ), + }, + { + Config: testAccAWSEmrClusterConfigInstanceGroupsUpdate(r), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSEmrClusterExists("aws_emr_cluster.tf-test-cluster", &cluster), + resource.TestCheckResourceAttr( + "aws_emr_cluster.tf-test-cluster", "instance_group.#", "2"), + ), + }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"configurations", "keep_job_flow_alive_when_no_steps"}, + }, + }, + }) +} + func TestAccAWSEMRCluster_instance_group_EBSVolumeType_st1(t *testing.T) { var cluster emr.Cluster r := acctest.RandInt() @@ -340,8 +374,6 @@ func TestAccAWSEMRCluster_terminationProtected(t *testing.T) { ImportStateVerify: true, ImportStateVerifyIgnore: []string{ "configurations", - "core_instance_count", - "core_instance_type", "kerberos_attributes.0.kdc_admin_password", "keep_job_flow_alive_when_no_steps", }, @@ -413,8 +445,6 @@ func TestAccAWSEMRCluster_visibleToAllUsers(t *testing.T) { ImportStateVerify: true, ImportStateVerifyIgnore: []string{ "configurations", - "core_instance_count", - "core_instance_type", "kerberos_attributes.0.kdc_admin_password", "keep_job_flow_alive_when_no_steps", }, @@ -2862,6 +2892,356 @@ resource "aws_iam_role_policy_attachment" "emr-autoscaling-role" { `, r) } +func testAccAWSEmrClusterConfigInstanceGroupsUpdate(r int) string { + return fmt.Sprintf(` +resource "aws_emr_cluster" "tf-test-cluster" { + name = "emr-test-%[1]d" + release_label = "emr-4.6.0" + applications = ["Spark"] + + 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}" + } + + instance_group = [ + { + instance_role = "CORE" + instance_type = "c4.large" + instance_count = "1" + ebs_config { + size = "40" + type = "gp2" + volumes_per_instance = 1 + } + bid_price = "0.30" + autoscaling_policy = < Date: Sun, 16 Dec 2018 23:09:29 -0700 Subject: [PATCH 13/20] Continuing work on emr clusters --- aws/resource_aws_emr_cluster.go | 84 +++++++++++++++----- aws/resource_aws_emr_cluster_migrate.go | 6 -- aws/resource_aws_emr_cluster_migrate_test.go | 2 +- 3 files changed, 64 insertions(+), 28 deletions(-) diff --git a/aws/resource_aws_emr_cluster.go b/aws/resource_aws_emr_cluster.go index 5576bac4e373..bf7949754888 100644 --- a/aws/resource_aws_emr_cluster.go +++ b/aws/resource_aws_emr_cluster.go @@ -1,7 +1,6 @@ package aws import ( - "bytes" "log" "encoding/json" @@ -15,7 +14,6 @@ import ( "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/private/protocol/json/jsonutil" "github.com/aws/aws-sdk-go/service/emr" - "github.com/hashicorp/terraform/helper/hashcode" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/helper/structure" @@ -203,19 +201,19 @@ func resourceAwsEMRCluster() *schema.Resource { "instance_group": { Type: schema.TypeSet, Optional: true, - ForceNew: true, - Computed: true, + // ForceNew: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "bid_price": { Type: schema.TypeString, Optional: true, - Required: false, + // ForceNew: true, }, "ebs_config": { Type: schema.TypeList, Optional: true, - ForceNew: true, + Computed: true, + // ForceNew: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "name": { @@ -248,6 +246,8 @@ func resourceAwsEMRCluster() *schema.Resource { Type: schema.TypeInt, Optional: true, Default: 0, + // TODO: Add the ability to update this attribute + //ForceNew: true, }, "autoscaling_policy": { Type: schema.TypeString, @@ -262,6 +262,7 @@ func resourceAwsEMRCluster() *schema.Resource { "instance_role": { Type: schema.TypeString, Required: true, + //ForceNew: true, ValidateFunc: validation.StringInSlice([]string{ emr.InstanceFleetTypeMaster, emr.InstanceFleetTypeCore, @@ -271,7 +272,7 @@ func resourceAwsEMRCluster() *schema.Resource { "instance_type": { Type: schema.TypeString, Required: true, - ForceNew: true, + //ForceNew: true, }, "name": { Type: schema.TypeString, @@ -280,7 +281,7 @@ func resourceAwsEMRCluster() *schema.Resource { }, }, }, - Set: resourceAwsEMRClusterInstanceGroupHash, + // Set: resourceAwsEMRClusterInstanceGroupHash, }, "bootstrap_action": { Type: schema.TypeSet, @@ -1080,17 +1081,12 @@ func flattenEmrStepSummary(stepSummary *emr.StepSummary) map[string]interface{} return m } -func flattenInstanceGroups(igs []*emr.InstanceGroup) (*schema.Set, error) { - instanceGroupSet := schema.Set{ - F: resourceAwsEMRClusterInstanceGroupHash, - } - +func flattenInstanceGroups(igs []*emr.InstanceGroup) ([]map[string]interface{}, error) { + instanceGroupSet := make([]map[string]interface{}, 0) for _, ig := range igs { attrs := map[string]interface{}{} if ig.BidPrice != nil { attrs["bid_price"] = *ig.BidPrice - } else { - attrs["bid_price"] = "" } attrs["ebs_config"] = flattenEBSConfig(ig.EbsBlockDevices) @@ -1099,11 +1095,56 @@ func flattenInstanceGroups(igs []*emr.InstanceGroup) (*schema.Set, error) { attrs["instance_type"] = *ig.InstanceType if ig.AutoScalingPolicy != nil { - autoscalingPolicyBytes, err := json.Marshal(ig.AutoScalingPolicy) - autoscalingPolicyString := string(autoscalingPolicyBytes) + // AutoScalingPolicy has an additional Status field that is causing diffs in the statefile. + // We are purposefully omitting that field here when we generate the autoscaling policy string + // for the statefile. + + for i, rule := range ig.AutoScalingPolicy.Rules { + for j, dimension := range rule.Trigger.CloudWatchAlarmDefinition.Dimensions { + if *dimension.Key == "JobFlowId" { + tmpDimensions := append(ig.AutoScalingPolicy.Rules[i].Trigger.CloudWatchAlarmDefinition.Dimensions[:j], ig.AutoScalingPolicy.Rules[i].Trigger.CloudWatchAlarmDefinition.Dimensions[j+1:]...) + ig.AutoScalingPolicy.Rules[i].Trigger.CloudWatchAlarmDefinition.Dimensions = tmpDimensions + } + } + if len(ig.AutoScalingPolicy.Rules[i].Trigger.CloudWatchAlarmDefinition.Dimensions) == 0 { + ig.AutoScalingPolicy.Rules[i].Trigger.CloudWatchAlarmDefinition.Dimensions = nil + } + } + + tmpAutoScalingPolicy := emr.AutoScalingPolicy{ + Constraints: ig.AutoScalingPolicy.Constraints, + Rules: ig.AutoScalingPolicy.Rules, + } + autoscalingPolicyConstraintsBytes, err := json.Marshal(tmpAutoScalingPolicy.Constraints) + if err != nil { + return nil, fmt.Errorf("error parsing EMR Cluster Instance Groups AutoScalingPolicy Constraints: %s", err) + } + autoscalingPolicyConstraintsString := string(autoscalingPolicyConstraintsBytes) + + autoscalingPolicyRulesBytes, err := json.Marshal(tmpAutoScalingPolicy.Rules) if err != nil { - return nil, fmt.Errorf("error parsing EMR Cluster Instance Groups AutoScalingPolicy: %s", err) + return nil, fmt.Errorf("error parsing EMR Cluster Instance Groups AutoScalingPolicy Rules: %s", err) + } + + var rules []map[string]interface{} + if err := json.Unmarshal(autoscalingPolicyRulesBytes, &rules); err != nil { + return nil, err } + + var cleanRules []map[string]interface{} + for _, rule := range rules { + cleanRules = append(cleanRules, removeNil(rule)) + } + + withoutNulls, err := json.Marshal(cleanRules) + if err != nil { + return nil, err + } + + autoscalingPolicyRulesString := string(withoutNulls) + + autoscalingPolicyString := fmt.Sprintf("{\"Constraints\":%s,\"Rules\":%s}", autoscalingPolicyConstraintsString, autoscalingPolicyRulesString) + attrs["autoscaling_policy"] = autoscalingPolicyString } else { attrs["autoscaling_policy"] = "" @@ -1112,10 +1153,10 @@ func flattenInstanceGroups(igs []*emr.InstanceGroup) (*schema.Set, error) { if attrs["name"] != nil { attrs["name"] = *ig.Name } - instanceGroupSet.Add(attrs) + instanceGroupSet = append(instanceGroupSet, attrs) } - return &instanceGroupSet, nil + return instanceGroupSet, nil } func flattenEBSConfig(ebsBlockDevices []*emr.EbsBlockDevice) []map[string]interface{} { @@ -1532,6 +1573,7 @@ func findMasterGroup(instanceGroups []*emr.InstanceGroup) *emr.InstanceGroup { return nil } +/* // EMRCluster always has an instance role of either master, core, or task // Name is optional for core and master(only group allowed for this type) but needed for task // since you can have multiple task instance groups. @@ -1543,4 +1585,4 @@ func resourceAwsEMRClusterInstanceGroupHash(v interface{}) int { buf.WriteString(fmt.Sprintf("%s", m["name"].(string))) } return hashcode.String(buf.String()) -} +}*/ diff --git a/aws/resource_aws_emr_cluster_migrate.go b/aws/resource_aws_emr_cluster_migrate.go index ff74f48337ec..a48a5e49e954 100644 --- a/aws/resource_aws_emr_cluster_migrate.go +++ b/aws/resource_aws_emr_cluster_migrate.go @@ -69,15 +69,9 @@ func migrateEMRClusterStateV0toV1(is *terraform.InstanceState) (*terraform.Insta for k, v := range newEBSConfigKeys { is.Attributes[k] = v - } log.Printf("[DEBUG] EMR Cluster Attributes after migration: %#v", is.Attributes) return is, nil } - -func updateV0ToV1EMRClusterEBSConfig(is *terraform.InstanceState) error { - - return nil -} diff --git a/aws/resource_aws_emr_cluster_migrate_test.go b/aws/resource_aws_emr_cluster_migrate_test.go index c0b30c04459e..b19942b63cd8 100644 --- a/aws/resource_aws_emr_cluster_migrate_test.go +++ b/aws/resource_aws_emr_cluster_migrate_test.go @@ -71,7 +71,7 @@ func TestAwsEMRClusterMigrateState_empty(t *testing.T) { // should handle non-nil but empty is = &terraform.InstanceState{} - is, err = resourceAwsEMRClusterMigrateState(0, is, meta) + _, err = resourceAwsEMRClusterMigrateState(0, is, meta) if err != nil { t.Fatalf("err: %#v", err) From a381f6b9d9f9f2f8865121c45ffa2a0b493a1d04 Mon Sep 17 00:00:00 2001 From: Matthew Frahry Date: Wed, 2 Jan 2019 10:42:53 -0700 Subject: [PATCH 14/20] adding customize diff --- aws/resource_aws_emr_cluster.go | 287 ++++++++++++++++++--------- aws/resource_aws_emr_cluster_test.go | 32 --- 2 files changed, 194 insertions(+), 125 deletions(-) diff --git a/aws/resource_aws_emr_cluster.go b/aws/resource_aws_emr_cluster.go index bf7949754888..d0dab41d66ea 100644 --- a/aws/resource_aws_emr_cluster.go +++ b/aws/resource_aws_emr_cluster.go @@ -1,6 +1,7 @@ package aws import ( + "bytes" "log" "encoding/json" @@ -14,6 +15,7 @@ import ( "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/private/protocol/json/jsonutil" "github.com/aws/aws-sdk-go/service/emr" + "github.com/hashicorp/terraform/helper/hashcode" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/helper/structure" @@ -29,8 +31,74 @@ func resourceAwsEMRCluster() *schema.Resource { Importer: &schema.ResourceImporter{ State: schema.ImportStatePassthrough, }, - SchemaVersion: 1, - MigrateState: resourceAwsEMRClusterMigrateState, + + // Write a customizeDiff to look at the instance groups and determin if an ebs_config is going + // from 0->1 and return computed?? Maybe? + CustomizeDiff: func(diff *schema.ResourceDiff, v interface{}) error { + if diff.HasChange("instance_group") { + o, n := diff.GetChange("instance_group") + oSet := o.(*schema.Set).List() + nSet := n.(*schema.Set).List() + + /* + if len(oSet) == len(nSet) { + for i, instanceGroup := range oSet { + oInstanceGroup := instanceGroup.(map[string]interface{}) + nInstanceGroup := nSet[i].(map[string]interface{}) + + oEBSConfig := oInstanceGroup["ebs_config"] + oEBSAttrs := oEBSConfig.(*schema.Set).List() + + nEBSConfig := nInstanceGroup["ebs_config"] + nEBSAttrs := nEBSConfig.(*schema.Set).List() + + // return fmt.Errorf("%+v", diff.GetChangedKeysPrefix(fmt.Sprintf("instance_group.%d", resourceAwsEMRClusterInstanceGroupHash(oInstanceGroup)))) + + if len(oEBSAttrs) != 1 && len(nEBSAttrs) != 0 { + for _, k := range diff.GetChangedKeysPrefix(fmt.Sprintf("instance_group.%d", resourceAwsEMRClusterInstanceGroupHash(oInstanceGroup))) { + if strings.HasSuffix(k, ".#") { + k = strings.TrimSuffix(k, ".#") + } + diff.ForceNew(k) + } + } + } + } // */ + + // Everything in instance group needs to be set to forcenew if the autoscaling policy doesn't change + if len(oSet) == len(nSet) { + for _, currInstanceGroup := range oSet { + for _, nextInstanceGroup := range nSet { + oInstanceGroup := currInstanceGroup.(map[string]interface{}) + nInstanceGroup := nextInstanceGroup.(map[string]interface{}) + + if oInstanceGroup["instance_role"].(string) == nInstanceGroup["instance_role"].(string) && oInstanceGroup["name"].(string) == nInstanceGroup["name"].(string) { + + oAutoScalingPolicy := oInstanceGroup["autoscaling_policy"].(string) + nAutoScalingPolicy := nInstanceGroup["autoscaling_policy"].(string) + + if oAutoScalingPolicy != "" && nAutoScalingPolicy != "" { + oJson, _ := structure.NormalizeJsonString(oAutoScalingPolicy) + nJson, _ := structure.NormalizeJsonString(nAutoScalingPolicy) + + if oJson == nJson { + for _, k := range diff.GetChangedKeysPrefix(fmt.Sprintf("instance_group.%d", resourceAwsEMRClusterInstanceGroupHash(oInstanceGroup))) { + if strings.HasSuffix(k, ".#") { + k = strings.TrimSuffix(k, ".#") + } + diff.ForceNew(k) + } + } + } + break + } + + } + } + } + } + return nil + }, Schema: map[string]*schema.Schema{ "name": { @@ -201,29 +269,22 @@ func resourceAwsEMRCluster() *schema.Resource { "instance_group": { Type: schema.TypeSet, Optional: true, - // ForceNew: true, + ForceNew: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "bid_price": { Type: schema.TypeString, Optional: true, - // ForceNew: true, }, "ebs_config": { - Type: schema.TypeList, + Type: schema.TypeSet, Optional: true, Computed: true, - // ForceNew: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ - "name": { - Type: schema.TypeString, - Computed: true, - }, "iops": { Type: schema.TypeInt, Optional: true, - Computed: true, }, "size": { Type: schema.TypeInt, @@ -241,13 +302,12 @@ func resourceAwsEMRCluster() *schema.Resource { }, }, }, + Set: resourceAwsEMRClusterEBSConfigHash, }, "instance_count": { Type: schema.TypeInt, Optional: true, Default: 0, - // TODO: Add the ability to update this attribute - //ForceNew: true, }, "autoscaling_policy": { Type: schema.TypeString, @@ -262,7 +322,6 @@ func resourceAwsEMRCluster() *schema.Resource { "instance_role": { Type: schema.TypeString, Required: true, - //ForceNew: true, ValidateFunc: validation.StringInSlice([]string{ emr.InstanceFleetTypeMaster, emr.InstanceFleetTypeCore, @@ -272,16 +331,14 @@ func resourceAwsEMRCluster() *schema.Resource { "instance_type": { Type: schema.TypeString, Required: true, - //ForceNew: true, }, "name": { Type: schema.TypeString, Optional: true, - ForceNew: true, }, }, }, - // Set: resourceAwsEMRClusterInstanceGroupHash, + Set: resourceAwsEMRClusterInstanceGroupHash, }, "bootstrap_action": { Type: schema.TypeSet, @@ -704,6 +761,7 @@ func resourceAwsEMRClusterRead(d *schema.ResourceData, meta interface{}) error { if masterGroup != nil { d.Set("master_instance_type", masterGroup.InstanceType) } + flattenedInstanceGroups, err := flattenInstanceGroups(instanceGroups) if err != nil { return fmt.Errorf("error flattening instance groups: %+v", err) @@ -1081,93 +1139,101 @@ func flattenEmrStepSummary(stepSummary *emr.StepSummary) map[string]interface{} return m } -func flattenInstanceGroups(igs []*emr.InstanceGroup) ([]map[string]interface{}, error) { - instanceGroupSet := make([]map[string]interface{}, 0) - for _, ig := range igs { - attrs := map[string]interface{}{} - if ig.BidPrice != nil { - attrs["bid_price"] = *ig.BidPrice - } - - attrs["ebs_config"] = flattenEBSConfig(ig.EbsBlockDevices) - attrs["instance_count"] = *ig.RequestedInstanceCount - attrs["instance_role"] = *ig.InstanceGroupType - attrs["instance_type"] = *ig.InstanceType - - if ig.AutoScalingPolicy != nil { - // AutoScalingPolicy has an additional Status field that is causing diffs in the statefile. - // We are purposefully omitting that field here when we generate the autoscaling policy string - // for the statefile. - - for i, rule := range ig.AutoScalingPolicy.Rules { - for j, dimension := range rule.Trigger.CloudWatchAlarmDefinition.Dimensions { - if *dimension.Key == "JobFlowId" { - tmpDimensions := append(ig.AutoScalingPolicy.Rules[i].Trigger.CloudWatchAlarmDefinition.Dimensions[:j], ig.AutoScalingPolicy.Rules[i].Trigger.CloudWatchAlarmDefinition.Dimensions[j+1:]...) - ig.AutoScalingPolicy.Rules[i].Trigger.CloudWatchAlarmDefinition.Dimensions = tmpDimensions - } - } - if len(ig.AutoScalingPolicy.Rules[i].Trigger.CloudWatchAlarmDefinition.Dimensions) == 0 { - ig.AutoScalingPolicy.Rules[i].Trigger.CloudWatchAlarmDefinition.Dimensions = nil - } - } +func flattenInstanceGroup(ig *emr.InstanceGroup) (map[string]interface{}, error) { + attrs := map[string]interface{}{} + if ig.BidPrice != nil { + attrs["bid_price"] = *ig.BidPrice + } - tmpAutoScalingPolicy := emr.AutoScalingPolicy{ - Constraints: ig.AutoScalingPolicy.Constraints, - Rules: ig.AutoScalingPolicy.Rules, - } - autoscalingPolicyConstraintsBytes, err := json.Marshal(tmpAutoScalingPolicy.Constraints) - if err != nil { - return nil, fmt.Errorf("error parsing EMR Cluster Instance Groups AutoScalingPolicy Constraints: %s", err) - } - autoscalingPolicyConstraintsString := string(autoscalingPolicyConstraintsBytes) + attrs["ebs_config"] = flattenEBSConfig(ig.EbsBlockDevices) + attrs["instance_count"] = int(*ig.RequestedInstanceCount) + attrs["instance_role"] = *ig.InstanceGroupType + attrs["instance_type"] = *ig.InstanceType - autoscalingPolicyRulesBytes, err := json.Marshal(tmpAutoScalingPolicy.Rules) - if err != nil { - return nil, fmt.Errorf("error parsing EMR Cluster Instance Groups AutoScalingPolicy Rules: %s", err) - } + if ig.AutoScalingPolicy != nil { + // AutoScalingPolicy has an additional Status field that is causing diffs in the statefile. + // We are purposefully omitting that field here when we generate the autoscaling policy string + // for the statefile. - var rules []map[string]interface{} - if err := json.Unmarshal(autoscalingPolicyRulesBytes, &rules); err != nil { - return nil, err + for i, rule := range ig.AutoScalingPolicy.Rules { + for j, dimension := range rule.Trigger.CloudWatchAlarmDefinition.Dimensions { + if *dimension.Key == "JobFlowId" { + tmpDimensions := append(ig.AutoScalingPolicy.Rules[i].Trigger.CloudWatchAlarmDefinition.Dimensions[:j], ig.AutoScalingPolicy.Rules[i].Trigger.CloudWatchAlarmDefinition.Dimensions[j+1:]...) + ig.AutoScalingPolicy.Rules[i].Trigger.CloudWatchAlarmDefinition.Dimensions = tmpDimensions + } } - - var cleanRules []map[string]interface{} - for _, rule := range rules { - cleanRules = append(cleanRules, removeNil(rule)) + if len(ig.AutoScalingPolicy.Rules[i].Trigger.CloudWatchAlarmDefinition.Dimensions) == 0 { + ig.AutoScalingPolicy.Rules[i].Trigger.CloudWatchAlarmDefinition.Dimensions = nil } + } - withoutNulls, err := json.Marshal(cleanRules) - if err != nil { - return nil, err - } + tmpAutoScalingPolicy := emr.AutoScalingPolicy{ + Constraints: ig.AutoScalingPolicy.Constraints, + Rules: ig.AutoScalingPolicy.Rules, + } + autoscalingPolicyConstraintsBytes, err := json.Marshal(tmpAutoScalingPolicy.Constraints) + if err != nil { + return nil, fmt.Errorf("error parsing EMR Cluster Instance Groups AutoScalingPolicy Constraints: %s", err) + } + autoscalingPolicyConstraintsString := string(autoscalingPolicyConstraintsBytes) - autoscalingPolicyRulesString := string(withoutNulls) + autoscalingPolicyRulesBytes, err := json.Marshal(tmpAutoScalingPolicy.Rules) + if err != nil { + return nil, fmt.Errorf("error parsing EMR Cluster Instance Groups AutoScalingPolicy Rules: %s", err) + } - autoscalingPolicyString := fmt.Sprintf("{\"Constraints\":%s,\"Rules\":%s}", autoscalingPolicyConstraintsString, autoscalingPolicyRulesString) + var rules []map[string]interface{} + if err := json.Unmarshal(autoscalingPolicyRulesBytes, &rules); err != nil { + return nil, err + } - attrs["autoscaling_policy"] = autoscalingPolicyString - } else { - attrs["autoscaling_policy"] = "" + var cleanRules []map[string]interface{} + for _, rule := range rules { + cleanRules = append(cleanRules, removeNil(rule)) } - if attrs["name"] != nil { - attrs["name"] = *ig.Name + withoutNulls, err := json.Marshal(cleanRules) + if err != nil { + return nil, err + } + + autoscalingPolicyRulesString := string(withoutNulls) + + autoscalingPolicyString := fmt.Sprintf("{\"Constraints\":%s,\"Rules\":%s}", autoscalingPolicyConstraintsString, autoscalingPolicyRulesString) + + attrs["autoscaling_policy"] = autoscalingPolicyString + } else { + attrs["autoscaling_policy"] = "" + } + + if attrs["name"] != nil { + attrs["name"] = *ig.Name + } + return attrs, nil +} + +func flattenInstanceGroups(igs []*emr.InstanceGroup) (*schema.Set, error) { + instanceGroupSet := []interface{}{} + for _, ig := range igs { + flattenedInstanceGroup, err := flattenInstanceGroup(ig) + if err != nil { + return nil, err } - instanceGroupSet = append(instanceGroupSet, attrs) + instanceGroupSet = append(instanceGroupSet, flattenedInstanceGroup) } - return instanceGroupSet, nil + return schema.NewSet(resourceAwsEMRClusterInstanceGroupHash, instanceGroupSet), nil } -func flattenEBSConfig(ebsBlockDevices []*emr.EbsBlockDevice) []map[string]interface{} { - ebsConfig := make([]map[string]interface{}, 0) +func flattenEBSConfig(ebsBlockDevices []*emr.EbsBlockDevice) *schema.Set { + ebsConfig := make([]interface{}, 0) for _, ebs := range ebsBlockDevices { ebsAttrs := make(map[string]interface{}) if ebs.VolumeSpecification.Iops != nil { - ebsAttrs["iops"] = *ebs.VolumeSpecification.Iops + ebsAttrs["iops"] = int(*ebs.VolumeSpecification.Iops) } if ebs.VolumeSpecification.SizeInGB != nil { - ebsAttrs["size"] = *ebs.VolumeSpecification.SizeInGB + ebsAttrs["size"] = int(*ebs.VolumeSpecification.SizeInGB) } if ebs.VolumeSpecification.VolumeType != nil { ebsAttrs["type"] = *ebs.VolumeSpecification.VolumeType @@ -1176,7 +1242,8 @@ func flattenEBSConfig(ebsBlockDevices []*emr.EbsBlockDevice) []map[string]interf ebsConfig = append(ebsConfig, ebsAttrs) } - return ebsConfig + + return schema.NewSet(resourceAwsEMRClusterEBSConfigHash, ebsConfig) } func flattenBootstrapArguments(actions []*emr.Command) []map[string]interface{} { @@ -1394,8 +1461,8 @@ func expandInstanceGroupConfigs(instanceGroupConfigs []interface{}) ([]*emr.Inst InstanceCount: aws.Int64(int64(configInstanceCount)), } - applyBidPrice(config, configAttributes) - applyEbsConfig(configAttributes, config) + expandBidPrice(config, configAttributes) + expandEbsConfig(configAttributes, config) if v, ok := configAttributes["autoscaling_policy"]; ok && v.(string) != "" { var autoScalingPolicy *emr.AutoScalingPolicy @@ -1415,7 +1482,7 @@ func expandInstanceGroupConfigs(instanceGroupConfigs []interface{}) ([]*emr.Inst return instanceGroupConfig, nil } -func applyBidPrice(config *emr.InstanceGroupConfig, configAttributes map[string]interface{}) { +func expandBidPrice(config *emr.InstanceGroupConfig, configAttributes map[string]interface{}) { if bidPrice, ok := configAttributes["bid_price"]; ok { if bidPrice != "" { config.BidPrice = aws.String(bidPrice.(string)) @@ -1426,12 +1493,12 @@ func applyBidPrice(config *emr.InstanceGroupConfig, configAttributes map[string] } } -func applyEbsConfig(configAttributes map[string]interface{}, config *emr.InstanceGroupConfig) { +func expandEbsConfig(configAttributes map[string]interface{}, config *emr.InstanceGroupConfig) { if rawEbsConfigs, ok := configAttributes["ebs_config"]; ok { ebsConfig := &emr.EbsConfiguration{} ebsBlockDeviceConfigs := make([]*emr.EbsBlockDeviceConfig, 0) - for _, rawEbsConfig := range rawEbsConfigs.([]interface{}) { + for _, rawEbsConfig := range rawEbsConfigs.(*schema.Set).List() { rawEbsConfig := rawEbsConfig.(map[string]interface{}) ebsBlockDeviceConfig := &emr.EbsBlockDeviceConfig{ VolumesPerInstance: aws.Int64(int64(rawEbsConfig["volumes_per_instance"].(int))), @@ -1573,7 +1640,6 @@ func findMasterGroup(instanceGroups []*emr.InstanceGroup) *emr.InstanceGroup { return nil } -/* // EMRCluster always has an instance role of either master, core, or task // Name is optional for core and master(only group allowed for this type) but needed for task // since you can have multiple task instance groups. @@ -1581,8 +1647,43 @@ func resourceAwsEMRClusterInstanceGroupHash(v interface{}) int { var buf bytes.Buffer m := v.(map[string]interface{}) buf.WriteString(fmt.Sprintf("%s-", m["instance_role"].(string))) - if m["name"] != nil { - buf.WriteString(fmt.Sprintf("%s", m["name"].(string))) + buf.WriteString(fmt.Sprintf("%s-", m["instance_type"].(string))) + buf.WriteString(fmt.Sprintf("%d-", m["instance_count"].(int))) + if v, ok := m["name"]; ok { + buf.WriteString(fmt.Sprintf("%s-", v.(string))) + } + if v, ok := m["bid_price"]; ok { + buf.WriteString(fmt.Sprintf("%s-", v.(string))) + } + + if v, ok := m["autoscaling_policy"]; ok { + pleaseWork, _ := structure.NormalizeJsonString(v.(string)) + buf.WriteString(fmt.Sprintf("%s-", pleaseWork)) + } + /* + + if v, ok := m["ebs_config"]; ok { + configs := v.(*schema.Set).List() + // Check the length of ebsConfigs to get around an ebs config being configured automatically when + // it isn't specified in Terraform + if len(configs) > 1 { + for _, ebsConfigs := range configs { + buf.WriteString(fmt.Sprintf("%d-", resourceAwsEMRClusterEBSConfigHash(ebsConfigs.(map[string]interface{})))) + } + } + }*/ + + return hashcode.String(buf.String()) +} + +func resourceAwsEMRClusterEBSConfigHash(v interface{}) int { + var buf bytes.Buffer + m := v.(map[string]interface{}) + buf.WriteString(fmt.Sprintf("%d-", m["size"].(int))) + buf.WriteString(fmt.Sprintf("%s-", m["type"].(string))) + buf.WriteString(fmt.Sprintf("%d-", m["volumes_per_instance"].(int))) + if v, ok := m["iops"]; ok && v.(int) != 0 { + buf.WriteString(fmt.Sprintf("%d-", v.(int))) } return hashcode.String(buf.String()) -}*/ +} diff --git a/aws/resource_aws_emr_cluster_test.go b/aws/resource_aws_emr_cluster_test.go index 45afe953356b..a4c0475f8c6e 100644 --- a/aws/resource_aws_emr_cluster_test.go +++ b/aws/resource_aws_emr_cluster_test.go @@ -719,8 +719,6 @@ resource "aws_emr_cluster" "test" { 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}" } @@ -946,36 +944,6 @@ resource "aws_main_route_table_association" "a" { route_table_id = "${aws_route_table.r.id}" } -resource "aws_security_group" "allow_all" { - name = "allow_all" - description = "Allow all inbound traffic" - vpc_id = "${aws_vpc.main.id}" - - ingress { - from_port = 0 - to_port = 0 - protocol = "-1" - cidr_blocks = ["0.0.0.0/0"] - } - - egress { - from_port = 0 - to_port = 0 - protocol = "-1" - cidr_blocks = ["0.0.0.0/0"] - } - - depends_on = ["aws_subnet.main"] - - lifecycle { - ignore_changes = ["ingress", "egress"] - } - - tags { - Name = "emr_test" - } -} - output "cluser_id" { value = "${aws_emr_cluster.test.id}" } From c2c9d2ad7dc4570ac7eab28bbd1e9d62af316981 Mon Sep 17 00:00:00 2001 From: Matthew Frahry Date: Thu, 3 Jan 2019 14:50:34 -0700 Subject: [PATCH 15/20] Fixing tests --- aws/resource_aws_emr_cluster.go | 3 +++ aws/resource_aws_emr_cluster_test.go | 30 ++++++++++++++-------------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/aws/resource_aws_emr_cluster.go b/aws/resource_aws_emr_cluster.go index 16f8f08aac1e..0745c0005620 100644 --- a/aws/resource_aws_emr_cluster.go +++ b/aws/resource_aws_emr_cluster.go @@ -209,11 +209,13 @@ func resourceAwsEMRCluster() *schema.Resource { Type: schema.TypeString, Optional: true, ForceNew: true, + Computed: true, }, "emr_managed_slave_security_group": { Type: schema.TypeString, Optional: true, ForceNew: true, + Computed: true, }, "instance_profile": { Type: schema.TypeString, @@ -270,6 +272,7 @@ func resourceAwsEMRCluster() *schema.Resource { Type: schema.TypeSet, Optional: true, ForceNew: true, + Computed: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "bid_price": { diff --git a/aws/resource_aws_emr_cluster_test.go b/aws/resource_aws_emr_cluster_test.go index a4c0475f8c6e..8ce46fb11698 100644 --- a/aws/resource_aws_emr_cluster_test.go +++ b/aws/resource_aws_emr_cluster_test.go @@ -1025,7 +1025,7 @@ resource "aws_security_group" "allow_all" { from_port = 0 to_port = 0 protocol = "-1" - cidr_blocks = ["0.0.0.0/0"] + self = true } egress { @@ -1338,7 +1338,7 @@ resource "aws_security_group" "allow_all" { from_port = 0 to_port = 0 protocol = "-1" - cidr_blocks = ["0.0.0.0/0"] + self = true } egress { @@ -1661,7 +1661,7 @@ resource "aws_security_group" "allow_all" { from_port = 0 to_port = 0 protocol = "-1" - cidr_blocks = ["0.0.0.0/0"] + self = true } egress { @@ -1949,7 +1949,7 @@ resource "aws_security_group" "allow_all" { from_port = 0 to_port = 0 protocol = "-1" - cidr_blocks = ["0.0.0.0/0"] + self = true } egress { @@ -2071,7 +2071,7 @@ resource "aws_security_group" "allow_all" { from_port = 0 to_port = 0 protocol = "-1" - cidr_blocks = ["0.0.0.0/0"] + self = true } egress { @@ -2445,7 +2445,7 @@ resource "aws_security_group" "allow_all" { from_port = 0 to_port = 0 protocol = "-1" - cidr_blocks = ["0.0.0.0/0"] + self = true } egress { @@ -2609,7 +2609,7 @@ resource "aws_security_group" "allow_all" { from_port = 0 to_port = 0 protocol = "-1" - cidr_blocks = ["0.0.0.0/0"] + self = true } egress { @@ -2959,7 +2959,7 @@ resource "aws_security_group" "allow_all" { from_port = 0 to_port = 0 protocol = "-1" - cidr_blocks = ["0.0.0.0/0"] + self = true } egress { @@ -3307,7 +3307,7 @@ resource "aws_security_group" "allow_all" { from_port = 0 to_port = 0 protocol = "-1" - cidr_blocks = ["0.0.0.0/0"] + self = true } egress { @@ -3609,7 +3609,7 @@ resource "aws_security_group" "allow_all" { from_port = 0 to_port = 0 protocol = "-1" - cidr_blocks = ["0.0.0.0/0"] + self = true } egress { @@ -3922,7 +3922,7 @@ resource "aws_security_group" "allow_all" { from_port = 0 to_port = 0 protocol = "-1" - cidr_blocks = ["0.0.0.0/0"] + self = true } egress { @@ -4229,7 +4229,7 @@ resource "aws_security_group" "allow_all" { from_port = 0 to_port = 0 protocol = "-1" - cidr_blocks = ["0.0.0.0/0"] + self = true } egress { @@ -4531,7 +4531,7 @@ resource "aws_security_group" "allow_all" { from_port = 0 to_port = 0 protocol = "-1" - cidr_blocks = ["0.0.0.0/0"] + self = true } egress { @@ -4839,7 +4839,7 @@ resource "aws_security_group" "allow_all" { from_port = 0 to_port = 0 protocol = "-1" - cidr_blocks = ["0.0.0.0/0"] + self = true } egress { @@ -5233,7 +5233,7 @@ resource "aws_security_group" "allow_all" { from_port = 0 to_port = 0 protocol = "-1" - cidr_blocks = ["0.0.0.0/0"] + self = true } egress { From c352a2dcae441553ba0f3365b4626717fa23935d Mon Sep 17 00:00:00 2001 From: Matthew Frahry Date: Sun, 6 Jan 2019 08:33:28 -0700 Subject: [PATCH 16/20] Finalizing tests --- aws/resource_aws_emr_cluster.go | 2 - aws/resource_aws_emr_cluster_test.go | 61 +++++++++++++--------------- 2 files changed, 28 insertions(+), 35 deletions(-) diff --git a/aws/resource_aws_emr_cluster.go b/aws/resource_aws_emr_cluster.go index 0745c0005620..0b50282310b4 100644 --- a/aws/resource_aws_emr_cluster.go +++ b/aws/resource_aws_emr_cluster.go @@ -209,13 +209,11 @@ func resourceAwsEMRCluster() *schema.Resource { Type: schema.TypeString, Optional: true, ForceNew: true, - Computed: true, }, "emr_managed_slave_security_group": { Type: schema.TypeString, Optional: true, ForceNew: true, - Computed: true, }, "instance_profile": { Type: schema.TypeString, diff --git a/aws/resource_aws_emr_cluster_test.go b/aws/resource_aws_emr_cluster_test.go index 8ce46fb11698..d9b682c7eae2 100644 --- a/aws/resource_aws_emr_cluster_test.go +++ b/aws/resource_aws_emr_cluster_test.go @@ -713,26 +713,22 @@ resource "aws_emr_cluster" "test" { core_instance_type = "c4.large" 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", @@ -747,15 +743,12 @@ resource "aws_emr_cluster" "test" { }, ] } - 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 = < Date: Sun, 6 Jan 2019 19:49:25 -0700 Subject: [PATCH 17/20] Additional tests and website additions --- aws/resource_aws_emr_cluster.go | 80 +++-- aws/resource_aws_emr_cluster_test.go | 378 +++++++++++++++++++++++ website/docs/r/emr_cluster.html.markdown | 2 + 3 files changed, 430 insertions(+), 30 deletions(-) diff --git a/aws/resource_aws_emr_cluster.go b/aws/resource_aws_emr_cluster.go index 0b50282310b4..e1d795651d1b 100644 --- a/aws/resource_aws_emr_cluster.go +++ b/aws/resource_aws_emr_cluster.go @@ -32,39 +32,12 @@ func resourceAwsEMRCluster() *schema.Resource { State: schema.ImportStatePassthrough, }, - // Write a customizeDiff to look at the instance groups and determin if an ebs_config is going - // from 0->1 and return computed?? Maybe? CustomizeDiff: func(diff *schema.ResourceDiff, v interface{}) error { if diff.HasChange("instance_group") { o, n := diff.GetChange("instance_group") oSet := o.(*schema.Set).List() nSet := n.(*schema.Set).List() - /* - if len(oSet) == len(nSet) { - for i, instanceGroup := range oSet { - oInstanceGroup := instanceGroup.(map[string]interface{}) - nInstanceGroup := nSet[i].(map[string]interface{}) - - oEBSConfig := oInstanceGroup["ebs_config"] - oEBSAttrs := oEBSConfig.(*schema.Set).List() - - nEBSConfig := nInstanceGroup["ebs_config"] - nEBSAttrs := nEBSConfig.(*schema.Set).List() - - // return fmt.Errorf("%+v", diff.GetChangedKeysPrefix(fmt.Sprintf("instance_group.%d", resourceAwsEMRClusterInstanceGroupHash(oInstanceGroup)))) - - if len(oEBSAttrs) != 1 && len(nEBSAttrs) != 0 { - for _, k := range diff.GetChangedKeysPrefix(fmt.Sprintf("instance_group.%d", resourceAwsEMRClusterInstanceGroupHash(oInstanceGroup))) { - if strings.HasSuffix(k, ".#") { - k = strings.TrimSuffix(k, ".#") - } - diff.ForceNew(k) - } - } - } - } */ - // Everything in instance group needs to be set to forcenew if the autoscaling policy doesn't change if len(oSet) == len(nSet) { for _, currInstanceGroup := range oSet { @@ -337,6 +310,10 @@ func resourceAwsEMRCluster() *schema.Resource { Type: schema.TypeString, Optional: true, }, + "id": { + Type: schema.TypeString, + Computed: true, + }, }, }, Set: resourceAwsEMRClusterInstanceGroupHash, @@ -936,6 +913,47 @@ func resourceAwsEMRClusterUpdate(d *schema.ResourceData, meta interface{}) error } } + if d.HasChange("instance_group") { + o, n := d.GetChange("instance_group") + oSet := o.(*schema.Set).List() + nSet := n.(*schema.Set).List() + for _, currInstanceGroup := range oSet { + for _, nextInstanceGroup := range nSet { + oInstanceGroup := currInstanceGroup.(map[string]interface{}) + nInstanceGroup := nextInstanceGroup.(map[string]interface{}) + + if oInstanceGroup["instance_role"].(string) == nInstanceGroup["instance_role"].(string) && oInstanceGroup["name"].(string) == nInstanceGroup["name"].(string) { + + if v, ok := nInstanceGroup["autoscaling_policy"]; ok && v.(string) != "" { + var autoScalingPolicy *emr.AutoScalingPolicy + + err := json.Unmarshal([]byte(v.(string)), &autoScalingPolicy) + + if err != nil { + return fmt.Errorf("error parsing EMR Auto Scaling Policy JSON for update: \n\n%s\n\n%s", v.(string), err) + } + + // TODO Confirm that the id for instance group is obtainable from the schema. If not, the user needs to run a terraform refresh + putAutoScalingPolicy := &emr.PutAutoScalingPolicyInput{ + ClusterId: aws.String(d.Id()), + AutoScalingPolicy: autoScalingPolicy, + InstanceGroupId: aws.String(oInstanceGroup["id"].(string)), + } + + _, errModify := conn.PutAutoScalingPolicy(putAutoScalingPolicy) + if errModify != nil { + log.Printf("[ERROR] %s", errModify) + return errModify + } + + break + } + } + + } + } + } + if err := setTagsEMR(conn, d); err != nil { return err } else { @@ -1146,16 +1164,17 @@ func flattenInstanceGroup(ig *emr.InstanceGroup) (map[string]interface{}, error) attrs["bid_price"] = *ig.BidPrice } + attrs["id"] = *ig.Id attrs["ebs_config"] = flattenEBSConfig(ig.EbsBlockDevices) attrs["instance_count"] = int(*ig.RequestedInstanceCount) attrs["instance_role"] = *ig.InstanceGroupType attrs["instance_type"] = *ig.InstanceType if ig.AutoScalingPolicy != nil { - // AutoScalingPolicy has an additional Status field that is causing diffs in the statefile. - // We are purposefully omitting that field here when we generate the autoscaling policy string + // AutoScalingPolicy has an additional Status field that is causing a new hashcode to be generated + // for `instance_group`. + // We are purposefully omitting that field here when we flatten the autoscaling policy string // for the statefile. - for i, rule := range ig.AutoScalingPolicy.Rules { for j, dimension := range rule.Trigger.CloudWatchAlarmDefinition.Dimensions { if *dimension.Key == "JobFlowId" { @@ -1210,6 +1229,7 @@ func flattenInstanceGroup(ig *emr.InstanceGroup) (map[string]interface{}, error) if attrs["name"] != nil { attrs["name"] = *ig.Name } + return attrs, nil } diff --git a/aws/resource_aws_emr_cluster_test.go b/aws/resource_aws_emr_cluster_test.go index d9b682c7eae2..bd0c63f1eecf 100644 --- a/aws/resource_aws_emr_cluster_test.go +++ b/aws/resource_aws_emr_cluster_test.go @@ -179,6 +179,36 @@ func TestAccAWSEMRCluster_instance_group_EBSVolumeType_st1(t *testing.T) { }) } +func TestAccAWSEMRCluster_updateAutoScalingPolicy(t *testing.T) { + var cluster emr.Cluster + r := acctest.RandInt() + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSEmrDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSEmrClusterConfigInstanceGroups_st1(r), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSEmrClusterExists("aws_emr_cluster.tf-test-cluster", &cluster), + ), + }, + { + Config: testAccAWSEmrClusterConfigInstanceGroups_updateAutoScalingPolicy(r), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSEmrClusterExists("aws_emr_cluster.tf-test-cluster", &cluster), + ), + }, + { + ResourceName: "aws_emr_cluster.tf-test-cluster", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"configurations", "keep_job_flow_alive_when_no_steps"}, + }, + }, + }) +} + func TestAccAWSEMRCluster_Kerberos_ClusterDedicatedKdc(t *testing.T) { var cluster emr.Cluster r := acctest.RandInt() @@ -3553,6 +3583,354 @@ resource "aws_iam_role_policy_attachment" "emr-autoscaling-role" { `, r) } +func testAccAWSEmrClusterConfigInstanceGroups_updateAutoScalingPolicy(r int) string { + return fmt.Sprintf(` +resource "aws_emr_cluster" "tf-test-cluster" { + name = "emr-test-%[1]d" + release_label = "emr-4.6.0" + applications = ["Spark"] + + 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}" + } + + instance_group = [ + { + instance_role = "CORE" + instance_type = "c4.large" + instance_count = "1" + ebs_config { + size = "500" + type = "st1" + volumes_per_instance = 1 + } + bid_price = "0.30" + autoscaling_policy = < Date: Sun, 6 Jan 2019 20:14:57 -0700 Subject: [PATCH 18/20] Removing additional files and extraneous comments --- aws/resource_aws_emr_cluster.go | 13 ++-- aws/resource_aws_emr_cluster_migrate.go | 77 ------------------- aws/resource_aws_emr_cluster_migrate_test.go | 79 -------------------- 3 files changed, 5 insertions(+), 164 deletions(-) delete mode 100644 aws/resource_aws_emr_cluster_migrate.go delete mode 100644 aws/resource_aws_emr_cluster_migrate_test.go diff --git a/aws/resource_aws_emr_cluster.go b/aws/resource_aws_emr_cluster.go index e1d795651d1b..993c7b2e9e00 100644 --- a/aws/resource_aws_emr_cluster.go +++ b/aws/resource_aws_emr_cluster.go @@ -928,12 +928,10 @@ func resourceAwsEMRClusterUpdate(d *schema.ResourceData, meta interface{}) error var autoScalingPolicy *emr.AutoScalingPolicy err := json.Unmarshal([]byte(v.(string)), &autoScalingPolicy) - if err != nil { return fmt.Errorf("error parsing EMR Auto Scaling Policy JSON for update: \n\n%s\n\n%s", v.(string), err) } - // TODO Confirm that the id for instance group is obtainable from the schema. If not, the user needs to run a terraform refresh putAutoScalingPolicy := &emr.PutAutoScalingPolicyInput{ ClusterId: aws.String(d.Id()), AutoScalingPolicy: autoScalingPolicy, @@ -942,8 +940,7 @@ func resourceAwsEMRClusterUpdate(d *schema.ResourceData, meta interface{}) error _, errModify := conn.PutAutoScalingPolicy(putAutoScalingPolicy) if errModify != nil { - log.Printf("[ERROR] %s", errModify) - return errModify + return fmt.Errorf("error updating autoscaling policy for instance group %q: %s", oInstanceGroup["id"].(string), errModify) } break @@ -1171,9 +1168,9 @@ func flattenInstanceGroup(ig *emr.InstanceGroup) (map[string]interface{}, error) attrs["instance_type"] = *ig.InstanceType if ig.AutoScalingPolicy != nil { - // AutoScalingPolicy has an additional Status field that is causing a new hashcode to be generated + // AutoScalingPolicy has an additional Status field and null values that are causing a new hashcode to be generated // for `instance_group`. - // We are purposefully omitting that field here when we flatten the autoscaling policy string + // We are purposefully omitting that field and the null values here when we flatten the autoscaling policy string // for the statefile. for i, rule := range ig.AutoScalingPolicy.Rules { for j, dimension := range rule.Trigger.CloudWatchAlarmDefinition.Dimensions { @@ -1685,8 +1682,8 @@ func resourceAwsEMRClusterInstanceGroupHash(v interface{}) int { if v, ok := m["ebs_config"]; ok { configs := v.(*schema.Set).List() - // Check the length of ebsConfigs to get around an ebs config being configured automatically when - // it isn't specified in Terraform + // There is an issue where an `ebs_config` is automatically configured when not specified in Terraform and + // this causes the hashcode to change. Instead, we'll ignore that configuration when setting up the hashcode. if len(configs) > 1 { for _, ebsConfigs := range configs { buf.WriteString(fmt.Sprintf("%d-", resourceAwsEMRClusterEBSConfigHash(ebsConfigs.(map[string]interface{})))) diff --git a/aws/resource_aws_emr_cluster_migrate.go b/aws/resource_aws_emr_cluster_migrate.go deleted file mode 100644 index a48a5e49e954..000000000000 --- a/aws/resource_aws_emr_cluster_migrate.go +++ /dev/null @@ -1,77 +0,0 @@ -package aws - -import ( - "fmt" - "log" - "strconv" - "strings" - - "github.com/hashicorp/terraform/terraform" -) - -func resourceAwsEMRClusterMigrateState( - v int, is *terraform.InstanceState, meta interface{}) (*terraform.InstanceState, error) { - switch v { - case 0: - log.Println("[INFO] Found AWS EMR Cluster State v0; migrating to v1") - return migrateEMRClusterStateV0toV1(is) - default: - return is, fmt.Errorf("Unexpected schema version: %d", v) - } -} - -func migrateEMRClusterStateV0toV1(is *terraform.InstanceState) (*terraform.InstanceState, error) { - if is.Empty() || is.Attributes == nil { - log.Println("[DEBUG] Empty EMR Cluster State; nothing to migrate.") - return is, nil - } - - ebsConfigCount := 0 - newEBSConfigKeys := make(map[string]string) - ebsConfigMapHashToInt := make(map[int]int) - keys := make([]string, len(is.Attributes)) - for k := range is.Attributes { - keys = append(keys, k) - } - for _, k := range keys { - if !strings.HasPrefix(k, "instance_group.") { - continue - } - - if !strings.Contains(k, "ebs_config") { - continue - } - - if strings.HasSuffix(k, "ebs_config.#") { - continue - } - - kParts := strings.Split(k, ".") - if len(kParts) != 5 { - return nil, fmt.Errorf("migration error: found `instance_group` key in unexpected format: %s", k) - } - instanceGroupHash, err := strconv.Atoi(kParts[1]) - if err != nil { - return nil, fmt.Errorf("migration error: found `instance_group` hash in unexpected format: %s", kParts[1]) - } - ebsConfigHash, err := strconv.Atoi(kParts[3]) - if err != nil { - return nil, fmt.Errorf("migration error: found `ebs_config` hash in unexpected format: %s", kParts[3]) - } - if _, ok := ebsConfigMapHashToInt[ebsConfigHash]; !ok { - ebsConfigMapHashToInt[ebsConfigHash] = ebsConfigCount - ebsConfigCount++ - } - newKey := fmt.Sprintf("instance_group.%d.ebs_config.%d.%s", instanceGroupHash, ebsConfigMapHashToInt[ebsConfigHash], kParts[4]) - newEBSConfigKeys[newKey] = is.Attributes[k] - delete(is.Attributes, k) - } - - for k, v := range newEBSConfigKeys { - is.Attributes[k] = v - } - - log.Printf("[DEBUG] EMR Cluster Attributes after migration: %#v", is.Attributes) - - return is, nil -} diff --git a/aws/resource_aws_emr_cluster_migrate_test.go b/aws/resource_aws_emr_cluster_migrate_test.go deleted file mode 100644 index b19942b63cd8..000000000000 --- a/aws/resource_aws_emr_cluster_migrate_test.go +++ /dev/null @@ -1,79 +0,0 @@ -package aws - -import ( - "testing" - - "github.com/hashicorp/terraform/terraform" -) - -func TestAwsEMRClusterMigrateState(t *testing.T) { - cases := map[string]struct { - StateVersion int - Attributes map[string]string - Expected map[string]string - Meta interface{} - }{ - "change `ebs_config` from set to list": { - StateVersion: 0, - Attributes: map[string]string{ - "instance_group.#": "1", - "instance_group.1693978638.ebs_config.#": "2", - "instance_group.1693978638.ebs_config.1693978638.name": "test1", - "instance_group.1693978638.ebs_config.1693978638.size": "10", - "instance_group.1693978638.ebs_config.112349887.name": "test2", - "instance_group.1693978638.ebs_config.112349887.size": "20", - }, - Expected: map[string]string{ - "instance_group.#": "1", - "instance_group.1693978638.ebs_config.#": "2", - "instance_group.1693978638.ebs_config.0.name": "test1", - "instance_group.1693978638.ebs_config.0.size": "10", - "instance_group.1693978638.ebs_config.1.name": "test2", - "instance_group.1693978638.ebs_config.1.size": "20", - }, - }, - } - for tn, tc := range cases { - is := &terraform.InstanceState{ - ID: "i-abc123", - Attributes: tc.Attributes, - } - is, err := resourceAwsEMRClusterMigrateState( - tc.StateVersion, is, tc.Meta) - - if err != nil { - t.Fatalf("bad: %s, err: %#v", tn, err) - } - - for k, v := range tc.Expected { - if is.Attributes[k] != v { - t.Fatalf( - "bad: %s\n\n expected: %#v -> %#v\n got: %#v -> %#v\n in: %#v", - tn, k, v, k, is.Attributes[k], is.Attributes) - } - } - } -} - -func TestAwsEMRClusterMigrateState_empty(t *testing.T) { - var is *terraform.InstanceState - var meta interface{} - - // should handle nil - is, err := resourceAwsEMRClusterMigrateState(0, is, meta) - - if err != nil { - t.Fatalf("err: %#v", err) - } - if is != nil { - t.Fatalf("expected nil instancestate, got: %#v", is) - } - - // should handle non-nil but empty - is = &terraform.InstanceState{} - _, err = resourceAwsEMRClusterMigrateState(0, is, meta) - - if err != nil { - t.Fatalf("err: %#v", err) - } -} From 609bb1e92787a88acb64de1c2f0503f17ccc4e8c Mon Sep 17 00:00:00 2001 From: Matthew Frahry Date: Tue, 8 Jan 2019 15:05:35 -0700 Subject: [PATCH 19/20] Simplifying loops --- aws/resource_aws_emr_cluster.go | 102 ++++++++++++++++++-------------- 1 file changed, 56 insertions(+), 46 deletions(-) diff --git a/aws/resource_aws_emr_cluster.go b/aws/resource_aws_emr_cluster.go index 993c7b2e9e00..dbae261227de 100644 --- a/aws/resource_aws_emr_cluster.go +++ b/aws/resource_aws_emr_cluster.go @@ -39,34 +39,44 @@ func resourceAwsEMRCluster() *schema.Resource { nSet := n.(*schema.Set).List() // Everything in instance group needs to be set to forcenew if the autoscaling policy doesn't change - if len(oSet) == len(nSet) { - for _, currInstanceGroup := range oSet { - for _, nextInstanceGroup := range nSet { - oInstanceGroup := currInstanceGroup.(map[string]interface{}) - nInstanceGroup := nextInstanceGroup.(map[string]interface{}) - - if oInstanceGroup["instance_role"].(string) == nInstanceGroup["instance_role"].(string) && oInstanceGroup["name"].(string) == nInstanceGroup["name"].(string) { - - oAutoScalingPolicy := oInstanceGroup["autoscaling_policy"].(string) - nAutoScalingPolicy := nInstanceGroup["autoscaling_policy"].(string) - - if oAutoScalingPolicy != "" && nAutoScalingPolicy != "" { - oJson, _ := structure.NormalizeJsonString(oAutoScalingPolicy) - nJson, _ := structure.NormalizeJsonString(nAutoScalingPolicy) - - if oJson == nJson { - for _, k := range diff.GetChangedKeysPrefix(fmt.Sprintf("instance_group.%d", resourceAwsEMRClusterInstanceGroupHash(oInstanceGroup))) { - if strings.HasSuffix(k, ".#") { - k = strings.TrimSuffix(k, ".#") - } - diff.ForceNew(k) - } - } - } - break - } + if len(oSet) != len(nSet) { + return nil + } + for _, currInstanceGroup := range oSet { + for _, nextInstanceGroup := range nSet { + oInstanceGroup := currInstanceGroup.(map[string]interface{}) + nInstanceGroup := nextInstanceGroup.(map[string]interface{}) + + if oInstanceGroup["instance_role"].(string) != nInstanceGroup["instance_role"].(string) || oInstanceGroup["name"].(string) != nInstanceGroup["name"].(string) { + continue + } + + oAutoScalingPolicy := oInstanceGroup["autoscaling_policy"].(string) + nAutoScalingPolicy := nInstanceGroup["autoscaling_policy"].(string) + + if oAutoScalingPolicy == "" && nAutoScalingPolicy == "" { + continue + } + oJSON, err := structure.NormalizeJsonString(oAutoScalingPolicy) + if err != nil { + return fmt.Errorf("error reading old json value: %s", err) + } + nJSON, err := structure.NormalizeJsonString(nAutoScalingPolicy) + if err != nil { + return fmt.Errorf("error reading new json value: %s", err) + } + + if oJSON != nJSON { + continue + } + for _, k := range diff.GetChangedKeysPrefix(fmt.Sprintf("instance_group.%d", resourceAwsEMRClusterInstanceGroupHash(oInstanceGroup))) { + if strings.HasSuffix(k, ".#") { + k = strings.TrimSuffix(k, ".#") + } + diff.ForceNew(k) } + break } } } @@ -922,32 +932,33 @@ func resourceAwsEMRClusterUpdate(d *schema.ResourceData, meta interface{}) error oInstanceGroup := currInstanceGroup.(map[string]interface{}) nInstanceGroup := nextInstanceGroup.(map[string]interface{}) - if oInstanceGroup["instance_role"].(string) == nInstanceGroup["instance_role"].(string) && oInstanceGroup["name"].(string) == nInstanceGroup["name"].(string) { + if oInstanceGroup["instance_role"].(string) != nInstanceGroup["instance_role"].(string) || oInstanceGroup["name"].(string) != nInstanceGroup["name"].(string) { + continue + } - if v, ok := nInstanceGroup["autoscaling_policy"]; ok && v.(string) != "" { - var autoScalingPolicy *emr.AutoScalingPolicy + if v, ok := nInstanceGroup["autoscaling_policy"]; ok && v.(string) != "" { + var autoScalingPolicy *emr.AutoScalingPolicy - err := json.Unmarshal([]byte(v.(string)), &autoScalingPolicy) - if err != nil { - return fmt.Errorf("error parsing EMR Auto Scaling Policy JSON for update: \n\n%s\n\n%s", v.(string), err) - } - - putAutoScalingPolicy := &emr.PutAutoScalingPolicyInput{ - ClusterId: aws.String(d.Id()), - AutoScalingPolicy: autoScalingPolicy, - InstanceGroupId: aws.String(oInstanceGroup["id"].(string)), - } + err := json.Unmarshal([]byte(v.(string)), &autoScalingPolicy) + if err != nil { + return fmt.Errorf("error parsing EMR Auto Scaling Policy JSON for update: \n\n%s\n\n%s", v.(string), err) + } - _, errModify := conn.PutAutoScalingPolicy(putAutoScalingPolicy) - if errModify != nil { - return fmt.Errorf("error updating autoscaling policy for instance group %q: %s", oInstanceGroup["id"].(string), errModify) - } + putAutoScalingPolicy := &emr.PutAutoScalingPolicyInput{ + ClusterId: aws.String(d.Id()), + AutoScalingPolicy: autoScalingPolicy, + InstanceGroupId: aws.String(oInstanceGroup["id"].(string)), + } - break + _, errModify := conn.PutAutoScalingPolicy(putAutoScalingPolicy) + if errModify != nil { + return fmt.Errorf("error updating autoscaling policy for instance group %q: %s", oInstanceGroup["id"].(string), errModify) } - } + break + } } + } } @@ -1213,7 +1224,6 @@ func flattenInstanceGroup(ig *emr.InstanceGroup) (map[string]interface{}, error) if err != nil { return nil, err } - autoscalingPolicyRulesString := string(withoutNulls) autoscalingPolicyString := fmt.Sprintf("{\"Constraints\":%s,\"Rules\":%s}", autoscalingPolicyConstraintsString, autoscalingPolicyRulesString) From 8e76a7f89ab2ed51e9b1f2653e64ca3b84886970 Mon Sep 17 00:00:00 2001 From: Matthew Frahry Date: Fri, 11 Jan 2019 06:51:38 -0700 Subject: [PATCH 20/20] Adding 0.12 supports and fixing docs --- aws/resource_aws_emr_cluster_test.go | 130 +++++++++++------------ website/docs/r/emr_cluster.html.markdown | 2 +- 2 files changed, 66 insertions(+), 66 deletions(-) diff --git a/aws/resource_aws_emr_cluster_test.go b/aws/resource_aws_emr_cluster_test.go index bd0c63f1eecf..0e87b18c4f79 100644 --- a/aws/resource_aws_emr_cluster_test.go +++ b/aws/resource_aws_emr_cluster_test.go @@ -1014,7 +1014,7 @@ resource "aws_emr_cluster" "tf-test-cluster" { core_instance_type = "c4.large" core_instance_count = 1 - tags { + tags = { role = "rolename" dns_zone = "env_zone" env = "env" @@ -1066,7 +1066,7 @@ resource "aws_security_group" "allow_all" { ignore_changes = ["ingress", "egress"] } - tags { + tags = { Name = "emr_test" } } @@ -1075,7 +1075,7 @@ resource "aws_vpc" "main" { cidr_block = "168.31.0.0/16" enable_dns_hostnames = true - tags { + tags = { Name = "terraform-testacc-emr-cluster" } } @@ -1084,7 +1084,7 @@ resource "aws_subnet" "main" { vpc_id = "${aws_vpc.main.id}" cidr_block = "168.31.0.0/20" - tags { + tags = { Name = "tf-acc-emr-cluster" } } @@ -1327,7 +1327,7 @@ EOF core_instance_type = "c4.large" core_instance_count = 1 - tags { + tags = { role = "rolename" dns_zone = "env_zone" env = "env" @@ -1379,7 +1379,7 @@ resource "aws_security_group" "allow_all" { ignore_changes = ["ingress", "egress"] } - tags { + tags = { Name = "emr_test" } } @@ -1388,7 +1388,7 @@ resource "aws_vpc" "main" { cidr_block = "168.31.0.0/16" enable_dns_hostnames = true - tags { + tags = { Name = "terraform-testacc-emr-cluster" } } @@ -1397,7 +1397,7 @@ resource "aws_subnet" "main" { vpc_id = "${aws_vpc.main.id}" cidr_block = "168.31.0.0/20" - tags { + tags = { Name = "tf-acc-emr-cluster" } } @@ -1702,7 +1702,7 @@ resource "aws_security_group" "allow_all" { ignore_changes = ["ingress", "egress"] } - tags { + tags = { Name = "emr_test" } } @@ -1711,7 +1711,7 @@ resource "aws_vpc" "main" { cidr_block = "168.31.0.0/16" enable_dns_hostnames = true - tags { + tags = { Name = "terraform-testacc-emr-cluster" } } @@ -1720,7 +1720,7 @@ resource "aws_subnet" "main" { vpc_id = "${aws_vpc.main.id}" cidr_block = "168.31.0.0/20" - tags { + tags = { Name = "tf-acc-emr-cluster" } } @@ -1990,7 +1990,7 @@ resource "aws_security_group" "allow_all" { ignore_changes = ["ingress", "egress"] } - tags { + tags = { Name = "emr_test" } } @@ -1999,7 +1999,7 @@ resource "aws_vpc" "main" { cidr_block = "10.0.0.0/16" enable_dns_hostnames = true - tags { + tags = { Name = "terraform-testacc-emr-cluster-kerberos-cluster-dedicated-kdc" } } @@ -2010,7 +2010,7 @@ resource "aws_subnet" "main" { cidr_block = "10.0.${count.index}.0/24" vpc_id = "${aws_vpc.main.id}" - tags { + tags = { Name = "tf-acc-emr-cluster-kerberos-cluster-dedicated-kdc" } } @@ -2018,7 +2018,7 @@ resource "aws_subnet" "main" { resource "aws_internet_gateway" "gw" { vpc_id = "${aws_vpc.main.id}" - tags { + tags = { Name = "terraform-testacc-emr-cluster-kerberos-cluster-dedicated-kdc" } } @@ -2063,7 +2063,7 @@ resource "aws_emr_cluster" "tf-test-cluster" { security_configuration = "${aws_emr_security_configuration.foo.name}" - tags { + tags = { role = "rolename" dns_zone = "env_zone" env = "env" @@ -2112,7 +2112,7 @@ resource "aws_security_group" "allow_all" { ignore_changes = ["ingress", "egress"] } - tags { + tags = { Name = "emr_test" } } @@ -2121,7 +2121,7 @@ resource "aws_vpc" "main" { cidr_block = "168.31.0.0/16" enable_dns_hostnames = true - tags { + tags = { Name = "terraform-testacc-emr-cluster-security-configuration" } } @@ -2130,7 +2130,7 @@ resource "aws_subnet" "main" { vpc_id = "${aws_vpc.main.id}" cidr_block = "168.31.0.0/20" - tags { + tags = { Name = "tf-acc-emr-cluster-security-configuration" } } @@ -2486,7 +2486,7 @@ resource "aws_security_group" "allow_all" { ignore_changes = ["ingress", "egress"] } - tags { + tags = { Name = "emr_test" } } @@ -2495,7 +2495,7 @@ resource "aws_vpc" "main" { cidr_block = "10.0.0.0/16" enable_dns_hostnames = true - tags { + tags = { Name = "terraform-testacc-emr-cluster-step" } } @@ -2506,7 +2506,7 @@ resource "aws_subnet" "main" { cidr_block = "10.0.${count.index}.0/24" vpc_id = "${aws_vpc.main.id}" - tags { + tags = { Name = "terraform-testacc-emr-cluster-step" } } @@ -2514,7 +2514,7 @@ resource "aws_subnet" "main" { resource "aws_internet_gateway" "gw" { vpc_id = "${aws_vpc.main.id}" - tags { + tags = { Name = "terraform-testacc-emr-cluster-step" } } @@ -2601,7 +2601,7 @@ EOT } ] - tags { + tags = { role = "rolename" dns_zone = "env_zone" env = "env" @@ -2650,7 +2650,7 @@ resource "aws_security_group" "allow_all" { ignore_changes = ["ingress", "egress"] } - tags { + tags = { Name = "emr_test" } } @@ -2659,7 +2659,7 @@ resource "aws_vpc" "main" { cidr_block = "168.31.0.0/16" enable_dns_hostnames = true - tags { + tags = { Name = "terraform-testacc-emr-cluster-instance-groups" } } @@ -2668,7 +2668,7 @@ resource "aws_subnet" "main" { vpc_id = "${aws_vpc.main.id}" cidr_block = "168.31.0.0/20" - tags { + tags = { Name = "tf-acc-emr-cluster-instance-groups" } } @@ -2951,7 +2951,7 @@ EOT } ] - tags { + tags = { role = "rolename" dns_zone = "env_zone" env = "env" @@ -3000,7 +3000,7 @@ resource "aws_security_group" "allow_all" { ignore_changes = ["ingress", "egress"] } - tags { + tags = { Name = "emr_test" } } @@ -3009,7 +3009,7 @@ resource "aws_vpc" "main" { cidr_block = "168.31.0.0/16" enable_dns_hostnames = true - tags { + tags = { Name = "terraform-testacc-emr-cluster-instance-groups" } } @@ -3018,7 +3018,7 @@ resource "aws_subnet" "main" { vpc_id = "${aws_vpc.main.id}" cidr_block = "168.31.0.0/20" - tags { + tags = { Name = "tf-acc-emr-cluster-instance-groups" } } @@ -3301,7 +3301,7 @@ EOT } ] - tags { + tags = { role = "rolename" dns_zone = "env_zone" env = "env" @@ -3348,7 +3348,7 @@ resource "aws_security_group" "allow_all" { ignore_changes = ["ingress", "egress"] } - tags { + tags = { Name = "emr_test" } } @@ -3357,7 +3357,7 @@ resource "aws_vpc" "main" { cidr_block = "168.31.0.0/16" enable_dns_hostnames = true - tags { + tags = { Name = "terraform-testacc-emr-cluster-instance-groups" } } @@ -3366,7 +3366,7 @@ resource "aws_subnet" "main" { vpc_id = "${aws_vpc.main.id}" cidr_block = "168.31.0.0/20" - tags { + tags = { Name = "tf-acc-emr-cluster-instance-groups" } } @@ -3649,7 +3649,7 @@ EOT } ] - tags { + tags = { role = "rolename" dns_zone = "env_zone" env = "env" @@ -3696,7 +3696,7 @@ resource "aws_security_group" "allow_all" { ignore_changes = ["ingress", "egress"] } - tags { + tags = { Name = "emr_test" } } @@ -3705,7 +3705,7 @@ resource "aws_vpc" "main" { cidr_block = "168.31.0.0/16" enable_dns_hostnames = true - tags { + tags = { Name = "terraform-testacc-emr-cluster-instance-groups" } } @@ -3714,7 +3714,7 @@ resource "aws_subnet" "main" { vpc_id = "${aws_vpc.main.id}" cidr_block = "168.31.0.0/20" - tags { + tags = { Name = "tf-acc-emr-cluster-instance-groups" } } @@ -3949,7 +3949,7 @@ resource "aws_emr_cluster" "tf-test-cluster" { core_instance_type = "c4.large" core_instance_count = 1 - tags { + tags = { role = "rolename" dns_zone = "env_zone" env = "env" @@ -3998,7 +3998,7 @@ resource "aws_security_group" "allow_all" { ignore_changes = ["ingress", "egress"] } - tags { + tags = { Name = "emr_test" } } @@ -4007,7 +4007,7 @@ resource "aws_vpc" "main" { cidr_block = "168.31.0.0/16" enable_dns_hostnames = true - tags { + tags = { Name = "terraform-testacc-emr-cluster-termination-policy" } } @@ -4016,7 +4016,7 @@ resource "aws_subnet" "main" { vpc_id = "${aws_vpc.main.id}" cidr_block = "168.31.0.0/20" - tags { + tags = { Name = "tf-acc-emr-cluster-termination-policy" } } @@ -4252,7 +4252,7 @@ resource "aws_emr_cluster" "tf-test-cluster" { core_instance_type = "c4.large" core_instance_count = 1 - tags { + tags = { role = "rolename" dns_zone = "env_zone" env = "env" @@ -4311,7 +4311,7 @@ resource "aws_security_group" "allow_all" { ignore_changes = ["ingress", "egress"] } - tags { + tags = { Name = "emr_test" } } @@ -4320,7 +4320,7 @@ resource "aws_vpc" "main" { cidr_block = "168.31.0.0/16" enable_dns_hostnames = true - tags { + tags = { Name = "terraform-testacc-emr-cluster-termination-policy" } } @@ -4329,7 +4329,7 @@ resource "aws_subnet" "main" { vpc_id = "${aws_vpc.main.id}" cidr_block = "168.31.0.0/20" - tags { + tags = { Name = "tf-acc-emr-cluster-termination-policy" } } @@ -4569,7 +4569,7 @@ resource "aws_emr_cluster" "tf-test-cluster" { core_instance_type = "c4.large" core_instance_count = 1 - tags { + tags = { role = "rolename" dns_zone = "env_zone" env = "env" @@ -4618,7 +4618,7 @@ resource "aws_security_group" "allow_all" { ignore_changes = ["ingress", "egress"] } - tags { + tags = { Name = "emr_test" } } @@ -4627,7 +4627,7 @@ resource "aws_vpc" "main" { cidr_block = "168.31.0.0/16" enable_dns_hostnames = true - tags { + tags = { Name = "terraform-testacc-emr-cluster-visible-to-all-users" } } @@ -4636,7 +4636,7 @@ resource "aws_subnet" "main" { vpc_id = "${aws_vpc.main.id}" cidr_block = "168.31.0.0/20" - tags { + tags = { Name = "tf-acc-emr-cluster-visible-to-all-users" } } @@ -4872,7 +4872,7 @@ resource "aws_emr_cluster" "tf-test-cluster" { core_instance_type = "c4.large" core_instance_count = 1 - tags { + tags = { dns_zone = "new_zone" Env = "production" name = "name-env" @@ -4920,7 +4920,7 @@ resource "aws_security_group" "allow_all" { ignore_changes = ["ingress", "egress"] } - tags { + tags = { Name = "emr_test" } } @@ -4929,7 +4929,7 @@ resource "aws_vpc" "main" { cidr_block = "168.31.0.0/16" enable_dns_hostnames = true - tags { + tags = { Name = "terraform-testacc-emr-cluster-updated-tags" } } @@ -4938,7 +4938,7 @@ resource "aws_subnet" "main" { vpc_id = "${aws_vpc.main.id}" cidr_block = "168.31.0.0/20" - tags { + tags = { Name = "tf-acc-emr-cluster-updated-tags" } } @@ -5178,7 +5178,7 @@ resource "aws_emr_cluster" "tf-test-cluster" { core_instance_type = "c4.large" core_instance_count = 1 - tags { + tags = { role = "rolename" dns_zone = "env_zone" env = "env" @@ -5228,7 +5228,7 @@ resource "aws_security_group" "allow_all" { ignore_changes = ["ingress", "egress"] } - tags { + tags = { Name = "emr_test" } } @@ -5237,7 +5237,7 @@ resource "aws_vpc" "main" { cidr_block = "168.31.0.0/16" enable_dns_hostnames = true - tags { + tags = { Name = "terraform-testacc-emr-cluster-updated-root-volume-size" } } @@ -5246,7 +5246,7 @@ resource "aws_subnet" "main" { vpc_id = "${aws_vpc.main.id}" cidr_block = "168.31.0.0/20" - tags { + tags = { Name = "tf-acc-emr-cluster-updated-root-volume-size" } } @@ -5472,7 +5472,7 @@ resource "aws_s3_bucket" "test" { resource "aws_vpc" "test" { cidr_block = "10.0.0.0/24" - tags { + tags = { Name = "terraform-testacc-emr-cluster-s3-logging" } } @@ -5480,7 +5480,7 @@ resource "aws_vpc" "test" { resource "aws_subnet" "test" { vpc_id = "${aws_vpc.test.id}" cidr_block = "10.0.0.0/24" - tags { + tags = { Name = "tf-acc-emr-cluster-s3-logging" } } @@ -5571,7 +5571,7 @@ resource "aws_emr_cluster" "tf-test-cluster" { core_instance_type = "c4.large" core_instance_count = 1 - tags { + tags = { role = "rolename" dns_zone = "env_zone" env = "env" @@ -5622,7 +5622,7 @@ resource "aws_security_group" "allow_all" { ignore_changes = ["ingress", "egress"] } - tags { + tags = { Name = "emr_test" } } @@ -5631,7 +5631,7 @@ resource "aws_vpc" "main" { cidr_block = "168.31.0.0/16" enable_dns_hostnames = true - tags { + tags = { Name = "terraform-testacc-emr-cluster-custom-ami-id" } } @@ -5640,7 +5640,7 @@ resource "aws_subnet" "main" { vpc_id = "${aws_vpc.main.id}" cidr_block = "168.31.0.0/20" - tags { + tags = { Name = "tf-acc-emr-cluster-custom-ami-id" } } diff --git a/website/docs/r/emr_cluster.html.markdown b/website/docs/r/emr_cluster.html.markdown index 0949d6139921..b5138ef3b461 100644 --- a/website/docs/r/emr_cluster.html.markdown +++ b/website/docs/r/emr_cluster.html.markdown @@ -322,7 +322,7 @@ In addition to all arguments above, the following attributes are exported: * `visible_to_all_users` - Indicates whether the job flow is visible to all IAM users of the AWS account associated with the job flow. * `tags` - The list of tags associated with a cluster. -For any instance_group the id is exported. e.g. aws_emr_cluster.instance_group.hashcode.id +For any instance_group the id is exported IN `aws_emr_cluster.instance_group.HASHCODE.id` format, e.g. `aws_emr_cluster.example.instance_group.12345678.id` ## Example bootable config