From d715af34db4165013c20639195bd9d17965bec5d Mon Sep 17 00:00:00 2001 From: ecrupper Date: Mon, 26 Jun 2023 11:33:26 -0500 Subject: [PATCH] fix(schedules): updated_at automatically updates to any change to row --- api/schedule/create.go | 2 +- api/schedule/update.go | 2 +- cmd/vela-server/schedule.go | 2 +- database/schedule/interface.go | 2 +- database/schedule/update.go | 15 ++++--- database/schedule/update_test.go | 76 +++++++++++++++++++++++++++++++- docker-compose.yml | 1 + 7 files changed, 88 insertions(+), 12 deletions(-) diff --git a/api/schedule/create.go b/api/schedule/create.go index c8eb92741..365df170e 100644 --- a/api/schedule/create.go +++ b/api/schedule/create.go @@ -170,7 +170,7 @@ func CreateSchedule(c *gin.Context) { dbSchedule.SetActive(true) // send API call to update the schedule - err = database.FromContext(c).UpdateSchedule(dbSchedule) + err = database.FromContext(c).UpdateSchedule(dbSchedule, true) if err != nil { retErr := fmt.Errorf("unable to set schedule %s to active: %w", dbSchedule.GetName(), err) diff --git a/api/schedule/update.go b/api/schedule/update.go index 646e55fc2..b0862132a 100644 --- a/api/schedule/update.go +++ b/api/schedule/update.go @@ -123,7 +123,7 @@ func UpdateSchedule(c *gin.Context) { } // update the schedule within the database - err = database.FromContext(c).UpdateSchedule(s) + err = database.FromContext(c).UpdateSchedule(s, true) if err != nil { retErr := fmt.Errorf("unable to update scheduled %s: %w", scheduleName, err) diff --git a/cmd/vela-server/schedule.go b/cmd/vela-server/schedule.go index ab0a56b90..a9fd38234 100644 --- a/cmd/vela-server/schedule.go +++ b/cmd/vela-server/schedule.go @@ -350,7 +350,7 @@ func processSchedule(s *library.Schedule, compiler compiler.Engine, database dat } // send API call to update schedule for ensuring scheduled_at field is set - err = database.UpdateSchedule(s) + err = database.UpdateSchedule(s, false) if err != nil { return fmt.Errorf("unable to update schedule %s/%s: %w", r.GetFullName(), s.GetName(), err) } diff --git a/database/schedule/interface.go b/database/schedule/interface.go index 8aa14efb5..afa163063 100644 --- a/database/schedule/interface.go +++ b/database/schedule/interface.go @@ -45,5 +45,5 @@ type ScheduleInterface interface { // ListSchedulesForRepo defines a function that gets a list of schedules by repo ID. ListSchedulesForRepo(*library.Repo, int, int) ([]*library.Schedule, int64, error) // UpdateSchedule defines a function that updates an existing schedule. - UpdateSchedule(*library.Schedule) error + UpdateSchedule(*library.Schedule, bool) error } diff --git a/database/schedule/update.go b/database/schedule/update.go index b69f8eff0..c60ae7568 100644 --- a/database/schedule/update.go +++ b/database/schedule/update.go @@ -13,7 +13,7 @@ import ( ) // UpdateSchedule updates an existing schedule in the database. -func (e *engine) UpdateSchedule(s *library.Schedule) error { +func (e *engine) UpdateSchedule(s *library.Schedule, config bool) error { e.logger.WithFields(logrus.Fields{ "schedule": s.GetName(), }).Tracef("updating schedule %s in the database", s.GetName()) @@ -27,9 +27,12 @@ func (e *engine) UpdateSchedule(s *library.Schedule) error { return err } - // send query to the database - return e.client. - Table(constants.TableSchedule). - Save(schedule). - Error + // if update is just setting the scheduled_at, then ignore updating other fields + if config { + err = e.client.Table(constants.TableSchedule).Save(schedule).Error + } else { + err = e.client.Table(constants.TableSchedule).Model(schedule).UpdateColumn("scheduled_at", s.GetScheduledAt()).Error + } + + return err } diff --git a/database/schedule/update_test.go b/database/schedule/update_test.go index 05fff4286..2554cefdc 100644 --- a/database/schedule/update_test.go +++ b/database/schedule/update_test.go @@ -11,7 +11,7 @@ import ( "github.com/DATA-DOG/go-sqlmock" ) -func TestSchedule_Engine_UpdateSchedule(t *testing.T) { +func TestSchedule_Engine_UpdateSchedule_Config(t *testing.T) { _repo := testRepo() _repo.SetID(1) _repo.SetOrg("foo") @@ -67,7 +67,79 @@ WHERE "id" = $10`). // run tests for _, test := range tests { t.Run(test.name, func(t *testing.T) { - err = test.database.UpdateSchedule(_schedule) + err = test.database.UpdateSchedule(_schedule, true) + + if test.failure { + if err == nil { + t.Errorf("UpdateSchedule for %s should have returned err", test.name) + } + + return + } + + if err != nil { + t.Errorf("UpdateSchedule for %s returned err: %v", test.name, err) + } + }) + } +} + +func TestSchedule_Engine_UpdateSchedule_NotConfig(t *testing.T) { + _repo := testRepo() + _repo.SetID(1) + _repo.SetOrg("foo") + _repo.SetName("bar") + _repo.SetFullName("foo/bar") + + _schedule := testSchedule() + _schedule.SetID(1) + _schedule.SetRepoID(1) + _schedule.SetName("nightly") + _schedule.SetEntry("0 0 * * *") + _schedule.SetCreatedAt(1) + _schedule.SetCreatedBy("user1") + _schedule.SetUpdatedAt(1) + _schedule.SetUpdatedBy("user2") + _schedule.SetScheduledAt(1) + + _postgres, _mock := testPostgres(t) + defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }() + + // ensure the mock expects the query + _mock.ExpectExec(`UPDATE "schedules" SET "scheduled_at"=$1 WHERE "id" = $2`). + WithArgs(1, 1). + WillReturnResult(sqlmock.NewResult(1, 1)) + + _sqlite := testSqlite(t) + defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }() + + err := _sqlite.CreateSchedule(_schedule) + if err != nil { + t.Errorf("unable to create test schedule for sqlite: %v", err) + } + + // setup tests + tests := []struct { + failure bool + name string + database *engine + }{ + { + failure: false, + name: "postgres", + database: _postgres, + }, + { + failure: false, + name: "sqlite3", + database: _sqlite, + }, + } + + // run tests + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + err = test.database.UpdateSchedule(_schedule, false) if test.failure { if err == nil { diff --git a/docker-compose.yml b/docker-compose.yml index d030743e6..9bb0b66f9 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -44,6 +44,7 @@ services: VELA_ENABLE_SECURE_COOKIE: 'false' VELA_REPO_ALLOWLIST: '*' VELA_SCHEDULE_ALLOWLIST: '*' + VELA_SCHEDULE_MINIMUM_FREQUENCY: 10m env_file: - .env restart: always