fleetdm / fleet

Open-source platform for IT, security, and infrastructure teams. (Linux, macOS, Chrome, Windows, cloud, data center)
https://fleetdm.com
Other
3.02k stars 419 forks source link

Update unreleased migration to avoid using a temporary DROP PRIMARY KEY #15619

Closed mna closed 9 months ago

mna commented 10 months ago

This can cause issues on deployments that use pxc_strict_mode with a Percona-modified version of mysql - it prevents existence of tables without primary keys (even if temporary).

See https://fleetdm.slack.com/archives/C019WG4GH0A/p1702404701144869?thread_ts=1702399444.800589&cid=C019WG4GH0A

mna commented 10 months ago

My understanding of the setting, reading this from percona's website:

Any undesirable operation performed on a table without an explicit primary key is denied and an error is logged.

Is that it would be ok to DROP and SET the primary key in the same ALTER statement (i.e. no "undesirable operation" would be performed on a table without a primary key). I'll try to run a test against such a percona mysql cluster with docker if possible.

mna commented 10 months ago

it would be ok to DROP and SET the primary key in the same ALTER statement

I'm fairly confident about that actually, given that the error we've seen in the 20231107130934 migration happened here, after the ALTER statements dropping of the PK, but when such a table's data got updated.

Also the Percona error message is:

Error 1105 (HY000): Percona-XtraDB-Cluster prohibits use of DML command on a table (tlsproddb.host_mdm_windows_profiles) without an explicit primary key with pxc_strict_mode = ENFORCING or MASTER)

So it would only error on DML commands. Another confirmation of that behavior is this recent-ish migration which also updates the PK of a table and we haven't heard of issues on this one (that I'm aware of, at least).

mna commented 10 months ago

For QA (@sabrinabuckets ): this is a DB migration fix, so I'd say to QA, get a fleet v4.41.1 running (ideally with some Apple MDM custom profiles) and then migrate the DB to the latest main (that includes this change) and see that the migration worked properly (i.e. if there were Apple profiles, they still show up after migration, and migration succeeded without issue).

Ideally, we'd run the migration on the "Percona XtraDB Cluster" version of mysql, which is the only place we've seen the migation issue, but I tried to set it up locally with docker without success (e.g. this didn't work for me: https://docs.percona.com/percona-xtradb-cluster/5.7/install/docker.html). But see my comment above, I'm fairly confident that the approach used in the fix will not cause this issue.

sabrinabuckets commented 9 months ago
bri@bris-mbp-2 fleet % FLEET_VERSION_A=v4.41.1 FLEET_VERSION_B=main go test ./test/upgrade                                                                                     
# github.com/fleetdm/fleet/v4/test/upgrade.test
ld: warning: ignoring duplicate libraries: '-lproc'
ok      github.com/fleetdm/fleet/v4/test/upgrade    76.589s
fleet-release commented 9 months ago

No more key drops, brief, Percona's strict mode eased, Stable as cloud's reef.