Skip to content

Commit

Permalink
Update resource for completion
Browse files Browse the repository at this point in the history 
* Add support for bid_price
* Convert EBS configuration into a force new (there is no way to update)
* Drop support for instance group types because they can't be managed
the same way.
  • Loading branch information
@nywilken
nywilken committed Apr 16, 2019
1 parent 4325f3e commit afa5085
Show file tree
Hide file tree
Showing 4 changed files with 146 additions and 25 deletions.
1 change: 1 addition & 0 deletions aws/resource_aws_emr_cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1262,6 +1262,7 @@ func flattenInstanceGroups(igs []*emr.InstanceGroup) (*schema.Set, error) {
}

func flattenEBSConfig(ebsBlockDevices []*emr.EbsBlockDevice) *schema.Set {

ebsConfig := make([]interface{}, 0)
for _, ebs := range ebsBlockDevices {
ebsAttrs := make(map[string]interface{})
Expand Down
14 changes: 11 additions & 3 deletions aws/resource_aws_emr_instance_group.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,22 +56,26 @@ func resourceAwsEMRInstanceGroup() *schema.Resource {
ForceNew: true,
},
"ebs_config": {
Type: schema.TypeSet,
Type: schema.TypeList,
Optional: true,
ForceNew: true,
Computed: true,
MinItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"iops": {
Type: schema.TypeInt,
ForceNew: true,
Optional: true,
},
"size": {
Type: schema.TypeInt,
ForceNew: true,
Required: true,
},
"type": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validateAwsEmrEbsVolumeType(),
},
"volumes_per_instance": {
Expand All @@ -92,7 +96,6 @@ func resourceAwsEMRInstanceGroup() *schema.Resource {
ForceNew: true,
Default: emr.InstanceGroupTypeTask,
ValidateFunc: validation.StringInSlice([]string{
emr.InstanceGroupTypeCore,
emr.InstanceGroupTypeTask}, false),
},
"instance_type": {
Expand Down Expand Up @@ -138,8 +141,10 @@ func resourceAwsEMRInstanceGroupCreate(d *schema.ResourceData, meta interface{})
groupConfig.AutoScalingPolicy = autoScalingPolicy
}

groupConfig.Market = aws.String(emr.MarketTypeOnDemand)
if v, ok := d.GetOk("bid_price"); ok {
groupConfig.BidPrice = aws.String(v.(string))
groupConfig.Market = aws.String(emr.MarketTypeSpot)
}

params := &emr.AddInstanceGroupsInput{
Expand Down Expand Up @@ -209,6 +214,7 @@ func resourceAwsEMRInstanceGroupRead(d *schema.ResourceData, meta interface{}) e
d.Set("autoscaling_policy", autoscalingPolicyString)

d.Set("bid_price", ig.BidPrice)
d.Set("ebs_config", flattenEBSConfig(ig.EbsBlockDevices))
d.Set("instance_count", ig.RequestedInstanceCount)
d.Set("instance_role", ig.InstanceGroupType)
d.Set("instance_type", ig.InstanceType)
Expand Down Expand Up @@ -238,6 +244,8 @@ func resourceAwsEMRInstanceGroupUpdate(d *schema.ResourceData, meta interface{})
},
}

// Give me some EBS config stuff.

_, err := conn.ModifyInstanceGroups(params)
if err != nil {
return fmt.Errorf("error modifying EMR Instance Group (%s): %s", d.Id(), err)
Expand Down
154 changes: 133 additions & 21 deletions aws/resource_aws_emr_instance_group_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
func TestAccAWSEMRInstanceGroup_basic(t *testing.T) {
var ig emr.InstanceGroup
rInt := acctest.RandInt()

resourceName := "aws_emr_instance_group.task"
resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Expand All @@ -26,7 +27,10 @@ func TestAccAWSEMRInstanceGroup_basic(t *testing.T) {
Config: testAccAWSEmrInstanceGroupConfig_basic(rInt),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEmrInstanceGroupExists(resourceName, &ig),
resource.TestCheckResourceAttr("aws_emr_instance_group.task", "instance_role", emr.InstanceGroupTypeTask),
resource.TestCheckResourceAttr(resourceName, "autoscaling_policy", ""),
resource.TestCheckResourceAttr(resourceName, "bid_price", ""),
resource.TestCheckResourceAttr(resourceName, "ebs_optimized", "false"),
resource.TestCheckResourceAttr(resourceName, "instance_role", emr.InstanceGroupTypeTask),
),
},
{
Expand All @@ -39,9 +43,63 @@ func TestAccAWSEMRInstanceGroup_basic(t *testing.T) {
})
}

func TestAccAWSEMRInstanceGroup_BidPrice(t *testing.T) {
var ig1, ig2 emr.InstanceGroup
rInt := acctest.RandInt()

resourceName := "aws_emr_instance_group.task"
resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSEmrInstanceGroupDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSEmrInstanceGroupConfig_basic(rInt),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEmrInstanceGroupExists(resourceName, &ig1),
resource.TestCheckResourceAttr(resourceName, "instance_role", emr.InstanceGroupTypeTask),
resource.TestCheckResourceAttr(resourceName, "bid_price", ""),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateIdFunc: testAccAWSEMRInstanceGroupResourceImportStateIdFunc(resourceName),
ImportStateVerify: true,
},
{
Config: testAccAWSEmrInstanceGroupConfig_BidPrice(rInt),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEmrInstanceGroupExists(resourceName, &ig2),
resource.TestCheckResourceAttr(resourceName, "instance_role", emr.InstanceGroupTypeTask),
resource.TestCheckResourceAttr(resourceName, "bid_price", "0.30"),
testAccAWSEMRInstanceGroupRecreated(t, &ig1, &ig2),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateIdFunc: testAccAWSEMRInstanceGroupResourceImportStateIdFunc(resourceName),
ImportStateVerify: true,
},
{
Config: testAccAWSEmrInstanceGroupConfig_basic(rInt),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEmrInstanceGroupExists(resourceName, &ig1),
resource.TestCheckResourceAttr(resourceName, "instance_role", emr.InstanceGroupTypeTask),
resource.TestCheckResourceAttr(resourceName, "bid_price", ""),
testAccAWSEMRInstanceGroupRecreated(t, &ig1, &ig2),
),
},
},
})
}

