Skip to content

Update Dockerfile to modify correct config file #199

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 7, 2016

Conversation

ltangvald
Copy link
Collaborator

MySQL 5.7.14 changed the structure of the configuration directory, so
/etc/mysql/my.cnf no longer contains the config options, but only includes other directories.
This caused the modification done to the config in Dockerfile to fail silently, creating issues
with any setups that required those changes, such as running with a different user.
Changed script to modify the new file /etc/mysql/mysql.conf.d/mysqld.cnf

Fixes issue #197

MySQL 5.7.14 changed the structure of the configuration directory, so
/etc/mysql/my.cnf no longer contains the config options, but only includes other directories.
This caused the modification done to the config in Dockerfile to fail silently, creating issues
with any setups that required those changes, such as running with a different user.
Changed script to modify the new file /etc/mysql/mysql.conf.d/mysqld.cnf
@yosifkit
Copy link
Member

yosifkit commented Aug 4, 2016

It looks like the default mysql install now listens publicly. We need it in the docker image, but won't that be unexpected for other users of the mysql apt repo?

Do you think we should also move the skip-host-cache\nskip-name-resolve to a separate config file so it is easier to remove? Hopefully to help with #154.

@ltangvald
Copy link
Collaborator Author

Yeah, that's a good point. It's more in line with the default config on other platforms, but it's potentially a breaking change and shouldn't be changed for a GA. Thanks for pointing it out :)
Moving the docker-specific options to a new file sounds like a good idea.

@tianon
Copy link
Member

tianon commented Sep 7, 2016

Arg, bind-address is now back, and breaking 5.7. 🍭

Merging as-is to fix that! (and blaming @yosifkit appropriately for pointing it out and probably prompting you fine upstream folks to fix the issue 😄)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants