From c54b262351289a05f65c5412a4693b1f89350284 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Wed, 23 Dec 2020 22:06:13 +0530 Subject: [PATCH] MM-31356: Add a minimum required version check for Postgres (#16597) * MM-31356: Add a minimum required version check for Postgres To keep conformance with our failing fast and obvious philosophy, we add a check to prevent Mattermost server from starting if the postgres version is below 10.0. This gives customers a chance to upgrade their database before upgrading their Mattermost version, than to run into weird compatibility issues after they have finished the upgrade. https://mattermost.atlassian.net/browse/MM-31356 ```release-note NONE ``` * fix lint errors * Use a function to pretty-print version string * rectify comment --- app/server.go | 14 ++++++++++++++ services/telemetry/telemetry.go | 2 +- services/telemetry/telemetry_test.go | 2 +- store/sqlstore/store.go | 28 ++++++++++++++++++++++++++-- store/sqlstore/store_test.go | 27 ++++++++++++++++++++++++++- store/store.go | 2 +- store/storetest/mocks/Store.go | 14 +++++++------- store/storetest/store.go | 2 +- 8 files changed, 77 insertions(+), 14 deletions(-) diff --git a/app/server.go b/app/server.go index 2052aed425..12670446cc 100644 --- a/app/server.go +++ b/app/server.go @@ -16,6 +16,7 @@ import ( "path" "path/filepath" "runtime" + "strconv" "strings" "sync" "sync/atomic" @@ -319,6 +320,19 @@ func NewServer(options ...Option) (*Server, error) { if s.newStore == nil { s.newStore = func() (store.Store, error) { s.sqlStore = sqlstore.New(s.Config().SqlSettings, s.Metrics) + if s.sqlStore.DriverName() == model.DATABASE_DRIVER_POSTGRES { + ver, err2 := s.sqlStore.GetDbVersion(true) + if err2 != nil { + return nil, errors.Wrap(err2, "cannot get DB version") + } + intVer, err2 := strconv.Atoi(ver) + if err2 != nil { + return nil, errors.Wrap(err2, "cannot parse DB version") + } + if intVer < sqlstore.MINIMUM_REQUIRED_POSTGRES_VERSION { + return nil, fmt.Errorf("minimum required postgres version is %s; found %s", sqlstore.VersionString(sqlstore.MINIMUM_REQUIRED_POSTGRES_VERSION), sqlstore.VersionString(intVer)) + } + } lcl, err2 := localcachelayer.NewLocalCacheLayer( retrylayer.New(s.sqlStore), diff --git a/services/telemetry/telemetry.go b/services/telemetry/telemetry.go index d0fb5f8093..0ca08a938f 100644 --- a/services/telemetry/telemetry.go +++ b/services/telemetry/telemetry.go @@ -896,7 +896,7 @@ func (ts *TelemetryService) trackServer() { data["system_admins"] = scr } - if scr, err := ts.dbStore.GetDbVersion(); err == nil { + if scr, err := ts.dbStore.GetDbVersion(false); err == nil { data["database_version"] = scr } diff --git a/services/telemetry/telemetry_test.go b/services/telemetry/telemetry_test.go index 6e6b0064d8..865282d7d1 100644 --- a/services/telemetry/telemetry_test.go +++ b/services/telemetry/telemetry_test.go @@ -70,7 +70,7 @@ func initializeMocks(cfg *model.Config) (*mocks.ServerIface, *storeMocks.Store, serverIfaceMock.On("HttpService").Return(httpservice.MakeHTTPService(configService)) storeMock := &storeMocks.Store{} - storeMock.On("GetDbVersion").Return("5.24.0", nil) + storeMock.On("GetDbVersion", false).Return("5.24.0", nil) systemStore := storeMocks.SystemStore{} props := model.StringMap{} diff --git a/store/sqlstore/store.go b/store/sqlstore/store.go index a50a733870..a2ba1594c1 100644 --- a/store/sqlstore/store.go +++ b/store/sqlstore/store.go @@ -9,6 +9,7 @@ import ( "encoding/json" "fmt" "os" + "strconv" "strings" "sync" "sync/atomic" @@ -38,6 +39,12 @@ const ( MYSQL_DUP_TABLE_ERROR_CODE = uint16(1050) // see https://dev.mysql.com/doc/mysql-errors/5.7/en/server-error-reference.html#error_er_table_exists_error DB_PING_ATTEMPTS = 18 DB_PING_TIMEOUT_SECS = 10 + // This is a numerical version string by postgres. The format is + // 2 characters for major, minor, and patch version prior to 10. + // After 10, it's major and minor only. + // 10.1 would be 100001. + // 9.6.3 would be 90603. + MINIMUM_REQUIRED_POSTGRES_VERSION = 100000 ) const ( @@ -321,10 +328,17 @@ func (ss *SqlStore) GetCurrentSchemaVersion() string { return version } -func (ss *SqlStore) GetDbVersion() (string, error) { +// GetDbVersion returns the version of the database being used. +// If numerical is set to true, it attempts to return a numerical version string +// that can be parsed by callers. +func (ss *SqlStore) GetDbVersion(numerical bool) (string, error) { var sqlVersion string if ss.DriverName() == model.DATABASE_DRIVER_POSTGRES { - sqlVersion = `SHOW server_version` + if numerical { + sqlVersion = `SHOW server_version_num` + } else { + sqlVersion = `SHOW server_version` + } } else if ss.DriverName() == model.DATABASE_DRIVER_MYSQL { sqlVersion = `SELECT version()` } else if ss.DriverName() == model.DATABASE_DRIVER_SQLITE { @@ -1347,3 +1361,13 @@ func IsDuplicate(err error) bool { return false } + +// VersionString converts an integer representation of a DB version +// to a pretty-printed string. +// Postgres doesn't follow three-part version numbers from 10.0 onwards: +// https://www.postgresql.org/docs/13/libpq-status.html#LIBPQ-PQSERVERVERSION. +func VersionString(v int) string { + minor := v % 10000 + major := v / 10000 + return strconv.Itoa(major) + "." + strconv.Itoa(minor) +} diff --git a/store/sqlstore/store_test.go b/store/sqlstore/store_test.go index 2ef881b949..822f615f99 100644 --- a/store/sqlstore/store_test.go +++ b/store/sqlstore/store_test.go @@ -369,7 +369,7 @@ func TestGetDbVersion(t *testing.T) { settings := makeSqlSettings(driver) store := New(*settings, nil) - version, err := store.GetDbVersion() + version, err := store.GetDbVersion(false) require.Nil(t, err) require.Regexp(t, regexp.MustCompile(`\d+\.\d+(\.\d+)?`), version) }) @@ -471,6 +471,31 @@ func TestIsDuplicate(t *testing.T) { } } +func TestVersionString(t *testing.T) { + versions := []struct { + input int + output string + }{ + { + input: 100000, + output: "10.0", + }, + { + input: 90603, + output: "9.603", + }, + { + input: 120005, + output: "12.5", + }, + } + + for _, v := range versions { + out := VersionString(v.input) + assert.Equal(t, v.output, out) + } +} + func makeSqlSettings(driver string) *model.SqlSettings { switch driver { case model.DATABASE_DRIVER_POSTGRES: diff --git a/store/store.go b/store/store.go index b67d13c4c3..5bc62b2814 100644 --- a/store/store.go +++ b/store/store.go @@ -61,7 +61,7 @@ type Store interface { DropAllTables() RecycleDBConnections(d time.Duration) GetCurrentSchemaVersion() string - GetDbVersion() (string, error) + GetDbVersion(numerical bool) (string, error) TotalMasterDbConnections() int TotalReadDbConnections() int TotalSearchDbConnections() int diff --git a/store/storetest/mocks/Store.go b/store/storetest/mocks/Store.go index 45ed48e2a9..5ac8b24f46 100644 --- a/store/storetest/mocks/Store.go +++ b/store/storetest/mocks/Store.go @@ -236,20 +236,20 @@ func (_m *Store) GetCurrentSchemaVersion() string { return r0 } -// GetDbVersion provides a mock function with given fields: -func (_m *Store) GetDbVersion() (string, error) { - ret := _m.Called() +// GetDbVersion provides a mock function with given fields: numerical +func (_m *Store) GetDbVersion(numerical bool) (string, error) { + ret := _m.Called(numerical) var r0 string - if rf, ok := ret.Get(0).(func() string); ok { - r0 = rf() + if rf, ok := ret.Get(0).(func(bool) string); ok { + r0 = rf(numerical) } else { r0 = ret.Get(0).(string) } var r1 error - if rf, ok := ret.Get(1).(func() error); ok { - r1 = rf() + if rf, ok := ret.Get(1).(func(bool) error); ok { + r1 = rf(numerical) } else { r1 = ret.Error(1) } diff --git a/store/storetest/store.go b/store/storetest/store.go index 1889d707c8..4af7b3fb65 100644 --- a/store/storetest/store.go +++ b/store/storetest/store.go @@ -95,7 +95,7 @@ func (s *Store) Close() { /* do nothing */ } func (s *Store) LockToMaster() { /* do nothing */ } func (s *Store) UnlockFromMaster() { /* do nothing */ } func (s *Store) DropAllTables() { /* do nothing */ } -func (s *Store) GetDbVersion() (string, error) { return "", nil } +func (s *Store) GetDbVersion(bool) (string, error) { return "", nil } func (s *Store) RecycleDBConnections(time.Duration) {} func (s *Store) TotalMasterDbConnections() int { return 1 } func (s *Store) TotalReadDbConnections() int { return 1 }