Merge branch 'master' into graham/add-placement-groups-to-odin

aws/asg/asg.go

@@ -270,32 +270,67 @@ func (s *ASG) Detach(asgc aws.ASGAPI) error {
 	return nil
 }
 
-func (s *ASG) IsDetached(asgc aws.ASGAPI) (bool, error) {
-	// No LBs? Skip!
-	if len(s.LoadBalancerNames) == 0 && len(s.TargetGroupARNs) == 0 {
-		return true, nil
+func (s *ASG) AttachedLBs(asgc aws.ASGAPI) ([]string, error) {
+	lbs := []string{}
+
+	tgs, err := s.attachedTargetGroups(asgc)
+	if err != nil {
+		return lbs, err
+	}
+
+	clbs, err := s.attachedClassicLBs(asgc)
+	if err != nil {
+		return lbs, err
 	}
 
-	removed := true
+	lbs = append(lbs, tgs...)
+	lbs = append(lbs, clbs...)
+
+	return lbs, err
+}
+
+func (s *ASG) attachedTargetGroups(asgc aws.ASGAPI) ([]string, error) {
+	lbs := []string{}
 
 	states, err := asgc.DescribeLoadBalancerTargetGroups(&autoscaling.DescribeLoadBalancerTargetGroupsInput{
 		AutoScalingGroupName: s.ServiceID(),
 	})
 
 	if err != nil {
-		return false, err
+		return lbs, err
 	}
 
-	// No LBs? Skip!
-	if len(states.LoadBalancerTargetGroups) == 0 {
-		return true, nil
+	for _, targetGroup := range states.LoadBalancerTargetGroups {
+		if *targetGroup.State == "Removed" {
+			continue
+		}
+
+		lbs = append(lbs, *targetGroup.LoadBalancerTargetGroupARN)
 	}
 
-	for _, targetGroup := range states.LoadBalancerTargetGroups {
-		removed = removed && (*targetGroup.State == "Removed")
+	return lbs, nil
+}
+
+func (s *ASG) attachedClassicLBs(asgc aws.ASGAPI) ([]string, error) {
+	lbs := []string{}
+
+	states, err := asgc.DescribeLoadBalancers(&autoscaling.DescribeLoadBalancersInput{
+		AutoScalingGroupName: s.ServiceID(),
+	})
+
+	if err != nil {
+		return lbs, err
+	}
+
+	for _, lb := range states.LoadBalancers {
+		if *lb.State == "Removed" {
+			continue
+		}
+
+		lbs = append(lbs, *lb.LoadBalancerName)
 	}
 
-	return removed, nil
+	return lbs, nil
 }
 
 // Teardown deletes the ASG with launch config and alarms

aws/asg/asg_test.go

@@ -3,6 +3,7 @@ package asg
 import (
 	"testing"
 
+	"github.com/aws/aws-sdk-go/service/autoscaling"
 	"github.com/coinbase/odin/aws/mocks"
 	"github.com/coinbase/step/utils/to"
 	"github.com/stretchr/testify/assert"
@@ -99,3 +100,54 @@ func Test_Teardown(t *testing.T) {
 	err = asgs[0].Teardown(asgc, cwc)
 	assert.NoError(t, err)
 }
+
+func Test_AttachedLBs(t *testing.T) {
+	asgc := &mocks.ASGClient{}
+
+	asgc.DescribeLoadBalancerTargetGroupsOutput = &autoscaling.DescribeLoadBalancerTargetGroupsOutput{
+		LoadBalancerTargetGroups: []*autoscaling.LoadBalancerTargetGroupState{
+			&autoscaling.LoadBalancerTargetGroupState{
+				LoadBalancerTargetGroupARN: to.Strp("arn"),
+				State: to.Strp("aaa"),
+			},
+		},
+	}
+
+	asgc.DescribeLoadBalancersOutput = &autoscaling.DescribeLoadBalancersOutput{
+		LoadBalancers: []*autoscaling.LoadBalancerState{
+			&autoscaling.LoadBalancerState{
+				LoadBalancerName: to.Strp("arn"),
+				State:            to.Strp("aaa"),
+			},
+		},
+	}
+
+	asgc.AddPreviousRuntimeResources("project", "config", "service1", "not_release")
+	asgs, err := ForProjectConfigNOTReleaseID(asgc, to.Strp("project"), to.Strp("config"), to.Strp("release"))
+
+	attached, err := asgs[0].AttachedLBs(asgc)
+	assert.NoError(t, err)
+	assert.Equal(t, 2, len(attached))
+
+	asgc.DescribeLoadBalancersOutput = &autoscaling.DescribeLoadBalancersOutput{
+		LoadBalancers: []*autoscaling.LoadBalancerState{
+			&autoscaling.LoadBalancerState{
+				LoadBalancerName: to.Strp("arn"),
+				State:            to.Strp("Removed"),
+			},
+		},
+	}
+
+	asgc.DescribeLoadBalancerTargetGroupsOutput = &autoscaling.DescribeLoadBalancerTargetGroupsOutput{
+		LoadBalancerTargetGroups: []*autoscaling.LoadBalancerTargetGroupState{
+			&autoscaling.LoadBalancerTargetGroupState{
+				LoadBalancerTargetGroupARN: to.Strp("arn"),
+				State: to.Strp("Removed"),
+			},
+		},
+	}
+
+	attached, err = asgs[0].AttachedLBs(asgc)
+	assert.NoError(t, err)
+	assert.Equal(t, 0, len(attached))
+}

aws/mocks/mock_asg.go

@@ -34,6 +34,7 @@ type ASGClient struct {
 	DescribePoliciesResp              map[string]*DescribePoliciesResponse
 
 	DescribeLoadBalancerTargetGroupsOutput *autoscaling.DescribeLoadBalancerTargetGroupsOutput
+	DescribeLoadBalancersOutput            *autoscaling.DescribeLoadBalancersOutput
 }
 
 func (m *ASGClient) init() {
@@ -237,3 +238,10 @@ func (m *ASGClient) DescribeLoadBalancerTargetGroups(input *autoscaling.Describe
 	}
 	return &autoscaling.DescribeLoadBalancerTargetGroupsOutput{}, nil
 }
+
+func (m *ASGClient) DescribeLoadBalancers(input *autoscaling.DescribeLoadBalancersInput) (*autoscaling.DescribeLoadBalancersOutput, error) {
+	if m.DescribeLoadBalancersOutput != nil {
+		return m.DescribeLoadBalancersOutput, nil
+	}
+	return &autoscaling.DescribeLoadBalancersOutput{}, nil
+}

deployer/machine.go

@@ -84,7 +84,7 @@ func StateMachine() (*machine.StateMachine, error) {
       "WaitForDeploy": {
         "Comment": "Give the Deploy time to boot instances",
         "Type": "Wait",
-        "Seconds" : 30,
+        "Seconds" : 90,
         "Next": "WaitForHealthy"
       },
       "WaitForHealthy": {
@@ -158,7 +158,7 @@ func StateMachine() (*machine.StateMachine, error) {
       "WaitDetachForSuccess": {
         "Comment": "Give detach a little time to do what it does",
         "Type": "Wait",
-        "SecondsPath" : "$.wait_for_detach",
+        "Seconds" : 5,
         "Next": "CleanUpSuccess"
       },
       "CleanUpSuccess": {
@@ -186,7 +186,7 @@ func StateMachine() (*machine.StateMachine, error) {
         "Retry": [{
           "Comment": "Retry on Detach Error",
           "ErrorEquals": ["DetachError"],
-          "MaxAttempts": 10,
+          "MaxAttempts": 30,
           "IntervalSeconds": 30
          },{
           "Comment": "Keep trying to Clean",
@@ -204,7 +204,7 @@ func StateMachine() (*machine.StateMachine, error) {
       "WaitDetachForFailure": {
         "Comment": "Give detach a little time to do what it does",
         "Type": "Wait",
-        "Seconds" : 10,
+        "Seconds" : 5,
         "Next": "CleanUpFailure"
       },
       "CleanUpFailure": {

deployer/models/release.go

@@ -30,10 +30,12 @@ type Release struct {
 	Healthy *bool `json:"healthy,omitempty"`
 
 	WaitForHealthy *int `json:"wait_for_healthy,omitempty"`
-	WaitForDetach  *int `json:"wait_for_detach,omitempty"`
 
 	// AWS Service is Downloaded
 	Services map[string]*Service `json:"services,omitempty"` // Downloaded From S3
+
+	// DEPRECATED: leave for backwards compatability
+	WaitForDetach *int `json:"wait_for_detach,omitempty"`
 }
 
 //////////
@@ -87,11 +89,6 @@ func (release *Release) SetDefaults() {
 
 	release.WaitForHealthy = to.Intp(waitForHealthy)
 
-	// Default to 20 if WaitForDetach
-	if release.WaitForDetach == nil || *release.WaitForDetach < 10 {
-		release.WaitForDetach = to.Intp(20)
-	}
-
 	if release.Healthy == nil {
 		release.Healthy = to.Boolp(false)
 	}

deployer/models/release_resources.go

@@ -313,12 +313,13 @@ func DetachAllASGs(asgc aws.ASGAPI, asgs []*asg.ASG) error {
 	}
 
 	for _, asg := range asgs {
-		d, err := asg.IsDetached(asgc)
+		attachedLBs, err := asg.AttachedLBs(asgc)
 		if err != nil {
 			return err
 		}
-		if !d {
-			return DetachError{fmt.Sprintf("asg %s not detached", *asg.ServiceID())}
+
+		if len(attachedLBs) != 0 {
+			return DetachError{fmt.Sprintf("asg %s not detached with lbs %s", *asg.ServiceID(), attachedLBs)}
 		}
 	}