search

sous-chefs / mysql

Development repository for the mysql cookbook
https://supermarket.chef.io/cookbooks/mysql
Apache License 2.0
337 stars 691 forks source link

Add mysql_user and mysql_database resources and remove Ubuntu 16.04 support #655

Closed EMH333 closed 3 years ago

EMH333 commented 3 years ago

Description

Adds mysql_user and mysql_database resources for creating, modifying and removing databases as well as database users. These resources were pulled from my https://github.com/sous-chefs/percona/pull/440 PR and modified to fit within this cookbook.

Edit: Also removes Ubuntu 16.04 support

Issues Resolved

Fixes #522

Check List

kitchen-porter commented 3 years ago
1 Warning
:warning: This is a big Pull Request.

Generated by :no_entry_sign: Danger

EMH333 commented 3 years ago

I'm unable to replicate the resources-57-ubuntu-1604 test failure locally so I suspect it's just being flaky. I'll wait till I get a review to rerun tests/dive deeper if needed.

ramereth commented 3 years ago

@EMH333 I'm able to replicate it when using kitchen-dokken but not with using kitchen-vagrant. Can you please see if you can too and figure out why that's happening?

ramereth commented 3 years ago

@EMH333 let's go ahead and remove Ubuntu 16.04 tests and see what we still need to fix to get this working. Looks like you still need to look into CentOS 8.

EMH333 commented 3 years ago

Wait to merge, so I can fix the grant issue here as well (https://github.com/sous-chefs/percona/pull/445)

EMH333 commented 3 years ago

I realized through the course of trying to find the converge error on Ubuntu 16.04 that the way this cookbook is set up and the way these resources work are fundamentally more different than I thought.

In the new resources, databases are selected via host/port or a socket for the default local server. Right now that doesn't match the way the rest of this cookbook is structured (multiple local db services, each with a different socket), and it might make sense to take the time to adapt it to that structure. The resources would have the option to take in the db service name, convert that to the proper socket file and use that instead of the conventional host/port or default socket. That logic is embedded within the resource, and it might take a lot of time to get right.

It wouldn't be a breaking change though, so we could just do a minor release later with that feature. And for people who just use a single database (named "default"), the current setup works perfectly fine.

Thoughts?

update: The decision made in the OSU OSL meeting is to merge this PR as is.

If requested at a later date, add support for multiple local instances, which would not be a breaking change.

kitchen-porter commented 3 years ago

Released as: 10.0.0