Fix manual maintenance mode (#763)

It incorrectly assumed the flag was being set by the caller, and
returned early. The return value is meant to be informative, rather than
acted upon. The tests only tested the return value.

Fixed the function to DTRT, and the tests to check the flag value as
well as the return value

Fixes #762

jobrunner/service.py

@@ -101,38 +101,42 @@ def maintenance_mode():
     manual_db_mode = get_flag_value("manual-db-maintenance")
     if manual_db_mode:
         log.info(f"manually set db mode: {DB_MAINTENANCE_MODE}")
-        return DB_MAINTENANCE_MODE
-
-    # detect db mode from TPP.
-    current = get_flag_value("mode")
-    ps = docker(
-        [
-            "run",
-            "--rm",
-            "-e",
-            "DATABASE_URL",
-            "ghcr.io/opensafely-core/cohortextractor",
-            "maintenance",
-            "--current-mode",
-            str(current),
-        ],
-        env={"DATABASE_URL": config.DATABASE_URLS["default"]},
-        check=True,
-        capture_output=True,
-        text=True,
-    )
-    last_line = ps.stdout.strip().split("\n")[-1]
-    if DB_MAINTENANCE_MODE in last_line:
-        if current != DB_MAINTENANCE_MODE:
-            log.warning("Enabling DB maintenance mode")
-        else:
-            log.warning("DB maintenance mode is currently enabled")
-        set_flag("mode", DB_MAINTENANCE_MODE)
+        mode = DB_MAINTENANCE_MODE
     else:
-        if current == DB_MAINTENANCE_MODE:
-            log.info("DB maintenance mode had ended")
-        set_flag("mode", None)
 
+        # detect db mode from TPP.
+        current = get_flag_value("mode")
+        ps = docker(
+            [
+                "run",
+                "--rm",
+                "-e",
+                "DATABASE_URL",
+                "ghcr.io/opensafely-core/cohortextractor",
+                "maintenance",
+                "--current-mode",
+                str(current),
+            ],
+            env={"DATABASE_URL": config.DATABASE_URLS["default"]},
+            check=True,
+            capture_output=True,
+            text=True,
+        )
+        last_line = ps.stdout.strip().split("\n")[-1]
+
+        if DB_MAINTENANCE_MODE in last_line:
+            if current != DB_MAINTENANCE_MODE:
+                log.warning("Enabling DB maintenance mode")
+            else:
+                log.warning("DB maintenance mode is currently enabled")
+
+            mode = DB_MAINTENANCE_MODE
+        else:
+            if current == DB_MAINTENANCE_MODE:
+                log.info("DB maintenance mode had ended")
+            mode = None
+
+    set_flag("mode", mode)
     mode = get_flag_value("mode")
     log.info(f"db mode: {mode}")
     return mode

tests/test_service.py

@@ -63,24 +63,28 @@ def test_maintenance_mode_off(mock_subprocess_run, db, db_config):
     ps = add_maintenance_command(mock_subprocess_run, current=None)
     ps.stdout = ""
     assert service.maintenance_mode() is None
+    assert queries.get_flag("mode").value is None
 
     queries.set_flag("mode", "db-maintenance")
     ps = add_maintenance_command(mock_subprocess_run, current="db-maintenance")
     ps.stdout = ""
     assert service.maintenance_mode() is None
+    assert queries.get_flag("mode").value is None
 
 
 def test_maintenance_mode_on(mock_subprocess_run, db, db_config):
     ps = add_maintenance_command(mock_subprocess_run, current=None)
     ps.stdout = "db-maintenance"
     ps.stderr = "other stuff"
     assert service.maintenance_mode() == "db-maintenance"
+    assert queries.get_flag("mode").value == "db-maintenance"
 
     queries.set_flag("mode", "db-maintenance")
     ps = add_maintenance_command(mock_subprocess_run, current="db-maintenance")
     ps.stdout = "db-maintenance"
     ps.stderr = "other stuff"
     assert service.maintenance_mode() == "db-maintenance"
+    assert queries.get_flag("mode").value == "db-maintenance"
 
 
 def test_maintenance_mode_error(mock_subprocess_run, db, db_config):
@@ -96,6 +100,7 @@ def test_maintenance_mode_error(mock_subprocess_run, db, db_config):
 def test_maintenance_mode_manual(db, db_config):
     queries.set_flag("manual-db-maintenance", "on")
     assert service.maintenance_mode() == "db-maintenance"
+    assert queries.get_flag("mode").value == "db-maintenance"
 
 
 @pytest.fixture