Skip to content
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.

Sign up for GitHub

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

Jump to bottom

nixos/mysql: run postStart as an unprivileged user #95231

Merged
merged 7 commits into from Aug 14, 2020
Merged

nixos/mysql: run postStart as an unprivileged user #95231

merged 7 commits into from Aug 14, 2020

Conversation

aanderse
Copy link
Member

@aanderse aanderse commented Aug 12, 2020 •
edited

Motivation for this change
  • general module cleanup
  • remove the need to run any part of this module as root
  • keep consistent with upstream
  • keep consistent with NixOS best practices

My intention is to squash e3c210d (nixos/mysql: run ExecStartPost as an unprivileged user) and a0539fbe8635205ce3e5d0bf4daba316a92651ba (nixos/mysql: move ExecStartPost into postStart) before merge.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

ping @thorstenweber83

@aanderse
Copy link
Member Author

@GrahamcOfBorg test mysql

@aanderse aanderse mentioned this pull request Aug 12, 2020
Closed
10 tasks
florianjacob
florianjacob approved these changes Aug 13, 2020
View reviewed changes
Copy link
Contributor

@florianjacob florianjacob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a read-only review, this looks like a well-done and necessary cleanup of the module.

aszlig
aszlig approved these changes Aug 13, 2020
View reviewed changes
nixos/modules/services/databases/mysql.nix
count=0
while [ ! -e /run/mysqld/mysqld.sock ]
do
if [ $count -eq 30 ]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know, that this has been like this before, but I think 30 seconds might not be quite enough, especially for large instances. I've got two servers in production which need several minutes to start up (they're using a custom MySQL NixOS module). They're MariaDB instances however and thus have support for notify, but longer startup times (especially if eg. innodb tables need to be recovered) could happen for MySQL as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aszlig if you want to provide a value for me I'd be happy to add a commit for you in my next PR against this module (which is coming soon). I've seen 600 seconds as a timeout on some other distros for mysql.

@aanderse
Copy link
Member Author

Merging based on review and feedback. Thanks all 🎉

@aanderse
Copy link
Member Author

@GrahamcOfBorg test mysql

@aanderse
Copy link
Member Author

The aarch64-linux variant of the test has been failing for a while... so not this PR 🤷‍♂️

@aanderse aanderse merged commit f1f4cc6 into NixOS:master Aug 14, 2020
@aanderse aanderse deleted the mysql-cleanup branch August 14, 2020 01:38
jtojnar added a commit to jtojnar/nixfiles that referenced this pull request Aug 23, 2020
@jtojnar
flake: update
b511a45 
Also had to run the following commands under `sudo mysql -u root`:

    CREATE USER IF NOT EXISTS 'mysql'@'localhost' identified with unix_socket;
    GRANT ALL PRIVILEGES ON *.* TO 'mysql'@'localhost' WITH GRANT OPTION;

Otherwise, mysql would fail to start in `ensureDatabases` with

    Aug 23 03:03:11 azazel mysql-post-start[254503]: + /nix/store/nr3klbhk0cni9v0azgc2kjnajv83k31r-mariadb-server-10.4.14/bin/mysql -N
    Aug 23 03:03:11 azazel mysql-post-start[254503]: ERROR 1044 (42000) at line 1: Access denied for user ''@'localhost' to database 'ostrov-tucnaku'

The solution is described in NixOS/nixpkgs#95231.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@florianjacob florianjacob florianjacob approved these changes

@aszlig aszlig aszlig approved these changes

@Izorkin Izorkin Izorkin approved these changes

@flokli flokli Awaiting requested review from flokli

Assignees
No one assigned
Labels
6.topic: nixos 8.has: changelog 8.has: documentation 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@aanderse @aszlig @florianjacob @Izorkin