From 4968657651b2da6a9e456a64be5ed21f40200f95 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Mon, 19 Jul 2021 14:36:04 +0530 Subject: [PATCH] MM-37034: Bump the MySQL min version to 5.7 (#17919) * MM-37034: Bump the MySQL min version to 5.7 https://mattermost.atlassian.net/browse/MM-37034 ```release-note Minimum required MySQL version is now 5.7 ``` * fix shadow warnings ```release-note NONE ``` * Bumped to 5.7.12 ```release-note NONE ``` * update comment ```release-note NONE ``` * refactor ```release-note NONE ``` * fix lint error ```release-note NONE ``` * fix2 ```release-note NONE ``` * add tests ```release-note NONE ``` * simplify ```release-note NONE ``` --- build/docker-compose.common.yml | 4 +- store/sqlstore/store.go | 99 +++++++++++++++++++++++++-------- store/sqlstore/store_test.go | 91 +++++++++++++++++++++++++++++- 3 files changed, 169 insertions(+), 25 deletions(-) diff --git a/build/docker-compose.common.yml b/build/docker-compose.common.yml index 048224cdd8..d1d44ec155 100644 --- a/build/docker-compose.common.yml +++ b/build/docker-compose.common.yml @@ -1,7 +1,7 @@ version: '2.4' services: mysql: - image: "mysql:5.6" + image: "mysql:5.7.12" restart: always networks: - mm-test @@ -19,7 +19,7 @@ services: volumes: - ./docker/mysql.conf.d/source.cnf:/etc/mysql/conf.d/mysql.cnf mysql-read-replica: - image: "mysql:5.7" + image: "mysql:5.7.12" restart: always networks: - mm-test diff --git a/store/sqlstore/store.go b/store/sqlstore/store.go index 024304a2b5..55b9b23a86 100644 --- a/store/sqlstore/store.go +++ b/store/sqlstore/store.go @@ -57,7 +57,9 @@ const ( // After 10, it's major and minor only. // 10.1 would be 100001. // 9.6.3 would be 90603. - MinimumRequiredPostgresVersion = 100000 + minimumRequiredPostgresVersion = 100000 + // major*1000 + minor*100 + patch + minimumRequiredMySQLVersion = 5712 migrationsDirectionUp migrationDirection = "up" migrationsDirectionDown migrationDirection = "down" @@ -179,24 +181,19 @@ func New(settings model.SqlSettings, metrics einterfaces.MetricsInterface) *SqlS store.initConnection() - if *settings.DriverName == model.DatabaseDriverPostgres { - ver, err := store.GetDbVersion(true) - if err != nil { - mlog.Critical("Cannot get DB version.", mlog.Err(err)) - os.Exit(ExitGenericFailure) - } - intVer, err := strconv.Atoi(ver) - if err != nil { - mlog.Critical("Cannot parse DB version.", mlog.Err(err)) - os.Exit(ExitGenericFailure) - } - if intVer < MinimumRequiredPostgresVersion { - mlog.Critical("Minimum Postgres version requirements not met.", mlog.String("Found", VersionString(intVer)), mlog.String("Wanted", VersionString(MinimumRequiredPostgresVersion))) - os.Exit(ExitGenericFailure) - } + ver, err := store.GetDbVersion(true) + if err != nil { + mlog.Critical("Error while getting DB version.", mlog.Err(err)) + os.Exit(ExitGenericFailure) } - err := store.migrate(migrationsDirectionUp) + ok, err := store.ensureMinimumDBVersion(ver) + if !ok { + mlog.Critical("Error while checking DB version.", mlog.Err(err)) + os.Exit(ExitGenericFailure) + } + + err = store.migrate(migrationsDirectionUp) if err != nil { mlog.Critical("Failed to apply database migrations.", mlog.Err(err)) os.Exit(ExitGenericFailure) @@ -1678,12 +1675,70 @@ func IsDuplicate(err error) bool { return false } -// VersionString converts an integer representation of a DB version +// ensureMinimumDBVersion gets the DB version and ensures it is +// above the required minimum version requirements. +func (ss *SqlStore) ensureMinimumDBVersion(ver string) (bool, error) { + switch *ss.settings.DriverName { + case model.DatabaseDriverPostgres: + intVer, err2 := strconv.Atoi(ver) + if err2 != nil { + return false, fmt.Errorf("cannot parse DB version: %v", err2) + } + if intVer < minimumRequiredPostgresVersion { + return false, fmt.Errorf("minimum Postgres version requirements not met. Found: %s, Wanted: %s", versionString(intVer, *ss.settings.DriverName), versionString(minimumRequiredPostgresVersion, *ss.settings.DriverName)) + } + case model.DatabaseDriverMysql: + // Usually a version string is of the form 5.6.49-log, 10.4.5-MariaDB etc. + if strings.Contains(strings.ToLower(ver), "maria") { + mlog.Debug("MariaDB detected. Skipping version check.") + return true, nil + } + parts := strings.Split(ver, "-") + if len(parts) < 1 { + return false, fmt.Errorf("cannot parse MySQL DB version: %s", ver) + } + // Get the major and minor versions. + versions := strings.Split(parts[0], ".") + if len(versions) < 3 { + return false, fmt.Errorf("cannot parse MySQL DB version: %s", ver) + } + majorVer, err2 := strconv.Atoi(versions[0]) + if err2 != nil { + return false, fmt.Errorf("cannot parse MySQL DB version: %s", err2) + } + minorVer, err2 := strconv.Atoi(versions[1]) + if err2 != nil { + return false, fmt.Errorf("cannot parse MySQL DB version: %s", err2) + } + patchVer, err2 := strconv.Atoi(versions[2]) + if err2 != nil { + return false, fmt.Errorf("cannot parse MySQL DB version: %s", err2) + } + intVer := majorVer*1000 + minorVer*100 + patchVer + if intVer < minimumRequiredMySQLVersion { + return false, fmt.Errorf("minimum MySQL version requirements not met. Found: %s, Wanted: %s", versionString(intVer, *ss.settings.DriverName), versionString(minimumRequiredMySQLVersion, *ss.settings.DriverName)) + } + } + return true, nil +} + +// 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) +// For MySQL, we consider a major*1000 + minor*100 + patch format. +func versionString(v int, driver string) string { + switch driver { + case model.DatabaseDriverPostgres: + minor := v % 10000 + major := v / 10000 + return strconv.Itoa(major) + "." + strconv.Itoa(minor) + case model.DatabaseDriverMysql: + minor := v % 1000 + major := v / 1000 + patch := minor % 100 + minor = minor / 100 + return strconv.Itoa(major) + "." + strconv.Itoa(minor) + "." + strconv.Itoa(patch) + } + return "" } diff --git a/store/sqlstore/store_test.go b/store/sqlstore/store_test.go index 7d54edeb01..752f46fc76 100644 --- a/store/sqlstore/store_test.go +++ b/store/sqlstore/store_test.go @@ -406,6 +406,81 @@ func TestGetDbVersion(t *testing.T) { } } +func TestEnsureMinimumDBVersion(t *testing.T) { + tests := []struct { + driver string + ver string + ok bool + err string + }{ + { + driver: model.DatabaseDriverPostgres, + ver: "100001", + ok: true, + err: "", + }, + { + driver: model.DatabaseDriverPostgres, + ver: "90603", + ok: false, + err: "minimum Postgres version requirements not met", + }, + { + driver: model.DatabaseDriverPostgres, + ver: "12.34.1", + ok: false, + err: "cannot parse DB version", + }, + { + driver: model.DatabaseDriverMysql, + ver: "10.4.5-MariaDB", + ok: true, + err: "", + }, + { + driver: model.DatabaseDriverMysql, + ver: "5.6.99-test", + ok: false, + err: "minimum MySQL version requirements not met", + }, + { + driver: model.DatabaseDriverMysql, + ver: "34-55.12", + ok: false, + err: "cannot parse MySQL DB version", + }, + { + driver: model.DatabaseDriverMysql, + ver: "8.0.0-log", + ok: true, + err: "", + }, + } + + pg := model.DatabaseDriverPostgres + pgSettings := &model.SqlSettings{ + DriverName: &pg, + } + my := model.DatabaseDriverMysql + mySettings := &model.SqlSettings{ + DriverName: &my, + } + for _, tc := range tests { + store := &SqlStore{} + switch tc.driver { + case pg: + store.settings = pgSettings + case my: + store.settings = mySettings + } + ok, err := store.ensureMinimumDBVersion(tc.ver) + assert.Equal(t, tc.ok, ok) + if tc.err != "" { + assert.Contains(t, err.Error(), tc.err) + } + } +} + func TestUpAndDownMigrations(t *testing.T) { testDrivers := []string{ model.DatabaseDriverPostgres, @@ -536,24 +611,38 @@ func TestIsDuplicate(t *testing.T) { func TestVersionString(t *testing.T) { versions := []struct { input int + driver string output string }{ { input: 100000, + driver: model.DatabaseDriverPostgres, output: "10.0", }, { input: 90603, + driver: model.DatabaseDriverPostgres, output: "9.603", }, { input: 120005, + driver: model.DatabaseDriverPostgres, output: "12.5", }, + { + input: 5708, + driver: model.DatabaseDriverMysql, + output: "5.7.8", + }, + { + input: 8000, + driver: model.DatabaseDriverMysql, + output: "8.0.0", + }, } for _, v := range versions { - out := VersionString(v.input) + out := versionString(v.input, v.driver) assert.Equal(t, v.output, out) } }