Use DiskPartitioner

Use the new DiskPartitioner class to create the partition layout.

This commit also moves the sleep() workaround to avoid the "device is
busy" problem into the DiskPartitioner wrapper.

Change-Id: I28bb17c7ce2262d3088f58ab4421145e8d9baa1f
Closes-Bug: #1297925

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

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)

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 = []
+    dp = disk_partitioner.DiskPartitioner(dev)
-
-    def add_partition(start, size, fs_type=''):
-        end = start + size
-        cmd.extend(['mkpart', 'primary', fs_type, str(start), str(end)])
-        return end
-
-    offset = 1
     if ephemeral_mb:
-        offset = add_partition(offset, ephemeral_mb)
+        dp.add_partition(ephemeral_mb)
-        offset = add_partition(offset, swap_mb, fs_type='linux-swap')
+        dp.add_partition(swap_mb, fs_type='linux-swap')
-        offset = add_partition(offset, root_mb)
+        dp.add_partition(root_mb)
     else:
-        offset = add_partition(offset, root_mb)
+        dp.add_partition(root_mb)
-        offset = add_partition(offset, swap_mb, fs_type='linux-swap')
+        dp.add_partition(swap_mb, fs_type='linux-swap')
-
+    dp.commit()
-    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)
 
 
 def is_block_device(dev):

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,
+        mock_exc.assert_called_once_with(*cmd, run_as_root=True,
-                                         run_as_root=True, attempts=3,
                                          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,
+        mock_exc.assert_called_once_with(*cmd, run_as_root=True,
-                                         run_as_root=True, attempts=3,
                                          check_exit_code=[0])

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)