fix: restore pre-stop stack state and runtime config path

zhaojing1987 · zhaojing1987 · commit ad111f5e83c6 · 2026-07-13T17:45:20.000+08:00
diff --git a/apphub/src/services/back_manager.py b/apphub/src/services/back_manager.py
@@ -357,9 +357,20 @@ def restore_backup(self, app_id: str, snapshot_id: str) -> None:
             if not extra_volumes:
                 raise CustomException(400, f"No valid volume mounts found for app: {app_id}", "No Volume Mounts")
 
-            # Stop containers before restore to release file locks and caches
             portainer = PortainerManager()
+            was_active = False
+            stack_id = None
             if endpoint_id:
+                try:
+                    stack_info = portainer.get_stack_by_name(app_id, endpoint_id)
+                    if stack_info:
+                        stack_id = stack_info.get("Id")
+                        was_active = stack_info.get("Status") == 1
+                except Exception as exc:
+                    logger.warning(f"Could not determine stack state for {app_id}: {exc}")
+
+            # Stop containers before restore to release file locks and caches
+            if endpoint_id and was_active:
                 try:
                     portainer.stop_stack(app_id, endpoint_id)
                     logger.access(f"Stopped containers for app {app_id} before restore")
@@ -391,23 +402,17 @@ def restore_backup(self, app_id: str, snapshot_id: str) -> None:
             if not summary_found:
                 raise CustomException(500, f"Restore incomplete — no summary returned for snapshot: {snapshot_id}", "Restore Failed")
 
-            # Start containers after restore via stack lifecycle APIs instead of
-            # starting every container individually. Some apps include one-shot
-            # init containers that should remain exited after a successful run.
+            # Use the stack state observed before restore. Once an active stack
+            # is stopped, Portainer reports it as inactive, which would wrongly
+            # push restores down the up_stack path and recreate containers.
             if endpoint_id:
                 try:
-                    stack_info = portainer.get_stack_by_name(app_id, endpoint_id)
-                    if stack_info is None:
-                        raise CustomException(404, "Not Found", f"Stack {app_id} not found")
-
-                    stack_status = stack_info.get("Status", 0)
-                    if stack_status == 2:
-                        stack_id = stack_info.get("Id")
-                        if stack_id is None:
-                            raise CustomException(404, "Not Found", f"Stack {app_id} has no Id")
+                    if was_active:
+                        portainer.start_stack(app_id, endpoint_id)
+                    elif stack_id is not None:
                         portainer.up_stack(stack_id, endpoint_id)
                     else:
-                        portainer.start_stack(app_id, endpoint_id)
+                        raise CustomException(404, "Not Found", f"Stack {app_id} not found")
 
                     self._ensure_restored_app_running(portainer, app_id, endpoint_id)
                     logger.access(f"Started containers for app {app_id} after restore")
diff --git a/apphub/tests/test_backup_restore.py b/apphub/tests/test_backup_restore.py
@@ -39,14 +39,23 @@
 
 
 class FakePortainer:
-    def __init__(self, stack_status, container_sequences):
+    def __init__(self, stack_status, container_sequences, stack_status_sequence=None):
         self.stack_status = stack_status
+        self.stack_status_sequence = list(stack_status_sequence or [])
         self.container_sequences = list(container_sequences)
         self.up_calls = []
         self.start_calls = []
+        self.stop_calls = []
 
     def get_stack_by_name(self, app_id, endpoint_id):
-        return {"Id": 9, "Name": app_id, "Status": self.stack_status}
+        if self.stack_status_sequence:
+            status = self.stack_status_sequence.pop(0)
+        else:
+            status = self.stack_status
+        return {"Id": 9, "Name": app_id, "Status": status}
+
+    def stop_stack(self, app_id, endpoint_id):
+        self.stop_calls.append((app_id, endpoint_id))
 
     def up_stack(self, stack_id, endpoint_id):
         self.up_calls.append((stack_id, endpoint_id))
@@ -115,6 +124,34 @@ def test_restore_start_uses_start_stack_for_active_stack(monkeypatch):
     assert portainer.up_calls == []
 
 
