From e320d5bb20b17d868803fad01bd78274c0b3f66e Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 30 Nov 2023 14:09:38 +0100 Subject: [PATCH 01/11] feat: purge old provisioner daemons --- coderd/database/dbauthz/dbauthz.go | 7 +++++++ coderd/database/dbmem/dbmem.go | 7 ++++++- coderd/database/dbmetrics/dbmetrics.go | 7 +++++++ coderd/database/dbmock/dbmock.go | 14 ++++++++++++++ coderd/database/querier.go | 5 +++++ coderd/database/queries.sql.go | 13 +++++++++++++ coderd/database/queries/provisionerdaemons.sql | 7 +++++++ 7 files changed, 59 insertions(+), 1 deletion(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 176c92ca19c78..0e9f9b6599452 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -777,6 +777,13 @@ func (q *querier) DeleteLicense(ctx context.Context, id int32) (int32, error) { return id, nil } +func (q *querier) DeleteOldProvisionerDaemons(ctx context.Context) error { + if err := q.authorizeContext(ctx, rbac.ActionDelete, rbac.ResourceSystem); err != nil { + return err + } + return q.db.DeleteOldProvisionerDaemons(ctx) +} + func (q *querier) DeleteOldWorkspaceAgentLogs(ctx context.Context) error { if err := q.authorizeContext(ctx, rbac.ActionDelete, rbac.ResourceSystem); err != nil { return err diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 3b78d67ebeba4..0e92a67e9da9d 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -1104,8 +1104,13 @@ func (q *FakeQuerier) DeleteLicense(_ context.Context, id int32) (int32, error) return 0, sql.ErrNoRows } +func (q *FakeQuerier) DeleteOldProvisionerDaemons(ctx context.Context) error { + // no-op + return nil +} + func (*FakeQuerier) DeleteOldWorkspaceAgentLogs(_ context.Context) error { - // noop + // no-op return nil } diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index e37153463a24a..4a8fc2f481737 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -197,6 +197,13 @@ func (m metricsStore) DeleteLicense(ctx context.Context, id int32) (int32, error return licenseID, err } +func (m metricsStore) DeleteOldProvisionerDaemons(ctx context.Context) error { + start := time.Now() + r0 := m.s.DeleteOldProvisionerDaemons(ctx) + m.queryLatencies.WithLabelValues("DeleteOldProvisionerDaemons").Observe(time.Since(start).Seconds()) + return r0 +} + func (m metricsStore) DeleteOldWorkspaceAgentLogs(ctx context.Context) error { start := time.Now() r0 := m.s.DeleteOldWorkspaceAgentLogs(ctx) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index bcc03a77ddd1a..c760185172a51 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -281,6 +281,20 @@ func (mr *MockStoreMockRecorder) DeleteLicense(arg0, arg1 interface{}) *gomock.C return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteLicense", reflect.TypeOf((*MockStore)(nil).DeleteLicense), arg0, arg1) } +// DeleteOldProvisionerDaemons mocks base method. +func (m *MockStore) DeleteOldProvisionerDaemons(arg0 context.Context) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteOldProvisionerDaemons", arg0) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteOldProvisionerDaemons indicates an expected call of DeleteOldProvisionerDaemons. +func (mr *MockStoreMockRecorder) DeleteOldProvisionerDaemons(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteOldProvisionerDaemons", reflect.TypeOf((*MockStore)(nil).DeleteOldProvisionerDaemons), arg0) +} + // DeleteOldWorkspaceAgentLogs mocks base method. func (m *MockStore) DeleteOldWorkspaceAgentLogs(arg0 context.Context) error { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index d1b3e070d8b13..612117facf0af 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -54,6 +54,11 @@ type sqlcQuerier interface { DeleteGroupMemberFromGroup(ctx context.Context, arg DeleteGroupMemberFromGroupParams) error DeleteGroupMembersByOrgAndUser(ctx context.Context, arg DeleteGroupMembersByOrgAndUserParams) error DeleteLicense(ctx context.Context, id int32) (int32, error) + // Delete provisioner daemons that have been created at least a week ago + // and have not connected to coderd since a week. + // A provisioner daemon with "zeroed" updated_at column indicates possible + // connectivity issues (no provisioner daemon activity since registration). + DeleteOldProvisionerDaemons(ctx context.Context) error // If an agent hasn't connected in the last 7 days, we purge it's logs. // Logs can take up a lot of space, so it's important we clean up frequently. DeleteOldWorkspaceAgentLogs(ctx context.Context) error diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 3604b7a58d0b1..b88758d030cce 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -2986,6 +2986,19 @@ func (q *sqlQuerier) GetParameterSchemasByJobID(ctx context.Context, jobID uuid. return items, nil } +const deleteOldProvisionerDaemons = `-- name: DeleteOldProvisionerDaemons :exec +DELETE FROM provisioner_daemons WHERE created_at < (NOW() - INTERVAL '7 days') AND updated_at < (NOW() - INTERVAL '7 days') +` + +// Delete provisioner daemons that have been created at least a week ago +// and have not connected to coderd since a week. +// A provisioner daemon with "zeroed" updated_at column indicates possible +// connectivity issues (no provisioner daemon activity since registration). +func (q *sqlQuerier) DeleteOldProvisionerDaemons(ctx context.Context) error { + _, err := q.db.ExecContext(ctx, deleteOldProvisionerDaemons) + return err +} + const getProvisionerDaemons = `-- name: GetProvisionerDaemons :many SELECT id, created_at, updated_at, name, provisioners, replica_id, tags diff --git a/coderd/database/queries/provisionerdaemons.sql b/coderd/database/queries/provisionerdaemons.sql index ccbbf9891b309..9d7024378595b 100644 --- a/coderd/database/queries/provisionerdaemons.sql +++ b/coderd/database/queries/provisionerdaemons.sql @@ -15,3 +15,10 @@ INSERT INTO ) VALUES ($1, $2, $3, $4, $5) RETURNING *; + +-- name: DeleteOldProvisionerDaemons :exec +-- Delete provisioner daemons that have been created at least a week ago +-- and have not connected to coderd since a week. +-- A provisioner daemon with "zeroed" updated_at column indicates possible +-- connectivity issues (no provisioner daemon activity since registration). +DELETE FROM provisioner_daemons WHERE created_at < (NOW() - INTERVAL '7 days') AND updated_at < (NOW() - INTERVAL '7 days'); From 6bf64d37c79f0bff6985a68af9f552b6f9157ef2 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 30 Nov 2023 14:31:49 +0100 Subject: [PATCH 02/11] fix: q --- coderd/database/dbmem/dbmem.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 0e92a67e9da9d..bfd76f9abb7e3 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -1104,7 +1104,7 @@ func (q *FakeQuerier) DeleteLicense(_ context.Context, id int32) (int32, error) return 0, sql.ErrNoRows } -func (q *FakeQuerier) DeleteOldProvisionerDaemons(ctx context.Context) error { +func (*FakeQuerier) DeleteOldProvisionerDaemons(ctx context.Context) error { // no-op return nil } From f44b62b3e1457383b821450cc6018faa47e87bd8 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 30 Nov 2023 14:57:28 +0100 Subject: [PATCH 03/11] fix: ctx --- coderd/database/dbmem/dbmem.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index bfd76f9abb7e3..a7e0090128fef 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -1104,7 +1104,7 @@ func (q *FakeQuerier) DeleteLicense(_ context.Context, id int32) (int32, error) return 0, sql.ErrNoRows } -func (*FakeQuerier) DeleteOldProvisionerDaemons(ctx context.Context) error { +func (*FakeQuerier) DeleteOldProvisionerDaemons(_ context.Context) error { // no-op return nil } From e83da4127a3f61ffee747a69c2a037d382050560 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 30 Nov 2023 15:21:21 +0100 Subject: [PATCH 04/11] implement DeleteOldProvisionerDaemons --- coderd/database/dbmem/dbmem.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index a7e0090128fef..af7f68944274f 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -1104,8 +1104,21 @@ func (q *FakeQuerier) DeleteLicense(_ context.Context, id int32) (int32, error) return 0, sql.ErrNoRows } -func (*FakeQuerier) DeleteOldProvisionerDaemons(_ context.Context) error { - // no-op +func (q *FakeQuerier) DeleteOldProvisionerDaemons(_ context.Context) error { + q.mutex.Lock() + defer q.mutex.Unlock() + + now := dbtime.Now() + weekInterval := 7 * 24 * time.Hour + + var validDaemons []database.ProvisionerDaemon + for _, p := range q.provisionerDaemons { + if !(p.CreatedAt.Before(now.Add(-weekInterval)) && p.UpdatedAt.Valid && p.UpdatedAt.Time.Before(now.Add(-weekInterval))) { + continue + } + validDaemons = append(validDaemons, p) + } + q.provisionerDaemons = validDaemons return nil } From 29a8ead12adc678ef2795f5ccff9dcbf9e8d05fb Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 30 Nov 2023 15:25:09 +0100 Subject: [PATCH 05/11] add to purge --- coderd/database/dbpurge/dbpurge.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/coderd/database/dbpurge/dbpurge.go b/coderd/database/dbpurge/dbpurge.go index b1062eee312ed..8968fd2fcf730 100644 --- a/coderd/database/dbpurge/dbpurge.go +++ b/coderd/database/dbpurge/dbpurge.go @@ -45,6 +45,9 @@ func New(ctx context.Context, logger slog.Logger, db database.Store) io.Closer { eg.Go(func() error { return db.DeleteOldWorkspaceAgentStats(ctx) }) + eg.Go(func() error { + return db.DeleteOldProvisionerDaemons(ctx) + }) err := eg.Wait() if err != nil { if errors.Is(err, context.Canceled) { From f4b83ad229384cde74e06043af4f697600174279 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 30 Nov 2023 15:44:15 +0100 Subject: [PATCH 06/11] Run every 10 mins, use doTick --- coderd/database/dbpurge/dbpurge.go | 53 +++++++++++++++++------------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/coderd/database/dbpurge/dbpurge.go b/coderd/database/dbpurge/dbpurge.go index 8968fd2fcf730..f8e2ca231983a 100644 --- a/coderd/database/dbpurge/dbpurge.go +++ b/coderd/database/dbpurge/dbpurge.go @@ -14,7 +14,7 @@ import ( ) const ( - delay = 24 * time.Hour + delay = 10 * time.Minute ) // New creates a new periodically purging database instance. @@ -23,40 +23,47 @@ const ( // This is for cleaning up old, unused resources from the database that take up space. func New(ctx context.Context, logger slog.Logger, db database.Store) io.Closer { closed := make(chan struct{}) + ctx, cancelFunc := context.WithCancel(ctx) //nolint:gocritic // The system purges old db records without user input. ctx = dbauthz.AsSystemRestricted(ctx) + + // Use time.Nanosecond to force an initial tick. It will be reset to the + // correct duration after executing once. + ticker := time.NewTicker(time.Nanosecond) + doTick := func() { + defer ticker.Reset(delay) + + var eg errgroup.Group + eg.Go(func() error { + return db.DeleteOldWorkspaceAgentLogs(ctx) + }) + eg.Go(func() error { + return db.DeleteOldWorkspaceAgentStats(ctx) + }) + eg.Go(func() error { + return db.DeleteOldProvisionerDaemons(ctx) + }) + err := eg.Wait() + if err != nil { + if errors.Is(err, context.Canceled) { + return + } + logger.Error(ctx, "failed to purge old database entries", slog.Error(err)) + } + } + go func() { defer close(closed) - - ticker := time.NewTicker(delay) defer ticker.Stop() for { select { case <-ctx.Done(): return case <-ticker.C: + ticker.Stop() + doTick() } - - var eg errgroup.Group - eg.Go(func() error { - return db.DeleteOldWorkspaceAgentLogs(ctx) - }) - eg.Go(func() error { - return db.DeleteOldWorkspaceAgentStats(ctx) - }) - eg.Go(func() error { - return db.DeleteOldProvisionerDaemons(ctx) - }) - err := eg.Wait() - if err != nil { - if errors.Is(err, context.Canceled) { - return - } - logger.Error(ctx, "failed to purge old database entries", slog.Error(err)) - } - - ticker.Reset(delay) } }() return &instance{ From 13b7a9937b718ca5ebc48fd88fc6f9a2c8c8180d Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 1 Dec 2023 12:43:56 +0100 Subject: [PATCH 07/11] purge tests --- coderd/database/dbmem/dbmem.go | 4 +- coderd/database/dbpurge/dbpurge_test.go | 58 +++++++++++++++++++ coderd/database/queries.sql.go | 7 ++- .../database/queries/provisionerdaemons.sql | 5 +- 4 files changed, 69 insertions(+), 5 deletions(-) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 0992b08735ef0..6777cdbbb2464 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -1118,10 +1118,11 @@ func (q *FakeQuerier) DeleteOldProvisionerDaemons(_ context.Context) error { now := dbtime.Now() weekInterval := 7 * 24 * time.Hour + weekAgo := now.Add(-weekInterval) var validDaemons []database.ProvisionerDaemon for _, p := range q.provisionerDaemons { - if !(p.CreatedAt.Before(now.Add(-weekInterval)) && p.UpdatedAt.Valid && p.UpdatedAt.Time.Before(now.Add(-weekInterval))) { + if p.CreatedAt.Before(weekAgo) && p.UpdatedAt.Valid && p.UpdatedAt.Time.Before(weekAgo) { continue } validDaemons = append(validDaemons, p) @@ -4863,6 +4864,7 @@ func (q *FakeQuerier) InsertProvisionerDaemon(_ context.Context, arg database.In Name: arg.Name, Provisioners: arg.Provisioners, Tags: arg.Tags, + UpdatedAt: arg.UpdatedAt, } q.provisionerDaemons = append(q.provisionerDaemons, daemon) return daemon, nil diff --git a/coderd/database/dbpurge/dbpurge_test.go b/coderd/database/dbpurge/dbpurge_test.go index 64fc74b4775ca..c5edf359f4bc9 100644 --- a/coderd/database/dbpurge/dbpurge_test.go +++ b/coderd/database/dbpurge/dbpurge_test.go @@ -2,15 +2,21 @@ package dbpurge_test import ( "context" + "database/sql" "testing" + "time" "go.uber.org/goleak" "github.com/stretchr/testify/require" "cdr.dev/slog/sloggers/slogtest" + "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbmem" "github.com/coder/coder/v2/coderd/database/dbpurge" + "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/testutil" ) func TestMain(m *testing.M) { @@ -24,3 +30,55 @@ func TestPurge(t *testing.T) { err := purger.Close() require.NoError(t, err) } + +func TestDeleteOldProvisionerDaemons(t *testing.T) { + t.Parallel() + + db, _ := dbtestutil.NewDB(t) + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + now := dbtime.Now() + + // given + _, err := db.InsertProvisionerDaemon(ctx, database.InsertProvisionerDaemonParams{ + Name: "external-0", + CreatedAt: now.Add(-7 * 24 * time.Hour), + UpdatedAt: sql.NullTime{Valid: true, Time: now.Add(-7 * 24 * time.Hour).Add(time.Minute)}, + }) + require.NoError(t, err) + _, err = db.InsertProvisionerDaemon(ctx, database.InsertProvisionerDaemonParams{ + Name: "external-1", + CreatedAt: now.Add(-8 * 24 * time.Hour), + UpdatedAt: sql.NullTime{Valid: true, Time: now.Add(-8 * 24 * time.Hour)}, + }) + require.NoError(t, err) + _, err = db.InsertProvisionerDaemon(ctx, database.InsertProvisionerDaemonParams{ + Name: "external-2", + CreatedAt: now.Add(-9 * 24 * time.Hour), + UpdatedAt: sql.NullTime{Valid: true, Time: now.Add(-9 * 24 * time.Hour)}, + }) + require.NoError(t, err) + _, err = db.InsertProvisionerDaemon(ctx, database.InsertProvisionerDaemonParams{ + Name: "external-3", + CreatedAt: now.Add(-6 * 24 * time.Hour), + UpdatedAt: sql.NullTime{Valid: true, Time: now.Add(-6 * 24 * time.Hour)}, + }) + require.NoError(t, err) + + // when + closer := dbpurge.New(ctx, logger, db) + defer closer.Close() + + // then + require.Eventually(t, func() bool { + daemons, err := db.GetProvisionerDaemons(ctx) + if err != nil { + return false + } + return len(daemons) == 2 + }, testutil.WaitShort, testutil.IntervalFast) + +} diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 4626a1791ceda..43fd8bc09b854 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3044,10 +3044,11 @@ INSERT INTO created_at, "name", provisioners, - tags + tags, + updated_at ) VALUES - ($1, $2, $3, $4, $5) RETURNING id, created_at, updated_at, name, provisioners, replica_id, tags + ($1, $2, $3, $4, $5, $6) RETURNING id, created_at, updated_at, name, provisioners, replica_id, tags ` type InsertProvisionerDaemonParams struct { @@ -3056,6 +3057,7 @@ type InsertProvisionerDaemonParams struct { Name string `db:"name" json:"name"` Provisioners []ProvisionerType `db:"provisioners" json:"provisioners"` Tags StringMap `db:"tags" json:"tags"` + UpdatedAt sql.NullTime `db:"updated_at" json:"updated_at"` } func (q *sqlQuerier) InsertProvisionerDaemon(ctx context.Context, arg InsertProvisionerDaemonParams) (ProvisionerDaemon, error) { @@ -3065,6 +3067,7 @@ func (q *sqlQuerier) InsertProvisionerDaemon(ctx context.Context, arg InsertProv arg.Name, pq.Array(arg.Provisioners), arg.Tags, + arg.UpdatedAt, ) var i ProvisionerDaemon err := row.Scan( diff --git a/coderd/database/queries/provisionerdaemons.sql b/coderd/database/queries/provisionerdaemons.sql index 9d7024378595b..1701a1cf8d746 100644 --- a/coderd/database/queries/provisionerdaemons.sql +++ b/coderd/database/queries/provisionerdaemons.sql @@ -11,10 +11,11 @@ INSERT INTO created_at, "name", provisioners, - tags + tags, + updated_at ) VALUES - ($1, $2, $3, $4, $5) RETURNING *; + ($1, $2, $3, $4, $5, $6) RETURNING *; -- name: DeleteOldProvisionerDaemons :exec -- Delete provisioner daemons that have been created at least a week ago From 142f2e237192e0b5e697e848cef5c7c59484f6b9 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 1 Dec 2023 12:48:33 +0100 Subject: [PATCH 08/11] fmt --- coderd/database/dbpurge/dbpurge_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/coderd/database/dbpurge/dbpurge_test.go b/coderd/database/dbpurge/dbpurge_test.go index c5edf359f4bc9..533a9e799edd6 100644 --- a/coderd/database/dbpurge/dbpurge_test.go +++ b/coderd/database/dbpurge/dbpurge_test.go @@ -80,5 +80,4 @@ func TestDeleteOldProvisionerDaemons(t *testing.T) { } return len(daemons) == 2 }, testutil.WaitShort, testutil.IntervalFast) - } From fe84029b928328ec98b0c6bef500c60b1e78fbf8 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 1 Dec 2023 13:09:37 +0100 Subject: [PATCH 09/11] Fix --- coderd/database/dbmem/dbmem.go | 2 +- coderd/database/dbpurge/dbpurge_test.go | 31 ++++++++++++------- coderd/database/queries.sql.go | 5 ++- .../database/queries/provisionerdaemons.sql | 5 ++- 4 files changed, 28 insertions(+), 15 deletions(-) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 6777cdbbb2464..4cae64776d4d9 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -1122,7 +1122,7 @@ func (q *FakeQuerier) DeleteOldProvisionerDaemons(_ context.Context) error { var validDaemons []database.ProvisionerDaemon for _, p := range q.provisionerDaemons { - if p.CreatedAt.Before(weekAgo) && p.UpdatedAt.Valid && p.UpdatedAt.Time.Before(weekAgo) { + if (p.CreatedAt.Before(weekAgo) && !p.UpdatedAt.Valid) || (p.UpdatedAt.Valid && p.UpdatedAt.Time.Before(weekAgo)) { continue } validDaemons = append(validDaemons, p) diff --git a/coderd/database/dbpurge/dbpurge_test.go b/coderd/database/dbpurge/dbpurge_test.go index 533a9e799edd6..6fed46f6e30c0 100644 --- a/coderd/database/dbpurge/dbpurge_test.go +++ b/coderd/database/dbpurge/dbpurge_test.go @@ -44,27 +44,34 @@ func TestDeleteOldProvisionerDaemons(t *testing.T) { // given _, err := db.InsertProvisionerDaemon(ctx, database.InsertProvisionerDaemonParams{ - Name: "external-0", - CreatedAt: now.Add(-7 * 24 * time.Hour), - UpdatedAt: sql.NullTime{Valid: true, Time: now.Add(-7 * 24 * time.Hour).Add(time.Minute)}, + // Provisioner daemon created 14 days ago, and checked in just before 7 days deadline. + Name: "external-0", + Provisioners: []database.ProvisionerType{"echo"}, + CreatedAt: now.Add(-14 * 24 * time.Hour), + UpdatedAt: sql.NullTime{Valid: true, Time: now.Add(-7 * 24 * time.Hour).Add(time.Minute)}, }) require.NoError(t, err) _, err = db.InsertProvisionerDaemon(ctx, database.InsertProvisionerDaemonParams{ - Name: "external-1", - CreatedAt: now.Add(-8 * 24 * time.Hour), - UpdatedAt: sql.NullTime{Valid: true, Time: now.Add(-8 * 24 * time.Hour)}, + // Provisioner daemon created 8 days ago, and checked in last time an hour after creation. + Name: "external-1", + Provisioners: []database.ProvisionerType{"echo"}, + CreatedAt: now.Add(-8 * 24 * time.Hour), + UpdatedAt: sql.NullTime{Valid: true, Time: now.Add(-8 * 24 * time.Hour).Add(time.Hour)}, }) require.NoError(t, err) _, err = db.InsertProvisionerDaemon(ctx, database.InsertProvisionerDaemonParams{ - Name: "external-2", - CreatedAt: now.Add(-9 * 24 * time.Hour), - UpdatedAt: sql.NullTime{Valid: true, Time: now.Add(-9 * 24 * time.Hour)}, + // Provisioner daemon created 9 days ago, and never checked in. + Name: "external-2", + Provisioners: []database.ProvisionerType{"echo"}, + CreatedAt: now.Add(-9 * 24 * time.Hour), }) require.NoError(t, err) _, err = db.InsertProvisionerDaemon(ctx, database.InsertProvisionerDaemonParams{ - Name: "external-3", - CreatedAt: now.Add(-6 * 24 * time.Hour), - UpdatedAt: sql.NullTime{Valid: true, Time: now.Add(-6 * 24 * time.Hour)}, + // Provisioner daemon created 6 days ago, and never checked in. + Name: "external-3", + Provisioners: []database.ProvisionerType{"echo"}, + CreatedAt: now.Add(-6 * 24 * time.Hour), + UpdatedAt: sql.NullTime{Valid: true, Time: now.Add(-6 * 24 * time.Hour)}, }) require.NoError(t, err) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 43fd8bc09b854..aa8f9c542f384 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -2987,7 +2987,10 @@ func (q *sqlQuerier) GetParameterSchemasByJobID(ctx context.Context, jobID uuid. } const deleteOldProvisionerDaemons = `-- name: DeleteOldProvisionerDaemons :exec -DELETE FROM provisioner_daemons WHERE created_at < (NOW() - INTERVAL '7 days') AND updated_at < (NOW() - INTERVAL '7 days') +DELETE FROM provisioner_daemons WHERE ( + (created_at < (NOW() - INTERVAL '7 days') AND updated_at IS NULL) OR + (updated_at IS NOT NULL AND updated_at < (NOW() - INTERVAL '7 days')) +) ` // Delete provisioner daemons that have been created at least a week ago diff --git a/coderd/database/queries/provisionerdaemons.sql b/coderd/database/queries/provisionerdaemons.sql index 1701a1cf8d746..ca8b5733ce6ef 100644 --- a/coderd/database/queries/provisionerdaemons.sql +++ b/coderd/database/queries/provisionerdaemons.sql @@ -22,4 +22,7 @@ VALUES -- and have not connected to coderd since a week. -- A provisioner daemon with "zeroed" updated_at column indicates possible -- connectivity issues (no provisioner daemon activity since registration). -DELETE FROM provisioner_daemons WHERE created_at < (NOW() - INTERVAL '7 days') AND updated_at < (NOW() - INTERVAL '7 days'); +DELETE FROM provisioner_daemons WHERE ( + (created_at < (NOW() - INTERVAL '7 days') AND updated_at IS NULL) OR + (updated_at IS NOT NULL AND updated_at < (NOW() - INTERVAL '7 days')) +); From c489c4a22d6644e2816c5bab733bc9fb0f29d1b1 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 1 Dec 2023 13:30:47 +0100 Subject: [PATCH 10/11] Fix: ID --- coderd/database/dbpurge/dbpurge_test.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/coderd/database/dbpurge/dbpurge_test.go b/coderd/database/dbpurge/dbpurge_test.go index 6fed46f6e30c0..bad857e586228 100644 --- a/coderd/database/dbpurge/dbpurge_test.go +++ b/coderd/database/dbpurge/dbpurge_test.go @@ -7,7 +7,9 @@ import ( "time" "go.uber.org/goleak" + "golang.org/x/exp/slices" + "github.com/google/uuid" "github.com/stretchr/testify/require" "cdr.dev/slog/sloggers/slogtest" @@ -45,6 +47,7 @@ func TestDeleteOldProvisionerDaemons(t *testing.T) { // given _, err := db.InsertProvisionerDaemon(ctx, database.InsertProvisionerDaemonParams{ // Provisioner daemon created 14 days ago, and checked in just before 7 days deadline. + ID: uuid.New(), Name: "external-0", Provisioners: []database.ProvisionerType{"echo"}, CreatedAt: now.Add(-14 * 24 * time.Hour), @@ -53,6 +56,7 @@ func TestDeleteOldProvisionerDaemons(t *testing.T) { require.NoError(t, err) _, err = db.InsertProvisionerDaemon(ctx, database.InsertProvisionerDaemonParams{ // Provisioner daemon created 8 days ago, and checked in last time an hour after creation. + ID: uuid.New(), Name: "external-1", Provisioners: []database.ProvisionerType{"echo"}, CreatedAt: now.Add(-8 * 24 * time.Hour), @@ -61,6 +65,7 @@ func TestDeleteOldProvisionerDaemons(t *testing.T) { require.NoError(t, err) _, err = db.InsertProvisionerDaemon(ctx, database.InsertProvisionerDaemonParams{ // Provisioner daemon created 9 days ago, and never checked in. + ID: uuid.New(), Name: "external-2", Provisioners: []database.ProvisionerType{"echo"}, CreatedAt: now.Add(-9 * 24 * time.Hour), @@ -68,6 +73,7 @@ func TestDeleteOldProvisionerDaemons(t *testing.T) { require.NoError(t, err) _, err = db.InsertProvisionerDaemon(ctx, database.InsertProvisionerDaemonParams{ // Provisioner daemon created 6 days ago, and never checked in. + ID: uuid.New(), Name: "external-3", Provisioners: []database.ProvisionerType{"echo"}, CreatedAt: now.Add(-6 * 24 * time.Hour), @@ -85,6 +91,13 @@ func TestDeleteOldProvisionerDaemons(t *testing.T) { if err != nil { return false } - return len(daemons) == 2 + return contains(daemons, "external-0") && + contains(daemons, "external-3") }, testutil.WaitShort, testutil.IntervalFast) } + +func contains(daemons []database.ProvisionerDaemon, name string) bool { + return slices.ContainsFunc(daemons, func(d database.ProvisionerDaemon) bool { + return d.Name == name + }) +} From a31dbe94951a6341bcbf3354d1fc4377e77d465c Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 1 Dec 2023 13:32:14 +0100 Subject: [PATCH 11/11] Fix: import --- coderd/database/dbpurge/dbpurge.go | 1 + coderd/database/dbpurge/dbpurge_test.go | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/coderd/database/dbpurge/dbpurge.go b/coderd/database/dbpurge/dbpurge.go index f8e2ca231983a..d3fc56a8c5f21 100644 --- a/coderd/database/dbpurge/dbpurge.go +++ b/coderd/database/dbpurge/dbpurge.go @@ -9,6 +9,7 @@ import ( "golang.org/x/sync/errgroup" "cdr.dev/slog" + "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" ) diff --git a/coderd/database/dbpurge/dbpurge_test.go b/coderd/database/dbpurge/dbpurge_test.go index bad857e586228..a7926a7079121 100644 --- a/coderd/database/dbpurge/dbpurge_test.go +++ b/coderd/database/dbpurge/dbpurge_test.go @@ -6,13 +6,13 @@ import ( "testing" "time" - "go.uber.org/goleak" - "golang.org/x/exp/slices" - "github.com/google/uuid" "github.com/stretchr/testify/require" + "go.uber.org/goleak" + "golang.org/x/exp/slices" "cdr.dev/slog/sloggers/slogtest" + "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbmem" "github.com/coder/coder/v2/coderd/database/dbpurge"