Merge ~dbungert/curtin:resize-vs-wipe into curtin:master

[Dan Bungert]

block/v2: resize-friendly ordering of wipe

Split from https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/419640 and is expected to have resolved the comments there.

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:d42e21af212f444cf8516df6014627da09122f89
https://jenkins.ubuntu.com/server/job/curtin-ci/291/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/291/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/291/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/291/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/291/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/291//rebuild

review: Needs Fixing (continuous-integration)

[Michael Hudson-Doyle (mwhudson)]

LGTM too. This is going to conflict with that previous branch I think? Which do you plan to land first?

review: Approve

[Dan Bungert (dbungert)]

Yes, all 3 branches will conflict with each other, sadly. I guess I should do
1) ntfs
2) handle no format action
3) wipe

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:84000f3fe8d6c926ff20013662c43a7f2286533f
https://jenkins.ubuntu.com/server/job/curtin-ci/296/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/296/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/296/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/296/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/296/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/296//rebuild

review: Approve (continuous-integration)

diff --git a/curtin/commands/block_meta_v2.py b/curtin/commands/block_meta_v2.py
index cc2639c..a095c8f 100644
--- a/curtin/commands/block_meta_v2.py
+++ b/curtin/commands/block_meta_v2.py
@@ -362,8 +362,20 @@ def disk_handler_v2(info, storage_config, handlers):
             resizes[entry.start] = _prepare_resize(storage_config, action,
                                                    table, part_info)
             preserved_offsets.add(entry.start)
-        wipe = wipes[entry.start] = _wipe_for_action(action)
-        if wipe is not None:
+        wipes[entry.start] = _wipe_for_action(action)
+
+    for kname, nr, offset, size in block.sysfs_partition_data(disk):
+        offset_sectors = table.bytes2sectors(offset)
+        resize = resizes.get(offset_sectors)
+        if resize and resize['direction'] == 'down':
+            perform_resize(kname, resize)
+
+    for kname, nr, offset, size in block.sysfs_partition_data(disk):
+        offset_sectors = table.bytes2sectors(offset)
+        if offset_sectors not in preserved_offsets:
+            # Do a superblock wipe of any partitions that are being deleted.
+            block.wipe_volume(block.kname_to_path(kname), 'superblock')
+        elif wipes.get(offset_sectors) is not None:
             # We do a quick wipe of where any new partitions will be,
             # because if there is bcache or other metadata there, this
             # can cause the partition to be used by a storage
@@ -371,27 +383,17 @@ def disk_handler_v2(info, storage_config, handlers):
             # wipe_volume call below. See
             # https://bugs.launchpad.net/curtin/+bug/1718699 for all
             # the gory details.
-            wipe_offset = table.sectors2bytes(entry.start)
-            LOG.debug('Wiping 1M on %s at offset %s', disk, wipe_offset)
-            block.zero_file_at_offsets(disk, [wipe_offset], exclusive=False)
-
-    for kname, nr, offset, size in block.sysfs_partition_data(disk):
-        offset_sectors = table.bytes2sectors(offset)
-        if offset_sectors not in preserved_offsets:
-            # Do a superblock wipe of any partitions that are being deleted.
-            block.wipe_volume(block.kname_to_path(kname), 'superblock')
-        resize = resizes.get(offset_sectors)
-        if resize and resize['direction'] == 'down':
-            perform_resize(kname, resize)
+            LOG.debug('Wiping 1M on %s at offset %s', disk, offset)
+            block.zero_file_at_offsets(disk, [offset], exclusive=False)
 
     table.apply(disk)
 
     for kname, number, offset, size in block.sysfs_partition_data(disk):
         offset_sectors = table.bytes2sectors(offset)
-        mode = wipes[offset_sectors]
-        if mode is not None:
+        wipe = wipes[offset_sectors]
+        if wipe is not None:
             # Wipe the new partitions as needed.
-            block.wipe_volume(block.kname_to_path(kname), mode)
+            block.wipe_volume(block.kname_to_path(kname), wipe)
         resize = resizes.get(offset_sectors)
         if resize and resize['direction'] == 'up':
             perform_resize(kname, resize)
diff --git a/tests/integration/test_block_meta.py b/tests/integration/test_block_meta.py
index 76a974b..8322062 100644
--- a/tests/integration/test_block_meta.py
+++ b/tests/integration/test_block_meta.py
@@ -820,3 +820,45 @@ class TestBlockMeta(IntegrationTestCase):
             self.assertEqual(9 << 20, _get_filesystem_size(dev, p1))
             self.assertEqual(136 << 20, _get_filesystem_size(dev, p5))
             self.assertEqual(50 << 20, _get_filesystem_size(dev, p6))
+
+    def test_split_and_wiping(self):
+        # regression test for a bug where a partition wipe would happen before
+        # a resize was performed, resulting in data loss.
+        img = self.tmp_path('image.img')
+        config = StorageConfigBuilder(version=2)
+        config.add_image(path=img, size='100M', ptable='gpt')
+        p1 = config.add_part(size=98 << 20, offset=1 << 20, number=1,
+                             fstype='ext4')
+        self.run_bm(config.render())
+        with loop_dev(img) as dev:
+            self.assertEqual(
+                summarize_partitions(dev), [
+                    PartData(number=1, offset=1 << 20, size=98 << 20),
+                ])
+            with self.mount(dev, p1) as mnt_point:
+                # Attempt to create files across the partition with gaps
+                for i in range(1, 41):
+                    with open(f'{mnt_point}/{str(i)}', 'wb') as fp:
+                        fp.write(bytes([i]) * (2 << 20))
+                for i in range(1, 41):
+                    if i % 5 != 0:
+                        os.remove(f'{mnt_point}/{str(i)}')
+
+        config = StorageConfigBuilder(version=2)
+        config.add_image(path=img, size='100M', ptable='gpt')
+        p1 = config.add_part(size=49 << 20, offset=1 << 20, number=1,
+                             fstype='ext4', resize=True)
+        config.set_preserve()
+        config.add_part(size=49 << 20, offset=50 << 20, number=2,
+                        fstype='ext4')
+        self.run_bm(config.render())
+        with loop_dev(img) as dev:
+            self.assertEqual(
+                summarize_partitions(dev), [
+                    PartData(number=1, offset=1 << 20, size=49 << 20),
+                    PartData(number=2, offset=50 << 20, size=49 << 20),
+                ])
+            with self.mount(dev, p1) as mnt_point:
+                for i in range(5, 41, 5):
+                    with open(f'{mnt_point}/{str(i)}', 'rb') as fp:
+                        self.assertEqual(bytes([i]) * (2 << 20), fp.read())