fix: port changes now propagate to containers via env file in-place writes
Root cause: write_env_file used os.replace() which creates a new inode. Docker file bind-mounts track the original inode at mount time, so the container's /app/.env.compose never saw updates — docker compose always read the stale port value and skipped container recreation. Fixes: - ip_utils.write_env_file: write in-place (open 'w') instead of os.replace() so Docker bind-mounted files see the update immediately - apply_pending_config: add --force-recreate to docker compose up for specific-container restarts, bypassing config-hash comparison as a belt-and-suspenders measure Tests added: - TestWriteEnvFileInPlace: verifies inode is preserved across writes - TestApplyPendingConfigForceRecreate: verifies --force-recreate is in the docker compose command for specific-container restarts Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
+1
-1
@@ -1024,7 +1024,7 @@ def apply_pending_config():
|
|||||||
'--project-directory', project_dir,
|
'--project-directory', project_dir,
|
||||||
'-f', '/app/docker-compose.yml',
|
'-f', '/app/docker-compose.yml',
|
||||||
'--env-file', '/app/.env.compose',
|
'--env-file', '/app/.env.compose',
|
||||||
'up', '-d', '--no-deps'] + containers,
|
'up', '-d', '--no-deps', '--force-recreate'] + containers,
|
||||||
capture_output=True, text=True, timeout=120,
|
capture_output=True, text=True, timeout=120,
|
||||||
)
|
)
|
||||||
if result.returncode != 0:
|
if result.returncode != 0:
|
||||||
|
|||||||
+5
-4
@@ -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():
|
for key, var in PORT_ENV_VAR_NAMES.items():
|
||||||
lines.append(f'{var}={merged_ports[key]}\n')
|
lines.append(f'{var}={merged_ports[key]}\n')
|
||||||
os.makedirs(os.path.dirname(os.path.abspath(path)), exist_ok=True)
|
os.makedirs(os.path.dirname(os.path.abspath(path)), exist_ok=True)
|
||||||
tmp = path + '.tmp'
|
content = ''.join(lines)
|
||||||
with open(tmp, 'w') as f:
|
# Write in-place (same inode) so Docker bind-mounted files see the update.
|
||||||
f.writelines(lines)
|
# os.replace() changes the inode which breaks file bind-mounts inside containers.
|
||||||
|
with open(path, 'w') as f:
|
||||||
|
f.write(content)
|
||||||
f.flush()
|
f.flush()
|
||||||
os.fsync(f.fileno())
|
os.fsync(f.fileno())
|
||||||
os.replace(tmp, path)
|
|
||||||
return True
|
return True
|
||||||
except Exception:
|
except Exception:
|
||||||
return False
|
return False
|
||||||
|
|||||||
@@ -214,5 +214,50 @@ class TestWriteEnvFilePorts(unittest.TestCase):
|
|||||||
self.assertIn(var + '=', content, f'{var} missing from .env')
|
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__':
|
if __name__ == '__main__':
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|||||||
@@ -288,5 +288,97 @@ class TestWireGuardPortPropagation(unittest.TestCase):
|
|||||||
mock_wg.apply_config.assert_not_called()
|
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__':
|
if __name__ == '__main__':
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|||||||
Reference in New Issue
Block a user