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

New Resource: aws_appautoscaling_scheduled_action #2231

Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
1 change: 1 addition & 0 deletions aws/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ func Provider() terraform.ResourceProvider {
"aws_app_cookie_stickiness_policy": resourceAwsAppCookieStickinessPolicy(),
"aws_appautoscaling_target": resourceAwsAppautoscalingTarget(),
"aws_appautoscaling_policy": resourceAwsAppautoscalingPolicy(),
"aws_appautoscaling_scheduled_action": resourceAwsAppautoscalingScheduledAction(),
"aws_athena_database": resourceAwsAthenaDatabase(),
"aws_athena_named_query": resourceAwsAthenaNamedQuery(),
"aws_autoscaling_attachment": resourceAwsAutoscalingAttachment(),
Expand Down
222 changes: 222 additions & 0 deletions aws/resource_aws_appautoscaling_scheduled_action.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,222 @@
package aws

import (
"fmt"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/service/applicationautoscaling"
"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/helper/schema"
)

const awsAppautoscalingScheduleTimeLayout = "2006-01-02T15:04:05Z"

func resourceAwsAppautoscalingScheduledAction() *schema.Resource {
return &schema.Resource{
Create: resourceAwsAppautoscalingScheduledActionCreate,
Read: resourceAwsAppautoscalingScheduledActionRead,
Delete: resourceAwsAppautoscalingScheduledActionDelete,

Schema: map[string]*schema.Schema{
"name": &schema.Schema{
Type: schema.TypeString,
Required: true,
ForceNew: true,
},
"service_namespace": &schema.Schema{
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) {
value := v.(string)
validServiceNamespaces := []string{"ecs", "elasticmapreduce", "ec2", "appstream", "dynamodb"}
Copy link
Member

Choose a reason for hiding this comment

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

As below - I'm afraid the list of valid service namespaces may/will grow and I'd like us to avoid the maintenance burden associated with it, if possible.

Copy link
Contributor Author

@atsushi-ishibashi atsushi-ishibashi Dec 2, 2017

Choose a reason for hiding this comment

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

Ok, I removed ValidateFunc.

for _, str := range validServiceNamespaces {
if value == str {
return
}
}
errors = append(errors, fmt.Errorf("expected %s to be one of %v, got %s", k, validServiceNamespaces, value))
return
},
},
"resource_id": &schema.Schema{
Type: schema.TypeString,
Required: true,
ForceNew: true,
},
"scalable_dimension": &schema.Schema{
Type: schema.TypeString,
Optional: true,
ForceNew: true,
ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) {
value := v.(string)
validDimensions := []string{"ecs:service:DesiredCount", "ec2:spot-fleet-request:TargetCapacity",
"elasticmapreduce:instancegroup:InstanceCount", "appstream:fleet:DesiredCapacity",
"dynamodb:table:ReadCapacityUnits", "dynamodb:table:WriteCapacityUnits",
"dynamodb:index:ReadCapacityUnits", "dynamodb:index:WriteCapacityUnits"}
Copy link
Member

Choose a reason for hiding this comment

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

While offline, plan-time validation is usually useful I'm afraid it could be counter-productive here as the list of valid dimensions may and will expand and it did recently. I think we'd play a constant catch-up with AWS here and rushing to cut a release just to support a new dimension just because of this strict validation.
Theoretically we could validate for something like [a-zA-Z]+:[a-zA-Z]+:[a-zA-Z]+ but frankly I'd just rather avoid the validation altogether here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

docs lists constant strings so I removed ValidateFunc.

for _, str := range validDimensions {
if value == str {
return
}
}
errors = append(errors, fmt.Errorf("expected %s to be one of %v, got %s", k, validDimensions, value))
return
},
},
"scalable_target_action": &schema.Schema{
Type: schema.TypeList,
Optional: true,
ForceNew: true,
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"max_capacity": &schema.Schema{
Type: schema.TypeInt,
Optional: true,
ForceNew: true,
},
"min_capacity": &schema.Schema{
Type: schema.TypeInt,
Optional: true,
ForceNew: true,
},
},
},
},
"schedule": &schema.Schema{
Type: schema.TypeString,
Optional: true,
ForceNew: true,
},
"start_time": &schema.Schema{
Type: schema.TypeString,
Optional: true,
ForceNew: true,
},
"end_time": &schema.Schema{
Type: schema.TypeString,
Optional: true,
ForceNew: true,
},
"arn": &schema.Schema{
Type: schema.TypeString,
Computed: true,
},
},
}
}

