Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EMR Cluster Import Support #6498

Merged
merged 24 commits into from
Jan 11, 2019
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
4fc9827
Add Importer to aws_emr_cluster.
stefansundin May 9, 2018
8c54895
Add import help to aws_emr_cluster documentation. Plus other minor ch…
stefansundin May 9, 2018
1637bb3
Fix typos in aws_emr_security_configuration documentation.
stefansundin May 9, 2018
6cbb601
Add acceptance test for import.
stefansundin May 9, 2018
2c1c04e
Make TestAccAWSEMRCluster_basic pass.
stefansundin Jun 4, 2018
015ae93
Remove duplicate code.
stefansundin Aug 12, 2018
d5760ad
Merge branch 'aws_emr_cluster-importer' of https://github.com/stefans…
bflad Oct 9, 2018
ff3c579
tests/resource/aws_emr_cluster: Add Import TestStep to all acceptance…
bflad Oct 9, 2018
2d2bb36
Merge branch 'master' into aws_emr_cluster-importer
mbfrahry Nov 14, 2018
b359cd4
Adding import support
Nov 16, 2018
9e50594
Updating tests
mbfrahry Nov 16, 2018
063db7f
Fixing tests
tracypholmes Nov 16, 2018
4387c3a
Finalizing tests
mbfrahry Nov 20, 2018
c833636
Removing comments
mbfrahry Nov 20, 2018
2fa32bd
Adding migration steps to EMR Cluster
mbfrahry Dec 10, 2018
adffd0d
Continuing work on emr clusters
mbfrahry Dec 17, 2018
a381f6b
adding customize diff
mbfrahry Jan 2, 2019
d3c6e20
Merging with master
mbfrahry Jan 2, 2019
c2c9d2a
Fixing tests
mbfrahry Jan 3, 2019
c352a2d
Finalizing tests
mbfrahry Jan 6, 2019
566e5b1
Additional tests and website additions
mbfrahry Jan 7, 2019
c0af366
Removing additional files and extraneous comments
mbfrahry Jan 7, 2019
609bb1e
Simplifying loops
mbfrahry Jan 8, 2019
8e76a7f
Adding 0.12 supports and fixing docs
mbfrahry Jan 11, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 105 additions & 42 deletions aws/resource_aws_emr_cluster.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package aws

import (
"bytes"
"log"

"encoding/json"
Expand All @@ -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"
Expand All @@ -26,6 +28,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,
Expand All @@ -39,9 +45,9 @@ func resourceAwsEMRCluster() *schema.Resource {
},
"master_instance_type": {
Type: schema.TypeString,
Required: false,
Optional: true,
ForceNew: true,
Computed: true,
},
"additional_info": {
Type: schema.TypeString,
Expand All @@ -61,8 +67,10 @@ func resourceAwsEMRCluster() *schema.Resource {
Computed: true,
},
"core_instance_count": {
Type: schema.TypeInt,
Optional: true,
Type: schema.TypeInt,
Optional: true,
ValidateFunc: validation.IntAtLeast(1),
Computed: true,
},
"cluster_state": {
Type: schema.TypeString,
Expand Down Expand Up @@ -194,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": {
Expand All @@ -202,14 +211,16 @@ func resourceAwsEMRCluster() *schema.Resource {
Required: false,
},
"ebs_config": {
Type: schema.TypeSet,
Type: schema.TypeList,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this change intentional? If so, we should also set MaxItems: 1 as well - otherwise we should undo this change as its not reliable to migrate a Terraform state and configuration from a multiple element TypeSet to TypeList.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was intentional because of another issue that cropped up. I've since fixed both of these issues and we're back to TypeSet on ebs_config

Optional: true,
ForceNew: true,
Computed: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"iops": {
Type: schema.TypeInt,
Optional: true,
Computed: true,
},
"size": {
Type: schema.TypeInt,
Expand Down Expand Up @@ -238,6 +249,7 @@ func resourceAwsEMRCluster() *schema.Resource {
Optional: true,
DiffSuppressFunc: suppressEquivalentJsonDiffs,
ValidateFunc: validation.ValidateJsonString,
Computed: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying not to introduce too much further scope creep into this pull request, but can you please add acceptance testing that attempts to update autoscaling_policy? I have a hunch that it will result in a permanent difference, especially with the new Set function. I think the same problem also applies to bid_price, instance_count, and especially instance_role. These attributes likely should be ForceNew: true if I'm right, but maybe I'm wrong.

I'm also not opposed to updating the resource documentation to discourage the usage of instance_group in preference of the (upcoming) aws_emr_instance_fleet and aws_emr_instance_group resources.

StateFunc: func(v interface{}) string {
jsonString, _ := structure.NormalizeJsonString(v)
return jsonString
Expand All @@ -264,6 +276,7 @@ func resourceAwsEMRCluster() *schema.Resource {
},
},
},
Set: resourceAwsEMRClusterHash,
},
"bootstrap_action": {
Type: schema.TypeSet,
Expand Down Expand Up @@ -430,14 +443,31 @@ func resourceAwsEMRClusterCreate(d *schema.ResourceData, meta interface{}) error
}

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)
}

var instanceProfile string
Expand All @@ -451,9 +481,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), ",")
Expand Down Expand Up @@ -601,6 +628,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")

Expand Down Expand Up @@ -664,9 +693,18 @@ func resourceAwsEMRClusterRead(d *schema.ResourceData, meta interface{}) error {
coreGroup := emrCoreInstanceGroup(instanceGroups)
if coreGroup != nil {
d.Set("core_instance_type", coreGroup.InstanceType)
d.Set("core_instance_count", coreGroup.RequestedInstanceCount)
}
if err := d.Set("instance_group", flattenInstanceGroups(instanceGroups)); err != nil {
log.Printf("[ERR] Error setting EMR instance groups: %s", err)
masterGroup := findMasterGroup(instanceGroups)
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)
}
if err := d.Set("instance_group", flattenedInstanceGroups); err != nil {
return fmt.Errorf("[ERR] Error setting EMR instance groups: %s", err)
}
}

Expand All @@ -682,19 +720,14 @@ 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)
}

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 {
Expand All @@ -708,7 +741,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 {
Expand All @@ -719,11 +752,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
Expand Down Expand Up @@ -901,7 +934,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
Expand Down Expand Up @@ -1043,7 +1076,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 {
Expand All @@ -1053,36 +1086,49 @@ 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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

range automatically can handle nil slices, so I don't believe this nil check is necessary: https://play.golang.org/p/-9ZKxKImPFH

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: We should probably provide some context about this error here:

Suggested change
return nil, err
return nil, fmt.Errorf("error parsing EMR Cluster Instance Groups AutoScalingPolicy: %s", err)

}
attrs["autoscaling_policy"] = autoscalingPolicyString
} else {
attrs["autoscaling_policy"] = ""
}

attrs["name"] = *ig.Name
if attrs["name"] != nil {
attrs["name"] = *ig.Name
}
result = append(result, attrs)
}

return result
return result, nil
}

func flattenBootstrapArguments(actions []*emr.Command) []map[string]interface{} {
Expand Down Expand Up @@ -1337,7 +1383,7 @@ 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{
VolumesPerInstance: aws.Int64(int64(rawEbsConfig["volumes_per_instance"].(int))),
Expand Down Expand Up @@ -1469,3 +1515,20 @@ func resourceAwsEMRClusterStateRefreshFunc(d *schema.ResourceData, meta interfac
return resp.Cluster, state, nil
}
}

func findMasterGroup(instanceGroups []*emr.InstanceGroup) *emr.InstanceGroup {
for _, group := range instanceGroups {
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())
}
Loading