fix: port changes now correctly queue pending restart for all services
Two bugs fixed: 1. calendar_manager and wireguard_manager (port-only) called _restart_container immediately in apply_config, bypassing the pending restart banner and restarting the container before the docker port binding in .env was updated — leaving the service broken until the banner was applied manually. apply_config now only updates the config file (radicale.conf / wg0.conf); the docker compose restart happens via the banner as intended. 2. Port change detection in update_config used `if old_val is not None` to guard against triggering on unchanged values. When a service's port was never explicitly saved (first time), old_val was None, so the pending restart was never queued. Fix: fall back to PORT_DEFAULTS[key] so the comparison is always against the effective current value. Add TestPortChangeDetection (5 tests) covering first-save and multi-service accumulation cases. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -209,5 +209,67 @@ class TestCancelPendingEndpoint(unittest.TestCase):
|
||||
self.assertEqual(data['changes'], [])
|
||||
|
||||
|
||||
class TestPortChangeDetection(unittest.TestCase):
|
||||
"""Test that port changes always trigger pending restart, even on first save."""
|
||||
|
||||
def setUp(self):
|
||||
app.config['TESTING'] = True
|
||||
self.client = app.test_client()
|
||||
_clear_pending_restart()
|
||||
# Remove any stored service configs so we start clean
|
||||
for key in ('calendar', 'files', 'wireguard', 'network', 'email'):
|
||||
config_manager.configs.pop(key, None)
|
||||
|
||||
def tearDown(self):
|
||||
_clear_pending_restart()
|
||||
for key in ('calendar', 'files', 'wireguard', 'network', 'email'):
|
||||
config_manager.configs.pop(key, None)
|
||||
|
||||
def _put_config(self, payload):
|
||||
return self.client.put('/api/config',
|
||||
data=json.dumps(payload),
|
||||
content_type='application/json')
|
||||
|
||||
def test_calendar_port_first_save_marks_pending(self):
|
||||
"""First-time calendar port save should still queue pending restart."""
|
||||
r = self._put_config({'calendar': {'port': 5233}})
|
||||
self.assertEqual(r.status_code, 200)
|
||||
p = config_manager.configs.get('_pending_restart', {})
|
||||
self.assertTrue(p.get('needs_restart'), 'pending restart not set on first calendar port save')
|
||||
self.assertIn('radicale', p.get('containers', []))
|
||||
|
||||
def test_files_port_first_save_marks_pending(self):
|
||||
"""First-time files (webdav) port save should queue pending restart."""
|
||||
r = self._put_config({'files': {'port': 8181}})
|
||||
self.assertEqual(r.status_code, 200)
|
||||
p = config_manager.configs.get('_pending_restart', {})
|
||||
self.assertTrue(p.get('needs_restart'))
|
||||
self.assertIn('webdav', p.get('containers', []))
|
||||
|
||||
def test_files_manager_port_first_save_marks_pending(self):
|
||||
r = self._put_config({'files': {'manager_port': 9090}})
|
||||
self.assertEqual(r.status_code, 200)
|
||||
p = config_manager.configs.get('_pending_restart', {})
|
||||
self.assertTrue(p.get('needs_restart'))
|
||||
self.assertIn('filegator', p.get('containers', []))
|
||||
|
||||
def test_multiple_service_port_changes_accumulate_containers(self):
|
||||
"""Saving two services should accumulate both containers in pending."""
|
||||
self._put_config({'calendar': {'port': 5233}})
|
||||
self._put_config({'files': {'port': 8181}})
|
||||
p = config_manager.configs.get('_pending_restart', {})
|
||||
self.assertTrue(p.get('needs_restart'))
|
||||
containers = p.get('containers', [])
|
||||
self.assertIn('radicale', containers)
|
||||
self.assertIn('webdav', containers)
|
||||
|
||||
def test_same_port_as_default_no_pending(self):
|
||||
"""Saving the default port value should NOT trigger pending restart."""
|
||||
r = self._put_config({'calendar': {'port': 5232}}) # 5232 is default
|
||||
self.assertEqual(r.status_code, 200)
|
||||
p = config_manager.configs.get('_pending_restart', {})
|
||||
self.assertFalse(p.get('needs_restart', False))
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
unittest.main()
|
||||
|
||||
Reference in New Issue
Block a user