+def test_restore_uses_pre_stop_stack_state_for_active_stack(monkeypatch):
+    manager = _build_manager()
+    portainer = FakePortainer(
+        stack_status=1,
+        stack_status_sequence=[1],
+        container_sequences=[[{"Names": ["/wordpress_demo"], "State": "running"}]],
+    )
+
+    monkeypatch.setattr(manager, '_check_repository', lambda: True)
+    monkeypatch.setattr(manager, 'list_snapshots', lambda app_id: [{"id": "snap-1", "short_id": "snap-1"}])
+    monkeypatch.setattr(manager, '_run_restic_container', lambda command, extra_volumes: '{"message_type":"summary"}')
+    monkeypatch.setattr(manager, '_ensure_restored_app_running', lambda *args, **kwargs: None)
+    monkeypatch.setattr(back_manager_module, 'AppManger', lambda: types.SimpleNamespace(
+        get_app_by_id=lambda app_id: types.SimpleNamespace(
+            endpointId=1,
+            volumes=[{"Mountpoint": "/var/lib/docker/volumes/wordpress_demo/_data", "Name": "wordpress_demo"}],
+        )
+    ))
+    monkeypatch.setattr(back_manager_module, 'PortainerManager', lambda: portainer)
+    monkeypatch.setattr(manager, '_resolve_host_path', lambda path: path)
+
+    manager.restore_backup('wordpress_demo', 'snap-1')
+
+    assert portainer.stop_calls == [('wordpress_demo', 1)]
+    assert portainer.start_calls == [('wordpress_demo', 1)]
+    assert portainer.up_calls == []
+
+
 def test_restore_validation_rejects_only_exited_runtime_containers(monkeypatch):
     manager = _build_manager()
     portainer = FakePortainer(
diff --git a/apphub/tests/test_portainer_manager.py b/apphub/tests/test_portainer_manager.py
@@ -1,12 +1,18 @@
 import sys
 import types
+import importlib.util
 from pathlib import Path
 
 PROJECT_ROOT = Path(__file__).resolve().parents[1]
 if str(PROJECT_ROOT) not in sys.path:
     sys.path.insert(0, str(PROJECT_ROOT))
 
-from src.services.portainer_manager import PortainerManager
+module_path = PROJECT_ROOT / 'src' / 'services' / 'portainer_manager.py'
+module_spec = importlib.util.spec_from_file_location('real_portainer_manager', module_path)
+portainer_manager_module = importlib.util.module_from_spec(module_spec)
+assert module_spec and module_spec.loader
+module_spec.loader.exec_module(portainer_manager_module)
+PortainerManager = portainer_manager_module.PortainerManager
 
 
 class FakeResponse:
@@ -29,6 +35,12 @@ def create_stack_standlone_repository(self, stack_name, endpoint_id, repository_
         return self.responses.pop(0)
 
 
+def _build_manager(fake_api):
+    manager = PortainerManager.__new__(PortainerManager)
+    manager.portainer = fake_api
+    return manager
+
+
 def test_create_stack_retries_after_cleaning_stale_compose_workspace(monkeypatch, tmp_path):
     workdir = tmp_path / 'compose' / '2'
     workdir.mkdir(parents=True)
@@ -39,8 +51,7 @@ def test_create_stack_retries_after_cleaning_stale_compose_workspace(monkeypatch
         FakeResponse(200, payload={'Id': 2}),
     ])
 
-    manager = object.__new__(PortainerManager)
-    manager.portainer = fake_api
+    manager = _build_manager(fake_api)
     monkeypatch.setattr(manager, '_extract_compose_workdir', lambda message: str(workdir))
 
     result = manager.create_stack_from_repository('moodle_qy7wv', 1, 'http://repo', 'user', 'pwd')
diff --git a/install/lib/upgrade-legacy.sh b/install/lib/upgrade-legacy.sh
@@ -510,9 +510,12 @@ data_root = Path(os.environ.get("WEBSOFT9_DATA_ROOT", "/opt/websoft9/data"))
 
 legacy_config_path = data_root / ".w9-migration/legacy-config.ini"
 legacy_daemon_path = data_root / ".w9-migration/legacy-daemon.json"
-# Respect the WEBSOFT9_APPHUB_CONFIG_PATH env var so migrated settings land in the
-# same persistent config file the rest of the system reads, not the bundled copy.
-runtime_config_path = Path(os.environ.get("WEBSOFT9_APPHUB_CONFIG_PATH", "/websoft9/apphub/src/config/config.ini"))
+# docker exec shells do not inherit the PID 1 AppHub config env vars reliably.
+# Write directly to the persistent host-backed runtime config path used by the platform.
+runtime_config_path = Path("/opt/websoft9/data/config/apphub/config.ini")
+legacy_runtime_config_path = Path("/data/config/apphub/config.ini")
+if not runtime_config_path.parent.exists() and legacy_runtime_config_path.parent.exists():
+  runtime_config_path = legacy_runtime_config_path
 # Bundled defaults shipped inside the image — used as a fallback when the
 # persistent config hasn't been bootstrapped yet (first migration).
 bundled_config_path = Path("/websoft9/apphub/src/config/config.ini")
diff --git a/scripts/platform-sync-config.sh b/scripts/platform-sync-config.sh
@@ -3,6 +3,11 @@
 set -euo pipefail
 
 mode="base"
+data_root="${WEBSOFT9_DATA_ROOT:-/opt/websoft9/data}"
+runtime_config_path="${WEBSOFT9_APPHUB_CONFIG_PATH:-$data_root/config/apphub/config.ini}"
+runtime_system_config_path="${WEBSOFT9_APPHUB_SYSTEM_CONFIG_PATH:-$data_root/config/apphub/system.ini}"
+bundled_config_path="/websoft9/apphub/src/config/config.ini"
+bundled_system_config_path="/websoft9/apphub/src/config/system.ini"
 
 while [[ $# -gt 0 ]]; do
   case "$1" in
@@ -21,19 +26,34 @@ set_config() {
   websoft9 setconfig --section "$1" --key "$2" --value "$3" >/dev/null
 }
 
+bootstrap_runtime_config() {
+  local source_path="$1"
+  local target_path="$2"
+
+  mkdir -p "$(dirname "$target_path")"
+  if [[ ! -f "$target_path" && -f "$source_path" ]]; then
+    cp -a "$source_path" "$target_path"
+  fi
+}
+
+ensure_runtime_config_files() {
+  bootstrap_runtime_config "$bundled_config_path" "$runtime_config_path"
+  bootstrap_runtime_config "$bundled_system_config_path" "$runtime_system_config_path"
+}
+
 set_config_if_missing() {
-  python3 - "$1" "$2" "$3" <<'PY'
+  python3 - "$runtime_config_path" "$1" "$2" "$3" <<'PY'
 import configparser
 import sys
 
-section, key, value = sys.argv[1:4]
+config_path, section, key, value = sys.argv[1:5]
 config = configparser.ConfigParser()
-config.read('/websoft9/apphub/src/config/config.ini')
+config.read(config_path)
 if not config.has_section(section):
     config.add_section(section)
 if not config.has_option(section, key):
     config.set(section, key, value)
-    with open('/websoft9/apphub/src/config/config.ini', 'w', encoding='utf-8') as file:
+    with open(config_path, 'w', encoding='utf-8') as file:
         config.write(file)
 PY
 }
@@ -43,18 +63,21 @@ set_system_config() {
 }
 
 sync_base() {
+  ensure_runtime_config_files
   set_config_if_missing platform_gateway https_enabled "${WEBSOFT9_PLATFORM_HTTPS_ENABLED:-false}"
-  set_config_if_missing platform_gateway ssl_cert "${WEBSOFT9_PLATFORM_GATEWAY_CERT_PATH:-/etc/custom/platform-gateway/ssl/websoft9-platform-gateway.cert}"
-  set_config_if_missing platform_gateway ssl_key "${WEBSOFT9_PLATFORM_GATEWAY_KEY_PATH:-/etc/custom/platform-gateway/ssl/websoft9-platform-gateway.key}"
+  set_config_if_missing platform_gateway ssl_cert "${WEBSOFT9_PLATFORM_GATEWAY_CERT_PATH:-$data_root/config/platform-gateway/ssl/websoft9-platform-gateway.cert}"
+  set_config_if_missing platform_gateway ssl_key "${WEBSOFT9_PLATFORM_GATEWAY_KEY_PATH:-$data_root/config/platform-gateway/ssl/websoft9-platform-gateway.key}"
   set_system_config docker_library path "${WEBSOFT9_LIBRARY_PATH:-/websoft9/library/apps}"
   set_system_config app_media path "${WEBSOFT9_MEDIA_PATH:-/websoft9/media/json}"
 }
 
 sync_credentials() {
-  python3 - <<'PY'
+  ensure_runtime_config_files
+  python3 - "$runtime_config_path" <<'PY'
 import configparser
+import sys
 
-config_path = '/websoft9/apphub/src/config/config.ini'
+config_path = sys.argv[1]
 config = configparser.ConfigParser()
 config.read(config_path, encoding='utf-8')
 
