From 1d2aa890adde29c4349805bda64d2c041c81805a Mon Sep 17 00:00:00 2001 From: Dan Bungert Date: Tue, 28 Sep 2021 13:05:23 -0600 Subject: [PATCH] delete_partition: allow when disk preserve False If disk.preserve is false, a delete partition may proceed. If we're actively removing partitions from a disk before clearing it, that's fine as well even though the preserve bit may yet be True. --- subiquity/common/filesystem/manipulator.py | 8 ++- subiquity/server/controllers/filesystem.py | 2 +- subiquity/tests/api/test_api.py | 82 ++++++++++++++++------ 3 files changed, 65 insertions(+), 27 deletions(-) diff --git a/subiquity/common/filesystem/manipulator.py b/subiquity/common/filesystem/manipulator.py index 1905a3fc..ea1f48a1 100644 --- a/subiquity/common/filesystem/manipulator.py +++ b/subiquity/common/filesystem/manipulator.py @@ -87,7 +87,9 @@ class FilesystemManipulator: self.create_filesystem(part, spec) return part - def delete_partition(self, part): + def delete_partition(self, part, override_preserve=False): + if not override_preserve and part.device.preserve: + raise Exception("cannot delete partitions from preserved disks") self.clear(part) self.model.remove_partition(part) @@ -137,7 +139,7 @@ class FilesystemManipulator: for v in raid._subvolumes: self.delete_raid(v) for p in list(raid.partitions()): - self.delete_partition(p) + self.delete_partition(p, True) for d in set(raid.devices) | set(raid.spare_devices): d.wipe = 'superblock' self.model.remove_raid(raid) @@ -192,7 +194,7 @@ class FilesystemManipulator: def reformat(self, disk): disk.grub_device = False for p in list(disk.partitions()): - self.delete_partition(p) + self.delete_partition(p, True) self.clear(disk) def partition_disk_handler(self, disk, partition, spec): diff --git a/subiquity/server/controllers/filesystem.py b/subiquity/server/controllers/filesystem.py index cad594d7..5ccc8386 100644 --- a/subiquity/server/controllers/filesystem.py +++ b/subiquity/server/controllers/filesystem.py @@ -356,7 +356,7 @@ class FilesystemController(SubiquityController, FilesystemManipulator): -> StorageResponseV2: partition = self.get_partition(data.disk_id, data.partition.number) data.partition.size = partition.size - self.delete_partition(partition) + self.delete_partition(partition, True) return await self.v2_add_partition_POST(data) @with_context(name='probe_once', description='restricted={restricted}') diff --git a/subiquity/tests/api/test_api.py b/subiquity/tests/api/test_api.py index af6cdd10..d77c60e6 100755 --- a/subiquity/tests/api/test_api.py +++ b/subiquity/tests/api/test_api.py @@ -56,6 +56,7 @@ class ClientException(Exception): class TestAPI(unittest.IsolatedAsyncioTestCase): machine_config = 'examples/simple.json' + need_spawn_server = True async def get(self, query, **kwargs): return await self.request('GET', query, **kwargs) @@ -98,7 +99,7 @@ class TestAPI(unittest.IsolatedAsyncioTestCase): except aiohttp.client_exceptions.ServerDisconnectedError: return - async def asyncSetUp(self): + async def spawn_server(self): # start server process env = os.environ.copy() env['SUBIQUITY_REPLAY_TIMESCALE'] = '100' @@ -108,19 +109,24 @@ class TestAPI(unittest.IsolatedAsyncioTestCase): self.proc = await astart_command(cmd, env=env) self.server = asyncio.create_task(self.proc.communicate()) + async def asyncSetUp(self): + if self.need_spawn_server: + await self.spawn_server() + # setup client conn = aiohttp.UnixConnector(path=socket_path) self.session = aiohttp.ClientSession(connector=conn) await self.server_startup() async def asyncTearDown(self): - await self.server_shutdown() - await self.session.close() - await self.server - try: - self.proc.kill() - except ProcessLookupError: - pass + if self.need_spawn_server: + await self.server_shutdown() + await self.session.close() + await self.server + try: + self.proc.kill() + except ProcessLookupError: + pass class TestSimple(TestAPI): @@ -213,28 +219,20 @@ class TestWin10Start(TestAPI): self.assertEqual('disk-sda', orig_resp['disks'][0]['id']) self.assertEqual(4, len(orig_resp['disks'][0]['partitions'])) + resp = await self.post('/storage/v2/reformat_disk', disk_id=disk_id) + self.assertEqual(0, len(resp['disks'][0]['partitions'])) + data = { 'disk_id': disk_id, 'partition': { 'size': -1, - 'number': 4, + 'number': 1, + 'format': 'ext3', + 'mount': '/', + 'grub_device': True, 'preserve': False, } } - resp = await self.post('/storage/v2/delete_partition', data) - self.assertEqual(3, len(resp['disks'][0]['partitions'])) - - resp = await self.post('/storage/v2/reformat_disk', disk_id=disk_id) - self.assertEqual(0, len(resp['disks'][0]['partitions'])) - - data['partition'] = { - 'size': -1, - 'number': 1, - 'format': 'ext3', - 'mount': '/', - 'grub_device': True, - 'preserve': False, - } add_resp = await self.post('/storage/v2/add_partition', data) self.assertEqual(2, len(add_resp['disks'][0]['partitions'])) self.assertEqual('ext3', @@ -249,6 +247,9 @@ class TestWin10Start(TestAPI): self.assertEqual(expected[key], actual[key]) self.assertEqual('ext4', resp['disks'][0]['partitions'][1]['format']) + resp = await self.post('/storage/v2/delete_partition', data) + self.assertEqual(1, len(resp['disks'][0]['partitions'])) + resp = await self.post('/storage/v2/reset') self.assertEqual(orig_resp, resp) @@ -278,3 +279,38 @@ class TestWin10Start(TestAPI): self.assertEqual(2, len(resp['disks'][0]['partitions'])) self.assertEqual('ext4', resp['disks'][0]['partitions'][1]['format']) self.assertEqual(42410704896, resp['disks'][0]['free_for_partitions']) + + @timeout(5) + async def test_v2_delete_requires_reformat(self): + disk_id = 'disk-sda' + data = { + 'disk_id': disk_id, + 'partition': { + 'size': -1, + 'number': 4, + 'mount': '/', + 'format': 'ext4', + 'preserve': False, + } + } + await self.post('/storage/v2/edit_partition', data) + with self.assertRaises(Exception): + await self.post('/storage/v2/delete_partition', data) + +# class TestDebug(TestAPI): +# machine_config = 'examples/win10.json' +# need_spawn_server = False + +# @unittest.skip("useful for interactive debug only") +# async def test_v2_delete_debug(self): +# data = { +# 'disk_id': 'disk-sda', +# 'partition': { +# 'size': -1, +# 'number': 4, +# 'mount': '/', +# 'format': 'ext4', +# 'preserve': False, +# } +# } +# await self.post('/storage/v2/delete_partition', data)