func TestAccAWSEMRInstanceGroup_AutoScalingPolicy(t *testing.T) {
var ig emr.InstanceGroup
rInt := acctest.RandInt()

resourceName := "aws_emr_instance_group.task"
resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Expand All @@ -50,18 +108,26 @@ func TestAccAWSEMRInstanceGroup_AutoScalingPolicy(t *testing.T) {
{
Config: testAccAWSEmrInstanceGroupConfig_AutoScalingPolicy(rInt, 1, 3),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEmrInstanceGroupExists("aws_emr_instance_group.task", &ig),
resource.TestCheckResourceAttr("aws_emr_instance_group.task", "instance_role", "TASK"),
resource.TestCheckResourceAttrSet("aws_emr_instance_group.task", "autoscaling_policy"),
testAccCheckAWSEmrInstanceGroupExists(resourceName, &ig),
resource.TestCheckResourceAttr(resourceName, "instance_role", "TASK"),
resource.TestCheckResourceAttrSet(resourceName, "autoscaling_policy"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateIdFunc: testAccAWSEMRInstanceGroupResourceImportStateIdFunc(resourceName),
ImportStateVerify: true,
},
},
})
}

func TestAccAWSEMRInstanceGroup_updateAutoScalingPolicy(t *testing.T) {
var ig emr.InstanceGroup
rInt := acctest.RandInt()

resourceName := "aws_emr_instance_group.task"
resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Expand All @@ -70,17 +136,23 @@ func TestAccAWSEMRInstanceGroup_updateAutoScalingPolicy(t *testing.T) {
{
Config: testAccAWSEmrInstanceGroupConfig_AutoScalingPolicy(rInt, 1, 3),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEmrInstanceGroupExists("aws_emr_instance_group.task", &ig),
resource.TestCheckResourceAttr("aws_emr_instance_group.task", "instance_role", "TASK"),
resource.TestCheckResourceAttrSet("aws_emr_instance_group.task", "autoscaling_policy"),
testAccCheckAWSEmrInstanceGroupExists(resourceName, &ig),
resource.TestCheckResourceAttr(resourceName, "instance_role", "TASK"),
resource.TestCheckResourceAttrSet(resourceName, "autoscaling_policy"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateIdFunc: testAccAWSEMRInstanceGroupResourceImportStateIdFunc(resourceName),
ImportStateVerify: true,
},
{
Config: testAccAWSEmrInstanceGroupConfig_AutoScalingPolicy(rInt, 2, 3),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEmrInstanceGroupExists("aws_emr_instance_group.task", &ig),
resource.TestCheckResourceAttr("aws_emr_instance_group.task", "instance_role", "TASK"),
resource.TestCheckResourceAttrSet("aws_emr_instance_group.task", "autoscaling_policy"),
testAccCheckAWSEmrInstanceGroupExists(resourceName, &ig),
resource.TestCheckResourceAttr(resourceName, "instance_role", "TASK"),
resource.TestCheckResourceAttrSet(resourceName, "autoscaling_policy"),
),
},
},
Expand All @@ -92,18 +164,26 @@ func TestAccAWSEMRInstanceGroup_updateAutoScalingPolicy(t *testing.T) {
func TestAccAWSEMRInstanceGroup_updateInstanceCount(t *testing.T) {
var ig emr.InstanceGroup
rInt := acctest.RandInt()

resourceName := "aws_emr_instance_group.task"
resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSEmrInstanceGroupDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSEmrInstanceGroupConfig_basic(rInt),
Check: testAccCheckAWSEmrInstanceGroupExists("aws_emr_instance_group.task", &ig),
Check: testAccCheckAWSEmrInstanceGroupExists(resourceName, &ig),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateIdFunc: testAccAWSEMRInstanceGroupResourceImportStateIdFunc(resourceName),
ImportStateVerify: true,
},
{
Config: testAccAWSEmrInstanceGroupConfig_zeroCount(rInt),
Check: testAccCheckAWSEmrInstanceGroupExists("aws_emr_instance_group.task", &ig),
Check: testAccCheckAWSEmrInstanceGroupExists(resourceName, &ig),
},
},
})
Expand All @@ -112,6 +192,8 @@ func TestAccAWSEMRInstanceGroup_updateInstanceCount(t *testing.T) {
func TestAccAWSEMRInstanceGroup_ebsConfig(t *testing.T) {
var ig emr.InstanceGroup
rInt := acctest.RandInt()

resourceName := "aws_emr_instance_group.task"
resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Expand All @@ -120,11 +202,23 @@ func TestAccAWSEMRInstanceGroup_ebsConfig(t *testing.T) {
{
Config: testAccAWSEmrInstanceGroupConfig_ebsConfig(rInt),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEmrInstanceGroupExists("aws_emr_instance_group.task", &ig),
resource.TestCheckResourceAttr(
"aws_emr_instance_group.task", "ebs_config.#", "1"),
resource.TestCheckResourceAttr(
"aws_emr_instance_group.task", "ebs_optimized", "true"),
testAccCheckAWSEmrInstanceGroupExists(resourceName, &ig),
resource.TestCheckResourceAttr(resourceName, "ebs_config.#", "1"),
resource.TestCheckResourceAttr(resourceName, "ebs_optimized", "true"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateIdFunc: testAccAWSEMRInstanceGroupResourceImportStateIdFunc(resourceName),
ImportStateVerify: true,
},
{
Config: testAccAWSEmrInstanceGroupConfig_ebsConfig(rInt),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEmrInstanceGroupExists(resourceName, &ig),
resource.TestCheckResourceAttr(resourceName, "ebs_config.#", "1"),
resource.TestCheckResourceAttr(resourceName, "ebs_optimized", "true"),
),
},
},
Expand Down Expand Up @@ -152,12 +246,10 @@ func testAccCheckAWSEmrInstanceGroupDestroy(s *terraform.State) error {
}
}

providerErr, ok := err.(awserr.Error)
if !ok {
if providerErr, ok := err.(awserr.Error); !ok {
log.Printf("[ERROR] %v", providerErr)
return err
}

log.Printf("[ERROR] %v", providerErr)
}

return nil
Expand Down Expand Up @@ -199,6 +291,15 @@ func testAccAWSEMRInstanceGroupResourceImportStateIdFunc(resourceName string) re
}
}

