From 355b16b9925e1779faa7b0508e584f4db759b201 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 13 Sep 2023 17:51:03 +0100 Subject: [PATCH 01/17] feat(coderd/database/dbtestutil): set a random timezone in the db --- coderd/database/dbtestutil/db.go | 80 +++++++++++++++++++++++++++++++- 1 file changed, 79 insertions(+), 1 deletion(-) diff --git a/coderd/database/dbtestutil/db.go b/coderd/database/dbtestutil/db.go index 00eae9dd11218..1dc725ef19409 100644 --- a/coderd/database/dbtestutil/db.go +++ b/coderd/database/dbtestutil/db.go @@ -3,7 +3,10 @@ package dbtestutil import ( "context" "database/sql" + "fmt" + "net/url" "os" + "strings" "testing" "github.com/stretchr/testify/require" @@ -19,9 +22,29 @@ func WillUsePostgres() bool { return os.Getenv("DB") != "" } -func NewDB(t testing.TB) (database.Store, pubsub.Pubsub) { +type options struct { + fixedTimezone bool +} + +type Option func(*options) + +// WithFixedTimezone disables the random timezone setting for the database. +// +// DEPRECATED: If you need to use this, you may have a timezone-related bug. +func WithFixedTimezone() Option { + return func(o *options) { + o.fixedTimezone = true + } +} + +func NewDB(t testing.TB, opts ...Option) (database.Store, pubsub.Pubsub) { t.Helper() + var o options + for _, opt := range opts { + opt(&o) + } + db := dbfake.New() ps := pubsub.NewInMemory() if WillUsePostgres() { @@ -35,6 +58,13 @@ func NewDB(t testing.TB) (database.Store, pubsub.Pubsub) { require.NoError(t, err) t.Cleanup(closePg) } + + if !o.fixedTimezone { + // To make sure we find timezone-related issues, we set the timezone of the database to a random one. + dbName := dbNameFromConnectionURL(t, connectionURL) + setRandDBTimezone(t, connectionURL, dbName) + } + sqlDB, err := sql.Open("postgres", connectionURL) require.NoError(t, err) t.Cleanup(func() { @@ -51,3 +81,51 @@ func NewDB(t testing.TB) (database.Store, pubsub.Pubsub) { return db, ps } + +// setRandDBTimezone sets the timezone of the database to the given timezone. +// The timezone change does not take effect until the next session, so +// we do this in our own connection +func setRandDBTimezone(t testing.TB, dbURL, dbname string) { + t.Helper() + + sqlDB, err := sql.Open("postgres", dbURL) + require.NoError(t, err) + defer func() { + _ = sqlDB.Close() + }() + + // Pick a random timezone. We can simply pick from pg_timezone_names. + var tz string + err = sqlDB.QueryRow("SELECT name FROM pg_timezone_names ORDER BY RANDOM() LIMIT 1").Scan(&tz) + require.NoError(t, err) + + // Set the timezone for the database. + t.Logf("setting timezone of database %q to %q", dbname, tz) + // We apparently can't use placeholders here, sadly. + // nolint: gosec // This is not user input and this is only executed in tests + _, err = sqlDB.Exec(fmt.Sprintf("ALTER DATABASE %s SET TIMEZONE TO %q", dbname, tz)) + require.NoError(t, err, "failed to set timezone for database") + + // Ensure the timezone was set. + // We need to reconnect for this. + _ = sqlDB.Close() + + sqlDB, err = sql.Open("postgres", dbURL) + defer func() { + _ = sqlDB.Close() + }() + require.NoError(t, err, "failed to reconnect to database") + var dbTz string + err = sqlDB.QueryRow("SHOW TIMEZONE").Scan(&dbTz) + require.NoError(t, err, "failed to get timezone from database") + require.Equal(t, tz, dbTz, "database timezone was not set correctly") +} + +// dbNameFromConnectionURL returns the database name from the given connection URL. +func dbNameFromConnectionURL(t testing.TB, connectionURL string) string { + // connectionURL is of the form postgres://user:pass@host:port/dbname + // We want to extract the dbname part. + u, err := url.Parse(connectionURL) + require.NoError(t, err) + return strings.TrimPrefix(u.Path, "/") +} From 986db8bc40eecc22dfbabb0e6449abceeba8e786 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 14 Sep 2023 09:25:12 +0100 Subject: [PATCH 02/17] remove check for tz after set --- coderd/database/dbtestutil/db.go | 23 ++++------------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/coderd/database/dbtestutil/db.go b/coderd/database/dbtestutil/db.go index 1dc725ef19409..a0e68afff3778 100644 --- a/coderd/database/dbtestutil/db.go +++ b/coderd/database/dbtestutil/db.go @@ -83,14 +83,14 @@ func NewDB(t testing.TB, opts ...Option) (database.Store, pubsub.Pubsub) { } // setRandDBTimezone sets the timezone of the database to the given timezone. -// The timezone change does not take effect until the next session, so -// we do this in our own connection func setRandDBTimezone(t testing.TB, dbURL, dbname string) { t.Helper() sqlDB, err := sql.Open("postgres", dbURL) require.NoError(t, err) defer func() { + // The updated timezone only comes into effect on reconnect, so close + // the DB after we're done. _ = sqlDB.Close() }() @@ -105,26 +105,11 @@ func setRandDBTimezone(t testing.TB, dbURL, dbname string) { // nolint: gosec // This is not user input and this is only executed in tests _, err = sqlDB.Exec(fmt.Sprintf("ALTER DATABASE %s SET TIMEZONE TO %q", dbname, tz)) require.NoError(t, err, "failed to set timezone for database") - - // Ensure the timezone was set. - // We need to reconnect for this. - _ = sqlDB.Close() - - sqlDB, err = sql.Open("postgres", dbURL) - defer func() { - _ = sqlDB.Close() - }() - require.NoError(t, err, "failed to reconnect to database") - var dbTz string - err = sqlDB.QueryRow("SHOW TIMEZONE").Scan(&dbTz) - require.NoError(t, err, "failed to get timezone from database") - require.Equal(t, tz, dbTz, "database timezone was not set correctly") } -// dbNameFromConnectionURL returns the database name from the given connection URL. +// dbNameFromConnectionURL returns the database name from the given connection URL, +// where connectionURL is of the form postgres://user:pass@host:port/dbname func dbNameFromConnectionURL(t testing.TB, connectionURL string) string { - // connectionURL is of the form postgres://user:pass@host:port/dbname - // We want to extract the dbname part. u, err := url.Parse(connectionURL) require.NoError(t, err) return strings.TrimPrefix(u.Path, "/") From fc3ba524b19b25fa4db94630991d0dfb2065ad8e Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 14 Sep 2023 10:43:19 +0100 Subject: [PATCH 03/17] clitest: fix timestamp regex to match non-UTC timezones --- cli/clitest/golden.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/clitest/golden.go b/cli/clitest/golden.go index 61dbb5e9f991a..2b7019cb7ef55 100644 --- a/cli/clitest/golden.go +++ b/cli/clitest/golden.go @@ -28,7 +28,7 @@ import ( // make update-golden-files var UpdateGoldenFiles = flag.Bool("update", false, "update .golden files") -var timestampRegex = regexp.MustCompile(`(?i)\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(.\d+)?Z`) +var timestampRegex = regexp.MustCompile(`(?i)\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(.\d+)?(Z|\+\d+:\d+)`) type CommandHelpCase struct { Name string From b3c559a18f660148593f6cd70fb543148717e748 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 14 Sep 2023 10:44:04 +0100 Subject: [PATCH 04/17] set fixed timezone in some existing unit tests --- coderd/workspaceagents_test.go | 4 ++++ coderd/workspaces_test.go | 3 +++ enterprise/coderd/templates_test.go | 4 ++++ enterprise/coderd/workspaces_test.go | 5 +++++ 4 files changed, 16 insertions(+) diff --git a/coderd/workspaceagents_test.go b/coderd/workspaceagents_test.go index d900ead4175cf..603ed62841f1b 100644 --- a/coderd/workspaceagents_test.go +++ b/coderd/workspaceagents_test.go @@ -23,6 +23,7 @@ import ( "github.com/coder/coder/v2/agent" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk/agentsdk" @@ -1184,8 +1185,11 @@ func TestWorkspaceAgent_LifecycleState(t *testing.T) { func TestWorkspaceAgent_Metadata(t *testing.T) { t.Parallel() + db, ps := dbtestutil.NewDB(t, dbtestutil.WithFixedTimezone()) client := coderdtest.New(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, + Database: db, + Pubsub: ps, }) user := coderdtest.CreateFirstUser(t, client) authToken := uuid.NewString() diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 9d4082d4e4e39..dfbafcc15b8ce 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -1510,8 +1510,11 @@ func TestWorkspaceFilterManual(t *testing.T) { t.Run("LastUsed", func(t *testing.T) { t.Parallel() + db, ps := dbtestutil.NewDB(t, dbtestutil.WithFixedTimezone()) client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, + Database: db, + Pubsub: ps, }) user := coderdtest.CreateFirstUser(t, client) authToken := uuid.NewString() diff --git a/enterprise/coderd/templates_test.go b/enterprise/coderd/templates_test.go index ad961368ee928..131534b206541 100644 --- a/enterprise/coderd/templates_test.go +++ b/enterprise/coderd/templates_test.go @@ -13,6 +13,7 @@ import ( "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/cryptorand" @@ -368,10 +369,13 @@ func TestTemplates(t *testing.T) { t.Run("UpdateLastUsedAt", func(t *testing.T) { t.Parallel() + db, ps := dbtestutil.NewDB(t, dbtestutil.WithFixedTimezone()) ctx := testutil.Context(t, testutil.WaitMedium) client, user := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ IncludeProvisionerDaemon: true, + Database: db, + Pubsub: ps, }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{ diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 2424b9ee8cc06..df429714cc1d8 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -15,6 +15,7 @@ import ( "github.com/coder/coder/v2/coderd/autobuild" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbtestutil" agplschedule "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/schedule/cron" "github.com/coder/coder/v2/coderd/util/ptr" @@ -641,9 +642,13 @@ func TestWorkspacesFiltering(t *testing.T) { dormantTTL := 24 * time.Hour + db, ps := dbtestutil.NewDB(t, dbtestutil.WithFixedTimezone()) + client, user := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ IncludeProvisionerDaemon: true, + Database: db, + Pubsub: ps, }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{ From f57dc9d06b8d3d6a9cc10c4a75614ed6b7a54f59 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 14 Sep 2023 11:19:39 +0100 Subject: [PATCH 05/17] fixup! set fixed timezone in some existing unit tests --- cli/clitest/golden.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/clitest/golden.go b/cli/clitest/golden.go index 2b7019cb7ef55..531aca6bc88b4 100644 --- a/cli/clitest/golden.go +++ b/cli/clitest/golden.go @@ -28,7 +28,7 @@ import ( // make update-golden-files var UpdateGoldenFiles = flag.Bool("update", false, "update .golden files") -var timestampRegex = regexp.MustCompile(`(?i)\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(.\d+)?(Z|\+\d+:\d+)`) +var timestampRegex = regexp.MustCompile(`(?i)\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(.\d+)?(Z|[+-]\d+:\d+)`) type CommandHelpCase struct { Name string From d3d12745b10155d31879b92bbd02f08c79c3f1f6 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 14 Sep 2023 11:28:16 +0100 Subject: [PATCH 06/17] fix timezone bug in ActivityBumpWorkspace query --- coderd/activitybump_test.go | 2 ++ coderd/database/queries.sql.go | 10 +++++----- coderd/database/queries/activitybump.sql | 10 +++++----- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/coderd/activitybump_test.go b/coderd/activitybump_test.go index 9d76a455a5350..39ac3302ebf19 100644 --- a/coderd/activitybump_test.go +++ b/coderd/activitybump_test.go @@ -30,6 +30,7 @@ func TestWorkspaceActivityBump(t *testing.T) { // doesn't use template autostop requirements and instead edits the // max_deadline on the build directly in the database. setupActivityTest := func(t *testing.T, deadline ...time.Duration) (client *codersdk.Client, workspace codersdk.Workspace, assertBumped func(want bool)) { + t.Helper() const ttl = time.Minute maxTTL := time.Duration(0) if len(deadline) > 0 { @@ -120,6 +121,7 @@ func TestWorkspaceActivityBump(t *testing.T) { _ = coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID) return client, workspace, func(want bool) { + t.Helper() if !want { // It is difficult to test the absence of a call in a non-racey // way. In general, it is difficult for the API to generate diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index eac279a1228d7..fb2f9e3552ca6 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -19,10 +19,10 @@ const activityBumpWorkspace = `-- name: ActivityBumpWorkspace :exec WITH latest AS ( SELECT workspace_builds.id::uuid AS build_id, - workspace_builds.deadline::timestamp AS build_deadline, - workspace_builds.max_deadline::timestamp AS build_max_deadline, + workspace_builds.deadline::timestamp with time zone AS build_deadline, + workspace_builds.max_deadline::timestamp with time zone AS build_max_deadline, workspace_builds.transition AS build_transition, - provisioner_jobs.completed_at::timestamp AS job_completed_at, + provisioner_jobs.completed_at::timestamp with time zone AS job_completed_at, (workspaces.ttl / 1000 / 1000 / 1000 || ' seconds')::interval AS ttl_interval FROM workspace_builds JOIN provisioner_jobs @@ -38,7 +38,7 @@ UPDATE SET updated_at = NOW(), deadline = CASE - WHEN l.build_max_deadline = '0001-01-01 00:00:00+00' + WHEN l.build_max_deadline AT TIME ZONE 'UTC' = '0001-01-01 00:00:00+00' THEN NOW() + l.ttl_interval ELSE LEAST(NOW() + l.ttl_interval, l.build_max_deadline) END @@ -46,7 +46,7 @@ FROM latest l WHERE wb.id = l.build_id AND l.job_completed_at IS NOT NULL AND l.build_transition = 'start' -AND l.build_deadline != '0001-01-01 00:00:00+00' +AND l.build_deadline AT TIME ZONE 'UTC' != '0001-01-01 00:00:00+00' AND l.build_deadline - (l.ttl_interval * 0.95) < NOW() ` diff --git a/coderd/database/queries/activitybump.sql b/coderd/database/queries/activitybump.sql index 5b6dab41dabe2..de3546e792b79 100644 --- a/coderd/database/queries/activitybump.sql +++ b/coderd/database/queries/activitybump.sql @@ -6,10 +6,10 @@ WITH latest AS ( SELECT workspace_builds.id::uuid AS build_id, - workspace_builds.deadline::timestamp AS build_deadline, - workspace_builds.max_deadline::timestamp AS build_max_deadline, + workspace_builds.deadline::timestamp with time zone AS build_deadline, + workspace_builds.max_deadline::timestamp with time zone AS build_max_deadline, workspace_builds.transition AS build_transition, - provisioner_jobs.completed_at::timestamp AS job_completed_at, + provisioner_jobs.completed_at::timestamp with time zone AS job_completed_at, (workspaces.ttl / 1000 / 1000 / 1000 || ' seconds')::interval AS ttl_interval FROM workspace_builds JOIN provisioner_jobs @@ -25,7 +25,7 @@ UPDATE SET updated_at = NOW(), deadline = CASE - WHEN l.build_max_deadline = '0001-01-01 00:00:00+00' + WHEN l.build_max_deadline AT TIME ZONE 'UTC' = '0001-01-01 00:00:00+00' THEN NOW() + l.ttl_interval ELSE LEAST(NOW() + l.ttl_interval, l.build_max_deadline) END @@ -34,7 +34,7 @@ WHERE wb.id = l.build_id AND l.job_completed_at IS NOT NULL AND l.build_transition = 'start' -- We only bump if workspace shutdown is manual. -AND l.build_deadline != '0001-01-01 00:00:00+00' +AND l.build_deadline AT TIME ZONE 'UTC' != '0001-01-01 00:00:00+00' -- We only bump when 5% of the deadline has elapsed. AND l.build_deadline - (l.ttl_interval * 0.95) < NOW() ; From 9f1f5c65a21925e9529bb6f8e3b40e51d4644641 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 14 Sep 2023 12:03:39 +0100 Subject: [PATCH 07/17] refactor to separate package, allow specifying tz --- coderd/database/dbtestutil/db.go | 34 +- coderd/database/dbtestutil/randtz/randtz.go | 1034 +++++++++++++++++++ coderd/workspaceagents_test.go | 2 +- coderd/workspaces_test.go | 2 +- enterprise/coderd/templates_test.go | 2 +- enterprise/coderd/workspaces_test.go | 2 +- 6 files changed, 1053 insertions(+), 23 deletions(-) create mode 100644 coderd/database/dbtestutil/randtz/randtz.go diff --git a/coderd/database/dbtestutil/db.go b/coderd/database/dbtestutil/db.go index a0e68afff3778..e3a9e546505f4 100644 --- a/coderd/database/dbtestutil/db.go +++ b/coderd/database/dbtestutil/db.go @@ -13,6 +13,7 @@ import ( "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbfake" + "github.com/coder/coder/v2/coderd/database/dbtestutil/randtz" "github.com/coder/coder/v2/coderd/database/postgres" "github.com/coder/coder/v2/coderd/database/pubsub" ) @@ -23,17 +24,18 @@ func WillUsePostgres() bool { } type options struct { - fixedTimezone bool + fixedTimezone string } type Option func(*options) -// WithFixedTimezone disables the random timezone setting for the database. +// WithTimezone sets the database to the defined timezone instead of +// to a random one. // -// DEPRECATED: If you need to use this, you may have a timezone-related bug. -func WithFixedTimezone() Option { +// Deprecated: If you need to use this, you may have a timezone-related bug. +func WithTimezone(tz string) Option { return func(o *options) { - o.fixedTimezone = true + o.fixedTimezone = tz } } @@ -59,11 +61,13 @@ func NewDB(t testing.TB, opts ...Option) (database.Store, pubsub.Pubsub) { t.Cleanup(closePg) } - if !o.fixedTimezone { - // To make sure we find timezone-related issues, we set the timezone of the database to a random one. - dbName := dbNameFromConnectionURL(t, connectionURL) - setRandDBTimezone(t, connectionURL, dbName) + if o.fixedTimezone == "" { + // To make sure we find timezone-related issues, we set the timezone + // of the database to a random one. + o.fixedTimezone = randtz.Name(t) } + dbName := dbNameFromConnectionURL(t, connectionURL) + setDBTimezone(t, connectionURL, dbName, o.fixedTimezone) sqlDB, err := sql.Open("postgres", connectionURL) require.NoError(t, err) @@ -83,7 +87,7 @@ func NewDB(t testing.TB, opts ...Option) (database.Store, pubsub.Pubsub) { } // setRandDBTimezone sets the timezone of the database to the given timezone. -func setRandDBTimezone(t testing.TB, dbURL, dbname string) { +func setDBTimezone(t testing.TB, dbURL, dbname, tz string) { t.Helper() sqlDB, err := sql.Open("postgres", dbURL) @@ -94,15 +98,7 @@ func setRandDBTimezone(t testing.TB, dbURL, dbname string) { _ = sqlDB.Close() }() - // Pick a random timezone. We can simply pick from pg_timezone_names. - var tz string - err = sqlDB.QueryRow("SELECT name FROM pg_timezone_names ORDER BY RANDOM() LIMIT 1").Scan(&tz) - require.NoError(t, err) - - // Set the timezone for the database. - t.Logf("setting timezone of database %q to %q", dbname, tz) - // We apparently can't use placeholders here, sadly. - // nolint: gosec // This is not user input and this is only executed in tests + // nolint: gosec // This unfortunately does not work with placeholders. _, err = sqlDB.Exec(fmt.Sprintf("ALTER DATABASE %s SET TIMEZONE TO %q", dbname, tz)) require.NoError(t, err, "failed to set timezone for database") } diff --git a/coderd/database/dbtestutil/randtz/randtz.go b/coderd/database/dbtestutil/randtz/randtz.go new file mode 100644 index 0000000000000..1a53bfaf725fd --- /dev/null +++ b/coderd/database/dbtestutil/randtz/randtz.go @@ -0,0 +1,1034 @@ +package randtz + +import ( + "math/rand" + "sync" + "testing" + "time" +) + +var ( + randTZName string + randTZNameOnce sync.Once +) + +// Name returns a random timezone name from the list of all +// timezones known to PostgreSQL. +func Name(t testing.TB) string { + t.Helper() + + randTZNameOnce.Do(func() { + // nolint: gosec // not used for cryptography + rnd := rand.New(rand.NewSource(time.Now().Unix())) + idx := rnd.Intn(len(tznames)) + randTZName = tznames[idx] + t.Logf("Random db timezone is %q\nIf you need a specific timezone, use dbtestutil.WithTimezone()", randTZName) + }) + + return randTZName +} + +// tznames is a list of all timezone names known to postgresql. +// The below list was generated with the query +// select name from pg_timezone_names order by name asc; +var tznames = []string{ + "Africa/Abidjan", + "Africa/Accra", + "Africa/Addis_Ababa", + "Africa/Algiers", + "Africa/Asmara", + "Africa/Asmera", + "Africa/Bamako", + "Africa/Bangui", + "Africa/Banjul", + "Africa/Bissau", + "Africa/Blantyre", + "Africa/Brazzaville", + "Africa/Bujumbura", + "Africa/Cairo", + "Africa/Casablanca", + "Africa/Ceuta", + "Africa/Conakry", + "Africa/Dakar", + "Africa/Dar_es_Salaam", + "Africa/Djibouti", + "Africa/Douala", + "Africa/El_Aaiun", + "Africa/Freetown", + "Africa/Gaborone", + "Africa/Harare", + "Africa/Johannesburg", + "Africa/Juba", + "Africa/Kampala", + "Africa/Khartoum", + "Africa/Kigali", + "Africa/Kinshasa", + "Africa/Lagos", + "Africa/Libreville", + "Africa/Lome", + "Africa/Luanda", + "Africa/Lubumbashi", + "Africa/Lusaka", + "Africa/Malabo", + "Africa/Maputo", + "Africa/Maseru", + "Africa/Mbabane", + "Africa/Mogadishu", + "Africa/Monrovia", + "Africa/Nairobi", + "Africa/Ndjamena", + "Africa/Niamey", + "Africa/Nouakchott", + "Africa/Ouagadougou", + "Africa/Porto-Novo", + "Africa/Sao_Tome", + "Africa/Timbuktu", + "Africa/Tripoli", + "Africa/Tunis", + "Africa/Windhoek", + "America/Adak", + "America/Anchorage", + "America/Anguilla", + "America/Antigua", + "America/Araguaina", + "America/Argentina/Buenos_Aires", + "America/Argentina/Catamarca", + "America/Argentina/ComodRivadavia", + "America/Argentina/Cordoba", + "America/Argentina/Jujuy", + "America/Argentina/La_Rioja", + "America/Argentina/Mendoza", + "America/Argentina/Rio_Gallegos", + "America/Argentina/Salta", + "America/Argentina/San_Juan", + "America/Argentina/San_Luis", + "America/Argentina/Tucuman", + "America/Argentina/Ushuaia", + "America/Aruba", + "America/Asuncion", + "America/Atikokan", + "America/Atka", + "America/Bahia", + "America/Bahia_Banderas", + "America/Barbados", + "America/Belem", + "America/Belize", + "America/Blanc-Sablon", + "America/Boa_Vista", + "America/Bogota", + "America/Boise", + "America/Buenos_Aires", + "America/Cambridge_Bay", + "America/Campo_Grande", + "America/Cancun", + "America/Caracas", + "America/Catamarca", + "America/Cayenne", + "America/Cayman", + "America/Chicago", + "America/Chihuahua", + "America/Ciudad_Juarez", + "America/Coral_Harbor", + "America/Cordoba", + "America/Costa_Rica", + "America/Creston", + "America/Cuiaba", + "America/Curacao", + "America/Danmarkshavn", + "America/Dawson", + "America/Dawson_Creek", + "America/Denver", + "America/Detroit", + "America/Dominica", + "America/Edmonton", + "America/Eirunepe", + "America/El_Salvador", + "America/Ensenada", + "America/Fortaleza", + "America/Fort_Nelson", + "America/Fort_Wayne", + "America/Glace_Bay", + "America/Godthab", + "America/Goose_Bay", + "America/Grand_Turk", + "America/Grenada", + "America/Guadeloupe", + "America/Guatemala", + "America/Guayaquil", + "America/Guyana", + "America/Halifax", + "America/Havana", + "America/Hermosillo", + "America/Indiana/Indianapolis", + "America/Indiana/Knox", + "America/Indiana/Marengo", + "America/Indiana/Petersburg", + "America/Indianapolis", + "America/Indiana/Tell_City", + "America/Indiana/Vevay", + "America/Indiana/Vincennes", + "America/Indiana/Winamac", + "America/Inuvik", + "America/Iqaluit", + "America/Jamaica", + "America/Jujuy", + "America/Juneau", + "America/Kentucky/Louisville", + "America/Kentucky/Monticello", + "America/Knox_IN", + "America/Kralendijk", + "America/La_Paz", + "America/Lima", + "America/Los_Angeles", + "America/Louisville", + "America/Lower_Princes", + "America/Maceio", + "America/Managua", + "America/Manaus", + "America/Marigot", + "America/Martinique", + "America/Matamoros", + "America/Mazatlan", + "America/Mendoza", + "America/Menominee", + "America/Merida", + "America/Metlakatla", + "America/Mexico_City", + "America/Miquelon", + "America/Moncton", + "America/Monterrey", + "America/Montevideo", + "America/Montreal", + "America/Montserrat", + "America/Nassau", + "America/New_York", + "America/Nipigon", + "America/Nome", + "America/Noronha", + "America/North_Dakota/Beulah", + "America/North_Dakota/Center", + "America/North_Dakota/New_Salem", + "America/Nuuk", + "America/Ojinaga", + "America/Panama", + "America/Pangnirtung", + "America/Paramaribo", + "America/Phoenix", + "America/Port-au-Prince", + "America/Porto_Acre", + "America/Port_of_Spain", + "America/Porto_Velho", + "America/Puerto_Rico", + "America/Punta_Arenas", + "America/Rainy_River", + "America/Rankin_Inlet", + "America/Recife", + "America/Regina", + "America/Resolute", + "America/Rio_Branco", + "America/Rosario", + "America/Santa_Isabel", + "America/Santarem", + "America/Santiago", + "America/Santo_Domingo", + "America/Sao_Paulo", + "America/Scoresbysund", + "America/Shiprock", + "America/Sitka", + "America/St_Barthelemy", + "America/St_Johns", + "America/St_Kitts", + "America/St_Lucia", + "America/St_Thomas", + "America/St_Vincent", + "America/Swift_Current", + "America/Tegucigalpa", + "America/Thule", + "America/Thunder_Bay", + "America/Tijuana", + "America/Toronto", + "America/Tortola", + "America/Vancouver", + "America/Virgin", + "America/Whitehorse", + "America/Winnipeg", + "America/Yakutat", + "America/Yellowknife", + "Antarctica/Casey", + "Antarctica/Davis", + "Antarctica/DumontDUrville", + "Antarctica/Macquarie", + "Antarctica/Mawson", + "Antarctica/McMurdo", + "Antarctica/Palmer", + "Antarctica/Rothera", + "Antarctica/South_Pole", + "Antarctica/Syowa", + "Antarctica/Troll", + "Antarctica/Vostok", + "Arctic/Longyearbyen", + "Asia/Aden", + "Asia/Almaty", + "Asia/Amman", + "Asia/Anadyr", + "Asia/Aqtau", + "Asia/Aqtobe", + "Asia/Ashgabat", + "Asia/Ashkhabad", + "Asia/Atyrau", + "Asia/Baghdad", + "Asia/Bahrain", + "Asia/Baku", + "Asia/Bangkok", + "Asia/Barnaul", + "Asia/Beirut", + "Asia/Bishkek", + "Asia/Brunei", + "Asia/Calcutta", + "Asia/Chita", + "Asia/Choibalsan", + "Asia/Chongqing", + "Asia/Chungking", + "Asia/Colombo", + "Asia/Dacca", + "Asia/Damascus", + "Asia/Dhaka", + "Asia/Dili", + "Asia/Dubai", + "Asia/Dushanbe", + "Asia/Famagusta", + "Asia/Gaza", + "Asia/Harbin", + "Asia/Hebron", + "Asia/Ho_Chi_Minh", + "Asia/Hong_Kong", + "Asia/Hovd", + "Asia/Irkutsk", + "Asia/Istanbul", + "Asia/Jakarta", + "Asia/Jayapura", + "Asia/Jerusalem", + "Asia/Kabul", + "Asia/Kamchatka", + "Asia/Karachi", + "Asia/Kashgar", + "Asia/Kathmandu", + "Asia/Katmandu", + "Asia/Khandyga", + "Asia/Kolkata", + "Asia/Krasnoyarsk", + "Asia/Kuala_Lumpur", + "Asia/Kuching", + "Asia/Kuwait", + "Asia/Macao", + "Asia/Macau", + "Asia/Magadan", + "Asia/Makassar", + "Asia/Manila", + "Asia/Muscat", + "Asia/Nicosia", + "Asia/Novokuznetsk", + "Asia/Novosibirsk", + "Asia/Omsk", + "Asia/Oral", + "Asia/Phnom_Penh", + "Asia/Pontianak", + "Asia/Pyongyang", + "Asia/Qatar", + "Asia/Qostanay", + "Asia/Qyzylorda", + "Asia/Rangoon", + "Asia/Riyadh", + "Asia/Saigon", + "Asia/Sakhalin", + "Asia/Samarkand", + "Asia/Seoul", + "Asia/Shanghai", + "Asia/Singapore", + "Asia/Srednekolymsk", + "Asia/Taipei", + "Asia/Tashkent", + "Asia/Tbilisi", + "Asia/Tehran", + "Asia/Tel_Aviv", + "Asia/Thimbu", + "Asia/Thimphu", + "Asia/Tokyo", + "Asia/Tomsk", + "Asia/Ujung_Pandang", + "Asia/Ulaanbaatar", + "Asia/Ulan_Bator", + "Asia/Urumqi", + "Asia/Ust-Nera", + "Asia/Vientiane", + "Asia/Vladivostok", + "Asia/Yakutsk", + "Asia/Yangon", + "Asia/Yekaterinburg", + "Asia/Yerevan", + "Atlantic/Azores", + "Atlantic/Bermuda", + "Atlantic/Canary", + "Atlantic/Cape_Verde", + "Atlantic/Faeroe", + "Atlantic/Faroe", + "Atlantic/Jan_Mayen", + "Atlantic/Madeira", + "Atlantic/Reykjavik", + "Atlantic/South_Georgia", + "Atlantic/Stanley", + "Atlantic/St_Helena", + "Australia/ACT", + "Australia/Adelaide", + "Australia/Brisbane", + "Australia/Broken_Hill", + "Australia/Canberra", + "Australia/Currie", + "Australia/Darwin", + "Australia/Eucla", + "Australia/Hobart", + "Australia/LHI", + "Australia/Lindeman", + "Australia/Lord_Howe", + "Australia/Melbourne", + "Australia/North", + "Australia/NSW", + "Australia/Perth", + "Australia/Queensland", + "Australia/South", + "Australia/Sydney", + "Australia/Tasmania", + "Australia/Victoria", + "Australia/West", + "Australia/Yancowinna", + "Brazil/Acre", + "Brazil/DeNoronha", + "Brazil/East", + "Brazil/West", + "Canada/Atlantic", + "Canada/Central", + "Canada/Eastern", + "Canada/Mountain", + "Canada/Newfoundland", + "Canada/Pacific", + "Canada/Saskatchewan", + "Canada/Yukon", + "CET", + "Chile/Continental", + "Chile/EasterIsland", + "CST6CDT", + "Cuba", + "EET", + "Egypt", + "Eire", + "EST", + "EST5EDT", + "Etc/GMT", + "Etc/GMT+0", + "Etc/GMT-0", + "Etc/GMT0", + "Etc/GMT+1", + "Etc/GMT-1", + "Etc/GMT+10", + "Etc/GMT-10", + "Etc/GMT+11", + "Etc/GMT-11", + "Etc/GMT+12", + "Etc/GMT-12", + "Etc/GMT-13", + "Etc/GMT-14", + "Etc/GMT+2", + "Etc/GMT-2", + "Etc/GMT+3", + "Etc/GMT-3", + "Etc/GMT+4", + "Etc/GMT-4", + "Etc/GMT+5", + "Etc/GMT-5", + "Etc/GMT+6", + "Etc/GMT-6", + "Etc/GMT+7", + "Etc/GMT-7", + "Etc/GMT+8", + "Etc/GMT-8", + "Etc/GMT+9", + "Etc/GMT-9", + "Etc/Greenwich", + "Etc/UCT", + "Etc/Universal", + "Etc/UTC", + "Etc/Zulu", + "Europe/Amsterdam", + "Europe/Andorra", + "Europe/Astrakhan", + "Europe/Athens", + "Europe/Belfast", + "Europe/Belgrade", + "Europe/Berlin", + "Europe/Bratislava", + "Europe/Brussels", + "Europe/Bucharest", + "Europe/Budapest", + "Europe/Busingen", + "Europe/Chisinau", + "Europe/Copenhagen", + "Europe/Dublin", + "Europe/Gibraltar", + "Europe/Guernsey", + "Europe/Helsinki", + "Europe/Isle_of_Man", + "Europe/Istanbul", + "Europe/Jersey", + "Europe/Kaliningrad", + "Europe/Kiev", + "Europe/Kirov", + "Europe/Lisbon", + "Europe/Ljubljana", + "Europe/London", + "Europe/Luxembourg", + "Europe/Madrid", + "Europe/Malta", + "Europe/Mariehamn", + "Europe/Minsk", + "Europe/Monaco", + "Europe/Moscow", + "Europe/Nicosia", + "Europe/Oslo", + "Europe/Paris", + "Europe/Podgorica", + "Europe/Prague", + "Europe/Riga", + "Europe/Rome", + "Europe/Samara", + "Europe/San_Marino", + "Europe/Sarajevo", + "Europe/Saratov", + "Europe/Simferopol", + "Europe/Skopje", + "Europe/Sofia", + "Europe/Stockholm", + "Europe/Tallinn", + "Europe/Tirane", + "Europe/Tiraspol", + "Europe/Ulyanovsk", + "Europe/Uzhgorod", + "Europe/Vaduz", + "Europe/Vatican", + "Europe/Vienna", + "Europe/Vilnius", + "Europe/Volgograd", + "Europe/Warsaw", + "Europe/Zagreb", + "Europe/Zaporozhye", + "Europe/Zurich", + "Factory", + "GB", + "GB-Eire", + "GMT", + "GMT+0", + "GMT-0", + "GMT0", + "Greenwich", + "Hongkong", + "HST", + "Iceland", + "Indian/Antananarivo", + "Indian/Chagos", + "Indian/Christmas", + "Indian/Cocos", + "Indian/Comoro", + "Indian/Kerguelen", + "Indian/Mahe", + "Indian/Maldives", + "Indian/Mauritius", + "Indian/Mayotte", + "Indian/Reunion", + "Iran", + "Israel", + "Jamaica", + "Japan", + "Kwajalein", + "Libya", + "localtime", + "MET", + "Mexico/BajaNorte", + "Mexico/BajaSur", + "Mexico/General", + "MST", + "MST7MDT", + "Navajo", + "NZ", + "NZ-CHAT", + "Pacific/Apia", + "Pacific/Auckland", + "Pacific/Bougainville", + "Pacific/Chatham", + "Pacific/Chuuk", + "Pacific/Easter", + "Pacific/Efate", + "Pacific/Enderbury", + "Pacific/Fakaofo", + "Pacific/Fiji", + "Pacific/Funafuti", + "Pacific/Galapagos", + "Pacific/Gambier", + "Pacific/Guadalcanal", + "Pacific/Guam", + "Pacific/Honolulu", + "Pacific/Johnston", + "Pacific/Kiritimati", + "Pacific/Kosrae", + "Pacific/Kwajalein", + "Pacific/Majuro", + "Pacific/Marquesas", + "Pacific/Midway", + "Pacific/Nauru", + "Pacific/Niue", + "Pacific/Norfolk", + "Pacific/Noumea", + "Pacific/Pago_Pago", + "Pacific/Palau", + "Pacific/Pitcairn", + "Pacific/Pohnpei", + "Pacific/Ponape", + "Pacific/Port_Moresby", + "Pacific/Rarotonga", + "Pacific/Saipan", + "Pacific/Samoa", + "Pacific/Tahiti", + "Pacific/Tarawa", + "Pacific/Tongatapu", + "Pacific/Truk", + "Pacific/Wake", + "Pacific/Wallis", + "Pacific/Yap", + "Poland", + "Portugal", + "posix/Africa/Abidjan", + "posix/Africa/Accra", + "posix/Africa/Addis_Ababa", + "posix/Africa/Algiers", + "posix/Africa/Asmara", + "posix/Africa/Asmera", + "posix/Africa/Bamako", + "posix/Africa/Bangui", + "posix/Africa/Banjul", + "posix/Africa/Bissau", + "posix/Africa/Blantyre", + "posix/Africa/Brazzaville", + "posix/Africa/Bujumbura", + "posix/Africa/Cairo", + "posix/Africa/Casablanca", + "posix/Africa/Ceuta", + "posix/Africa/Conakry", + "posix/Africa/Dakar", + "posix/Africa/Dar_es_Salaam", + "posix/Africa/Djibouti", + "posix/Africa/Douala", + "posix/Africa/El_Aaiun", + "posix/Africa/Freetown", + "posix/Africa/Gaborone", + "posix/Africa/Harare", + "posix/Africa/Johannesburg", + "posix/Africa/Juba", + "posix/Africa/Kampala", + "posix/Africa/Khartoum", + "posix/Africa/Kigali", + "posix/Africa/Kinshasa", + "posix/Africa/Lagos", + "posix/Africa/Libreville", + "posix/Africa/Lome", + "posix/Africa/Luanda", + "posix/Africa/Lubumbashi", + "posix/Africa/Lusaka", + "posix/Africa/Malabo", + "posix/Africa/Maputo", + "posix/Africa/Maseru", + "posix/Africa/Mbabane", + "posix/Africa/Mogadishu", + "posix/Africa/Monrovia", + "posix/Africa/Nairobi", + "posix/Africa/Ndjamena", + "posix/Africa/Niamey", + "posix/Africa/Nouakchott", + "posix/Africa/Ouagadougou", + "posix/Africa/Porto-Novo", + "posix/Africa/Sao_Tome", + "posix/Africa/Timbuktu", + "posix/Africa/Tripoli", + "posix/Africa/Tunis", + "posix/Africa/Windhoek", + "posix/America/Adak", + "posix/America/Anchorage", + "posix/America/Anguilla", + "posix/America/Antigua", + "posix/America/Araguaina", + "posix/America/Argentina/Buenos_Aires", + "posix/America/Argentina/Catamarca", + "posix/America/Argentina/ComodRivadavia", + "posix/America/Argentina/Cordoba", + "posix/America/Argentina/Jujuy", + "posix/America/Argentina/La_Rioja", + "posix/America/Argentina/Mendoza", + "posix/America/Argentina/Rio_Gallegos", + "posix/America/Argentina/Salta", + "posix/America/Argentina/San_Juan", + "posix/America/Argentina/San_Luis", + "posix/America/Argentina/Tucuman", + "posix/America/Argentina/Ushuaia", + "posix/America/Aruba", + "posix/America/Asuncion", + "posix/America/Atikokan", + "posix/America/Atka", + "posix/America/Bahia", + "posix/America/Bahia_Banderas", + "posix/America/Barbados", + "posix/America/Belem", + "posix/America/Belize", + "posix/America/Blanc-Sablon", + "posix/America/Boa_Vista", + "posix/America/Bogota", + "posix/America/Boise", + "posix/America/Buenos_Aires", + "posix/America/Cambridge_Bay", + "posix/America/Campo_Grande", + "posix/America/Cancun", + "posix/America/Caracas", + "posix/America/Catamarca", + "posix/America/Cayenne", + "posix/America/Cayman", + "posix/America/Chicago", + "posix/America/Chihuahua", + "posix/America/Ciudad_Juarez", + "posix/America/Coral_Harbor", + "posix/America/Cordoba", + "posix/America/Costa_Rica", + "posix/America/Creston", + "posix/America/Cuiaba", + "posix/America/Curacao", + "posix/America/Danmarkshavn", + "posix/America/Dawson", + "posix/America/Dawson_Creek", + "posix/America/Denver", + "posix/America/Detroit", + "posix/America/Dominica", + "posix/America/Edmonton", + "posix/America/Eirunepe", + "posix/America/El_Salvador", + "posix/America/Ensenada", + "posix/America/Fortaleza", + "posix/America/Fort_Nelson", + "posix/America/Fort_Wayne", + "posix/America/Glace_Bay", + "posix/America/Godthab", + "posix/America/Goose_Bay", + "posix/America/Grand_Turk", + "posix/America/Grenada", + "posix/America/Guadeloupe", + "posix/America/Guatemala", + "posix/America/Guayaquil", + "posix/America/Guyana", + "posix/America/Halifax", + "posix/America/Havana", + "posix/America/Hermosillo", + "posix/America/Indiana/Indianapolis", + "posix/America/Indiana/Knox", + "posix/America/Indiana/Marengo", + "posix/America/Indiana/Petersburg", + "posix/America/Indianapolis", + "posix/America/Indiana/Tell_City", + "posix/America/Indiana/Vevay", + "posix/America/Indiana/Vincennes", + "posix/America/Indiana/Winamac", + "posix/America/Inuvik", + "posix/America/Iqaluit", + "posix/America/Jamaica", + "posix/America/Jujuy", + "posix/America/Juneau", + "posix/America/Kentucky/Louisville", + "posix/America/Kentucky/Monticello", + "posix/America/Knox_IN", + "posix/America/Kralendijk", + "posix/America/La_Paz", + "posix/America/Lima", + "posix/America/Los_Angeles", + "posix/America/Louisville", + "posix/America/Lower_Princes", + "posix/America/Maceio", + "posix/America/Managua", + "posix/America/Manaus", + "posix/America/Marigot", + "posix/America/Martinique", + "posix/America/Matamoros", + "posix/America/Mazatlan", + "posix/America/Mendoza", + "posix/America/Menominee", + "posix/America/Merida", + "posix/America/Metlakatla", + "posix/America/Mexico_City", + "posix/America/Miquelon", + "posix/America/Moncton", + "posix/America/Monterrey", + "posix/America/Montevideo", + "posix/America/Montreal", + "posix/America/Montserrat", + "posix/America/Nassau", + "posix/America/New_York", + "posix/America/Nipigon", + "posix/America/Nome", + "posix/America/Noronha", + "posix/America/North_Dakota/Beulah", + "posix/America/North_Dakota/Center", + "posix/America/North_Dakota/New_Salem", + "posix/America/Nuuk", + "posix/America/Ojinaga", + "posix/America/Panama", + "posix/America/Pangnirtung", + "posix/America/Paramaribo", + "posix/America/Phoenix", + "posix/America/Port-au-Prince", + "posix/America/Porto_Acre", + "posix/America/Port_of_Spain", + "posix/America/Porto_Velho", + "posix/America/Puerto_Rico", + "posix/America/Punta_Arenas", + "posix/America/Rainy_River", + "posix/America/Rankin_Inlet", + "posix/America/Recife", + "posix/America/Regina", + "posix/America/Resolute", + "posix/America/Rio_Branco", + "posix/America/Rosario", + "posix/America/Santa_Isabel", + "posix/America/Santarem", + "posix/America/Santiago", + "posix/America/Santo_Domingo", + "posix/America/Sao_Paulo", + "posix/America/Scoresbysund", + "posix/America/Shiprock", + "posix/America/Sitka", + "posix/America/St_Barthelemy", + "posix/America/St_Johns", + "posix/America/St_Kitts", + "posix/America/St_Lucia", + "posix/America/St_Thomas", + "posix/America/St_Vincent", + "posix/America/Swift_Current", + "posix/America/Tegucigalpa", + "posix/America/Thule", + "posix/America/Thunder_Bay", + "posix/America/Tijuana", + "posix/America/Toronto", + "posix/America/Tortola", + "posix/America/Vancouver", + "posix/America/Virgin", + "posix/America/Whitehorse", + "posix/America/Winnipeg", + "posix/America/Yakutat", + "posix/America/Yellowknife", + "posix/Antarctica/Casey", + "posix/Antarctica/Davis", + "posix/Antarctica/DumontDUrville", + "posix/Antarctica/Macquarie", + "posix/Antarctica/Mawson", + "posix/Antarctica/McMurdo", + "posix/Antarctica/Palmer", + "posix/Antarctica/Rothera", + "posix/Antarctica/South_Pole", + "posix/Antarctica/Syowa", + "posix/Antarctica/Troll", + "posix/Antarctica/Vostok", + "posix/Arctic/Longyearbyen", + "posix/Asia/Aden", + "posix/Asia/Almaty", + "posix/Asia/Amman", + "posix/Asia/Anadyr", + "posix/Asia/Aqtau", + "posix/Asia/Aqtobe", + "posix/Asia/Ashgabat", + "posix/Asia/Ashkhabad", + "posix/Asia/Atyrau", + "posix/Asia/Baghdad", + "posix/Asia/Bahrain", + "posix/Asia/Baku", + "posix/Asia/Bangkok", + "posix/Asia/Barnaul", + "posix/Asia/Beirut", + "posix/Asia/Bishkek", + "posix/Asia/Brunei", + "posix/Asia/Calcutta", + "posix/Asia/Chita", + "posix/Asia/Choibalsan", + "posix/Asia/Chongqing", + "posix/Asia/Chungking", + "posix/Asia/Colombo", + "posix/Asia/Dacca", + "posix/Asia/Damascus", + "posix/Asia/Dhaka", + "posix/Asia/Dili", + "posix/Asia/Dubai", + "posix/Asia/Dushanbe", + "posix/Asia/Famagusta", + "posix/Asia/Gaza", + "posix/Asia/Harbin", + "posix/Asia/Hebron", + "posix/Asia/Ho_Chi_Minh", + "posix/Asia/Hong_Kong", + "posix/Asia/Hovd", + "posix/Asia/Irkutsk", + "posix/Asia/Istanbul", + "posix/Asia/Jakarta", + "posix/Asia/Jayapura", + "posix/Asia/Jerusalem", + "posix/Asia/Kabul", + "posix/Asia/Kamchatka", + "posix/Asia/Karachi", + "posix/Asia/Kashgar", + "posix/Asia/Kathmandu", + "posix/Asia/Katmandu", + "posix/Asia/Khandyga", + "posix/Asia/Kolkata", + "posix/Asia/Krasnoyarsk", + "posix/Asia/Kuala_Lumpur", + "posix/Asia/Kuching", + "posix/Asia/Kuwait", + "posix/Asia/Macao", + "posix/Asia/Macau", + "posix/Asia/Magadan", + "posix/Asia/Makassar", + "posix/Asia/Manila", + "posix/Asia/Muscat", + "posix/Asia/Nicosia", + "posix/Asia/Novokuznetsk", + "posix/Asia/Novosibirsk", + "posix/Asia/Omsk", + "posix/Asia/Oral", + "posix/Asia/Phnom_Penh", + "posix/Asia/Pontianak", + "posix/Asia/Pyongyang", + "posix/Asia/Qatar", + "posix/Asia/Qostanay", + "posix/Asia/Qyzylorda", + "posix/Asia/Rangoon", + "posix/Asia/Riyadh", + "posix/Asia/Saigon", + "posix/Asia/Sakhalin", + "posix/Asia/Samarkand", + "posix/Asia/Seoul", + "posix/Asia/Shanghai", + "posix/Asia/Singapore", + "posix/Asia/Srednekolymsk", + "posix/Asia/Taipei", + "posix/Asia/Tashkent", + "posix/Asia/Tbilisi", + "posix/Asia/Tehran", + "posix/Asia/Tel_Aviv", + "posix/Asia/Thimbu", + "posix/Asia/Thimphu", + "posix/Asia/Tokyo", + "posix/Asia/Tomsk", + "posix/Asia/Ujung_Pandang", + "posix/Asia/Ulaanbaatar", + "posix/Asia/Ulan_Bator", + "posix/Asia/Urumqi", + "posix/Asia/Ust-Nera", + "posix/Asia/Vientiane", + "posix/Asia/Vladivostok", + "posix/Asia/Yakutsk", + "posix/Asia/Yangon", + "posix/Asia/Yekaterinburg", + "posix/Asia/Yerevan", + "posix/Atlantic/Azores", + "posix/Atlantic/Bermuda", + "posix/Atlantic/Canary", + "posix/Atlantic/Cape_Verde", + "posix/Atlantic/Faeroe", + "posix/Atlantic/Faroe", + "posix/Atlantic/Jan_Mayen", + "posix/Atlantic/Madeira", + "posix/Atlantic/Reykjavik", + "posix/Atlantic/South_Georgia", + "posix/Atlantic/Stanley", + "posix/Atlantic/St_Helena", + "posix/Australia/ACT", + "posix/Australia/Adelaide", + "posix/Australia/Brisbane", + "posix/Australia/Broken_Hill", + "posix/Australia/Canberra", + "posix/Australia/Currie", + "posix/Australia/Darwin", + "posix/Australia/Eucla", + "posix/Australia/Hobart", + "posix/Australia/LHI", + "posix/Australia/Lindeman", + "posix/Australia/Lord_Howe", + "posix/Australia/Melbourne", + "posix/Australia/North", + "posix/Australia/NSW", + "posix/Australia/Perth", + "posix/Australia/Queensland", + "posix/Australia/South", + "posix/Australia/Sydney", + "posix/Australia/Tasmania", + "posix/Australia/Victoria", + "posix/Australia/West", + "posix/Australia/Yancowinna", + "posix/Brazil/Acre", + "posix/Brazil/DeNoronha", + "posix/Brazil/East", + "posix/Brazil/West", + "posix/Canada/Atlantic", + "posix/Canada/Central", + "posix/Canada/Eastern", + "posix/Canada/Mountain", + "posix/Canada/Newfoundland", + "posix/Canada/Pacific", + "posix/Canada/Saskatchewan", + "posix/Canada/Yukon", + "posix/CET", + "posix/Chile/Continental", + "posix/Chile/EasterIsland", + "posix/CST6CDT", + "posix/Cuba", + "posix/EET", + "posix/Egypt", + "posix/Eire", + "posix/EST", + "posix/EST5EDT", + "posix/Etc/GMT", + "posix/Etc/GMT+0", + "posix/Etc/GMT-0", + "posix/Etc/GMT0", + "posix/Etc/GMT+1", + "posix/Etc/GMT-1", + "posix/Etc/GMT+10", + "posix/Etc/GMT-10", + "posix/Etc/GMT+11", + "posix/Etc/GMT-11", + "posix/Etc/GMT+12", + "posix/Etc/GMT-12", + "posix/Etc/GMT-13", + "posix/Etc/GMT-14", + "posix/Etc/GMT+2", + "posix/Etc/GMT-2", + "posix/Etc/GMT+3", + "posix/Etc/GMT-3", + "posix/Etc/GMT+4", + "posix/Etc/GMT-4", + "posix/Etc/GMT+5", + "posix/Etc/GMT-5", + "posix/Etc/GMT+6", + "posix/Etc/GMT-6", + "posix/Etc/GMT+7", + "posix/Etc/GMT-7", + "posix/Etc/GMT+8", + "posix/Etc/GMT-8", + "posix/Etc/GMT+9", + "posix/Etc/GMT-9", + "posix/Etc/Greenwich", + "posix/Etc/UCT", + "posix/Etc/Universal", + "posix/Etc/UTC", + "posix/Etc/Zulu", + "posix/Europe/Amsterdam", +} diff --git a/coderd/workspaceagents_test.go b/coderd/workspaceagents_test.go index 603ed62841f1b..a60ef39e53f9e 100644 --- a/coderd/workspaceagents_test.go +++ b/coderd/workspaceagents_test.go @@ -1185,7 +1185,7 @@ func TestWorkspaceAgent_LifecycleState(t *testing.T) { func TestWorkspaceAgent_Metadata(t *testing.T) { t.Parallel() - db, ps := dbtestutil.NewDB(t, dbtestutil.WithFixedTimezone()) + db, ps := dbtestutil.NewDB(t, dbtestutil.WithTimezone("UTC")) client := coderdtest.New(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, Database: db, diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index dfbafcc15b8ce..5290940b86a55 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -1510,7 +1510,7 @@ func TestWorkspaceFilterManual(t *testing.T) { t.Run("LastUsed", func(t *testing.T) { t.Parallel() - db, ps := dbtestutil.NewDB(t, dbtestutil.WithFixedTimezone()) + db, ps := dbtestutil.NewDB(t, dbtestutil.WithTimezone("UTC")) client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, Database: db, diff --git a/enterprise/coderd/templates_test.go b/enterprise/coderd/templates_test.go index 131534b206541..cc2e8dc1a4e0e 100644 --- a/enterprise/coderd/templates_test.go +++ b/enterprise/coderd/templates_test.go @@ -369,7 +369,7 @@ func TestTemplates(t *testing.T) { t.Run("UpdateLastUsedAt", func(t *testing.T) { t.Parallel() - db, ps := dbtestutil.NewDB(t, dbtestutil.WithFixedTimezone()) + db, ps := dbtestutil.NewDB(t, dbtestutil.WithTimezone("UTC")) ctx := testutil.Context(t, testutil.WaitMedium) client, user := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index df429714cc1d8..ae628a1422b8f 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -642,7 +642,7 @@ func TestWorkspacesFiltering(t *testing.T) { dormantTTL := 24 * time.Hour - db, ps := dbtestutil.NewDB(t, dbtestutil.WithFixedTimezone()) + db, ps := dbtestutil.NewDB(t, dbtestutil.WithTimezone("UTC")) client, user := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ From 7c2ff2f83268ab6fdf80067843de0fc829380753 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 14 Sep 2023 12:46:01 +0100 Subject: [PATCH 08/17] set db timezone to canada/newfoundland by default --- coderd/database/dbtestutil/db.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/coderd/database/dbtestutil/db.go b/coderd/database/dbtestutil/db.go index e3a9e546505f4..4ba67714f6aa5 100644 --- a/coderd/database/dbtestutil/db.go +++ b/coderd/database/dbtestutil/db.go @@ -13,7 +13,6 @@ import ( "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbfake" - "github.com/coder/coder/v2/coderd/database/dbtestutil/randtz" "github.com/coder/coder/v2/coderd/database/postgres" "github.com/coder/coder/v2/coderd/database/pubsub" ) @@ -63,8 +62,12 @@ func NewDB(t testing.TB, opts ...Option) (database.Store, pubsub.Pubsub) { if o.fixedTimezone == "" { // To make sure we find timezone-related issues, we set the timezone - // of the database to a random one. - o.fixedTimezone = randtz.Name(t) + // of the database to a non-UTC one. + // The below was picked due to the following properties: + // - It has a non-UTC offset + // - It has a fractional hour UTC offset + // - It includes a daylight savings time component + o.fixedTimezone = "Canada/Newfoundland" } dbName := dbNameFromConnectionURL(t, connectionURL) setDBTimezone(t, connectionURL, dbName, o.fixedTimezone) From eecfcdb2701e44a98954fdb221719aa199ae6edb Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 14 Sep 2023 12:46:15 +0100 Subject: [PATCH 09/17] test activity bump in multiple timezones --- coderd/activitybump_internal_test.go | 230 ++++++++++++++------------- 1 file changed, 121 insertions(+), 109 deletions(-) diff --git a/coderd/activitybump_internal_test.go b/coderd/activitybump_internal_test.go index 81fb8aaf36c34..aff7a38143d71 100644 --- a/coderd/activitybump_internal_test.go +++ b/coderd/activitybump_internal_test.go @@ -22,6 +22,15 @@ import ( func Test_ActivityBumpWorkspace(t *testing.T) { t.Parallel() + // We test the below in multiple timezones + timezones := []string{ + "Asia/Kolkata", + "Canada/Newfoundland", + "Europe/Paris", + "US/Central", + "UTC", + } + for _, tt := range []struct { name string transition database.WorkspaceTransition @@ -92,117 +101,120 @@ func Test_ActivityBumpWorkspace(t *testing.T) { }, } { tt := tt - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - var ( - now = dbtime.Now() - ctx = testutil.Context(t, testutil.WaitShort) - log = slogtest.Make(t, nil) - db, _ = dbtestutil.NewDB(t) - org = dbgen.Organization(t, db, database.Organization{}) - user = dbgen.User(t, db, database.User{ - Status: database.UserStatusActive, - }) - _ = dbgen.OrganizationMember(t, db, database.OrganizationMember{ - UserID: user.ID, - OrganizationID: org.ID, - }) - templateVersion = dbgen.TemplateVersion(t, db, database.TemplateVersion{ - OrganizationID: org.ID, - CreatedBy: user.ID, - }) - template = dbgen.Template(t, db, database.Template{ - OrganizationID: org.ID, - ActiveVersionID: templateVersion.ID, - CreatedBy: user.ID, - }) - ws = dbgen.Workspace(t, db, database.Workspace{ - OwnerID: user.ID, - OrganizationID: org.ID, - TemplateID: template.ID, - Ttl: sql.NullInt64{Valid: true, Int64: int64(tt.workspaceTTL)}, - }) - job = dbgen.ProvisionerJob(t, db, database.ProvisionerJob{ - OrganizationID: org.ID, - CompletedAt: tt.jobCompletedAt, - }) - _ = dbgen.WorkspaceResource(t, db, database.WorkspaceResource{ - JobID: job.ID, + for _, tz := range timezones { + tz := tz + t.Run(tt.name+"/"+tz, func(t *testing.T) { + t.Parallel() + + var ( + now = dbtime.Now() + ctx = testutil.Context(t, testutil.WaitShort) + log = slogtest.Make(t, nil) + db, _ = dbtestutil.NewDB(t, dbtestutil.WithTimezone(tz)) + org = dbgen.Organization(t, db, database.Organization{}) + user = dbgen.User(t, db, database.User{ + Status: database.UserStatusActive, + }) + _ = dbgen.OrganizationMember(t, db, database.OrganizationMember{ + UserID: user.ID, + OrganizationID: org.ID, + }) + templateVersion = dbgen.TemplateVersion(t, db, database.TemplateVersion{ + OrganizationID: org.ID, + CreatedBy: user.ID, + }) + template = dbgen.Template(t, db, database.Template{ + OrganizationID: org.ID, + ActiveVersionID: templateVersion.ID, + CreatedBy: user.ID, + }) + ws = dbgen.Workspace(t, db, database.Workspace{ + OwnerID: user.ID, + OrganizationID: org.ID, + TemplateID: template.ID, + Ttl: sql.NullInt64{Valid: true, Int64: int64(tt.workspaceTTL)}, + }) + job = dbgen.ProvisionerJob(t, db, database.ProvisionerJob{ + OrganizationID: org.ID, + CompletedAt: tt.jobCompletedAt, + }) + _ = dbgen.WorkspaceResource(t, db, database.WorkspaceResource{ + JobID: job.ID, + }) + buildID = uuid.New() + ) + + var buildNumber int32 = 1 + // Insert a number of previous workspace builds. + for i := 0; i < 5; i++ { + insertPrevWorkspaceBuild(t, db, org.ID, templateVersion.ID, ws.ID, database.WorkspaceTransitionStart, buildNumber) + buildNumber++ + insertPrevWorkspaceBuild(t, db, org.ID, templateVersion.ID, ws.ID, database.WorkspaceTransitionStop, buildNumber) + buildNumber++ + } + + // dbgen.WorkspaceBuild automatically sets deadline to now+1 hour if not set + var buildDeadline time.Time + if tt.buildDeadlineOffset != nil { + buildDeadline = now.Add(*tt.buildDeadlineOffset) + } + var maxDeadline time.Time + if tt.maxDeadlineOffset != nil { + maxDeadline = now.Add(*tt.maxDeadlineOffset) + } + err := db.InsertWorkspaceBuild(ctx, database.InsertWorkspaceBuildParams{ + ID: buildID, + CreatedAt: dbtime.Now(), + UpdatedAt: dbtime.Now(), + BuildNumber: buildNumber, + InitiatorID: user.ID, + Reason: database.BuildReasonInitiator, + WorkspaceID: ws.ID, + JobID: job.ID, + TemplateVersionID: templateVersion.ID, + Transition: tt.transition, + Deadline: buildDeadline, + MaxDeadline: maxDeadline, }) - buildID = uuid.New() - ) - - var buildNumber int32 = 1 - // Insert a number of previous workspace builds. - for i := 0; i < 5; i++ { - insertPrevWorkspaceBuild(t, db, org.ID, templateVersion.ID, ws.ID, database.WorkspaceTransitionStart, buildNumber) - buildNumber++ - insertPrevWorkspaceBuild(t, db, org.ID, templateVersion.ID, ws.ID, database.WorkspaceTransitionStop, buildNumber) - buildNumber++ - } - - // dbgen.WorkspaceBuild automatically sets deadline to now+1 hour if not set - var buildDeadline time.Time - if tt.buildDeadlineOffset != nil { - buildDeadline = now.Add(*tt.buildDeadlineOffset) - } - var maxDeadline time.Time - if tt.maxDeadlineOffset != nil { - maxDeadline = now.Add(*tt.maxDeadlineOffset) - } - err := db.InsertWorkspaceBuild(ctx, database.InsertWorkspaceBuildParams{ - ID: buildID, - CreatedAt: dbtime.Now(), - UpdatedAt: dbtime.Now(), - BuildNumber: buildNumber, - InitiatorID: user.ID, - Reason: database.BuildReasonInitiator, - WorkspaceID: ws.ID, - JobID: job.ID, - TemplateVersionID: templateVersion.ID, - Transition: tt.transition, - Deadline: buildDeadline, - MaxDeadline: maxDeadline, + require.NoError(t, err, "unexpected error inserting workspace build") + bld, err := db.GetWorkspaceBuildByID(ctx, buildID) + require.NoError(t, err, "unexpected error fetching inserted workspace build") + + // Validate our initial state before bump + require.Equal(t, tt.transition, bld.Transition, "unexpected transition before bump") + require.Equal(t, tt.jobCompletedAt.Time.UTC(), job.CompletedAt.Time.UTC(), "unexpected job completed at before bump") + require.Equal(t, buildDeadline.UTC(), bld.Deadline.UTC(), "unexpected build deadline before bump") + require.Equal(t, maxDeadline.UTC(), bld.MaxDeadline.UTC(), "unexpected max deadline before bump") + require.Equal(t, tt.workspaceTTL, time.Duration(ws.Ttl.Int64), "unexpected workspace TTL before bump") + + workaroundWindowsTimeResolution(t) + + // Bump duration is measured from the time of the bump, so we measure from here. + start := dbtime.Now() + activityBumpWorkspace(ctx, log, db, bld.WorkspaceID) + elapsed := time.Since(start) + if elapsed > 15*time.Second { + t.Logf("warning: activityBumpWorkspace took longer than 15 seconds: %s", elapsed) + } + // The actual bump could have happened anywhere in the elapsed time, so we + // guess at the approximate time of the bump. + approxBumpTime := start.Add(elapsed / 2) + + // Validate our state after bump + updatedBuild, err := db.GetLatestWorkspaceBuildByWorkspaceID(ctx, bld.WorkspaceID) + require.NoError(t, err, "unexpected error getting latest workspace build") + if tt.expectedBump == 0 { + require.Equal(t, bld.UpdatedAt.UTC(), updatedBuild.UpdatedAt.UTC(), "should not have bumped updated_at") + require.Equal(t, bld.Deadline.UTC(), updatedBuild.Deadline.UTC(), "should not have bumped deadline") + } else { + require.NotEqual(t, bld.UpdatedAt.UTC(), updatedBuild.UpdatedAt.UTC(), "should have bumped updated_at") + expectedDeadline := approxBumpTime.Add(tt.expectedBump).UTC() + // Note: if CI is especially slow, this test may fail. There is an internal 15-second + // deadline in activityBumpWorkspace, so we allow the same window here. + require.WithinDuration(t, expectedDeadline, updatedBuild.Deadline.UTC(), 15*time.Second, "unexpected deadline after bump") + } }) - require.NoError(t, err, "unexpected error inserting workspace build") - bld, err := db.GetWorkspaceBuildByID(ctx, buildID) - require.NoError(t, err, "unexpected error fetching inserted workspace build") - - // Validate our initial state before bump - require.Equal(t, tt.transition, bld.Transition, "unexpected transition before bump") - require.Equal(t, tt.jobCompletedAt.Time.UTC(), job.CompletedAt.Time.UTC(), "unexpected job completed at before bump") - require.Equal(t, buildDeadline.UTC(), bld.Deadline.UTC(), "unexpected build deadline before bump") - require.Equal(t, maxDeadline.UTC(), bld.MaxDeadline.UTC(), "unexpected max deadline before bump") - require.Equal(t, tt.workspaceTTL, time.Duration(ws.Ttl.Int64), "unexpected workspace TTL before bump") - - workaroundWindowsTimeResolution(t) - - // Bump duration is measured from the time of the bump, so we measure from here. - start := dbtime.Now() - activityBumpWorkspace(ctx, log, db, bld.WorkspaceID) - elapsed := time.Since(start) - if elapsed > 15*time.Second { - t.Logf("warning: activityBumpWorkspace took longer than 15 seconds: %s", elapsed) - } - // The actual bump could have happened anywhere in the elapsed time, so we - // guess at the approximate time of the bump. - approxBumpTime := start.Add(elapsed / 2) - - // Validate our state after bump - updatedBuild, err := db.GetLatestWorkspaceBuildByWorkspaceID(ctx, bld.WorkspaceID) - require.NoError(t, err, "unexpected error getting latest workspace build") - if tt.expectedBump == 0 { - require.Equal(t, bld.UpdatedAt.UTC(), updatedBuild.UpdatedAt.UTC(), "should not have bumped updated_at") - require.Equal(t, bld.Deadline.UTC(), updatedBuild.Deadline.UTC(), "should not have bumped deadline") - } else { - require.NotEqual(t, bld.UpdatedAt.UTC(), updatedBuild.UpdatedAt.UTC(), "should have bumped updated_at") - expectedDeadline := approxBumpTime.Add(tt.expectedBump).UTC() - // Note: if CI is especially slow, this test may fail. There is an internal 15-second - // deadline in activityBumpWorkspace, so we allow the same window here. - require.WithinDuration(t, expectedDeadline, updatedBuild.Deadline.UTC(), 15*time.Second, "unexpected deadline after bump") - } - }) + } } } From d327131c3ac503b7a659a7bf1734befee33076df Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 14 Sep 2023 12:48:51 +0100 Subject: [PATCH 10/17] remove unnecessary IN TIME ZONE UTC in activitybump query --- coderd/database/queries.sql.go | 4 ++-- coderd/database/queries/activitybump.sql | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index fb2f9e3552ca6..67b7b782b9e29 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -38,7 +38,7 @@ UPDATE SET updated_at = NOW(), deadline = CASE - WHEN l.build_max_deadline AT TIME ZONE 'UTC' = '0001-01-01 00:00:00+00' + WHEN l.build_max_deadline = '0001-01-01 00:00:00+00' THEN NOW() + l.ttl_interval ELSE LEAST(NOW() + l.ttl_interval, l.build_max_deadline) END @@ -46,7 +46,7 @@ FROM latest l WHERE wb.id = l.build_id AND l.job_completed_at IS NOT NULL AND l.build_transition = 'start' -AND l.build_deadline AT TIME ZONE 'UTC' != '0001-01-01 00:00:00+00' +AND l.build_deadline != '0001-01-01 00:00:00+00' AND l.build_deadline - (l.ttl_interval * 0.95) < NOW() ` diff --git a/coderd/database/queries/activitybump.sql b/coderd/database/queries/activitybump.sql index de3546e792b79..9b8e358e19000 100644 --- a/coderd/database/queries/activitybump.sql +++ b/coderd/database/queries/activitybump.sql @@ -25,7 +25,7 @@ UPDATE SET updated_at = NOW(), deadline = CASE - WHEN l.build_max_deadline AT TIME ZONE 'UTC' = '0001-01-01 00:00:00+00' + WHEN l.build_max_deadline = '0001-01-01 00:00:00+00' THEN NOW() + l.ttl_interval ELSE LEAST(NOW() + l.ttl_interval, l.build_max_deadline) END @@ -34,7 +34,7 @@ WHERE wb.id = l.build_id AND l.job_completed_at IS NOT NULL AND l.build_transition = 'start' -- We only bump if workspace shutdown is manual. -AND l.build_deadline AT TIME ZONE 'UTC' != '0001-01-01 00:00:00+00' +AND l.build_deadline != '0001-01-01 00:00:00+00' -- We only bump when 5% of the deadline has elapsed. AND l.build_deadline - (l.ttl_interval * 0.95) < NOW() ; From 76809840f98b6242373a9c28ba64aec08689d173 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 14 Sep 2023 15:20:29 +0100 Subject: [PATCH 11/17] clarify use of multiple timezones in test --- coderd/activitybump_internal_test.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/coderd/activitybump_internal_test.go b/coderd/activitybump_internal_test.go index aff7a38143d71..18a02bc2d2a39 100644 --- a/coderd/activitybump_internal_test.go +++ b/coderd/activitybump_internal_test.go @@ -22,13 +22,14 @@ import ( func Test_ActivityBumpWorkspace(t *testing.T) { t.Parallel() - // We test the below in multiple timezones + // We test the below in multiple timezones specifically + // chosen to trigger timezone-related bugs. timezones := []string{ - "Asia/Kolkata", - "Canada/Newfoundland", - "Europe/Paris", - "US/Central", - "UTC", + "Asia/Kolkata", // No DST, positive fractional offset + "Canada/Newfoundland", // DST, negative fractional offset + "Europe/Paris", // DST, positive offset + "US/Arizona", // No DST, negative offset + "UTC", // Baseline } for _, tt := range []struct { From d051bbbbad0d11a7f10e19a15f84c1c4b86e0112 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 14 Sep 2023 15:21:07 +0100 Subject: [PATCH 12/17] remove deprecation and add linter rule --- coderd/database/dbtestutil/db.go | 9 +++------ coderd/workspaceagents_test.go | 1 + coderd/workspaces_test.go | 2 ++ enterprise/coderd/templates_test.go | 1 + enterprise/coderd/workspaces_test.go | 1 + scripts/rules.go | 13 +++++++++++++ 6 files changed, 21 insertions(+), 6 deletions(-) diff --git a/coderd/database/dbtestutil/db.go b/coderd/database/dbtestutil/db.go index 4ba67714f6aa5..49057fc3f6582 100644 --- a/coderd/database/dbtestutil/db.go +++ b/coderd/database/dbtestutil/db.go @@ -28,10 +28,7 @@ type options struct { type Option func(*options) -// WithTimezone sets the database to the defined timezone instead of -// to a random one. -// -// Deprecated: If you need to use this, you may have a timezone-related bug. +// WithTimezone sets the database to the defined timezone. func WithTimezone(tz string) Option { return func(o *options) { o.fixedTimezone = tz @@ -90,14 +87,14 @@ func NewDB(t testing.TB, opts ...Option) (database.Store, pubsub.Pubsub) { } // setRandDBTimezone sets the timezone of the database to the given timezone. +// Note that the updated timezone only comes into effect on reconnect, so we +// create our own connnection for this and close the DB after we're done. func setDBTimezone(t testing.TB, dbURL, dbname, tz string) { t.Helper() sqlDB, err := sql.Open("postgres", dbURL) require.NoError(t, err) defer func() { - // The updated timezone only comes into effect on reconnect, so close - // the DB after we're done. _ = sqlDB.Close() }() diff --git a/coderd/workspaceagents_test.go b/coderd/workspaceagents_test.go index a60ef39e53f9e..6dfddfd1cb952 100644 --- a/coderd/workspaceagents_test.go +++ b/coderd/workspaceagents_test.go @@ -1185,6 +1185,7 @@ func TestWorkspaceAgent_LifecycleState(t *testing.T) { func TestWorkspaceAgent_Metadata(t *testing.T) { t.Parallel() + // nolint:gocritic // https://github.com/coder/coder/issues/9682 db, ps := dbtestutil.NewDB(t, dbtestutil.WithTimezone("UTC")) client := coderdtest.New(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 5290940b86a55..3b4f3d69df5b3 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -1510,6 +1510,8 @@ func TestWorkspaceFilterManual(t *testing.T) { t.Run("LastUsed", func(t *testing.T) { t.Parallel() + + // nolint:gocritic // https://github.com/coder/coder/issues/9682 db, ps := dbtestutil.NewDB(t, dbtestutil.WithTimezone("UTC")) client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, diff --git a/enterprise/coderd/templates_test.go b/enterprise/coderd/templates_test.go index cc2e8dc1a4e0e..49dff3bd58565 100644 --- a/enterprise/coderd/templates_test.go +++ b/enterprise/coderd/templates_test.go @@ -369,6 +369,7 @@ func TestTemplates(t *testing.T) { t.Run("UpdateLastUsedAt", func(t *testing.T) { t.Parallel() + // nolint:gocritic // https://github.com/coder/coder/issues/9682 db, ps := dbtestutil.NewDB(t, dbtestutil.WithTimezone("UTC")) ctx := testutil.Context(t, testutil.WaitMedium) client, user := coderdenttest.New(t, &coderdenttest.Options{ diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index ae628a1422b8f..324e7aa5a0d22 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -642,6 +642,7 @@ func TestWorkspacesFiltering(t *testing.T) { dormantTTL := 24 * time.Hour + // nolint:gocritic // https://github.com/coder/coder/issues/9682 db, ps := dbtestutil.NewDB(t, dbtestutil.WithTimezone("UTC")) client, user := coderdenttest.New(t, &coderdenttest.Options{ diff --git a/scripts/rules.go b/scripts/rules.go index 2154cc828a21a..ab5ef0b43008b 100644 --- a/scripts/rules.go +++ b/scripts/rules.go @@ -395,3 +395,16 @@ func slogError(m dsl.Matcher) { Where(m["name"].Const && m["value"].Type.Is("error") && !m["name"].Text.Matches(`^"internal_error"$`)). Report(`Error should be logged using "slog.Error" instead.`) } + +// withTimezoneUTC ensures that we don't just sprinkle dbtestutil.WithTimezone("UTC") about +// to work around real timezone bugs in our code. +// +//nolint:unused,deadcode,varnamelen +func withTimezoneUTC(m dsl.Matcher) { + m.Match( + `dbtestutil.WithTimezone($tz)`, + ).Where( + m["tz"].Text.Matches(`[uU][tT][cC]"$`), + ).Report(`Setting database timezone to UTC may mask timezone-related bugs.`). + At(m["tz"]) +} From da059dc7c81245a0c3f4add78803f4fee4dc0876 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 14 Sep 2023 15:27:16 +0100 Subject: [PATCH 13/17] fix typo --- coderd/database/dbtestutil/db.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/dbtestutil/db.go b/coderd/database/dbtestutil/db.go index 49057fc3f6582..a65527a05c6d3 100644 --- a/coderd/database/dbtestutil/db.go +++ b/coderd/database/dbtestutil/db.go @@ -88,7 +88,7 @@ func NewDB(t testing.TB, opts ...Option) (database.Store, pubsub.Pubsub) { // setRandDBTimezone sets the timezone of the database to the given timezone. // Note that the updated timezone only comes into effect on reconnect, so we -// create our own connnection for this and close the DB after we're done. +// create our own connection for this and close the DB after we're done. func setDBTimezone(t testing.TB, dbURL, dbname, tz string) { t.Helper() From 0d425ba5f574e4d45421d364287d715ac2599f65 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 14 Sep 2023 15:34:31 +0100 Subject: [PATCH 14/17] bump allowed withinDuration for TestWorkpaceActivityBump --- coderd/activitybump_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/activitybump_test.go b/coderd/activitybump_test.go index 39ac3302ebf19..19ddf928d4e69 100644 --- a/coderd/activitybump_test.go +++ b/coderd/activitybump_test.go @@ -153,7 +153,7 @@ func TestWorkspaceActivityBump(t *testing.T) { require.Equal(t, workspace.LatestBuild.Deadline.Time, workspace.LatestBuild.MaxDeadline.Time) return } - require.WithinDuration(t, dbtime.Now().Add(ttl), workspace.LatestBuild.Deadline.Time, 3*time.Second) + require.WithinDuration(t, dbtime.Now().Add(ttl), workspace.LatestBuild.Deadline.Time, testutil.WaitShort) } } From d97ff2f88d911219d40e9d3f59aeb2e00f016949 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 14 Sep 2023 16:34:03 +0100 Subject: [PATCH 15/17] make Test_ActivityBumpWorkspace more flake-resistant --- coderd/activitybump_internal_test.go | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/coderd/activitybump_internal_test.go b/coderd/activitybump_internal_test.go index 18a02bc2d2a39..a864aae25c8c3 100644 --- a/coderd/activitybump_internal_test.go +++ b/coderd/activitybump_internal_test.go @@ -193,27 +193,28 @@ func Test_ActivityBumpWorkspace(t *testing.T) { // Bump duration is measured from the time of the bump, so we measure from here. start := dbtime.Now() activityBumpWorkspace(ctx, log, db, bld.WorkspaceID) - elapsed := time.Since(start) - if elapsed > 15*time.Second { - t.Logf("warning: activityBumpWorkspace took longer than 15 seconds: %s", elapsed) - } - // The actual bump could have happened anywhere in the elapsed time, so we - // guess at the approximate time of the bump. - approxBumpTime := start.Add(elapsed / 2) + end := dbtime.Now() // Validate our state after bump updatedBuild, err := db.GetLatestWorkspaceBuildByWorkspaceID(ctx, bld.WorkspaceID) require.NoError(t, err, "unexpected error getting latest workspace build") + require.Equal(t, bld.MaxDeadline.UTC(), updatedBuild.MaxDeadline.UTC(), "max_deadline should not have changed") if tt.expectedBump == 0 { require.Equal(t, bld.UpdatedAt.UTC(), updatedBuild.UpdatedAt.UTC(), "should not have bumped updated_at") require.Equal(t, bld.Deadline.UTC(), updatedBuild.Deadline.UTC(), "should not have bumped deadline") - } else { - require.NotEqual(t, bld.UpdatedAt.UTC(), updatedBuild.UpdatedAt.UTC(), "should have bumped updated_at") - expectedDeadline := approxBumpTime.Add(tt.expectedBump).UTC() - // Note: if CI is especially slow, this test may fail. There is an internal 15-second - // deadline in activityBumpWorkspace, so we allow the same window here. - require.WithinDuration(t, expectedDeadline, updatedBuild.Deadline.UTC(), 15*time.Second, "unexpected deadline after bump") + return + } + require.NotEqual(t, bld.UpdatedAt.UTC(), updatedBuild.UpdatedAt.UTC(), "should have bumped updated_at") + if tt.maxDeadlineOffset != nil { + require.Equal(t, bld.MaxDeadline.UTC(), updatedBuild.MaxDeadline.UTC(), "new deadline must equal original max deadline") + return } + + // Assert that the bump occurred between start and end. + expectedDeadlineStart := start.Add(tt.expectedBump) + expectedDeadlineEnd := end.Add(tt.expectedBump) + require.GreaterOrEqual(t, updatedBuild.Deadline, expectedDeadlineStart, "new deadline should be greater than or equal to start") + require.LessOrEqual(t, updatedBuild.Deadline, expectedDeadlineEnd, "new deadline should be lesser than or equal to end") }) } } From f18c6b503448e9ae5d5d761375b9bba54ece2e55 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 14 Sep 2023 20:50:29 +0100 Subject: [PATCH 16/17] replace conditional sleep with a slower unconditional sleep that should hopefully be greater than win32 max timer resolution --- coderd/activitybump_internal_test.go | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/coderd/activitybump_internal_test.go b/coderd/activitybump_internal_test.go index a864aae25c8c3..8c77ae2a11dd1 100644 --- a/coderd/activitybump_internal_test.go +++ b/coderd/activitybump_internal_test.go @@ -2,7 +2,6 @@ package coderd import ( "database/sql" - "runtime" "testing" "time" @@ -188,7 +187,10 @@ func Test_ActivityBumpWorkspace(t *testing.T) { require.Equal(t, maxDeadline.UTC(), bld.MaxDeadline.UTC(), "unexpected max deadline before bump") require.Equal(t, tt.workspaceTTL, time.Duration(ws.Ttl.Int64), "unexpected workspace TTL before bump") - workaroundWindowsTimeResolution(t) + // Wait a bit before bumping as dbtime is rounded to the nearest millisecond. + // This should also hopefully be enough for Windows time resolution to register + // a tick (win32 max timer resolution is apparently between 0.5 and 15.6ms) + <-time.After(testutil.IntervalFast) // Bump duration is measured from the time of the bump, so we measure from here. start := dbtime.Now() @@ -237,11 +239,3 @@ func insertPrevWorkspaceBuild(t *testing.T, db database.Store, orgID, tvID, work Transition: transition, }) } - -func workaroundWindowsTimeResolution(t *testing.T) { - t.Helper() - if runtime.GOOS == "windows" { - t.Logf("workaround: sleeping for a short time to avoid time resolution issues on Windows") - <-time.After(testutil.IntervalSlow) - } -} From 5a8aa8b810d924a699d5ab61533a7c77b99c97ce Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 14 Sep 2023 20:51:18 +0100 Subject: [PATCH 17/17] modify logic slightly in TestWorkspaceActivityBump from "deadline must equal max_deadline" to actually be "deadline must not exceed max_deadline" as comments suggest --- coderd/activitybump_test.go | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/coderd/activitybump_test.go b/coderd/activitybump_test.go index 19ddf928d4e69..024803157b784 100644 --- a/coderd/activitybump_test.go +++ b/coderd/activitybump_test.go @@ -136,21 +136,29 @@ func TestWorkspaceActivityBump(t *testing.T) { return } + var updatedAfter time.Time // The Deadline bump occurs asynchronously. require.Eventuallyf(t, func() bool { workspace, err = client.Workspace(ctx, workspace.ID) require.NoError(t, err) - return workspace.LatestBuild.Deadline.Time != firstDeadline + updatedAfter = dbtime.Now() + if workspace.LatestBuild.Deadline.Time == firstDeadline { + updatedAfter = time.Now() + return false + } + return true }, testutil.WaitLong, testutil.IntervalFast, "deadline %v never updated", firstDeadline, ) + require.Greater(t, workspace.LatestBuild.Deadline.Time, updatedAfter) + // If the workspace has a max deadline, the deadline must not exceed // it. - if maxTTL != 0 && dbtime.Now().Add(ttl).After(workspace.LatestBuild.MaxDeadline.Time) { - require.Equal(t, workspace.LatestBuild.Deadline.Time, workspace.LatestBuild.MaxDeadline.Time) + if workspace.LatestBuild.MaxDeadline.Valid { + require.LessOrEqual(t, workspace.LatestBuild.Deadline.Time, workspace.LatestBuild.MaxDeadline.Time) return } require.WithinDuration(t, dbtime.Now().Add(ttl), workspace.LatestBuild.Deadline.Time, testutil.WaitShort) @@ -212,12 +220,6 @@ func TestWorkspaceActivityBump(t *testing.T) { require.NoError(t, err) _ = sshConn.Close() - assertBumped(true) - - // Double check that the workspace build's deadline is equal to the - // max deadline. - workspace, err = client.Workspace(ctx, workspace.ID) - require.NoError(t, err) - require.Equal(t, workspace.LatestBuild.Deadline.Time, workspace.LatestBuild.MaxDeadline.Time) + assertBumped(true) // also asserts max ttl not exceeded }) }