diff --git a/aws/asg/asg.go b/aws/asg/asg.go index 7325c93..0bb6a90 100644 --- a/aws/asg/asg.go +++ b/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 diff --git a/aws/asg/asg_test.go b/aws/asg/asg_test.go index 0bc4493..e5632d3 100644 --- a/aws/asg/asg_test.go +++ b/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)) +} diff --git a/aws/mocks/mock_asg.go b/aws/mocks/mock_asg.go index 3027e10..4955bf9 100644 --- a/aws/mocks/mock_asg.go +++ b/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 +} diff --git a/deployer/machine.go b/deployer/machine.go index d9d1b2f..1b86ab4 100644 --- a/deployer/machine.go +++ b/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": { diff --git a/deployer/models/release.go b/deployer/models/release.go index 7f1e063..79096d7 100644 --- a/deployer/models/release.go +++ b/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) } diff --git a/deployer/models/release_resources.go b/deployer/models/release_resources.go index 98db876..9a05ee0 100644 --- a/deployer/models/release_resources.go +++ b/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)} } }