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

@@ -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)
 		}

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)
 

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{

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)
 }

database/worker/list.go

@@ -4,37 +4,40 @@ package worker
 
 import (
 	"context"
+	"fmt"
+	"strconv"
 
 	"github.com/go-vela/types/constants"
 	"github.com/go-vela/types/database"
 	"github.com/go-vela/types/library"
 )
 
 // 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
 	}

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)
 			}
 		})
 	}