fleet
fleet copied to clipboard
Support MySQL 8 and drop support for MySQL 5.7
Goal
| User story |
|---|
| As a Fleet contributor, |
| I want to update Fleet to support MySQL 8 and drop support for MySQL 5.7 |
| so that I can ship stable Fleet releases that use MySQL 8 features. |
Context
- Requestor(s): @zwass
- Product designer: @lukeheath
Changes
Product
There's a huge chunk of work that has to do with communication:
- [ ] Documenting this in fleetdm.com
- [ ] Document this as part of the release notes
- [ ] Let all our customers and community know
- [ ] Support any customer that needs help with the migration
Engineering
All our queries are compatible with MySQL 8. We shouldn't scope modifying any of queries to make use of the MySQL 8 features.
- [ ] Modify the root
docker-compose.ymlfile to use MySQL 8 by default - [ ] Delete the GitHub action that tests in MySQL 5.7
- [ ] Document the supported version in our internal docs
- [ ] Let all the team know so they can update their local environments
ℹ️ Please read this issue carefully and understand it. Pay special attention to UI wireframes, especially "dev notes".
QA
Risk assessment
- Risk level: Low but needs really good communication
Manual testing steps
- Step 1
- Step 2
- Step 3
Testing notes
Confirmation
- [ ] Engineer (@____): Added comment to user story confirming successful completion of QA.
- [ ] QA (@____): Added comment to user story confirming successful completion of QA.
What will need to change in the docs?
@nonpunctual search shows some instances of 5.7 referenced in the docs: https://github.com/search?q=repo%3Afleetdm%2Ffleet%20mysql%205.7&type=code
(It also pulls up some code comments where we are doing extra work to support 5.7)
@zwass as you asked for pain points related to mysql 5.7, there's a recent one where mysql 8 supports more advanced check constraints that 5.7 parses syntactically, but otherwise ignores: https://github.com/fleetdm/fleet/blob/2d4a183789c5336373665516839b9314cd056a1e/server/datastore/mysql/migrations/tables/20240126020642_AddMDMProfileLabelsTable.go#L47-L51
And another point that comes to mind, that is not an issue per se at the moment but could become one if we maintained 5.7 support, is for the INSERT .. ON DUPLICATE KEY statements that we use a lot. The UPDATE part of the statement must use the form VALUES(column_name) to set the column to whatever the value was provided to the INSERT part in mysql 5.7 and that's supported in mysql 8.0 but it became officially deprecated in 8.0.20: https://dev.mysql.com/doc/refman/8.0/en/insert-on-duplicate.html (see the "NOTE" part mid-page).
As they document:
deprecated beginning with MySQL 8.0.20, and is subject to removal in a future version of MySQL. Instead, use row and column aliases, as described in the next few paragraphs of this section.
The row and column aliases is not supported by 5.7, so there could be a point where supporting 5.7 prevents us from supporting 8.X (that is, the point at which mysql decides to stop supporting VALUES(...)).
a few more:
- For labels, software titles and hosts we're doing variations of this due to the lack of JSON aggregation functions:
https://github.com/fleetdm/fleet/blob/f131d35807868a7e1f08efbedd9b8c9ac2f8b6f2/server/datastore/mysql/hosts.go#L980
- Also due to the lack of JSON aggregation functions, in other places we do manipulations like:
https://github.com/fleetdm/fleet/blob/f131d35807868a7e1f08efbedd9b8c9ac2f8b6f2/server/datastore/mysql/software_titles.go#L117-L126
- Lack of CTEs makes us do various tricks like complex subqueries
Thanks all! Really helpful examples / pain points above. Let's weigh it during feature fest next week.
Remove MySQL 5.7 support from documentation. Start using MySQL 8 features and drop code that supports 5.7.
Can we remove 5.7 from the docs and start using MySQL 8 w/o dropping code that supports 5.7?
(drop code later) Trying to think of ways to make the first pass work smaller.
Does that even make sense to do?
@noahtalerman @zwass Maybe in the docs where 5.7 is referenced the 1st step should be to add note or text block that says its EOL & won't be supported in upcoming versions without a specific time or version reference? We could put the same note at every mention...
Hey @zwass, @mna, @roperzh heads up, this story was prioritized during feature fest.
Aiming to ship an improvement in the next 6 weeks.
Moved the original issue description here:
Problem
MySQL 5.7 has been retired and is no longer receiving security updates. In Fleet's Terraform and Cloud instances, MySQL 8.0 is used. Continuing to document support for MySQL 5.7 may encourage our users to use the (insecure) older database version, and also constrains our own development efforts (eg. @getvictor mentioned poor unicode support in 5.7 that prevents us from using newer features that will work more like users expect).
Potential solutions
- Check in with self-hosted customers and work with them on an upgrade plan.
- Remove MySQL 5.7 support from documentation.
- Start using MySQL 8 features and drop code that supports 5.7.
@noahtalerman I am happy to help with this if there is an action item for me here otherwise can you let me know if there isn't one so I can unsubscribe from issue? Thanks all!
Hey @nonpunctual, I don't think we need any action from you at this time.
If it's helpful, I don't think you need to subscribe to any issues in fleetdm/fleet or fleetdm/confidential.
With the Toast app in Slack, you get notified every time you're @ mentioned in an issue in fleetdm/fleet or fleetdm/confidential.
another data point, we just spent almost a whole day with Sarah chasing a bug that's only present in MySQL 5.7.17
https://github.com/fleetdm/fleet/blob/cab98fe4c31bccd17ad2e6b0063619539c803812/server/datastore/mysql/apple_mdm.go#L2389-L2398
Hey @zwass heads up, we didn't get to this estimated in the last design sprint.
Bringing it back to feature fest.
What's the business impact of this change? How does it affect our users?
@noahtalerman please see numerous customer and business impacting issues above.
@lukeheath not sure if there's a process for engineering to prioritize things like this but sounds like it may be useful to do it sooner than later.
@zwass Will do, agree we want to action this soon. I'll speak to Noah later today about what's needed to get this estimated.
Hey @lukeheath want to take over as PM/PD for this one? :)
If yes, please add it to the drafting board and assign yourself.
Removing this from the feature fest board.
@noahtalerman Sure, happy to! I'll drive this forward next week.
@georgekarrv I'm not sure what additional specs this story needs, so I moved to the "Specified" column so y'all can consider it during the next estimation. This could go to either product group, but because @mna and @roperzh have been most involved in this issue I thought they'd be best suited to implement.
Absolutely, we will bring this around at our next estimation session.
@lukeheath @georgekarrv @mna All our queries are compatible with MySQL 8.
IMO we shouldn't scope modifying the list of queries we listed above to make use of the MySQL 8 features.
Engineering wise, I think what should be done is mostly cleanup (unless I'm missing something):
- Modify the root
docker-compose.ymlfile to use MySQL 8 by default - Delete the GitHub action that tests in MySQL 5.7
- Document the supported version in our internal docs
There's a huge chunk of work that has to do with communication:
- Documenting this in fleetdm.com
- Document this as part of the release notes
- Let all our customers and community know
- Support any customer that needs help with the migration
Thanks! @roperzh or @georgekarrv Please update the ticket description to reflect the research above so it's at the top and easy to see.
@lukeheath @georgekarrv I updated the issue, I put the communication changes under "Product", lmk if there's a better section for that or if it should be handled by Engineering.
Hey team! Please add your planning poker estimate with Zenhub @dantecatalfamo @ghernandez345 @gillespi314 @mna @roperzh
Another example where dropping MySQL 5.7 support would have lead to simpler code: https://github.com/fleetdm/fleet/pull/19654/files#diff-b2dbd5eb9d667567e564e494777c3d3662362ba5300cf6cb3669df059d51f73aR19
I will argue this should have a P2 next sprint if we don't finish it this week.
As a related ask, can we specify the minimum supported patch version of MySQL 8.0? An example:
MySQL 8.0.19, you can specify a time zone offset when inserting TIMESTAMP and DATETIME values into a table
@zayhanlon just a heads up this will merge on monday so it will 'officially' be dropped in 4.55.0 (mid sprint release)
i'm going to notify the customer and community base early @georgekarrv @ddribeiro @ksatter @pacamaster FYI
QA Notes:
- Restarted my docker environment (after merging latest
main) to update my MySQL version and confirmed I'm now on8.0.36 - I'll continue testing issues for this sprint and will run smoke tests as well
@xpkoala if you haven't already, it would be great if you were on ver8 too to test against EPOPS
Smoke Tests completed for both teams and passed.