Skip to content

Commit

Permalink
aws: undeprecate min_elb_capacity; restore min capacity waiting
Browse files Browse the repository at this point in the history
It was a mistake to switched fully to `==` when activating waiting for
capacity on updates in #3947. Users that didn't set `min_elb_capacity ==
desired_capacity` and instead treated it as an actual "minimum" would
see timeouts for every create, since their target numbers would never be
reached exactly.

Here, we fix that regression by restoring the minimum waiting behavior
during creates.

In order to preserve all the stated behavior, I had to split out
different criteria for create and update, criteria which are now
exhaustively unit tested.

The set of fields that affect capacity waiting behavior has become a bit
of a mess. Next major release I'd like to rework all of these into a
more consistently named block of config. For now, just getting the
behavior correct and documented.

(Also removes all the fixed names from the ASG tests as I was hitting
collision issues running them over here.)

Fixes #4792
  • Loading branch information
phinze committed Jan 27, 2016
1 parent 97ea366 commit c70eab6
Show file tree
Hide file tree
Showing 6 changed files with 395 additions and 109 deletions.
97 changes: 4 additions & 93 deletions builtin/providers/aws/resource_aws_autoscaling_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,8 @@ func resourceAwsAutoscalingGroup() *schema.Resource {
},

"min_elb_capacity": &schema.Schema{
Type: schema.TypeInt,
Optional: true,
Deprecated: "Please use 'wait_for_elb_capacity' instead.",
Type: schema.TypeInt,
Optional: true,
},

"min_size": &schema.Schema{
Expand Down Expand Up @@ -222,7 +221,7 @@ func resourceAwsAutoscalingGroupCreate(d *schema.ResourceData, meta interface{})
d.SetId(d.Get("name").(string))
log.Printf("[INFO] AutoScaling Group ID: %s", d.Id())

if err := waitForASGCapacity(d, meta); err != nil {
if err := waitForASGCapacity(d, meta, capacitySatifiedCreate); err != nil {
return err
}

Expand Down Expand Up @@ -377,7 +376,7 @@ func resourceAwsAutoscalingGroupUpdate(d *schema.ResourceData, meta interface{})
}

if shouldWaitForCapacity {
waitForASGCapacity(d, meta)
waitForASGCapacity(d, meta, capacitySatifiedUpdate)
}

return resourceAwsAutoscalingGroupRead(d, meta)
Expand Down Expand Up @@ -513,94 +512,6 @@ func resourceAwsAutoscalingGroupDrain(d *schema.ResourceData, meta interface{})
})
}

// Waits for a minimum number of healthy instances to show up as healthy in the
// ASG before continuing. Waits up to `waitForASGCapacityTimeout` for
// "desired_capacity", or "min_size" if desired capacity is not specified.
//
// If "wait_for_elb_capacity" is specified, will also wait for that number of
// instances to show up InService in all attached ELBs. See "Waiting for
// Capacity" in docs for more discussion of the feature.
func waitForASGCapacity(d *schema.ResourceData, meta interface{}) error {
wantASG := d.Get("min_size").(int)
if v := d.Get("desired_capacity").(int); v > 0 {
wantASG = v
}
wantELB := d.Get("wait_for_elb_capacity").(int)

// Covers deprecated field support
wantELB += d.Get("min_elb_capacity").(int)

wait, err := time.ParseDuration(d.Get("wait_for_capacity_timeout").(string))
if err != nil {
return err
}

if wait == 0 {
log.Printf("[DEBUG] Capacity timeout set to 0, skipping capacity waiting.")
return nil
}

log.Printf("[DEBUG] Waiting %s for capacity: %d ASG, %d ELB",
wait, wantASG, wantELB)

return resource.Retry(wait, func() error {
g, err := getAwsAutoscalingGroup(d, meta)
if err != nil {
return resource.RetryError{Err: err}
}
if g == nil {
return nil
}
lbis, err := getLBInstanceStates(g, meta)
if err != nil {
return resource.RetryError{Err: err}
}

haveASG := 0
haveELB := 0

for _, i := range g.Instances {
if i.HealthStatus == nil || i.InstanceId == nil || i.LifecycleState == nil {
continue
}

if !strings.EqualFold(*i.HealthStatus, "Healthy") {
continue
}

if !strings.EqualFold(*i.LifecycleState, "InService") {
continue
}

haveASG++

if wantELB > 0 {
inAllLbs := true
for _, states := range lbis {
state, ok := states[*i.InstanceId]
if !ok || !strings.EqualFold(state, "InService") {
inAllLbs = false
}
}
if inAllLbs {
haveELB++
}
}
}

log.Printf("[DEBUG] %q Capacity: %d/%d ASG, %d/%d ELB",
d.Id(), haveASG, wantASG, haveELB, wantELB)

if haveASG == wantASG && haveELB == wantELB {
return nil
}

return fmt.Errorf(
"Still waiting for %q instances. Current/Desired: %d/%d ASG, %d/%d ELB",
d.Id(), haveASG, wantASG, haveELB, wantELB)
})
}

// Returns a mapping of the instance states of all the ELBs attached to the
// provided ASG.
//
Expand Down
4 changes: 0 additions & 4 deletions builtin/providers/aws/resource_aws_autoscaling_group_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -585,14 +585,12 @@ resource "aws_subnet" "alt" {
}
resource "aws_launch_configuration" "foobar" {
name = "vpc-asg-test"
image_id = "ami-b5b3fc85"
instance_type = "t2.micro"
}
resource "aws_autoscaling_group" "bar" {
availability_zones = ["us-west-2a"]
name = "vpc-asg-test"
max_size = 2
min_size = 1
health_check_grace_period = 300
Expand Down Expand Up @@ -631,7 +629,6 @@ resource "aws_subnet" "alt" {
}
resource "aws_launch_configuration" "foobar" {
name = "vpc-asg-test"
image_id = "ami-b5b3fc85"
instance_type = "t2.micro"
}
Expand All @@ -641,7 +638,6 @@ resource "aws_autoscaling_group" "bar" {
"${aws_subnet.main.id}",
"${aws_subnet.alt.id}",
]
name = "vpc-asg-test"
max_size = 2
min_size = 1
health_check_grace_period = 300
Expand Down
129 changes: 129 additions & 0 deletions builtin/providers/aws/resource_aws_autoscaling_group_waiting.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
package aws

import (
"fmt"
"log"
"strings"
"time"

"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/helper/schema"
)

// waitForASGCapacityTimeout gathers the current numbers of healthy instances
// in the ASG and its attached ELBs and yields these numbers to a
// capacitySatifiedFunction. Loops for up to wait_for_capacity_timeout until
// the capacitySatisfiedFunc returns true.
//
// See "Waiting for Capacity" in docs for more discussion of the feature.
func waitForASGCapacity(
d *schema.ResourceData,
meta interface{},
satisfiedFunc capacitySatisfiedFunc) error {
wait, err := time.ParseDuration(d.Get("wait_for_capacity_timeout").(string))
if err != nil {
return err
}

if wait == 0 {
log.Printf("[DEBUG] Capacity timeout set to 0, skipping capacity waiting.")
return nil
}

log.Printf("[DEBUG] Waiting on %s for capacity...", d.Id())

return resource.Retry(wait, func() error {
g, err := getAwsAutoscalingGroup(d, meta)
if err != nil {
return resource.RetryError{Err: err}
}
if g == nil {
return nil
}
lbis, err := getLBInstanceStates(g, meta)
if err != nil {
return resource.RetryError{Err: err}
}

haveASG := 0
haveELB := 0

for _, i := range g.Instances {
if i.HealthStatus == nil || i.InstanceId == nil || i.LifecycleState == nil {
continue
}

if !strings.EqualFold(*i.HealthStatus, "Healthy") {
continue
}

if !strings.EqualFold(*i.LifecycleState, "InService") {
continue
}

haveASG++

inAllLbs := true
for _, states := range lbis {
state, ok := states[*i.InstanceId]
if !ok || !strings.EqualFold(state, "InService") {
inAllLbs = false
}
}
if inAllLbs {
haveELB++
}
}

satisfied, reason := satisfiedFunc(d, haveASG, haveELB)

log.Printf("[DEBUG] %q Capacity: %d ASG, %d ELB, satisfied: %t, reason: %q",
d.Id(), haveASG, haveELB, satisfied, reason)

if satisfied {
return nil
}

return fmt.Errorf("%q: Waiting up to %s: %s", d.Id(), wait, reason)
})
}

type capacitySatisfiedFunc func(*schema.ResourceData, int, int) (bool, string)

// capacitySatifiedCreate treats all targets as minimums
func capacitySatifiedCreate(d *schema.ResourceData, haveASG, haveELB int) (bool, string) {
minASG := d.Get("min_size").(int)
if wantASG := d.Get("desired_capacity").(int); wantASG > 0 {
minASG = wantASG
}
if haveASG < minASG {
return false, fmt.Sprintf(
"Need at least %d healthy instances in ASG, have %d", minASG, haveASG)
}
minELB := d.Get("min_elb_capacity").(int)
if wantELB := d.Get("wait_for_elb_capacity").(int); wantELB > 0 {
minELB = wantELB
}
if haveELB < minELB {
return false, fmt.Sprintf(
"Need at least %d healthy instances in ELB, have %d", minELB, haveELB)
}
return true, ""
}

// capacitySatifiedUpdate only cares about specific targets
func capacitySatifiedUpdate(d *schema.ResourceData, haveASG, haveELB int) (bool, string) {
if wantASG := d.Get("desired_capacity").(int); wantASG > 0 {
if haveASG != wantASG {
return false, fmt.Sprintf(
"Need exactly %d healthy instances in ASG, have %d", wantASG, haveASG)
}
}
if wantELB := d.Get("wait_for_elb_capacity").(int); wantELB > 0 {
if haveELB != wantELB {
return false, fmt.Sprintf(
"Need exactly %d healthy instances in ELB, have %d", wantELB, haveELB)
}
}
return true, ""
}
Loading

0 comments on commit c70eab6

Please sign in to comment.