From ff2fd6c4e5720c4ef0d549431397f813b769c6e9 Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Sat, 31 Aug 2019 20:08:49 +0200 Subject: [PATCH 1/3] nixos/redis: unbreak module The redis module currently fails to start up, most likely due to running a chown as non-root in preStart. While at it, I hardcoded it to use systemd's StateDirectory and DynamicUser to manage directory permissions, removed the unused appendOnlyFilename option, and the pidFile option. We properly tell redis now it's daemonized, and it'll use notify support to signal readiness. --- nixos/doc/manual/release-notes/rl-1909.xml | 5 ++ nixos/modules/rename.nix | 9 +++- nixos/modules/services/databases/redis.nix | 58 +++++----------------- 3 files changed, 25 insertions(+), 47 deletions(-) diff --git a/nixos/doc/manual/release-notes/rl-1909.xml b/nixos/doc/manual/release-notes/rl-1909.xml index f831cfcdc57..14fafabde8e 100644 --- a/nixos/doc/manual/release-notes/rl-1909.xml +++ b/nixos/doc/manual/release-notes/rl-1909.xml @@ -599,6 +599,11 @@ package and crashplan-small-business service have been removed from nixpkgs due to lack of maintainer. + + The redis module was hardcoded to use the redis user, + /run/redis as runtime directory and + /var/lib/redis as state directory. + diff --git a/nixos/modules/rename.nix b/nixos/modules/rename.nix index 1048c2af2ea..9e0ab60ca67 100644 --- a/nixos/modules/rename.nix +++ b/nixos/modules/rename.nix @@ -256,7 +256,7 @@ with lib; # binfmt (mkRenamedOptionModule [ "boot" "binfmtMiscRegistrations" ] [ "boot" "binfmt" "registrations" ]) - + # ACME (mkRemovedOptionModule [ "security" "acme" "directory"] "ACME Directory is now hardcoded to /var/lib/acme and its permisisons are managed by systemd. See https://github.com/NixOS/nixpkgs/issues/53852 for more info.") (mkRemovedOptionModule [ "security" "acme" "preDelay"] "This option has been removed. If you want to make sure that something executes before certificates are provisioned, add a RequiredBy=acme-\${cert}.service to the service you want to execute before the cert renewal") @@ -285,6 +285,13 @@ with lib; throw "services.redshift.longitude is set to null, you can remove this" else builtins.fromJSON value)) + # Redis + (mkRemovedOptionModule [ "services" "redis" "user" ] "The redis module now is hardcoded to the redis user.") + (mkRemovedOptionModule [ "services" "redis" "dbpath" ] "The redis module now uses /var/lib/redis as data directory.") + (mkRemovedOptionModule [ "services" "redis" "dbFilename" ] "The redis module now uses /var/lib/redis/dump.rdb as database dump location.") + (mkRemovedOptionModule [ "services" "redis" "appendOnlyFilename" ] "This option was never used.") + (mkRemovedOptionModule [ "services" "redis" "pidFile" ] "This option was removed.") + ] ++ (forEach [ "blackboxExporter" "collectdExporter" "fritzboxExporter" "jsonExporter" "minioExporter" "nginxExporter" "nodeExporter" "snmpExporter" "unifiExporter" "varnishExporter" ] diff --git a/nixos/modules/services/databases/redis.nix b/nixos/modules/services/databases/redis.nix index a11c8ff1275..9c389d80a6d 100644 --- a/nixos/modules/services/databases/redis.nix +++ b/nixos/modules/services/databases/redis.nix @@ -8,17 +8,19 @@ let condOption = name: value: if value != null then "${name} ${toString value}" else ""; redisConfig = pkgs.writeText "redis.conf" '' - pidfile ${cfg.pidFile} port ${toString cfg.port} ${condOption "bind" cfg.bind} ${condOption "unixsocket" cfg.unixSocket} + daemonize yes + supervised systemd loglevel ${cfg.logLevel} logfile ${cfg.logfile} syslog-enabled ${redisBool cfg.syslog} + pidfile /run/redis/redis.pid databases ${toString cfg.databases} ${concatMapStrings (d: "save ${toString (builtins.elemAt d 0)} ${toString (builtins.elemAt d 1)}\n") cfg.save} - dbfilename ${cfg.dbFilename} - dir ${toString cfg.dbpath} + dbfilename dump.rdb + dir /var/lib/redis ${if cfg.slaveOf != null then "slaveof ${cfg.slaveOf.ip} ${toString cfg.slaveOf.port}" else ""} ${condOption "masterauth" cfg.masterAuth} ${condOption "requirepass" cfg.requirePass} @@ -55,18 +57,6 @@ in description = "Which Redis derivation to use."; }; - user = mkOption { - type = types.str; - default = "redis"; - description = "User account under which Redis runs."; - }; - - pidFile = mkOption { - type = types.path; - default = "/var/lib/redis/redis.pid"; - description = ""; - }; - port = mkOption { type = types.int; default = 6379; @@ -100,7 +90,7 @@ in type = with types; nullOr path; default = null; description = "The path to the socket to bind to."; - example = "/run/redis.sock"; + example = "/run/redis/redis.sock"; }; logLevel = mkOption { @@ -136,18 +126,6 @@ in example = [ [900 1] [300 10] [60 10000] ]; }; - dbFilename = mkOption { - type = types.str; - default = "dump.rdb"; - description = "The filename where to dump the DB."; - }; - - dbpath = mkOption { - type = types.path; - default = "/var/lib/redis"; - description = "The DB will be written inside this directory, with the filename specified using the 'dbFilename' configuration."; - }; - slaveOf = mkOption { default = null; # { ip, port } description = "An attribute set with two attributes: ip and port to which this redis instance acts as a slave."; @@ -175,12 +153,6 @@ in description = "By default data is only periodically persisted to disk, enable this option to use an append-only file for improved persistence."; }; - appendOnlyFilename = mkOption { - type = types.str; - default = "appendonly.aof"; - description = "Filename for the append-only file (stored inside of dbpath)"; - }; - appendFsync = mkOption { type = types.str; default = "everysec"; # no, always, everysec @@ -222,19 +194,15 @@ in allowedTCPPorts = [ cfg.port ]; }; - users.users.redis = - { name = cfg.user; - description = "Redis database user"; - }; + users.users.redis.description = "Redis database user"; environment.systemPackages = [ cfg.package ]; systemd.services.disable-transparent-huge-pages = { - enable = config.services.redis.enable; description = "Disable Transparent Huge Pages (required by Redis)"; before = [ "redis.service" ]; wantedBy = [ "redis.service" ]; - script = "echo never >/sys/kernel/mm/transparent_hugepage/enabled"; + script = "echo never > /sys/kernel/mm/transparent_hugepage/enabled"; serviceConfig.Type = "oneshot"; }; @@ -244,14 +212,12 @@ in wantedBy = [ "multi-user.target" ]; after = [ "network.target" ]; - preStart = '' - install -d -m0700 -o ${cfg.user} ${cfg.dbpath} - chown -R ${cfg.user} ${cfg.dbpath} - ''; - serviceConfig = { ExecStart = "${cfg.package}/bin/redis-server ${redisConfig}"; - User = cfg.user; + RuntimeDirectory = "redis"; + StateDirectory = "redis"; + Type = "notify"; + User = "redis"; }; }; From 8680f72c880fcf573b42e9203339653a9af411bc Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Sun, 1 Sep 2019 14:12:10 +0200 Subject: [PATCH 2/3] nixos/redis: add changelog for #67768 --- nixos/doc/manual/release-notes/rl-1909.xml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/nixos/doc/manual/release-notes/rl-1909.xml b/nixos/doc/manual/release-notes/rl-1909.xml index 14fafabde8e..3bb5bd84e0c 100644 --- a/nixos/doc/manual/release-notes/rl-1909.xml +++ b/nixos/doc/manual/release-notes/rl-1909.xml @@ -603,6 +603,9 @@ The redis module was hardcoded to use the redis user, /run/redis as runtime directory and /var/lib/redis as state directory. + Note that the NixOS module for Redis now disables kernel support for Transparent Huge Pages (THP), + because this features causes major performance problems for Redis, + e.g. (https://redis.io/topics/latency). From c00c4b1940afe842675e32b2e69c3b13f7035643 Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Sat, 31 Aug 2019 20:17:33 +0200 Subject: [PATCH 3/3] nixos/redis: add test --- nixos/tests/all-tests.nix | 1 + nixos/tests/redis.nix | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+) create mode 100644 nixos/tests/redis.nix diff --git a/nixos/tests/all-tests.nix b/nixos/tests/all-tests.nix index 14dca7409c4..8ee4dfbf13b 100644 --- a/nixos/tests/all-tests.nix +++ b/nixos/tests/all-tests.nix @@ -234,6 +234,7 @@ in rabbitmq = handleTest ./rabbitmq.nix {}; radarr = handleTest ./radarr.nix {}; radicale = handleTest ./radicale.nix {}; + redis = handleTest ./redis.nix {}; redmine = handleTest ./redmine.nix {}; roundcube = handleTest ./roundcube.nix {}; rspamd = handleTest ./rspamd.nix {}; diff --git a/nixos/tests/redis.nix b/nixos/tests/redis.nix new file mode 100644 index 00000000000..325d93424dd --- /dev/null +++ b/nixos/tests/redis.nix @@ -0,0 +1,26 @@ +import ./make-test.nix ({ pkgs, ...} : { + name = "redis"; + meta = with pkgs.stdenv.lib.maintainers; { + maintainers = [ flokli ]; + }; + + nodes = { + machine = + { pkgs, ... }: + + { + services.redis.enable = true; + services.redis.unixSocket = "/run/redis/redis.sock"; + }; + }; + + testScript = '' + startAll; + + $machine->waitForUnit("redis"); + $machine->waitForOpenPort("6379"); + + $machine->succeed("redis-cli ping | grep PONG"); + $machine->succeed("redis-cli -s /run/redis/redis.sock ping | grep PONG"); + ''; +})