Merge ~mwhudson/curtin:image-action into curtin:master

Proposed by Michael Hudson-Doyle
Status: Merged
Approved by: Paride Legovini
Approved revision: 9e9a7cc092519e71782ac62389cdac12d33b1ab3
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~mwhudson/curtin:image-action
Merge into: curtin:master
Diff against target: 305 lines (+206/-6)
5 files modified
curtin/block/__init__.py (+2/-1)
curtin/commands/block_meta.py (+41/-3)
tests/integration/__init__.py (+3/-0)
tests/integration/test_block_meta.py (+156/-0)
tests/unittests/test_commands_block_meta.py (+4/-2)
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Approve
Dan Bungert Approve
Review via email: mp+409326@code.launchpad.net

Commit message

Add integration tests for some partitioning operations

This adds a new test directory, tests/integration, which sits
between the unittests and the vmtests in some sense. I've added a
way for curtin storage operations to operate on a file-backed
loop device rather than a real block device and written some
tests to cover the behaviour of partition_handler by actually
running block meta and then inspecting the resulting disk image
to check the results match expectations.

This is all motivated by the changes I want to make to support
editing partition tables.

Nothing runs these tests automatically yet, need to think about
that. Currently you can run them by hand with:

  sudo python3 -m pytest tests/integration/test_block_meta.py

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Dan Bungert (dbungert) :
review: Needs Information
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Thanks for the comments, pushed a new revision

Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Ryan Harper (raharper) :
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Thanks for the comments, pushed a commit which addresses most of them.

Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Dan Bungert (dbungert) wrote :

Thanks for this, I think it will be quite helpful.

