7f08d0ebd8
Closes #236062 The PR #236062 was submitted because of the following problem: a synapse instance was running in a NixOS container attached to the host network and a postgresql instance on the host as database. In this setup, synapse connected to its DB via 127.0.0.1, but the DB wasn't locally set up and thus not configured in NixOS (i.e. `config.services.postgresql.enable` was `false`). This caused the assertion removed in this patch to fail. Over three years ago this assertion was introduced when this module stopped doing autoconfiguration of postgresql entirely[1] because a breaking change in synapse couldn't be managed via an auto-upgrade on our side. To make sure people don't deploy their DB away by accident, this assertion was introduced. Nowadays this doesn't serve any value anymore because people with existing instances should've upgraded by now (otherwise it's their job to carefully read the release notes when missing upgrades for several years) and people deploying fresh instances are instructed by the docs to also configure postgresql[2]. Instead, it only causes issues in corner cases like #236062, so after some discussion in that PR I think it's time to remove the assertion altogether. Also, there's no `Requires=` for `postgresql.service` in the systemd units which means that it's not strictly guaranteed that the DB is up when synapse starts up. This is fixed now by adding `requires`. To avoid being bitten by above mentioned cases again, this only happens if `config.services.postgresql.enable` is `true`. If somebody uses a non-local postgresql, but has also deployed a local postgresql instance on the synapse server (rather unlikely IMHO), it's their job to opt out of this behavior with `mkForce` (this is precisely one of the use-cases `mkForce` and friends were built for IMHO). [1] https://github.com/NixOS/nixpkgs/pull/80447 [2] https://nixos.org/manual/nixos/stable/#module-services-matrix-synapse |
||
|---|---|---|
| .. | ||
| doc/manual | ||
| lib | ||
| maintainers | ||
| modules | ||
| tests | ||
| COPYING | ||
| default.nix | ||
| README.md | ||
| release-combined.nix | ||
| release-small.nix | ||
| release.nix | ||
NixOS
NixOS is a Linux distribution based on the purely functional package management system Nix. More information can be found at https://nixos.org/nixos and in the manual in doc/manual.
Testing changes
You can add new module to your NixOS configuration file (usually it’s /etc/nixos/configuration.nix). And do sudo nixos-rebuild test -I nixpkgs=<path to your local nixpkgs folder> --fast.
Reviewing contributions
When changing the bootloader installation process, extra care must be taken. Grub installations cannot be rolled back, hence changes may break people’s installations forever. For any non-trivial change to the bootloader please file a PR asking for review, especially from @edolstra.
Module updates
Module updates are submissions changing modules in some ways. These often contains changes to the options or introduce new options.
Reviewing process:
- Ensure that the module maintainers are notified.
- CODEOWNERS will make GitHub notify users based on the submitted changes, but it can happen that it misses some of the package maintainers.
- Ensure that the module tests, if any, are succeeding.
- Ensure that the introduced options are correct.
- Type should be appropriate (string related types differs in their merging capabilities,
loaOfandstringtypes are deprecated). - Description, default and example should be provided.
- Type should be appropriate (string related types differs in their merging capabilities,
- Ensure that option changes are backward compatible.
mkRenamedOptionModuleWithprovides a way to make option changes backward compatible.
- Ensure that removed options are declared with
mkRemovedOptionModule - Ensure that changes that are not backward compatible are mentioned in release notes.
- Ensure that documentations affected by the change is updated.
Sample template for a module update review is provided below.
##### Reviewed points
- [ ] changes are backward compatible
- [ ] removed options are declared with `mkRemovedOptionModule`
- [ ] changes that are not backward compatible are documented in release notes
- [ ] module tests succeed on ARCHITECTURE
- [ ] options types are appropriate
- [ ] options description is set
- [ ] options example is provided
- [ ] documentation affected by the changes is updated
##### Possible improvements
##### Comments
New modules
New modules submissions introduce a new module to NixOS.
Reviewing process:
- Ensure that the module tests, if any, are succeeding.
- Ensure that the introduced options are correct.
- Type should be appropriate (string related types differs in their merging capabilities,
loaOfandstringtypes are deprecated). - Description, default and example should be provided.
- Type should be appropriate (string related types differs in their merging capabilities,
- Ensure that module
metafield is present- Maintainers should be declared in
meta.maintainers. - Module documentation should be declared with
meta.doc.
- Maintainers should be declared in
- Ensure that the module respect other modules functionality.
- For example, enabling a module should not open firewall ports by default.
Sample template for a new module review is provided below.
##### Reviewed points
- [ ] module path fits the guidelines
- [ ] module tests succeed on ARCHITECTURE
- [ ] options have appropriate types
- [ ] options have default
- [ ] options have example
- [ ] options have descriptions
- [ ] No unneeded package is added to environment.systemPackages
- [ ] meta.maintainers is set
- [ ] module documentation is declared in meta.doc
##### Possible improvements
##### Comments