Skip to content

Commit

Permalink
Refactror redundant code
Browse files Browse the repository at this point in the history
  • Loading branch information
abicky committed May 24, 2021
1 parent 356e0f8 commit 4fbb251
Show file tree
Hide file tree
Showing 10 changed files with 35 additions and 60 deletions.
5 changes: 2 additions & 3 deletions cmd/reduceclustercapacity.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ func reduceClusterCapacity(cmd *cobra.Command, args []string) error {
return newRuntimeError("failed to initialize a session: %w", err)
}

var drainer capacity.Drainer
drainer, err = capacity.NewDrainer(cluster, ecsconst.MaxListableContainerInstances, ecs.New(sess))
drainer, err := capacity.NewDrainer(cluster, ecsconst.MaxListableContainerInstances, ecs.New(sess))
if err != nil {
return newRuntimeError("failed to initialize a Drainer: %w", err)
}
Expand Down Expand Up @@ -96,7 +95,7 @@ func reduceClusterCapacity(cmd *cobra.Command, args []string) error {
return newRuntimeError("failed to create an event rule for interruption warnings: %w", err)
}

if err = sfr.ReduceCapacity(amount, drainer, capacity.NewSQSQueuePoller(queueURL, sqsSvc)); err != nil {
if err := sfr.ReduceCapacity(amount, drainer, capacity.NewSQSQueuePoller(queueURL, sqsSvc)); err != nil {
return newRuntimeError("failed to reduce the cluster capacity: %w", err)
}

Expand Down
6 changes: 2 additions & 4 deletions cmd/replaceautoscalinggroupinstances.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,12 @@ func replaceAutoScalingGroupInstances(cmd *cobra.Command, args []string) error {
return newRuntimeError("failed to initialize a session: %w", err)
}

var asg *capacity.AutoScalingGroup
asg, err = capacity.NewAutoScalingGroup(name, autoscaling.New(sess), ec2.New(sess))
asg, err := capacity.NewAutoScalingGroup(name, autoscaling.New(sess), ec2.New(sess))
if err != nil {
return newRuntimeError("failed to initialize a AutoScalingGroup: %w", err)
}

var drainer capacity.Drainer
drainer, err = capacity.NewDrainer(cluster, batchSize, ecs.New(sess))
drainer, err := capacity.NewDrainer(cluster, batchSize, ecs.New(sess))
if err != nil {
return newRuntimeError("failed to initialize a Drainer: %w", err)
}
Expand Down
6 changes: 2 additions & 4 deletions cmd/terminatespotfleetinstances.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,12 @@ func terminateSpotFleetInstances(cmd *cobra.Command, args []string) error {
return newRuntimeError("failed to initialize a session: %w", err)
}

var sfr *capacity.SpotFleetRequest
sfr, err = capacity.NewSpotFleetRequest(id, ec2.New(sess))
sfr, err := capacity.NewSpotFleetRequest(id, ec2.New(sess))
if err != nil {
return newRuntimeError("failed to initialize a SpotFleetRequest: %w", err)
}

var drainer capacity.Drainer
drainer, err = capacity.NewDrainer(cluster, batchSize, ecs.New(sess))
drainer, err := capacity.NewDrainer(cluster, batchSize, ecs.New(sess))
if err != nil {
return newRuntimeError("failed to initialize a Drainer: %w", err)
}
Expand Down
24 changes: 10 additions & 14 deletions internal/capacity/autoscalinggroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ type AutoScalingGroup struct {

func NewAutoScalingGroup(name string, asSvc autoscalingiface.AutoScalingAPI, ec2Svc ec2iface.EC2API) (*AutoScalingGroup, error) {
asg := AutoScalingGroup{asSvc: asSvc, ec2Svc: ec2Svc, name: name}
err := asg.reload()
if err != nil {
if err := asg.reload(); err != nil {
return nil, err
}
return &asg, nil
Expand Down Expand Up @@ -87,35 +86,32 @@ func (asg *AutoScalingGroup) reload() error {
return xerrors.Errorf("the auto scaling group \"%s\" doesn't exist", asg.name)
}

group := resp.AutoScalingGroups[0]
originalDesiredCapacity := *group.DesiredCapacity
originalMaxSize := *group.MaxSize
var stateSavedAt time.Time
for _, t := range group.Tags {
asg.Group = resp.AutoScalingGroups[0]
asg.OriginalDesiredCapacity = asg.DesiredCapacity
asg.OriginalMaxSize = asg.MaxSize
for _, t := range asg.Tags {
switch *t.Key {
case "ecsmec:OriginalDesiredCapacity":
originalDesiredCapacity, err = strconv.ParseInt(*t.Value, 10, 64)
originalDesiredCapacity, err := strconv.ParseInt(*t.Value, 10, 64)
if err != nil {
return xerrors.Errorf("ecsmec:OriginalDesiredCapacity is invalid (%s): %w", *t.Value, err)
}
asg.OriginalDesiredCapacity = &originalDesiredCapacity
case "ecsmec:OriginalMaxSize":
originalMaxSize, err = strconv.ParseInt(*t.Value, 10, 64)
originalMaxSize, err := strconv.ParseInt(*t.Value, 10, 64)
if err != nil {
return xerrors.Errorf("ecsmec:OriginalMaxSize is invalid (%s): %w", *t.Value, err)
}
asg.OriginalMaxSize = &originalMaxSize
case "ecsmec:StateSavedAt":
stateSavedAt, err = time.Parse(time.RFC3339, *t.Value)
stateSavedAt, err := time.Parse(time.RFC3339, *t.Value)
if err != nil {
return xerrors.Errorf("ecsmec:StateSavedAt is invalid (%s): %w", *t.Value, err)
}
asg.StateSavedAt = &stateSavedAt
}
}

asg.Group = group
asg.OriginalDesiredCapacity = &originalDesiredCapacity
asg.OriginalMaxSize = &originalMaxSize

return nil
}

Expand Down
15 changes: 5 additions & 10 deletions internal/capacity/autoscalinggroup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -439,8 +439,7 @@ func TestAutoScalingGroup_ReplaceInstances(t *testing.T) {
t.Fatal(err)
}

err = group.ReplaceInstances(drainerMock)
if err != nil {
if err := group.ReplaceInstances(drainerMock); err != nil {
t.Errorf("err = %#v; want nil", err)
}
})
Expand Down Expand Up @@ -517,8 +516,7 @@ func TestAutoScalingGroup_ReplaceInstances(t *testing.T) {
t.Fatal(err)
}

err = group.ReplaceInstances(drainerMock)
if err != nil {
if err := group.ReplaceInstances(drainerMock); err != nil {
t.Errorf("err = %#v; want nil", err)
}
})
Expand Down Expand Up @@ -584,8 +582,7 @@ func TestAutoScalingGroup_ReplaceInstances(t *testing.T) {
t.Fatal(err)
}

err = group.ReplaceInstances(drainerMock)
if err != nil {
if err := group.ReplaceInstances(drainerMock); err != nil {
t.Errorf("err = %#v; want nil", err)
}
})
Expand Down Expand Up @@ -660,8 +657,7 @@ func TestAutoScalingGroup_ReplaceInstances(t *testing.T) {
t.Fatal(err)
}

err = group.ReplaceInstances(drainerMock)
if err != nil {
if err := group.ReplaceInstances(drainerMock); err != nil {
t.Errorf("err = %#v; want nil", err)
}
})
Expand Down Expand Up @@ -745,8 +741,7 @@ func TestAutoScalingGroup_ReduceCapacity(t *testing.T) {
t.Fatal(err)
}

