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.

Sign up for GitHub

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

r/aws_appautoscaling_target: Support updating max_capacity, min_capacity, and role_arn attributes #2542

Closed
wants to merge 2 commits into from
Closed

r/aws_appautoscaling_target: Support updating max_capacity, min_capacity, and role_arn attributes #2542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8a6fad5
r/aws_appautoscaling_target: Support updating max_capacity, min_capac…
bflad Dec 5, 2017
6333865
r/aws_appautoscaling_target: #2542 review comments
bflad Dec 11, 2017
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
51 changes: 46 additions & 5 deletions aws/resource_aws_appautoscaling_target.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,17 @@ func resourceAwsAppautoscalingTarget() *schema.Resource {
return &schema.Resource{
Create: resourceAwsAppautoscalingTargetCreate,
Read: resourceAwsAppautoscalingTargetRead,
Update: resourceAwsAppautoscalingTargetUpdate,
Delete: resourceAwsAppautoscalingTargetDelete,

Schema: map[string]*schema.Schema{
"max_capacity": {
Type: schema.TypeInt,
Required: true,
ForceNew: true,
},
"min_capacity": {
Type: schema.TypeInt,
Required: true,
ForceNew: true,
},
"resource_id": {
Type: schema.TypeString,
Expand All @@ -38,7 +37,6 @@ func resourceAwsAppautoscalingTarget() *schema.Resource {
"role_arn": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
},
"scalable_dimension": {
Type: schema.TypeString,
Expand Down Expand Up @@ -74,8 +72,8 @@ func resourceAwsAppautoscalingTargetCreate(d *schema.ResourceData, meta interfac
_, err = conn.RegisterScalableTarget(&targetOpts)

if err != nil {
if awsErr, ok := err.(awserr.Error); ok && awsErr.Code() == "ValidationException" {
log.Printf("[DEBUG] Retrying creation of Application Autoscaling Scalable Target due to possible issues with IAM: %s", awsErr)
if isAWSErr(err, "ValidationException", "Unable to assume IAM role") {
Copy link
Member

Choose a reason for hiding this comment

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

👍 More precise matching, yey

log.Printf("[DEBUG] Retrying creation of Application Autoscaling Scalable Target due to possible issues with IAM: %s", err)
return resource.RetryableError(err)
}
return resource.NonRetryableError(err)
Expand Down Expand Up @@ -118,6 +116,49 @@ func resourceAwsAppautoscalingTargetRead(d *schema.ResourceData, meta interface{
return nil
}

func resourceAwsAppautoscalingTargetUpdate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).appautoscalingconn

input := &applicationautoscaling.RegisterScalableTargetInput{
ResourceId: aws.String(d.Get("resource_id").(string)),
ScalableDimension: aws.String(d.Get("scalable_dimension").(string)),
ServiceNamespace: aws.String(d.Get("service_namespace").(string)),
}

if d.HasChange("max_capacity") {
input.MaxCapacity = aws.Int64(int64(d.Get("max_capacity").(int)))
}

if d.HasChange("min_capacity") {
input.MinCapacity = aws.Int64(int64(d.Get("min_capacity").(int)))
}

if d.HasChange("role_arn") {
input.RoleARN = aws.String(d.Get("role_arn").(string))
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add the role_arn update in tests so that we ensure it's all ok?
Seems rather good for me otherwise :)

}

log.Printf("[DEBUG] Updating Application Autoscaling Target: %#v", input)
var err error
err = resource.Retry(1*time.Minute, func() *resource.RetryError {
_, err = conn.RegisterScalableTarget(input)

if err != nil {
if isAWSErr(err, "ValidationException", "Unable to assume IAM role") {
log.Printf("[DEBUG] Retrying creation of Application Autoscaling Scalable Target due to possible issues with IAM: %s", err)
return resource.RetryableError(err)
}
return resource.NonRetryableError(err)
}

return nil
})
if err != nil {
return fmt.Errorf("Error updating application autoscaling target: %s", err)
}

return resourceAwsAppautoscalingTargetRead(d, meta)
}

func resourceAwsAppautoscalingTargetDelete(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).appautoscalingconn

Expand Down
7 changes: 6 additions & 1 deletion aws/resource_aws_appautoscaling_target_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package aws

import (
"fmt"
"regexp"
"testing"

"github.com/aws/aws-sdk-go/aws"
Expand All @@ -27,6 +28,8 @@ func TestAccAWSAppautoScalingTarget_basic(t *testing.T) {
Config: testAccAWSAppautoscalingTargetConfig(randClusterName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSAppautoscalingTargetExists("aws_appautoscaling_target.bar", &target),
resource.TestMatchResourceAttr("aws_appautoscaling_target.bar", "role_arn",
regexp.MustCompile(fmt.Sprintf("^arn:aws:iam::[0-9]{12}:role/autoscalerole%s$", randClusterName))),
resource.TestCheckResourceAttr("aws_appautoscaling_target.bar", "service_namespace", "ecs"),
resource.TestCheckResourceAttr("aws_appautoscaling_target.bar", "scalable_dimension", "ecs:service:DesiredCount"),
resource.TestCheckResourceAttr("aws_appautoscaling_target.bar", "min_capacity", "1"),
Expand All @@ -40,6 +43,8 @@ func TestAccAWSAppautoScalingTarget_basic(t *testing.T) {
testAccCheckAWSAppautoscalingTargetExists("aws_appautoscaling_target.bar", &target),
resource.TestCheckResourceAttr("aws_appautoscaling_target.bar", "min_capacity", "2"),
resource.TestCheckResourceAttr("aws_appautoscaling_target.bar", "max_capacity", "8"),
resource.TestMatchResourceAttr("aws_appautoscaling_target.bar", "role_arn",
regexp.MustCompile(fmt.Sprintf("^arn:aws:iam::[0-9]{12}:role/autoscaleroleupdate%s$", randClusterName))),
),
},
},
Expand Down Expand Up @@ -288,7 +293,7 @@ func testAccAWSAppautoscalingTargetConfigUpdate(
randClusterName string) string {
return fmt.Sprintf(`
resource "aws_iam_role" "autoscale_role" {
name = "autoscalerole%s"
name = "autoscaleroleupdate%s"
path = "/"

assume_role_policy = <<EOF
Expand Down