From 44c232bbeb22bfd8a4188952c8dc98f1b8d43cb7 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Fri, 4 Jun 2021 17:28:20 +0200
Subject: [PATCH 1/2] nixos/postgresqlBackup: Use PATH for readability

(cherry picked from commit c586e42763e0f093d16b4b655759cb340171ad42)
---
 nixos/modules/services/backup/postgresql-backup.nix | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/nixos/modules/services/backup/postgresql-backup.nix b/nixos/modules/services/backup/postgresql-backup.nix
index 9da2d522a68..8857335a6e5 100644
--- a/nixos/modules/services/backup/postgresql-backup.nix
+++ b/nixos/modules/services/backup/postgresql-backup.nix
@@ -14,15 +14,17 @@ let
 
       requires = [ "postgresql.service" ];
 
+      path = [ pkgs.coreutils pkgs.gzip config.services.postgresql.package ];
+
       script = ''
         umask 0077 # ensure backup is only readable by postgres user
 
         if [ -e ${cfg.location}/${db}.sql.gz ]; then
-          ${pkgs.coreutils}/bin/mv ${cfg.location}/${db}.sql.gz ${cfg.location}/${db}.prev.sql.gz
+          mv ${cfg.location}/${db}.sql.gz ${cfg.location}/${db}.prev.sql.gz
         fi
 
         ${dumpCmd} | \
-          ${pkgs.gzip}/bin/gzip -c > ${cfg.location}/${db}.sql.gz
+          gzip -c > ${cfg.location}/${db}.sql.gz
       '';
 
       serviceConfig = {
@@ -113,12 +115,12 @@ in {
     })
     (mkIf (cfg.enable && cfg.backupAll) {
       systemd.services.postgresqlBackup =
-        postgresqlBackupService "all" "${config.services.postgresql.package}/bin/pg_dumpall";
+        postgresqlBackupService "all" "pg_dumpall";
     })
     (mkIf (cfg.enable && !cfg.backupAll) {
       systemd.services = listToAttrs (map (db:
         let
-          cmd = "${config.services.postgresql.package}/bin/pg_dump ${cfg.pgdumpOptions} ${db}";
+          cmd = "pg_dump ${cfg.pgdumpOptions} ${db}";
         in {
           name = "postgresqlBackup-${db}";
           value = postgresqlBackupService db cmd;

From 809cc5bf28c21fbc4947614fd13ece86730a3233 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Fri, 4 Jun 2021 17:34:26 +0200
Subject: [PATCH 2/2] nixos/postgresqlBackup: Only replace backup when
 successful

Previously, a failed backup would always overwrite ${db}.sql.gz,
because the bash `>` redirect truncates the file; even if the
backup was going to fail.
On the next run, the ${db}.prev.sql.gz backup would be
overwritten by the bad ${db}.sql.gz.

Now, if the backup fails, the ${db}.in-progress.sql.gz is in an
unknown state, but ${db}.sql.gz will not be written.
On the next run, ${db}.prev.sql.gz (our only good backup) will
not be overwritten because ${db}.sql.gz does not exist.

(cherry picked from commit 81c8189a841728a813bcde8604b80427fcf33522)
---
 .../services/backup/postgresql-backup.nix     |  6 ++++-
 nixos/tests/postgresql.nix                    | 22 +++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/nixos/modules/services/backup/postgresql-backup.nix b/nixos/modules/services/backup/postgresql-backup.nix
index 8857335a6e5..f658eb756f7 100644
--- a/nixos/modules/services/backup/postgresql-backup.nix
+++ b/nixos/modules/services/backup/postgresql-backup.nix
@@ -17,6 +17,8 @@ let
       path = [ pkgs.coreutils pkgs.gzip config.services.postgresql.package ];
 
       script = ''
+        set -e -o pipefail
+
         umask 0077 # ensure backup is only readable by postgres user
 
         if [ -e ${cfg.location}/${db}.sql.gz ]; then
@@ -24,7 +26,9 @@ let
         fi
 
         ${dumpCmd} | \
-          gzip -c > ${cfg.location}/${db}.sql.gz
+          gzip -c > ${cfg.location}/${db}.in-progress.sql.gz
+
+        mv ${cfg.location}/${db}.in-progress.sql.gz ${cfg.location}/${db}.sql.gz
       '';
 
       serviceConfig = {
diff --git a/nixos/tests/postgresql.nix b/nixos/tests/postgresql.nix
index 091e64294ac..0369a070719 100644
--- a/nixos/tests/postgresql.nix
+++ b/nixos/tests/postgresql.nix
@@ -73,8 +73,30 @@ let
           machine.succeed(
               "systemctl start ${backupService}.service",
               "zcat /var/backup/postgresql/${backupName}.sql.gz | grep '<test>ok</test>'",
+              "ls -hal /var/backup/postgresql/ >/dev/console",
               "stat -c '%a' /var/backup/postgresql/${backupName}.sql.gz | grep 600",
           )
+      with subtest("Backup service fails gracefully"):
+          # Sabotage the backup process
+          machine.succeed("rm /run/postgresql/.s.PGSQL.5432")
+          machine.fail(
+              "systemctl start ${backupService}.service",
+          )
+          machine.succeed(
+              "ls -hal /var/backup/postgresql/ >/dev/console",
+              "zcat /var/backup/postgresql/${backupName}.prev.sql.gz | grep '<test>ok</test>'",
+              "stat /var/backup/postgresql/${backupName}.in-progress.sql.gz",
+          )
+          # In a previous version, the second run would overwrite prev.sql.gz,
+          # so we test a second run as well.
+          machine.fail(
+              "systemctl start ${backupService}.service",
+          )
+          machine.succeed(
+              "stat /var/backup/postgresql/${backupName}.in-progress.sql.gz",
+              "zcat /var/backup/postgresql/${backupName}.prev.sql.gz | grep '<test>ok</test>'",
+          )
+
 
       with subtest("Initdb works"):
           machine.succeed("sudo -u postgres initdb -D /tmp/testpostgres2")