diff --git a/etc/ironic/rootwrap.d/ironic-utils.filters b/etc/ironic/rootwrap.d/ironic-utils.filters index 0ee273b1e4..513900ee98 100644 --- a/etc/ironic/rootwrap.d/ironic-utils.filters +++ b/etc/ironic/rootwrap.d/ironic-utils.filters @@ -4,10 +4,12 @@ [Filters] # ironic/drivers/modules/deploy_utils.py iscsiadm: CommandFilter, iscsiadm, root -parted: CommandFilter, parted, root dd: CommandFilter, dd, root blkid: CommandFilter, blkid, root # ironic/common/utils.py mkswap: CommandFilter, mkswap, root mkfs: CommandFilter, mkfs, root + +# ironic/common/disk_partitioner.py +parted: CommandFilter, parted, root diff --git a/ironic/common/disk_partitioner.py b/ironic/common/disk_partitioner.py index 04352ff4ea..61d061f71a 100644 --- a/ironic/common/disk_partitioner.py +++ b/ironic/common/disk_partitioner.py @@ -13,6 +13,8 @@ # License for the specific language governing permissions and limitations # under the License. +import time + from ironic.common import utils @@ -90,3 +92,7 @@ class DiskPartitioner(object): start = end self._exec(*cmd_args) + # TODO(lucasagomes): Do not sleep, use another mechanism to avoid + # the "device is busy" problem. lsof, fuser... + # avoid "device is busy" + time.sleep(3) diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index bad6458477..4703dbc67e 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -20,6 +20,7 @@ import socket import stat import time +from ironic.common import disk_partitioner from ironic.common import exception from ironic.common import utils from ironic.openstack.common import excutils @@ -80,27 +81,15 @@ def delete_iscsi(portal_address, portal_port, target_iqn): def make_partitions(dev, root_mb, swap_mb, ephemeral_mb): """Create partitions for root and swap on a disk device.""" - cmd = [] - - def add_partition(start, size, fs_type=''): - end = start + size - cmd.extend(['mkpart', 'primary', fs_type, str(start), str(end)]) - return end - - offset = 1 + dp = disk_partitioner.DiskPartitioner(dev) if ephemeral_mb: - offset = add_partition(offset, ephemeral_mb) - offset = add_partition(offset, swap_mb, fs_type='linux-swap') - offset = add_partition(offset, root_mb) + dp.add_partition(ephemeral_mb) + dp.add_partition(swap_mb, fs_type='linux-swap') + dp.add_partition(root_mb) else: - offset = add_partition(offset, root_mb) - offset = add_partition(offset, swap_mb, fs_type='linux-swap') - - utils.execute('parted', '-a', 'optimal', '-s', dev, '--', 'mklabel', - 'msdos', 'unit', 'MiB', *cmd, run_as_root=True, attempts=3, - check_exit_code=[0]) - # avoid "device is busy" - time.sleep(3) + dp.add_partition(root_mb) + dp.add_partition(swap_mb, fs_type='linux-swap') + dp.commit() def is_block_device(dev): diff --git a/ironic/tests/drivers/test_deploy_utils.py b/ironic/tests/drivers/test_deploy_utils.py index d81ba8e29a..0079df04a8 100644 --- a/ironic/tests/drivers/test_deploy_utils.py +++ b/ironic/tests/drivers/test_deploy_utils.py @@ -14,6 +14,8 @@ # License for the specific language governing permissions and limitations # under the License. +import time + import fixtures import mock import os @@ -403,6 +405,7 @@ class WorkOnDiskTestCase(tests_base.TestCase): self.swap_mb, ephemeral_mb) +@mock.patch.object(time, 'sleep', lambda _: None) @mock.patch.object(common_utils, 'execute') class MakePartitionsTestCase(tests_base.TestCase): @@ -413,7 +416,7 @@ class MakePartitionsTestCase(tests_base.TestCase): self.swap_mb = 512 self.ephemeral_mb = 0 self.parted_static_cmd = ['parted', '-a', 'optimal', '-s', self.dev, - '--', 'mklabel', 'msdos', 'unit', 'MiB'] + '--', 'unit', 'MiB', 'mklabel', 'msdos'] def test_make_partitions(self, mock_exc): expected_mkpart = ['mkpart', 'primary', '', '1', '1025', @@ -421,8 +424,7 @@ class MakePartitionsTestCase(tests_base.TestCase): cmd = self.parted_static_cmd + expected_mkpart utils.make_partitions(self.dev, self.root_mb, self.swap_mb, self.ephemeral_mb) - mock_exc.assert_called_once_with(*cmd, - run_as_root=True, attempts=3, + mock_exc.assert_called_once_with(*cmd, run_as_root=True, check_exit_code=[0]) def test_make_partitions_with_ephemeral(self, mock_exc): @@ -433,6 +435,5 @@ class MakePartitionsTestCase(tests_base.TestCase): cmd = self.parted_static_cmd + expected_mkpart utils.make_partitions(self.dev, self.root_mb, self.swap_mb, self.ephemeral_mb) - mock_exc.assert_called_once_with(*cmd, - run_as_root=True, attempts=3, + mock_exc.assert_called_once_with(*cmd, run_as_root=True, check_exit_code=[0]) diff --git a/ironic/tests/test_disk_partitioner.py b/ironic/tests/test_disk_partitioner.py index ea9f998e88..7c4df007f9 100644 --- a/ironic/tests/test_disk_partitioner.py +++ b/ironic/tests/test_disk_partitioner.py @@ -13,6 +13,8 @@ # License for the specific language governing permissions and limitations # under the License. +import time + import mock from testtools.matchers import HasLength @@ -21,14 +23,10 @@ from ironic.common import utils from ironic.tests import base +@mock.patch.object(time, 'sleep', lambda _: None) +@mock.patch.object(utils, 'execute', lambda _: None) class DiskPartitionerTestCase(base.TestCase): - def setUp(self): - super(DiskPartitionerTestCase, self).setUp() - self.mock_exc = mock.patch.object(utils, 'execute') - self.mock_exc.start() - self.addCleanup(self.mock_exc.stop) - def test_add_partition(self): dp = disk_partitioner.DiskPartitioner('/dev/fake') dp.add_partition(1024)