nixos/redis: enable multiple instances of redis-server

[ju1m]

Motivation for this change

Revamping of services.redis to support per-application redis-server as it's supposed to be used since Redis databases are only mere key prefixes without any configuration or ACL of their own.
This is was extracted from #133984 to do a proper review.

Things done

• nix -L run -f . nixosTests.redis
• nix -L run -f . nixosTests.txredisapi

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all packages 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/)
• 21.11 Release Notes (or backporting 21.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/628

[aanderse]

Might be a bit late in the release cycle to merge this for 21.11.

[Izorkin]

It seems to me that the option looks bad:

services.redis.servers."".enable = true;

Сan try like this?:

services.redis.servers."default".enable = true;

or:

services.redis.servers.enable = true;
services.redis.servers."test1".enable = true;
services.redis.servers."test2".enable = true;

[ju1m]

It seems to me that the option looks bad:

services.redis.servers."".enable = true;

@Izorkin, indeed, I find it bad looking too, but to my mind it is a reminder that redis-server is not meant to be a global service, and that you should specify a server dedicated to your usage. I mean, naming it "" or "default" leads to the same bad practice this PR wants to get rid of.

Сan try like this?:

services.redis.servers."default".enable = true;

The thing is that mkRenamedOptionModule-ing using "default" instead of "" would break backward compatibility of the Unix user, group and socket path, unless the "default" server is singled out to behave like the "" server, but then what to do with the "" server? making it a synonym? disallowing it? I prefer to avoid special cases and keep the current 1-to-1 mapping.

or:

services.redis.servers.enable = true;
services.redis.servers."test1".enable = true;
services.redis.servers."test2".enable = true;

Not sure it can be done and that would be confusing because services.redis.servers is an attrset, so I would prefer to keep it simple.
What could be done is using mkAliasOptionModule instead of mkRenamedOptionModule, but again this would weaken the goal of this PR to get rid of any such thing as a global redis-server.

[tomberek]

Re-visit/re-review this now that 21.11 is complete?

[symphorien]

I'm really not familiar with redis, but I left two small comments

[symphorien]

why this default?

[ju1m]

For backward compatibility: previously redis was accessible over TCP by default, but this is not (easily) possible with multiple instances due to the need to allocate a TCP port.

[aanderse]

ping @roberth because of #148456

[roberth]

I don't think this interacts with #148456 more than any other module so that's fine.
Peeking at the PR, it seems that "" instead of "default" could be a decent choice if it means that the module logic is simpler and more predictable.
Please do add instructions to the release notes and explain the occurrences of "" using inline comments, so other maintainers know what's going on.

That all said, I will leave the review to maintainers who use redis.

[ju1m]

• Add release note.
• Fix description of services.redis.extraConfig.
• Rebase on latest master.

[tomberek]

I'll merge this after the weekend it there is no further interest or discussion. This will provide a long time to fix any issues prior to the next release.

[ju1m]

@lunik1, it has, my mistake, I've proposed a fix there: #156004

[yu-re-ka]

#158945