fix(backup): check the schedule of a host backup job before saving it

.github/scripts/tests/test_backup_schedule_validation.py

@@ -0,0 +1,44 @@
+"""A schedule systemd rejects leaves a timer that never fires and reports no
+error, so the wizard asks for it again instead of saving it."""
+from pathlib import Path
+import subprocess
+import tempfile
+import unittest
+
+SCHEDULER = Path(__file__).resolve().parents[3] / 'scripts/backup_restore/backup_scheduler.sh'
+ANALYZE = '#!/bin/sh\ncase "$2" in "daily 02:00") exit 1;; esac\nexit 0\n'
+
+
+def function(name):
+    text = SCHEDULER.read_text()
+    start = text.index(name + '() {')
+    return text[start:text.index('\n}', start) + 2] + '\n'
+
+
+class ScheduleValidation(unittest.TestCase):
+    def valid(self, expression, **environment):
+        with tempfile.TemporaryDirectory() as directory:
+            analyze = Path(directory) / 'systemd-analyze'
+            analyze.write_text(ANALYZE)
+            analyze.chmod(0o755)
+            return subprocess.run(
+                ['/bin/bash', '-c', function('_calendar_is_valid') + '_calendar_is_valid "$1"', 'test', expression],
+                env={'PATH': f'{directory}:/usr/bin:/bin', **environment},
+            ).returncode == 0
+
+    def test_systemd_decides_what_is_valid(self):
+        self.assertTrue(self.valid('02:30'))
+        self.assertFalse(self.valid('daily 02:00'))
+
+    def test_a_scheduler_without_systemctl_is_not_checked(self):
+        self.assertTrue(self.valid('daily 02:00', PMX_BACKUP_NO_SYSTEMCTL='1'))
+
+    def test_the_wizard_asks_again_before_saving_the_job(self):
+        text = SCHEDULER.read_text()
+        check = text.index('while ! _calendar_is_valid "$on_calendar"; do')
+        self.assertLess(check, text.index('"ON_CALENDAR=$on_calendar"'))
+        self.assertGreater(check, text.index('on_calendar=$(dialog'))
+
+
+if __name__ == '__main__':
+    unittest.main()

lang/es.json

@@ -2385,6 +2385,7 @@
   "Example: cp /etc/pve/lxc/110.conf /etc/pve/lxc/110.conf.bak": "Ejemplo: cp /etc/pve/lxc/110.conf /etc/pve/lxc/110.conf.bak",
   "Example: cp /etc/pve/lxc/114.conf /etc/pve/lxc/114.conf.bak": "Ejemplo: cp /etc/pve/lxc/114.conf /etc/pve/lxc/114.conf.bak",
   "Example: daily or Mon..Fri 03:00": "Ejemplo: diario o lunes a viernes 03:00",
+  "Example: daily, 02:30 or Mon..Fri 03:00": "Ejemplo: daily, 02:30 o Mon..Fri 03:00",
   "Example: mkdir -p /tmp/lxc_convert_114": "Ejemplo: mkdir -p /tmp/lxc_convert_114",
   "Example: pct config 110 | grep unprivileged": "Ejemplo: pct config 110 | grep unprivileged",
   "Example: pct stop 110": "Ejemplo: pct stop 110",
@@ -7487,6 +7488,7 @@
   "This removes the storage from Proxmox. The iSCSI target is not affected.": "Esto elimina el almacenamiento de Proxmox. El destino iSCSI no se ve afectado.",
   "This replaces the currently installed keyfile.": "Esto reemplaza el archivo de claves instalado actualmente.",
   "This returns available IQNs.": "Esto devuelve los IQN disponibles.",
+  "This schedule is not valid. Examples: daily, 02:30 or Mon..Fri 03:00": "Esta programación no es válida. Ejemplos: daily, 02:30 o Mon..Fri 03:00",
   "This script cannot be executed from the Proxmox web terminal.": "Este script no se puede ejecutar desde la terminal web de Proxmox.",
   "This script must be run as root.": "Este script debe ejecutarse como root.",
   "This script must be run on a Proxmox host.": "Este script debe ejecutarse en un host Proxmox.",

lang/sk.json

@@ -2388,6 +2388,7 @@
   "Example: cp /etc/pve/lxc/110.conf /etc/pve/lxc/110.conf.bak": "Príklad: cp /etc/pve/lxc/110.conf /etc/pve/lxc/110.conf.bak",
   "Example: cp /etc/pve/lxc/114.conf /etc/pve/lxc/114.conf.bak": "Príklad: cp /etc/pve/lxc/114.conf /etc/pve/lxc/114.conf.bak",
   "Example: daily or Mon..Fri 03:00": "Príklad: daily alebo Mon..Fri 03:00",
