Skip to content

Commit

Permalink
Support for MySQL 8.4.2 (fleetdm#21364)
Browse files Browse the repository at this point in the history
fleetdm#21270

The main change for MySQL 8.4.2 is that foreign key constraints are
stricter:
https://dev.mysql.com/doc/refman/8.4/en/server-system-variables.html#sysvar_restrict_fk_on_non_standard_key

Also, most replica-related commands have been renamed.

# Checklist for submitter

- [x] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/Committing-Changes.md#changes-files)
for more information.
- [x] Added/updated tests
- [x] If database migrations are included, checked table schema to
confirm autoupdate
- [x] Manual QA for all new/changed functionality
  • Loading branch information
getvictor authored Aug 16, 2024
1 parent 84ee756 commit 4eb7253
Show file tree
Hide file tree
Showing 11 changed files with 143 additions and 37 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test-go.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ jobs:
suite: ["integration", "core"]
os: [ubuntu-latest]
go-version: ['${{ vars.GO_VERSION }}']
mysql: ["mysql:8.0.36"]
mysql: ["mysql:8.0.36", "mysql:8.4.2"]
continue-on-error: ${{ matrix.suite == 'integration' }} # Since integration tests have a higher chance of failing, often for unrelated reasons, we don't want to fail the whole job if they fail
runs-on: ${{ matrix.os }}

Expand Down
1 change: 1 addition & 0 deletions changes/21270-mysql-8.4.2
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Added support for MySQL 8.4.2 LTS
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ func Up_20220915165115(tx *sql.Tx) error {
// Apply MDM Core schema.
_, err = tx.Exec(nanoSchema)
if err != nil {
return fmt.Errorf("failed to apply nanomdm schema: %w", err)
return fmt.Errorf("failed to apply nanomdm core schema: %w", err)
}

// Apply MDM DEP schema.
_, err = tx.Exec(depSchema)
if err != nil {
return fmt.Errorf("failed to apply nanomdm schema: %w", err)
return fmt.Errorf("failed to apply nanomdm dep schema: %w", err)
}

// Add Fleet domain tables.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ CREATE TABLE nano_users (
CHECK (user_authenticate_digest IS NULL OR user_authenticate_digest != '')
);

ALTER TABLE nano_users ADD CONSTRAINT idx_unique_id UNIQUE (id);

/* This table represents enrollments which are an amalgamation of
* both device and user enrollments.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,19 @@ func Up_20240730374423(tx *sql.Tx) error {
return fmt.Errorf("updating platform in vpp_apps: %w", err)
}

// Drop foreign keys first so they don't interfere with updating primary key.
_, err = tx.Exec(`ALTER TABLE vpp_apps_teams DROP FOREIGN KEY vpp_apps_teams_ibfk_1`)
if err != nil {
return fmt.Errorf("updating foreign key in vpp_apps: %w", err)
}

// We drop this foreign key in this migration (for MySQL 8.4). It will be added back in the next migration.
_, err = tx.Exec(`
ALTER TABLE host_vpp_software_installs DROP FOREIGN KEY host_vpp_software_installs_ibfk_2`)
if err != nil {
return fmt.Errorf("drop foreign key in host_vpp_software_installs: %w", err)
}

_, err = tx.Exec(`ALTER TABLE vpp_apps DROP PRIMARY KEY, ADD PRIMARY KEY (adam_id, platform)`)
if err != nil {
return fmt.Errorf("updating primary key in vpp_apps: %w", err)
Expand Down Expand Up @@ -57,7 +70,7 @@ func Up_20240730374423(tx *sql.Tx) error {
if err != nil {
return fmt.Errorf("updating key in vpp_apps: %w", err)
}
_, err = tx.Exec(`ALTER TABLE vpp_apps_teams DROP FOREIGN KEY vpp_apps_teams_ibfk_1, ADD FOREIGN KEY vpp_apps_teams_ibfk_1 (adam_id, platform) REFERENCES vpp_apps (adam_id, platform) ON DELETE CASCADE`)
_, err = tx.Exec(`ALTER TABLE vpp_apps_teams ADD FOREIGN KEY vpp_apps_teams_ibfk_3 (adam_id, platform) REFERENCES vpp_apps (adam_id, platform) ON DELETE CASCADE`)
if err != nil {
return fmt.Errorf("updating foreign key in vpp_apps: %w", err)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,16 @@ func Up_20240801115359(tx *sql.Tx) error {
if err != nil {
return fmt.Errorf("updating key in host_vpp_software_installs: %w", err)
}
if indexExistsTx(tx, "host_vpp_software_installs", "host_vpp_software_installs_ibfk_2") {
_, err = tx.Exec(`
ALTER TABLE host_vpp_software_installs DROP FOREIGN KEY host_vpp_software_installs_ibfk_2`)
if err != nil {
return fmt.Errorf("updating foreign key in host_vpp_software_installs: %w", err)
}
}
_, err = tx.Exec(`
ALTER TABLE host_vpp_software_installs DROP FOREIGN KEY host_vpp_software_installs_ibfk_2,
ADD FOREIGN KEY host_vpp_software_installs_ibfk_3 (adam_id, platform) REFERENCES vpp_apps (adam_id, platform) ON DELETE CASCADE`)
ALTER TABLE host_vpp_software_installs
ADD CONSTRAINT host_vpp_software_installs_ibfk_3 FOREIGN KEY (adam_id, platform) REFERENCES vpp_apps (adam_id, platform) ON DELETE CASCADE`)
if err != nil {
return fmt.Errorf("updating foreign key in host_vpp_software_installs: %w", err)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package tables

import (
"database/sql"
"fmt"
)

func init() {
MigrationClient.AddMigration(Up_20240816103247, Down_20240816103247)
}

func Up_20240816103247(tx *sql.Tx) error {

// This constraint is required for MySQL 8.4.2 because nano_enrollments foreign key expects nano_users.id to be unique.
if !indexExistsTx(tx, "nano_users", "idx_unique_id") {
_, err := tx.Exec(`ALTER TABLE nano_users ADD CONSTRAINT idx_unique_id UNIQUE (id)`)
if err != nil {
return fmt.Errorf("adding unique index to nano_users: %w", err)
}
}

return nil
}

func Down_20240816103247(tx *sql.Tx) error {
return nil
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package tables

import (
"testing"

"github.com/stretchr/testify/require"
)

func TestUp_20240816103247(t *testing.T) {
db := applyUpToPrev(t)

// Apply current migration
applyNext(t, db)

// Check if the index exists
var indexExists bool
err := db.QueryRow(`
SELECT 1 FROM information_schema.statistics
WHERE table_schema = DATABASE()
AND table_name = 'nano_users'
AND index_name = 'idx_unique_id'
`).Scan(&indexExists)

require.NoError(t, err)
require.True(t, indexExists, "Index idx_unique_id should exist")
}
16 changes: 16 additions & 0 deletions server/datastore/mysql/migrations/tables/migration.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,22 @@ AND index_name = ?
return count > 0
}

func indexExistsTx(tx *sql.Tx, table, index string) bool {
var count int
err := tx.QueryRow(`
SELECT COUNT(1)
FROM INFORMATION_SCHEMA.STATISTICS
WHERE table_schema = DATABASE()
AND table_name = ?
AND index_name = ?
`, table, index).Scan(&count)
if err != nil {
return false
}

return count > 0
}

// updateAppConfigJSON updates the `json_value` stored in the `app_config_json` after applying the
// supplied callback to the current config object.
func updateAppConfigJSON(tx *sql.Tx, fn func(config *fleet.AppConfig) error) error {
Expand Down
5 changes: 3 additions & 2 deletions server/datastore/mysql/schema.sql

Large diffs are not rendered by default.

72 changes: 43 additions & 29 deletions server/datastore/mysql/testing_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,14 +235,14 @@ func setupRealReplica(t testing.TB, testName string, ds *Datastore, options *dbO

t.Cleanup(
func() {
// Stop slave
// Stop replica
if out, err := exec.Command(
"docker", "compose", "exec", "-T", "mysql_replica_test",
// Command run inside container
"mysql",
"-u"+testUsername, "-p"+testPassword,
"-e",
"STOP SLAVE; RESET SLAVE ALL;",
"STOP REPLICA; RESET REPLICA ALL;",
).CombinedOutput(); err != nil {
t.Log(err)
t.Log(string(out))
Expand All @@ -262,25 +262,40 @@ func setupRealReplica(t testing.TB, testName string, ds *Datastore, options *dbO
_, err = ds.primary.ExecContext(ctx, "FLUSH PRIVILEGES")
require.NoError(t, err)

// Retrieve master binary log coordinates
ms, err := ds.MasterStatus(ctx)
require.NoError(t, err)

// Get MySQL version
var version string
err = ds.primary.GetContext(ctx, &version, "SELECT VERSION()")
require.NoError(t, err)
using57 := strings.HasPrefix(version, "5.7")
extraMasterOptions := ""
if !using57 {
extraMasterOptions = "GET_MASTER_PUBLIC_KEY=1," // needed for MySQL 8.0 caching_sha2_password authentication
}

// Retrieve master binary log coordinates
ms, err := ds.MasterStatus(ctx, version)
require.NoError(t, err)

mu.Lock()
databasesToReplicate = strings.TrimPrefix(databasesToReplicate+fmt.Sprintf(", `%s`", testName), ",")
mu.Unlock()

// Configure slave and start replication
setSourceStmt := fmt.Sprintf(`
CHANGE REPLICATION SOURCE TO
GET_SOURCE_PUBLIC_KEY=1,
SOURCE_HOST='mysql_test',
SOURCE_USER='%s',
SOURCE_PASSWORD='%s',
SOURCE_LOG_FILE='%s',
SOURCE_LOG_POS=%d
`, replicaUser, replicaPassword, ms.File, ms.Position)
if strings.HasPrefix(version, "8.0") {
setSourceStmt = fmt.Sprintf(`
CHANGE MASTER TO
GET_MASTER_PUBLIC_KEY=1,
MASTER_HOST='mysql_test',
MASTER_USER='%s',
MASTER_PASSWORD='%s',
MASTER_LOG_FILE='%s',
MASTER_LOG_POS=%d
`, replicaUser, replicaPassword, ms.File, ms.Position)
}

// Configure replica and start replication
if out, err := exec.Command(
"docker", "compose", "exec", "-T", "mysql_replica_test",
// Command run inside container
Expand All @@ -289,18 +304,12 @@ func setupRealReplica(t testing.TB, testName string, ds *Datastore, options *dbO
"-e",
fmt.Sprintf(
`
STOP SLAVE;
RESET SLAVE ALL;
STOP REPLICA;
RESET REPLICA ALL;
CHANGE REPLICATION FILTER REPLICATE_DO_DB = ( %s );
CHANGE MASTER TO
%s
MASTER_HOST='mysql_test',
MASTER_USER='%s',
MASTER_PASSWORD='%s',
MASTER_LOG_FILE='%s',
MASTER_LOG_POS=%d;
START SLAVE;
`, databasesToReplicate, extraMasterOptions, replicaUser, replicaPassword, ms.File, ms.Position,
%s;
START REPLICA;
`, databasesToReplicate, setSourceStmt,
),
).CombinedOutput(); err != nil {
t.Error(err)
Expand Down Expand Up @@ -454,7 +463,7 @@ func CreateMySQLDSWithReplica(t *testing.T, opts *DatastoreTestOptions) *Datasto
ds = createMySQLDSWithOptions(t, opts)
status, err := ds.ReplicaStatus(context.Background())
require.NoError(t, err)
if status["Replica_SQL_Running"] != "Yes" && status["Slave_SQL_Running"] != "Yes" {
if status["Replica_SQL_Running"] != "Yes" {
t.Logf("create replica attempt: %d replica status: %+v", attempt, status)
if lastErr, ok := status["Last_Error"]; ok && lastErr != "" {
t.Logf("replica not running after attempt %d; Last_Error: %s", attempt, lastErr)
Expand Down Expand Up @@ -793,10 +802,15 @@ type MasterStatus struct {
Position uint64
}

func (ds *Datastore) MasterStatus(ctx context.Context) (MasterStatus, error) {
rows, err := ds.writer(ctx).Query("SHOW MASTER STATUS")
func (ds *Datastore) MasterStatus(ctx context.Context, mysqlVersion string) (MasterStatus, error) {
stmt := "SHOW BINARY LOG STATUS"
if strings.HasPrefix(mysqlVersion, "8.0") {
stmt = "SHOW MASTER STATUS"
}

rows, err := ds.writer(ctx).Query(stmt)
if err != nil {
return MasterStatus{}, ctxerr.Wrap(ctx, err, "show master status")
return MasterStatus{}, ctxerr.Wrap(ctx, err, stmt)
}
defer rows.Close()

Expand Down Expand Up @@ -841,7 +855,7 @@ func (ds *Datastore) MasterStatus(ctx context.Context) (MasterStatus, error) {
}

func (ds *Datastore) ReplicaStatus(ctx context.Context) (map[string]interface{}, error) {
rows, err := ds.reader(ctx).QueryContext(ctx, "SHOW SLAVE STATUS")
rows, err := ds.reader(ctx).QueryContext(ctx, "SHOW REPLICA STATUS")
if err != nil {
return nil, ctxerr.Wrap(ctx, err, "show replica status")
}
Expand Down

0 comments on commit 4eb7253

Please sign in to comment.