diff --git a/api/app.py b/api/app.py index 5274b97..a3732db 100644 --- a/api/app.py +++ b/api/app.py @@ -1024,7 +1024,7 @@ def apply_pending_config(): '--project-directory', project_dir, '-f', '/app/docker-compose.yml', '--env-file', '/app/.env.compose', - 'up', '-d', '--no-deps'] + containers, + 'up', '-d', '--no-deps', '--force-recreate'] + containers, capture_output=True, text=True, timeout=120, ) if result.returncode != 0: diff --git a/api/ip_utils.py b/api/ip_utils.py index 0837cb2..2f98cd9 100644 --- a/api/ip_utils.py +++ b/api/ip_utils.py @@ -233,12 +233,13 @@ def write_env_file(ip_range: str, path: str, ports: Optional[Dict[str, int]] = N for key, var in PORT_ENV_VAR_NAMES.items(): lines.append(f'{var}={merged_ports[key]}\n') os.makedirs(os.path.dirname(os.path.abspath(path)), exist_ok=True) - tmp = path + '.tmp' - with open(tmp, 'w') as f: - f.writelines(lines) + content = ''.join(lines) + # Write in-place (same inode) so Docker bind-mounted files see the update. + # os.replace() changes the inode which breaks file bind-mounts inside containers. + with open(path, 'w') as f: + f.write(content) f.flush() os.fsync(f.fileno()) - os.replace(tmp, path) return True except Exception: return False diff --git a/tests/test_ip_utils.py b/tests/test_ip_utils.py index 53e1ad2..5a5aa7f 100644 --- a/tests/test_ip_utils.py +++ b/tests/test_ip_utils.py @@ -214,5 +214,50 @@ class TestWriteEnvFilePorts(unittest.TestCase): self.assertIn(var + '=', content, f'{var} missing from .env') +class TestWriteEnvFileInPlace(unittest.TestCase): + """write_env_file must update the file in-place (same inode) so Docker + file bind-mounts inside containers see the change immediately. + os.replace() would create a new inode and the bind-mount would remain + pointing at the stale inode.""" + + def setUp(self): + self.tmp = tempfile.mkdtemp() + self.env_path = os.path.join(self.tmp, '.env') + # Pre-create the file so it has an initial inode + with open(self.env_path, 'w') as f: + f.write('INITIAL=1\n') + self.initial_inode = os.stat(self.env_path).st_ino + + def tearDown(self): + import shutil + shutil.rmtree(self.tmp) + + def test_same_inode_after_write(self): + """Inode must NOT change after write_env_file — bind-mounts track the inode.""" + ip_utils.write_env_file('172.20.0.0/16', self.env_path) + after_inode = os.stat(self.env_path).st_ino + self.assertEqual(self.initial_inode, after_inode, + 'write_env_file changed the file inode — Docker bind-mounts ' + 'would not see the update') + + def test_same_inode_after_port_change(self): + """Inode must be preserved even when port values change.""" + ip_utils.write_env_file('172.20.0.0/16', self.env_path, {'wg_port': 51820}) + inode_first = os.stat(self.env_path).st_ino + ip_utils.write_env_file('172.20.0.0/16', self.env_path, {'wg_port': 51821}) + inode_second = os.stat(self.env_path).st_ino + self.assertEqual(inode_first, inode_second, + 'write_env_file changed inode on second write') + self.assertIn('WG_PORT=51821', open(self.env_path).read()) + + def test_content_visible_via_open_after_write(self): + """After write_env_file the new content is immediately readable through + the same file descriptor path (same inode).""" + ip_utils.write_env_file('172.20.0.0/16', self.env_path, {'wg_port': 9999}) + content = open(self.env_path).read() + self.assertIn('WG_PORT=9999', content) + self.assertNotIn('INITIAL=1', content) + + if __name__ == '__main__': unittest.main() diff --git a/tests/test_wireguard_endpoints.py b/tests/test_wireguard_endpoints.py index baa9b64..17e55a3 100644 --- a/tests/test_wireguard_endpoints.py +++ b/tests/test_wireguard_endpoints.py @@ -288,5 +288,97 @@ class TestWireGuardPortPropagation(unittest.TestCase): mock_wg.apply_config.assert_not_called() +class TestApplyPendingConfigForceRecreate(unittest.TestCase): + """ + POST /api/config/apply for specific containers (not '*') must pass + --force-recreate to docker compose so that port-binding changes actually + take effect even if Docker's config-hash comparison misses them. + + The config-hash issue arises from Docker file bind-mounts: the env file + inside the container is mounted to a specific inode; if the host file was + ever replaced (new inode), the container's bind-mount stays on the old + inode and docker compose sees stale values. --force-recreate bypasses + the hash comparison entirely. + """ + + def setUp(self): + app.config['TESTING'] = True + self.client = app.test_client() + + @patch('app._clear_pending_restart') + @patch('app.config_manager') + def test_apply_pending_uses_force_recreate(self, mock_cm, mock_clear): + """apply_pending_config for specific containers must include --force-recreate.""" + mock_cm.configs = { + '_pending_restart': { + 'needs_restart': True, + 'containers': ['wireguard'], + 'network_recreate': False, + } + } + + captured_target = {} + + def patched_thread(target=None, daemon=False, **kw): + captured_target['fn'] = target + t = MagicMock() + t.start = lambda: None + return t + + with patch('app.threading.Thread', side_effect=patched_thread): + r = self.client.post('/api/config/apply') + + self.assertEqual(r.status_code, 200) + self.assertIn('fn', captured_target) + + # Execute the captured _do_apply and verify subprocess call includes --force-recreate + with patch('subprocess.run') as mock_run, \ + patch('time.sleep'): + mock_run.return_value = MagicMock(returncode=0, stderr='') + captured_target['fn']() + + call_args = mock_run.call_args + self.assertIsNotNone(call_args, 'subprocess.run was not called in _do_apply') + cmd = call_args[0][0] + self.assertIn('--force-recreate', cmd, + f'--force-recreate missing from docker compose command: {cmd}') + self.assertIn('wireguard', cmd) + + @patch('app._clear_pending_restart') + @patch('app.config_manager') + def test_apply_pending_all_services_no_force_recreate(self, mock_cm, mock_clear): + """All-services restart ('*') uses a helper container (Popen), not subprocess.run.""" + mock_cm.configs = { + '_pending_restart': { + 'needs_restart': True, + 'containers': ['*'], + 'network_recreate': False, + } + } + + captured_target = {} + + def patched_thread(target=None, daemon=False, **kw): + captured_target['fn'] = target + t = MagicMock() + t.start = lambda: None + return t + + with patch('app.threading.Thread', side_effect=patched_thread): + r = self.client.post('/api/config/apply') + + self.assertEqual(r.status_code, 200) + self.assertIn('fn', captured_target) + + # For '*', _do_apply spawns a helper container via Popen, not subprocess.run + with patch('subprocess.Popen') as mock_popen, \ + patch('subprocess.run') as mock_run: + mock_popen.return_value = MagicMock() + captured_target['fn']() + + mock_run.assert_not_called() + mock_popen.assert_called_once() + + if __name__ == '__main__': unittest.main()