summary refs log tree commit diff
path: root/nixos
diff options
context:
space:
mode:
authorMaximilian Bosch <maximilian@mbosch.me>2023-10-09 13:47:51 +0200
committerMaximilian Bosch <maximilian@mbosch.me>2023-10-09 14:06:50 +0200
commit7f08d0ebd8ebc54c2da12dd339efcf06e6a63d8c (patch)
tree86b7939a9de4b7f293588fded3c6872e8bba084a /nixos
parent67252742c17b87e04bda10babbdadbe872c7f084 (diff)
downloadnixpkgs-7f08d0ebd8ebc54c2da12dd339efcf06e6a63d8c.tar
nixpkgs-7f08d0ebd8ebc54c2da12dd339efcf06e6a63d8c.tar.gz
nixpkgs-7f08d0ebd8ebc54c2da12dd339efcf06e6a63d8c.tar.bz2
nixpkgs-7f08d0ebd8ebc54c2da12dd339efcf06e6a63d8c.tar.lz
nixpkgs-7f08d0ebd8ebc54c2da12dd339efcf06e6a63d8c.tar.xz
nixpkgs-7f08d0ebd8ebc54c2da12dd339efcf06e6a63d8c.tar.zst
nixpkgs-7f08d0ebd8ebc54c2da12dd339efcf06e6a63d8c.zip
nixos/matrix-synapse: drop old DB check assertion, actually require DB to be up
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
Diffstat (limited to 'nixos')
-rw-r--r--nixos/modules/services/matrix/synapse.nix23
1 files changed, 5 insertions, 18 deletions
diff --git a/nixos/modules/services/matrix/synapse.nix b/nixos/modules/services/matrix/synapse.nix
index 1354a8cb58b..1fb89ec4b24 100644
--- a/nixos/modules/services/matrix/synapse.nix
+++ b/nixos/modules/services/matrix/synapse.nix
@@ -12,7 +12,9 @@ let
 
   usePostgresql = cfg.settings.database.name == "psycopg2";
   hasLocalPostgresDB = let args = cfg.settings.database.args; in
-    usePostgresql && (!(args ? host) || (elem args.host [ "localhost" "127.0.0.1" "::1" ]));
+    usePostgresql
+    && (!(args ? host) || (elem args.host [ "localhost" "127.0.0.1" "::1" ]))
+    && config.services.postgresql.enable;
   hasWorkers = cfg.workers != { };
 
   listenerSupportsResource = resource: listener:
@@ -945,23 +947,6 @@ in {
         '';
       }
       {
-        assertion = hasLocalPostgresDB -> config.services.postgresql.enable;
-        message = ''
-          Cannot deploy matrix-synapse with a configuration for a local postgresql database
-            and a missing postgresql service. Since 20.03 it's mandatory to manually configure the
-            database (please read the thread in https://github.com/NixOS/nixpkgs/pull/80447 for
-            further reference).
-
-            If you
-            - try to deploy a fresh synapse, you need to configure the database yourself. An example
-              for this can be found in <nixpkgs/nixos/tests/matrix/synapse.nix>
-            - update your existing matrix-synapse instance, you simply need to add `services.postgresql.enable = true`
-              to your configuration.
-
-          For further information about this update, please read the release-notes of 20.03 carefully.
-        '';
-      }
-      {
         assertion = hasWorkers -> cfg.settings.redis.enabled;
         message = ''
           Workers for matrix-synapse require configuring a redis instance. This can be done
@@ -1034,9 +1019,11 @@ in {
             partOf = [ "matrix-synapse.target" ];
             wantedBy = [ "matrix-synapse.target" ];
             unitConfig.ReloadPropagatedFrom = "matrix-synapse.target";
+            requires = optional hasLocalPostgresDB "postgresql.service";
           }
           else {
             after = [ "network-online.target" ] ++ optional hasLocalPostgresDB "postgresql.service";
+            requires = optional hasLocalPostgresDB "postgresql.service";
             wantedBy = [ "multi-user.target" ];
           };
         baseServiceConfig = {