err = group.ReduceCapacity(int64(len(instancesToTerminate)), drainerMock)
if err != nil {
if err := group.ReduceCapacity(int64(len(instancesToTerminate)), drainerMock); err != nil {
t.Errorf("err = %#v; want nil", err)
}

Expand Down
2 changes: 1 addition & 1 deletion internal/capacity/drainer.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func (d *drainer) drainContainerInstances(arns []*string, wait bool) error {
// Stop tasks manually because tasks that don't belong to a service won't stop
// even after their cluster instance's status becomes "DRAINING"
log.Printf("Stop the task \"%s\"\n", *t.TaskArn)
_, err = d.ecsSvc.StopTask(&ecs.StopTaskInput{
_, err := d.ecsSvc.StopTask(&ecs.StopTaskInput{
Cluster: t.ClusterArn,
Reason: aws.String("Task stopped by ecsmec"),
Task: t.TaskArn,
Expand Down
12 changes: 4 additions & 8 deletions internal/capacity/drainer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,7 @@ func TestDrainer_Drain(t *testing.T) {
t.Fatal(err)
}

err = drainer.Drain(instanceIDs)
if err != nil {
if err := drainer.Drain(instanceIDs); err != nil {
t.Errorf("err = %#v; want nil", err)
}
})
Expand Down Expand Up @@ -125,8 +124,7 @@ func TestDrainer_Drain(t *testing.T) {
t.Fatal(err)
}

err = drainer.Drain(instanceIDs)
if err == nil {
if err := drainer.Drain(instanceIDs); err == nil {
t.Errorf("err = nil; want non-nil")
}
})
Expand Down Expand Up @@ -225,8 +223,7 @@ func TestDrainer_ProcessInterruptions(t *testing.T) {
t.Fatal(err)
}

var entries []*sqs.DeleteMessageBatchRequestEntry
entries, err = drainer.ProcessInterruptions(messages)
entries, err := drainer.ProcessInterruptions(messages)
if err != nil {
t.Errorf("err = %#v; want nil", err)
}
Expand Down Expand Up @@ -264,8 +261,7 @@ func TestDrainer_ProcessInterruptions(t *testing.T) {
t.Fatal(err)
}

var entries []*sqs.DeleteMessageBatchRequestEntry
entries, err = drainer.ProcessInterruptions(messages)
entries, err := drainer.ProcessInterruptions(messages)
if err != nil {
t.Errorf("err = nil; want non-nil")
}
Expand Down
3 changes: 1 addition & 2 deletions internal/capacity/poller.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,7 @@ func (p *SQSQueuePoller) PollOnce(callback func([]*sqs.Message) ([]*sqs.DeleteMe
return xerrors.Errorf("failed to receive messages: %w", err)
}

var entries []*sqs.DeleteMessageBatchRequestEntry
entries, err = callback(resp.Messages)
entries, err := callback(resp.Messages)
if err != nil {
return err
}
Expand Down
15 changes: 5 additions & 10 deletions internal/capacity/spotfleetrequest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@ func TestSpotFleetRequest_TerminateAllInstances(t *testing.T) {
t.Fatal(err)
}

err = sfr.TerminateAllInstances(drainerMock)
if err != nil {
if err := sfr.TerminateAllInstances(drainerMock); err != nil {
t.Errorf("err = %#v; want nil", err)
}
})
Expand Down Expand Up @@ -110,8 +109,7 @@ func TestSpotFleetRequest_TerminateAllInstances(t *testing.T) {
t.Fatal(err)
}

err = sfr.TerminateAllInstances(drainerMock)
if err == nil {
if err := sfr.TerminateAllInstances(drainerMock); err == nil {
t.Errorf("err = nil; want non-nil")
}
})
Expand Down Expand Up @@ -236,8 +234,7 @@ func TestSpotFleetRequest_ReduceCapacity(t *testing.T) {
t.Fatal(err)
}

err = sfr.ReduceCapacity(tt.amount, drainerMock, pollerMock)
if err != nil {
if err := sfr.ReduceCapacity(tt.amount, drainerMock, pollerMock); err != nil {
t.Errorf("err = %#v; want nil", err)
}
})
Expand Down Expand Up @@ -268,8 +265,7 @@ func TestSpotFleetRequest_ReduceCapacity(t *testing.T) {
t.Fatal(err)
}

err = sfr.ReduceCapacity(1, drainerMock, pollerMock)
if err != nil {
if err := sfr.ReduceCapacity(1, drainerMock, pollerMock); err != nil {
t.Errorf("err = %#v; want nil", err)
}
})
Expand Down Expand Up @@ -362,8 +358,7 @@ func TestSpotFleetRequest_ReduceCapacity(t *testing.T) {
t.Fatal(err)
}

err = sfr.ReduceCapacity(4, drainerMock, pollerMock)
if err == nil {
if err := sfr.ReduceCapacity(4, drainerMock, pollerMock); err == nil {
t.Errorf("err = nil; want non-nil")
}
})
Expand Down
7 changes: 3 additions & 4 deletions internal/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,11 @@ func (s *Service) copy(cluster string, serviceName string, overrides Definition)
}

func (s *Service) createAndWaitUntilStable(config *ecs.CreateServiceInput) error {
_, err := s.ecsSvc.CreateService(config)
if err != nil {
if _, err := s.ecsSvc.CreateService(config); err != nil {
return xerrors.Errorf("failed to create the service \"%s\": %w", *config.ServiceName, err)
}

err = s.ecsSvc.WaitUntilServicesStable(&ecs.DescribeServicesInput{
err := s.ecsSvc.WaitUntilServicesStable(&ecs.DescribeServicesInput{
Cluster: config.Cluster,
Services: []*string{config.ServiceName},
})
Expand Down Expand Up @@ -141,7 +140,7 @@ func (s *Service) stopAndWaitUntilStopped(cluster string, serviceName string) er
}

for arns := range sliceutil.ChunkSlice(taskArns, ecsconst.MaxDescribableTasks) {
err = s.ecsSvc.WaitUntilTasksStopped(&ecs.DescribeTasksInput{
err := s.ecsSvc.WaitUntilTasksStopped(&ecs.DescribeTasksInput{
Cluster: aws.String(cluster),
Tasks: arns,
})
Expand Down

0 comments on commit 4fbb251

Please sign in to comment.