func resourceAwsAppautoscalingScheduledActionCreate(d *schema.ResourceData, meta interface{}) error {
Copy link
Member

Choose a reason for hiding this comment

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

Per docs I think we should support updates of this resource and basically just rename this function to something like resourceAwsAppautoscalingScheduledActionCreateOrUpdate or resourceAwsAppautoscalingScheduledActionPut and reference it in Update too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK👍

conn := meta.(*AWSClient).appautoscalingconn

input := &applicationautoscaling.PutScheduledActionInput{
ScheduledActionName: aws.String(d.Get("name").(string)),
ServiceNamespace: aws.String(d.Get("service_namespace").(string)),
ResourceId: aws.String(d.Get("resource_id").(string)),
}
if v, ok := d.GetOk("scalable_dimension"); ok {
input.ScalableDimension = aws.String(v.(string))
}
if v, ok := d.GetOk("schedule"); ok {
input.Schedule = aws.String(v.(string))
}
if v, ok := d.GetOk("scalable_target_action"); ok {
sta := &applicationautoscaling.ScalableTargetAction{}
raw := v.([]interface{})[0].(map[string]interface{})
if max, ok := raw["max_capacity"]; ok {
sta.MaxCapacity = aws.Int64(int64(max.(int)))
}
if min, ok := raw["min_capacity"]; ok {
sta.MaxCapacity = aws.Int64(int64(min.(int)))
}
input.ScalableTargetAction = sta
}
if v, ok := d.GetOk("start_time"); ok {
t, err := time.Parse(awsAppautoscalingScheduleTimeLayout, v.(string))
if err != nil {
return fmt.Errorf("Error Parsing Appautoscaling Scheduled Action Start Time: %s", err.Error())
}
input.StartTime = aws.Time(t)
}
if v, ok := d.GetOk("end_time"); ok {
t, err := time.Parse(awsAppautoscalingScheduleTimeLayout, v.(string))
if err != nil {
return fmt.Errorf("Error Parsing Appautoscaling Scheduled Action End Time: %s", err.Error())
}
input.EndTime = aws.Time(t)
}

err := resource.Retry(5*time.Minute, func() *resource.RetryError {
_, err := conn.PutScheduledAction(input)
if err != nil {
if aerr, ok := err.(awserr.Error); ok {
switch aerr.Code() {
case applicationautoscaling.ErrCodeObjectNotFoundException:
return resource.RetryableError(err)
default:
return resource.NonRetryableError(err)
}
Copy link
Member

Choose a reason for hiding this comment

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

Q: Is this related to eventual consistency?

Regardless of the reason I think the above conditional starting on L153 can be simplified to something like this:

if err != nil {
  if isAWSErr(err, applicationautoscaling.ErrCodeObjectNotFoundException, "") {
    return resource.RetryableError(err)
  }
  return resource.NonRetryableError(err)
}

Copy link
Contributor Author

@atsushi-ishibashi atsushi-ishibashi Dec 2, 2017

Choose a reason for hiding this comment

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

I assumed that soon after creating dynamo table and on...
I simplified handling error.

}
return resource.NonRetryableError(err)
}
return nil
})

if err != nil {
return err
}

d.SetId(d.Get("name").(string))
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the ID consist of name + service namespace as that's how we look it up? In other words my question is: Can there be more than 1 scheduled action of the same name? If not then ignore me.

Copy link
Contributor Author

@atsushi-ishibashi atsushi-ishibashi Dec 2, 2017

Choose a reason for hiding this comment

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

According to DeleteScheduledActionInput docs, ResourceId, ScheduledActionName, ServiceNamespace are required so we have to generate id from these three.
Should I convert to hash because it becomes long?

Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest we make it something like this:

d.SetId(name+"-"+serviceNamespace+"-"+resourceId)

assuming the underscore has no special meaning in either of these fields.

return resourceAwsAppautoscalingScheduledActionRead(d, meta)
}

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

saName := d.Id()
input := &applicationautoscaling.DescribeScheduledActionsInput{
ScheduledActionNames: []*string{aws.String(saName)},
ServiceNamespace: aws.String(d.Get("service_namespace").(string)),
}
resp, err := conn.DescribeScheduledActions(input)
if err != nil {
return err
}
if len(resp.ScheduledActions) < 1 {
d.SetId("")
return nil
}
if len(resp.ScheduledActions) != 1 {
return fmt.Errorf("Number of Scheduled Action (%s) isn't one, got %d", saName, len(resp.ScheduledActions))
Copy link
Member

Choose a reason for hiding this comment

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

Grammar nitpick, do you mind changing this to Expected 1 scheduled action under %s, found %d?

}
if *resp.ScheduledActions[0].ScheduledActionName != saName {
return fmt.Errorf("Scheduled Action (%s) not found", saName)
}
d.Set("arn", resp.ScheduledActions[0].ScheduledActionARN)
return nil
}

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

input := &applicationautoscaling.DeleteScheduledActionInput{
ScheduledActionName: aws.String(d.Get("name").(string)),
ServiceNamespace: aws.String(d.Get("service_namespace").(string)),
ResourceId: aws.String(d.Get("resource_id").(string)),
}
if v, ok := d.GetOk("scalable_dimension"); ok {
input.ScalableDimension = aws.String(v.(string))
}
_, err := conn.DeleteScheduledAction(input)
if err != nil {
if aerr, ok := err.(awserr.Error); ok && aerr.Code() == applicationautoscaling.ErrCodeObjectNotFoundException {
d.SetId("")
return nil
}
return err
}
d.SetId("")
return nil
}
Loading