func testAccAWSEMRInstanceGroupRecreated(t *testing.T, before, after *emr.InstanceGroup) resource.TestCheckFunc {
return func(s *terraform.State) error {
if *before.Id == *after.Id {
t.Fatalf("Expected change of Instance Group Ids, but both were %v", before.Id)
}
return nil
}
}

const testAccAWSEmrInstanceGroupBase = `
resource "aws_security_group" "allow_all" {
name = "allow_all"
Expand Down Expand Up @@ -484,6 +585,17 @@ func testAccAWSEmrInstanceGroupConfig_basic(r int) string {
`, r)
}

func testAccAWSEmrInstanceGroupConfig_BidPrice(r int) string {
return fmt.Sprintf(testAccAWSEmrInstanceGroupBase+`
resource "aws_emr_instance_group" "task" {
cluster_id = "${aws_emr_cluster.tf-test-cluster.id}"
bid_price = "0.30"
instance_count = 1
instance_type = "c4.large"
}
`, r)
}

func testAccAWSEmrInstanceGroupConfig_AutoScalingPolicy(r, min, max int) string {
return fmt.Sprintf(testAccAWSEmrInstanceGroupBase+`
resource "aws_emr_instance_group" "task" {
Expand Down
2 changes: 1 addition & 1 deletion website/docs/r/emr_instance_group.html.markdown
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ The following arguments are supported:

* `name` (Required) Human friendly name given to the instance group. Changing this forces a new resource to be created.
* `cluster_id` (Required) ID of the EMR Cluster to attach to. Changing this forces a new resource to be created.
* `instance_role` (Optional) The EC2 instance role for all instances in the instance group. Valid values are CORE, TASK. Default `"TASK"`
* `instance_role` (Optional) The EC2 instance role for all instances in the instance group. Default `"TASK"`
* `instance_type` (Required) The EC2 instance type for all instances in the instance group. Changing this forces a new resource to be created.
* `instance_count` (optional) target number of instances for the instance group. defaults to 0.
* `bid_price` - (Optional) If set, the bid price for each EC2 instance in the instance group, expressed in USD. By setting this attribute, the instance group is being declared as a Spot Instance, and will implicitly create a Spot request. Leave this blank to use On-Demand Instances.
Expand Down

0 comments on commit afa5085

Please sign in to comment.