fix: make node working directory deletion robust

The rmtree error handler in BaseNode.delete() was chmod'ing the failed
path to S_IWRITE (0o200). On POSIX this strips the search permission
from directories, turning a transient deletion failure into a directory
that can no longer be traversed or deleted. The node deletion itself
silently "succeeds" (rmtree gives up once its error handler returns)
and a later project deletion then fails with EACCES. The handler also
never retried the failed operation, so it did not help on Windows
either (the platform it was written for).

The transient failure exists in practice: a concurrent MD5 checksum
computation caching its result in the node directory (e.g. a properties
request racing the deletion) can recreate a file after rmtree has
listed the directory, making the final rmdir fail with ENOTEMPTY.

- add the missing user permissions instead of replacing the whole mode,
  and retry the failed unlink/rmdir
- retry the whole deletion a few times to absorb files recreated while
  the directory is being deleted
- raise a ComputeError when the directory cannot be fully deleted
  instead of failing silently
This commit is contained in:
YueGuobin committed 2026-09-15 01:23:31 +08:00
1 parent 1c57bd7dca
commit 84e7ead465
3 files changed
+136 -4

No files matched your search

+13 -2
View File
@@ -21,6 +21,7 @@ import pytest
import pytest_asyncio
import uuid
import os
import shutil
from types import SimpleNamespace
from unittest.mock import patch
@@ -2111,12 +2112,22 @@ async def test_close_reclaims_node_directory(vm, port_manager):
@pytest.mark.asyncio
async def test_delete_retries_after_reclaim(vm):
real_rmtree = shutil.rmtree
calls = 0
def rmtree_first_fails_then_deletes(directory, onerror=None, **kwargs):
nonlocal calls
calls += 1
if calls == 1:
raise OSError("permission denied")
real_rmtree(directory, onerror=onerror)
with patch.object(vm, "close", new_callable=AsyncioMagicMock):
with patch.object(vm, "_reclaim_directory_ownership", new_callable=AsyncioMagicMock, return_value=True) as mock_reclaim:
# First rmtree hits the root-owned leftovers, the retry (after
# the reclaim) succeeds.
# the reclaim) succeeds and really deletes the directory.
with patch("gns3server.compute.base_node.shutil.rmtree",
side_effect=[OSError("permission denied"), None]) as mock_rmtree:
side_effect=rmtree_first_fails_then_deletes) as mock_rmtree:
await vm.delete()
assert mock_rmtree.call_count == 2
mock_reclaim.assert_called_once_with(vm.working_dir)