From 49066352a370681644875dc85512736b29a4234f Mon Sep 17 00:00:00 2001 From: Easton Crupper <65553218+ecrupper@users.noreply.github.com> Date: Fri, 5 Jan 2024 09:55:32 -0500 Subject: [PATCH] enhance(api/workers): add filters to list workers (#1029) * enhance(api/workers): add filters to list workers * add buffer to checked in for integration testing * test filters and fix swagger spec * change active to boolean type * remove default --- api/metrics.go | 2 +- api/worker/list.go | 40 ++++++++++++++++++++- database/integration_test.go | 6 ++-- database/worker/interface.go | 2 +- database/worker/list.go | 31 ++++++++-------- database/worker/list_test.go | 68 +++++++++++++++++++++++++++--------- 6 files changed, 112 insertions(+), 37 deletions(-) diff --git a/api/metrics.go b/api/metrics.go index b3dbbeb9f..afad74583 100644 --- a/api/metrics.go +++ b/api/metrics.go @@ -422,7 +422,7 @@ func recordGauges(c *gin.Context) { // worker_build_limit, active_worker_count, inactive_worker_count, idle_worker_count, available_worker_count, busy_worker_count, error_worker_count if q.WorkerBuildLimit || q.ActiveWorkerCount || q.InactiveWorkerCount || q.IdleWorkerCount || q.AvailableWorkerCount || q.BusyWorkerCount || q.ErrorWorkerCount { // send API call to capture the workers - workers, err := database.FromContext(c).ListWorkers(ctx) + workers, err := database.FromContext(c).ListWorkers(ctx, "all", time.Now().Unix(), 0) if err != nil { logrus.Errorf("unable to get workers: %v", err) } diff --git a/api/worker/list.go b/api/worker/list.go index 6b24484a6..f402144da 100644 --- a/api/worker/list.go +++ b/api/worker/list.go @@ -5,6 +5,8 @@ package worker import ( "fmt" "net/http" + "strconv" + "time" "github.com/gin-gonic/gin" "github.com/go-vela/server/database" @@ -20,6 +22,20 @@ import ( // --- // produces: // - application/json +// parameters: +// - in: query +// name: active +// description: Filter workers based on active status +// type: boolean +// - in: query +// name: checked_in_before +// description: filter workers that have checked in before a certain time +// type: integer +// - in: query +// name: checked_in_after +// description: filter workers that have checked in after a certain time +// type: integer +// default: 0 // security: // - ApiKeyAuth: [] // responses: @@ -48,7 +64,29 @@ func ListWorkers(c *gin.Context) { "user": u.GetName(), }).Info("reading workers") - w, err := database.FromContext(c).ListWorkers(ctx) + active := c.Query("active") + + // capture before query parameter if present, default to now + before, err := strconv.ParseInt(c.DefaultQuery("checked_in_before", strconv.FormatInt(time.Now().UTC().Unix(), 10)), 10, 64) + if err != nil { + retErr := fmt.Errorf("unable to convert `checked_in_before` query parameter: %w", err) + + util.HandleError(c, http.StatusBadRequest, retErr) + + return + } + + // capture after query parameter if present, default to 0 + after, err := strconv.ParseInt(c.DefaultQuery("checked_in_after", "0"), 10, 64) + if err != nil { + retErr := fmt.Errorf("unable to convert `checked_in_after` query parameter: %w", err) + + util.HandleError(c, http.StatusBadRequest, retErr) + + return + } + + w, err := database.FromContext(c).ListWorkers(ctx, active, before, after) if err != nil { retErr := fmt.Errorf("unable to get workers: %w", err) diff --git a/database/integration_test.go b/database/integration_test.go index 818ba081f..49b844fda 100644 --- a/database/integration_test.go +++ b/database/integration_test.go @@ -1938,7 +1938,7 @@ func testWorkers(t *testing.T, db Interface, resources *Resources) { methods["CountWorkers"] = true // list the workers - list, err := db.ListWorkers(context.TODO()) + list, err := db.ListWorkers(context.TODO(), "all", time.Now().Unix(), 0) if err != nil { t.Errorf("unable to list workers: %v", err) } @@ -2450,7 +2450,7 @@ func newResources() *Resources { workerOne.SetRunningBuildIDs([]string{"12345"}) workerOne.SetLastBuildStartedAt(time.Now().UTC().Unix()) workerOne.SetLastBuildFinishedAt(time.Now().UTC().Unix()) - workerOne.SetLastCheckedIn(time.Now().UTC().Unix()) + workerOne.SetLastCheckedIn(time.Now().UTC().Unix() - 60) workerOne.SetBuildLimit(1) workerTwo := new(library.Worker) @@ -2464,7 +2464,7 @@ func newResources() *Resources { workerTwo.SetRunningBuildIDs([]string{"12345"}) workerTwo.SetLastBuildStartedAt(time.Now().UTC().Unix()) workerTwo.SetLastBuildFinishedAt(time.Now().UTC().Unix()) - workerTwo.SetLastCheckedIn(time.Now().UTC().Unix()) + workerTwo.SetLastCheckedIn(time.Now().UTC().Unix() - 60) workerTwo.SetBuildLimit(1) return &Resources{ diff --git a/database/worker/interface.go b/database/worker/interface.go index c594c556a..589e8971b 100644 --- a/database/worker/interface.go +++ b/database/worker/interface.go @@ -37,7 +37,7 @@ type WorkerInterface interface { // GetWorkerForHostname defines a function that gets a worker by hostname. GetWorkerForHostname(context.Context, string) (*library.Worker, error) // ListWorkers defines a function that gets a list of all workers. - ListWorkers(context.Context) ([]*library.Worker, error) + ListWorkers(context.Context, string, int64, int64) ([]*library.Worker, error) // UpdateWorker defines a function that updates an existing worker. UpdateWorker(context.Context, *library.Worker) (*library.Worker, error) } diff --git a/database/worker/list.go b/database/worker/list.go index dbf8ff2a7..bb8169d61 100644 --- a/database/worker/list.go +++ b/database/worker/list.go @@ -4,6 +4,8 @@ package worker import ( "context" + "fmt" + "strconv" "github.com/go-vela/types/constants" "github.com/go-vela/types/database" @@ -11,30 +13,31 @@ import ( ) // ListWorkers gets a list of all workers from the database. -func (e *engine) ListWorkers(ctx context.Context) ([]*library.Worker, error) { +func (e *engine) ListWorkers(ctx context.Context, active string, before, after int64) ([]*library.Worker, error) { e.logger.Trace("listing all workers from the database") // variables to store query results and return value - count := int64(0) w := new([]database.Worker) workers := []*library.Worker{} - // count the results - count, err := e.CountWorkers(ctx) - if err != nil { - return nil, err - } + // build query with checked in constraints + query := e.client.Table(constants.TableWorker). + Where("last_checked_in < ?", before). + Where("last_checked_in > ?", after) + + // if active can be parsed as a boolean, add to query + if b, err := strconv.ParseBool(active); err == nil { + // convert bool to 0/1 for Sqlite + qBool := 0 + if b { + qBool = 1 + } - // short-circuit if there are no results - if count == 0 { - return workers, nil + query.Where("active = ?", fmt.Sprintf("%d", qBool)) } // send query to the database and store result in variable - err = e.client. - Table(constants.TableWorker). - Find(&w). - Error + err := query.Find(&w).Error if err != nil { return nil, err } diff --git a/database/worker/list_test.go b/database/worker/list_test.go index 9c5331fe4..4a5988228 100644 --- a/database/worker/list_test.go +++ b/database/worker/list_test.go @@ -4,44 +4,51 @@ package worker import ( "context" - "reflect" "testing" + "time" "github.com/DATA-DOG/go-sqlmock" "github.com/go-vela/types/library" + "github.com/google/go-cmp/cmp" ) func TestWorker_Engine_ListWorkers(t *testing.T) { + older := time.Now().Unix() - 60 + newer := time.Now().Unix() - 30 // setup types _workerOne := testWorker() _workerOne.SetID(1) _workerOne.SetHostname("worker_0") _workerOne.SetAddress("localhost") _workerOne.SetActive(true) + _workerOne.SetLastCheckedIn(newer) _workerTwo := testWorker() _workerTwo.SetID(2) _workerTwo.SetHostname("worker_1") _workerTwo.SetAddress("localhost") _workerTwo.SetActive(true) + _workerTwo.SetLastCheckedIn(older) + + _workerThree := testWorker() + _workerThree.SetID(3) + _workerThree.SetHostname("worker_2") + _workerThree.SetAddress("localhost") + _workerThree.SetActive(false) + _workerThree.SetLastCheckedIn(newer) _postgres, _mock := testPostgres(t) defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() // create expected result in mock - _rows := sqlmock.NewRows([]string{"count"}).AddRow(2) - - // ensure the mock expects the query - _mock.ExpectQuery(`SELECT count(*) FROM "workers"`).WillReturnRows(_rows) - - // create expected result in mock - _rows = sqlmock.NewRows( + _rows := sqlmock.NewRows( []string{"id", "hostname", "address", "routes", "active", "status", "last_status_update_at", "running_build_ids", "last_build_started_at", "last_build_finished_at", "last_checked_in", "build_limit"}). - AddRow(1, "worker_0", "localhost", nil, true, nil, 0, nil, 0, 0, 0, 0). - AddRow(2, "worker_1", "localhost", nil, true, nil, 0, nil, 0, 0, 0, 0) + AddRow(1, "worker_0", "localhost", nil, true, nil, 0, nil, 0, 0, newer, 0). + AddRow(2, "worker_1", "localhost", nil, true, nil, 0, nil, 0, 0, older, 0). + AddRow(3, "worker_2", "localhost", nil, false, nil, 0, nil, 0, 0, newer, 0) // ensure the mock expects the query - _mock.ExpectQuery(`SELECT * FROM "workers"`).WillReturnRows(_rows) + _mock.ExpectQuery(`SELECT * FROM "workers" WHERE last_checked_in < $1 AND last_checked_in > $2`).WillReturnRows(_rows) _sqlite := testSqlite(t) defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }() @@ -56,22 +63,49 @@ func TestWorker_Engine_ListWorkers(t *testing.T) { t.Errorf("unable to create test worker for sqlite: %v", err) } + _, err = _sqlite.CreateWorker(context.TODO(), _workerThree) + if err != nil { + t.Errorf("unable to create test worker for sqlite: %v", err) + } + // setup tests tests := []struct { failure bool + before int64 + active string name string database *engine want []*library.Worker }{ { failure: false, - name: "postgres", + before: newer, + active: "all", + name: "sqlite3 before filter", + database: _sqlite, + want: []*library.Worker{_workerTwo}, + }, + { + failure: false, + before: newer + 1, + active: "all", + name: "postgres catch all", database: _postgres, - want: []*library.Worker{_workerOne, _workerTwo}, + want: []*library.Worker{_workerOne, _workerTwo, _workerThree}, + }, + { + failure: false, + before: newer + 1, + active: "all", + name: "sqlite3 catch all", + database: _sqlite, + want: []*library.Worker{_workerOne, _workerTwo, _workerThree}, }, { failure: false, - name: "sqlite3", + before: newer + 1, + active: "true", + name: "sqlite3 active filter", database: _sqlite, want: []*library.Worker{_workerOne, _workerTwo}, }, @@ -80,7 +114,7 @@ func TestWorker_Engine_ListWorkers(t *testing.T) { // run tests for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got, err := test.database.ListWorkers(context.TODO()) + got, err := test.database.ListWorkers(context.TODO(), test.active, test.before, 0) if test.failure { if err == nil { @@ -94,8 +128,8 @@ func TestWorker_Engine_ListWorkers(t *testing.T) { t.Errorf("ListWorkers for %s returned err: %v", test.name, err) } - if !reflect.DeepEqual(got, test.want) { - t.Errorf("ListWorkers for %s is %v, want %v", test.name, got, test.want) + if diff := cmp.Diff(test.want, got); diff != "" { + t.Errorf("ListWorkers() mismatch (-want +got):\n%s", diff) } }) }