review: Approve
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) :
review: Approve (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

Autolanding: FAILED
Approved revid is not set in launchpad. This is most likely a launchpad issue and re-approve should fix it. There is also a chance (although a very small one) this is a permission problem of the ps-jenkins bot
https://jenkins.ubuntu.com/server/job/curtin-autoland-test/138/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-autoland-test/nodes=metal-amd64/138/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-autoland-test/nodes=metal-arm64/138/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-autoland-test/nodes=metal-ppc64el/138/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-autoland-test/nodes=metal-s390x/138/

review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/curtin/block/__init__.py b/curtin/block/__init__.py
2index a33ccf2..5046eb9 100644
3--- a/curtin/block/__init__.py
4+++ b/curtin/block/__init__.py
5@@ -132,7 +132,8 @@ def partition_kname(disk_kname, partition_number):
6 os.path.realpath('%s-part%s' % (disk_link,
7 partition_number)))
8
9- for dev_type in ['bcache', 'nvme', 'mmcblk', 'cciss', 'mpath', 'md']:
10+ for dev_type in ['bcache', 'nvme', 'mmcblk', 'cciss', 'mpath', 'md',
11+ 'loop']:
12 if disk_kname.startswith(dev_type):
13 partition_number = "p%s" % partition_number
14 break
15diff --git a/curtin/commands/block_meta.py b/curtin/commands/block_meta.py
16index aae85c6..dc7efd4 100644
17--- a/curtin/commands/block_meta.py
18+++ b/curtin/commands/block_meta.py
19@@ -72,6 +72,8 @@ CMD_ARGUMENTS = (
20 'choices': ['ext4', 'ext3'], 'default': None}),
21 ('--umount', {'help': 'unmount any mounted filesystems before exit',
22 'action': 'store_true', 'default': False}),
23+ ('--testmode', {'help': 'enable some test actions',
24+ 'action': 'store_true', 'default': False}),
25 ('mode', {'help': 'meta-mode to use',
26 'choices': [CUSTOM, SIMPLE, SIMPLE_BOOT]}),
27 )
28@@ -81,7 +83,10 @@ CMD_ARGUMENTS = (
29 @logged_time("BLOCK_META")
30 def block_meta(args):
31 # main entry point for the block-meta command.
32- state = util.load_command_environment(strict=True)
33+ if args.testmode:
34+ state = {}
35+ else:
36+ state = util.load_command_environment(strict=True)
37 cfg = config.load_command_config(args, state)
38 dd_images = util.get_dd_images(cfg.get('sources', {}))
39
40@@ -99,7 +104,8 @@ def block_meta(args):
41 args.devices = devices
42
43 LOG.debug('clearing devices=%s', devices)
44- meta_clear(devices, state.get('report_stack_prefix', ''))
45+ if devices:
46+ meta_clear(devices, state.get('report_stack_prefix', ''))
47
48 # dd-images requires use of meta_simple
49 if len(dd_images) > 0 and args.force_mode is False:
50@@ -517,6 +523,9 @@ def get_path_to_storage_volume(volume, storage_config):
51 volume_path = block.kname_to_path(bcache_kname)
52 LOG.debug('got bcache volume path %s', volume_path)
53
54+ elif vol.get('type') == 'image':
55+ volume_path = vol['dev']
56+
57 else:
58 raise NotImplementedError("cannot determine the path to storage \
59 volume '%s' with type '%s'" % (volume, vol.get('type')))
60@@ -530,6 +539,28 @@ def get_path_to_storage_volume(volume, storage_config):
61 return volume_path
62
63
64+DEVS = set()
65+
66+
67+def image_handler(info, storage_config):
68+ path = info['path']
69+ if os.path.exists(path):
70+ os.unlink(path)
71+ try:
72+ with open(path, 'wb') as fp:
73+ fp.truncate(int(util.human2bytes(info['size'])))
74+ dev = util.subp([
75+ 'losetup', '--show', '--find', path],
76+ capture=True)[0].strip()
77+ except BaseException:
78+ if os.path.exists(path):
79+ os.unlink(path)
80+ raise
81+ info['dev'] = dev
82+ DEVS.add(dev)
83+ disk_handler(info, storage_config)
84+
85+
86 def dasd_handler(info, storage_config):
87 """ Prepare the specified dasd device per configuration
88
89@@ -1967,7 +1998,11 @@ def meta_custom(args):
90 'zpool': zpool_handler,
91 }
92
93- state = util.load_command_environment(strict=True)
94+ if args.testmode:
95+ command_handlers['image'] = image_handler
96+ state = {}
97+ else:
98+ state = util.load_command_environment(strict=True)
99 cfg = config.load_command_config(args, state)
100
101 storage_config_dict = extract_storage_ordered_dict(cfg)
102@@ -1992,6 +2027,9 @@ def meta_custom(args):
103 (item_id, type(error).__name__, error))
104 raise
105
106+ if args.testmode:
107+ util.subp(['losetup', '--detach'] + list(DEVS))
108+
109 if args.umount:
110 util.do_umount(state['target'], recursive=True)
111 return 0
112diff --git a/tests/integration/__init__.py b/tests/integration/__init__.py
113new file mode 100644
114index 0000000..5e43585
115--- /dev/null
116+++ b/tests/integration/__init__.py
117@@ -0,0 +1,3 @@
118+# This file is part of curtin. See LICENSE file for copyright and license info.
119+
120+# This directory contains tests that require root to run, essentially.
121diff --git a/tests/integration/test_block_meta.py b/tests/integration/test_block_meta.py
122new file mode 100644
123index 0000000..f8aa6f3
124--- /dev/null
125+++ b/tests/integration/test_block_meta.py
126@@ -0,0 +1,156 @@
127+# This file is part of curtin. See LICENSE file for copyright and license info.
128+
129+from collections import namedtuple
130+import contextlib
131+import sys
132+import yaml
133+
134+from curtin import block, udev, util
135+
136+from tests.unittests.helpers import CiTestCase
137+
138+
139+class IntegrationTestCase(CiTestCase):
140+ allowed_subp = True
141+
142+
143+@contextlib.contextmanager
144+def loop_dev(image):
145+ dev = util.subp(
146+ ['losetup', '--show', '--find', '--partscan', image],
147+ capture=True, decode='ignore')[0].strip()
148+ try:
149+ udev.udevadm_trigger([dev])
150+ yield dev
151+ finally:
152+ util.subp(['losetup', '--detach', dev])
153+
154+
155+PartData = namedtuple("PartData", ('number', 'offset', 'size'))
156+
157+
158+def summarize_partitions(dev):
159+ # We don't care about the kname
160+ return sorted(
161+ [PartData(*d[1:]) for d in block.sysfs_partition_data(dev)])
162+
163+
164+class StorageConfigBuilder:
165+
166+ def __init__(self):
167+ self.config = []
168+ self.cur_image = None
169+
170+ def render(self):
171+ return {
172+ 'storage': {
173+ 'config': self.config,
174+ },
175+ }
176+
177+ def add_image(self, *, path, size, **kw):
178+ action = {
179+ 'type': 'image',
180+ 'id': 'id' + str(len(self.config)),
181+ 'path': path,
182+ 'size': size,
183+ }
184+ action.update(**kw)
185+ self.cur_image = action['id']
186+ self.config.append(action)
187+
188+ def add_part(self, *, size, **kw):
189+ if self.cur_image is None:
190+ raise Exception("no current image")
191+ action = {
192+ 'type': 'partition',
193+ 'id': 'id' + str(len(self.config)),
194+ 'device': self.cur_image,
195+ 'size': size,
196+ }
197+ action.update(**kw)
198+ self.config.append(action)
199+
200+
201+class TestBlockMeta(IntegrationTestCase):
202+
203+ def run_bm(self, config):
204+ config_path = self.tmp_path('config.yaml')
205+ with open(config_path, 'w') as fp:
206+ yaml.dump(config, fp)
207+ cmd = [
208+ sys.executable, '-m', 'curtin', '--showtrace', '-vv',
209+ '-c', config_path, 'block-meta', '--testmode', 'custom',
210+ ]
211+ util.subp(cmd)
212+
213+ def _test_default_offsets(self, ptable):
214+ psize = 40 << 20
215+ img = self.tmp_path('image.img')
216+ config = StorageConfigBuilder()
217+ config.add_image(path=img, size='100M', ptable=ptable)
218+ config.add_part(size=psize, number=1)
219+ config.add_part(size=psize, number=2)
220+ self.run_bm(config.render())
221+
222+ with loop_dev(img) as dev:
223+ self.assertEqual(
224+ summarize_partitions(dev), [
225+ PartData(
226+ number=1, offset=1 << 20, size=psize),
227+ PartData(
228+ number=2, offset=(1 << 20) + psize, size=psize),
229+ ])
230+
231+ def test_default_offsets_gpt(self):
232+ self._test_default_offsets('gpt')
233+
234+ def test_default_offsets_msdos(self):
235+ self._test_default_offsets('msdos')
236+
237+ def _test_non_default_numbering(self, ptable):
238+ psize = 40 << 20
239+ img = self.tmp_path('image.img')
240+ config = StorageConfigBuilder()
241+ config.add_image(path=img, size='100M', ptable=ptable)
242+ config.add_part(size=psize, number=1)
243+ config.add_part(size=psize, number=4)
244+ self.run_bm(config.render())
245+
246+ with loop_dev(img) as dev:
247+ self.assertEqual(
248+ summarize_partitions(dev), [
249+ PartData(
250+ number=1, offset=1 << 20, size=psize),
251+ PartData(
252+ number=4, offset=(1 << 20) + psize, size=psize),
253+ ])
254+
255+ def test_non_default_numbering_gpt(self):
256+ self._test_non_default_numbering('gpt')
257+
258+ def BROKEN_test_non_default_numbering_msdos(self):
259+ self._test_non_default_numbering('msdos')
260+
261+ def test_logical(self):
262+ img = self.tmp_path('image.img')
263+ config = StorageConfigBuilder()
264+ config.add_image(path=img, size='100M', ptable='msdos')
265+ config.add_part(size='50M', number=1, flag='extended')
266+ config.add_part(size='10M', number=5, flag='logical')
267+ config.add_part(size='10M', number=6, flag='logical')
268+ self.run_bm(config.render())
269+
270+ with loop_dev(img) as dev:
271+ self.assertEqual(
272+ summarize_partitions(dev), [
273+ # extended partitions get a strange size in sysfs
274+ PartData(number=1, offset=1 << 20, size=1 << 10),
275+ PartData(number=5, offset=2 << 20, size=10 << 20),
276+ # part 5 takes us to 12 MiB offset, curtin leaves a 1 MiB
277+ # gap.
278+ PartData(number=6, offset=13 << 20, size=10 << 20),
279+ ])
280+
281+ p1kname = block.partition_kname(block.path_to_kname(dev), 1)
282+ self.assertTrue(block.is_extended_partition('/dev/' + p1kname))
283diff --git a/tests/unittests/test_commands_block_meta.py b/tests/unittests/test_commands_block_meta.py
284index 48a3217..c299df0 100644
285--- a/tests/unittests/test_commands_block_meta.py
286+++ b/tests/unittests/test_commands_block_meta.py
287@@ -201,7 +201,8 @@ class TestBlockMetaSimple(CiTestCase):
288 mock_write_image.return_value = devname
289
290 args = Namespace(target=self.target, devices=None, mode=None,
291- boot_fstype=None, fstype=None, force_mode=False)
292+ boot_fstype=None, fstype=None, force_mode=False,
293+ testmode=True)
294
295 block_meta.block_meta(args)
296
297@@ -258,7 +259,8 @@ class TestBlockMetaSimple(CiTestCase):
298 mock_write_image.return_value = devname
299
300 args = Namespace(target=None, devices=None, mode='custom',
301- boot_fstype=None, fstype=None, force_mode=False)
302+ boot_fstype=None, fstype=None, force_mode=False,
303+ testmode=True)
304
305 block_meta.block_meta(args)
306

Subscribers

People subscribed via source and target branches