+  "Example: daily, 02:30 or Mon..Fri 03:00": "Príklad: daily, 02:30 alebo Mon..Fri 03:00",
   "Example: mkdir -p /tmp/lxc_convert_114": "Príklad: mkdir -p /tmp/lxc_convert_114",
   "Example: pct config 110 | grep unprivileged": "Príklad: pct config 110 | grep unprivileged",
   "Example: pct stop 110": "Príklad: pct stop 110",
@@ -7514,6 +7515,7 @@
   "This removes the storage from Proxmox. The iSCSI target is not affected.": "Týmto sa úložisko odstráni z Proxmoxu. iSCSI target sa nezmení.",
   "This replaces the currently installed keyfile.": "Týmto sa nahradí aktuálne nainštalovaný keyfile.",
   "This returns available IQNs.": "Týmto sa zobrazia dostupné IQN.",
+  "This schedule is not valid. Examples: daily, 02:30 or Mon..Fri 03:00": "Tento plán nie je platný. Príklady: daily, 02:30 alebo Mon..Fri 03:00",
   "This script cannot be executed from the Proxmox web terminal.": "Tento skript sa nedá spustiť z webového terminálu Proxmoxu.",
   "This script must be run as root.": "Tento skript musí byť spustený ako root.",
   "This script must be run on a Proxmox host.": "Tento skript musí byť spustený na Proxmox serveri.",

scripts/backup_restore/backup_scheduler.sh

@@ -107,6 +107,12 @@ _job_is_attached() {
 
 # Reads a key=val pair from the job .env file (handles `printf %q`
 # quoting that _write_job_env produces).
+_calendar_is_valid() {
+  [[ "${PMX_BACKUP_NO_SYSTEMCTL:-0}" == "1" ]] && return 0
+  command -v systemd-analyze >/dev/null 2>&1 || return 0
+  systemd-analyze calendar "$1" >/dev/null 2>&1
+}
+
 _job_env_get() {
   local id="$1" key="$2" f raw
   f=$(_job_file "$id")
@@ -315,12 +321,10 @@ _reconcile_restored_jobs() {
     fi
 
     on_calendar=$(_job_env_get "$id" ON_CALENDAR || echo "")
-    if [[ "${PMX_BACKUP_NO_SYSTEMCTL:-0}" != "1" ]] && command -v systemd-analyze >/dev/null 2>&1; then
-      if ! systemd-analyze calendar "$on_calendar" >/dev/null 2>&1; then
-        echo "${id}: invalid systemd OnCalendar expression." >&2
-        invalid=$((invalid + 1))
-        continue
-      fi
+    if ! _calendar_is_valid "$on_calendar"; then
+      echo "${id}: invalid systemd OnCalendar expression." >&2
+      invalid=$((invalid + 1))
+      continue
     fi
 
     if ! _write_job_units "$id" "$on_calendar"; then
@@ -582,9 +586,18 @@ _create_job() {
   fi
 
   on_calendar=$(dialog --backtitle "ProxMenux" --title "$(translate "Schedule")" \
-    --inputbox "$(translate "systemd OnCalendar expression")"$'\n'"$(translate "Example: daily or Mon..Fri 03:00")" \
+    --inputbox "$(translate "systemd OnCalendar expression")"$'\n'"$(translate "Example: daily, 02:30 or Mon..Fri 03:00")" \
     11 72 "daily" 3>&1 1>&2 2>&3) || return 1
   [[ -z "$on_calendar" ]] && return 1
+  # A timer with an invalid expression never fires and reports no error.
+  while ! _calendar_is_valid "$on_calendar"; do
+    dialog --backtitle "ProxMenux" --title "$(translate "Schedule")" \
+      --msgbox "$(translate "This schedule is not valid. Examples: daily, 02:30 or Mon..Fri 03:00")" 8 72
+    on_calendar=$(dialog --backtitle "ProxMenux" --title "$(translate "Schedule")" \
+      --inputbox "$(translate "systemd OnCalendar expression")"$'\n'"$(translate "Example: daily, 02:30 or Mon..Fri 03:00")" \
+      11 72 "$on_calendar" 3>&1 1>&2 2>&3) || return 1
+    [[ -z "$on_calendar" ]] && return 1
+  done
 
   profile_mode=$(dialog --backtitle "ProxMenux" --title "$(translate "Profile")" \
     --menu "\n$(translate "Select backup profile:")" 12 68 4 \