From 589caeac5fde1533afd56e8bf2288e72b42c71df Mon Sep 17 00:00:00 2001 From: Eoin McAfee Date: Mon, 3 Apr 2023 11:19:42 +0100 Subject: [PATCH] fixes issue in idle termination logic --- engine/planner.go | 4 +- engine/planner_test.go | 209 ++++++++++++++++++++++++----------------- 2 files changed, 124 insertions(+), 89 deletions(-) diff --git a/engine/planner.go b/engine/planner.go index f71b10e6..ef9470ba 100644 --- a/engine/planner.go +++ b/engine/planner.go @@ -192,7 +192,7 @@ func (p *planner) mark(ctx context.Context, n int) error { } // if there are no idle servers, there are no servers - // to retire and we can exit. + // to retire, we can exit. if len(idle) == 0 { logger.Debugln("no idle servers to shutdown") } @@ -265,7 +265,7 @@ func (p *planner) listBusy(ctx context.Context) (map[string]struct{}, error) { if p.match(stage) == false { continue } - if stage.Status == drone.StatusRunning { + if stage.Status == drone.StatusRunning || stage.Status == drone.StatusPending { busy[stage.Machine] = struct{}{} } } diff --git a/engine/planner_test.go b/engine/planner_test.go index 01b8787d..1ee59a80 100644 --- a/engine/planner_test.go +++ b/engine/planner_test.go @@ -479,93 +479,128 @@ func TestPlan_ExcludePendingWhenTerminating(t *testing.T) { } } -// func TestListBusy(t *testing.T) { -// controller := gomock.NewController(t) -// defer controller.Finish() - -// client := mocks.NewMockClient(controller) -// client.EXPECT().Build("octocat", "hello-world", 1).Return(&drone.Build{ -// Procs: []*drone.Proc{ -// {PID: 1, Machine: "machine1"}, -// {PID: 2, Machine: "machine2"}, -// }, -// }, nil) -// client.EXPECT().BuildQueue().Return([]*drone.Activity{ -// {Status: drone.StatusPending}, -// {Status: drone.StatusRunning, Owner: "octocat", Name: "hello-world", Number: 1}, -// }, nil) - -// scaler := Scaler{Client: client} -// busy, err := scaler.listBusy(context.TODO()) -// if err != nil { -// t.Error(err) -// return -// } -// if got, want := len(busy), 2; got != want { -// t.Errorf("Want busy server count %d, got %d", want, got) -// } -// if _, ok := busy["machine1"]; !ok { -// t.Errorf("Expected server not in busy list") -// } -// if _, ok := busy["machine2"]; !ok { -// t.Errorf("Expected server not in busy list") -// } -// } - -// func TestCapacity(t *testing.T) { -// controller := gomock.NewController(t) -// defer controller.Finish() - -// servers := []*autoscaler.Server{ -// {Name: "server1", Capacity: 4}, -// {Name: "server2", Capacity: 3}, -// {Name: "server3", Capacity: 2}, -// {Name: "server4", Capacity: 1}, -// } - -// store := mocks.NewMockServerStore(controller) -// store.EXPECT().List(gomock.Any()).Return(servers, nil) - -// scaler := Scaler{Servers: store} -// capacity, count, err := scaler.capacity(context.TODO()) -// if err != nil { -// t.Error(err) -// return -// } -// if got, want := capacity, 10; got != want { -// t.Errorf("Want capacity count %d, got %d", want, got) -// } -// if got, want := count, 4; got != want { -// t.Errorf("Want server count %d, got %d", want, got) -// } -// } - -// func TestCount(t *testing.T) { -// controller := gomock.NewController(t) -// defer controller.Finish() - -// client := mocks.NewMockClient(controller) -// client.EXPECT().BuildQueue().Return([]*drone.Activity{ -// {Status: drone.StatusPending}, -// {Status: drone.StatusPending}, -// {Status: drone.StatusPending}, -// {Status: drone.StatusRunning}, -// {Status: drone.StatusRunning}, -// }, nil) - -// scaler := Scaler{Client: client} -// pending, running, err := scaler.count(context.TODO()) -// if err != nil { -// t.Error(err) -// return -// } -// if got, want := pending, 3; got != want { -// t.Errorf("Want pending count %d, got %d", want, got) -// } -// if got, want := running, 2; got != want { -// t.Errorf("Want running count %d, got %d", want, got) -// } -// } +func TestListBusy(t *testing.T) { + controller := gomock.NewController(t) + defer controller.Finish() + + client := mocks.NewMockClient(controller) + client.EXPECT().Queue().Return([]*drone.Stage{ + {Status: drone.StatusPending, Machine: "machine1"}, + {Status: drone.StatusRunning, Machine: "machine2"}, + }, nil) + + p := planner{ + client: client, + } + + busy, err := p.listBusy(context.TODO()) + if err != nil { + t.Error(err) + } + if got, want := len(busy), 2; got != want { + t.Errorf("Want busy server count %d, got %d", want, got) + } + if _, ok := busy["machine1"]; !ok { + t.Errorf("Expected server not in busy list") + } + if _, ok := busy["machine2"]; !ok { + t.Errorf("Expected server not in busy list") + } +} + +func TestListBusyWithPendingAndRunning(t *testing.T) { + controller := gomock.NewController(t) + defer controller.Finish() + + client := mocks.NewMockClient(controller) + client.EXPECT().Queue().Return([]*drone.Stage{ + {Status: drone.StatusPending, Machine: "machine1"}, + {Status: drone.StatusRunning, Machine: "machine2"}, + {Status: drone.StatusPending, Machine: "machine3"}, + }, nil) + + p := planner{ + client: client, + } + + busy, err := p.listBusy(context.TODO()) + if err != nil { + t.Error(err) + } + if got, want := len(busy), 3; got != want { + t.Errorf("Want busy server count %d, got %d", want, got) + } + if _, ok := busy["machine1"]; !ok { + t.Errorf("Machine1 not found in the busy server list") + } + if _, ok := busy["machine2"]; !ok { + t.Errorf("Machine2 not found in the busy server list") + } + if _, ok := busy["machine3"]; !ok { + t.Errorf("Machine3 not found in the busy server list") + } +} + +func TestCapacity(t *testing.T) { + controller := gomock.NewController(t) + defer controller.Finish() + + servers := []*autoscaler.Server{ + {Name: "server1", Capacity: 4}, + {Name: "server2", Capacity: 3}, + {Name: "server3", Capacity: 2}, + {Name: "server4", Capacity: 1}, + } + + store := mocks.NewMockServerStore(controller) + store.EXPECT().List(gomock.Any()).Return(servers, nil) + + p := planner{ + servers: store, + } + + capacity, count, err := p.capacity(context.TODO()) + if err != nil { + t.Error(err) + return + } + if got, want := capacity, 10; got != want { + t.Errorf("Want capacity count %d, got %d", want, got) + } + if got, want := count, 4; got != want { + t.Errorf("Want server count %d, got %d", want, got) + } +} + +func TestCount(t *testing.T) { + controller := gomock.NewController(t) + defer controller.Finish() + + client := mocks.NewMockClient(controller) + client.EXPECT().Queue().Return([]*drone.Stage{ + {Status: drone.StatusPending}, + {Status: drone.StatusPending}, + {Status: drone.StatusPending}, + {Status: drone.StatusRunning}, + {Status: drone.StatusRunning}, + }, nil) + + p := planner{ + client: client, + } + + pending, running, err := p.count(context.TODO()) + if err != nil { + t.Error(err) + return + } + if got, want := pending, 3; got != want { + t.Errorf("Want pending count %d, got %d", want, got) + } + if got, want := running, 2; got != want { + t.Errorf("Want running count %d, got %d", want, got) + } +} func TestMatch(t *testing.T) { tests := []struct {