From 9a95e1451236064459f34ece1ef2f4edc6635fa7 Mon Sep 17 00:00:00 2001
From: Uros Orozel <uros.orozel@gmail.com>
Date: Fri, 29 Mar 2019 04:00:52 +0000
Subject: [PATCH] Include partiton name and flags from parted output

Currently list_partitions method fails if partition
name is defined on any GPT partition, it also only
extract one partition flag.

Updated _PARTED_PRINT_RE is now set to capture
partition name/description and partition flags.

Change-Id: I40410622829453a485aa33047848922ab134d725
Task: 30250
Story: 2005322
---
 ironic_lib/disk_utils.py                      |  8 ++--
 ironic_lib/tests/test_disk_utils.py           | 44 ++++++++++++++++++-
 ...tend-list-partitions-b71f81c77f6ecfdb.yaml |  6 +++
 3 files changed, 53 insertions(+), 5 deletions(-)
 create mode 100644 releasenotes/notes/extend-list-partitions-b71f81c77f6ecfdb.yaml

diff --git a/ironic_lib/disk_utils.py b/ironic_lib/disk_utils.py
index d53df175..f4273c7f 100644
--- a/ironic_lib/disk_utils.py
+++ b/ironic_lib/disk_utils.py
@@ -67,7 +67,7 @@ CONF.register_opts(opts, group='disk_utils')
 LOG = logging.getLogger(__name__)
 
 _PARTED_PRINT_RE = re.compile(r"^(\d+):([\d\.]+)MiB:"
-                              "([\d\.]+)MiB:([\d\.]+)MiB:(\w*)::(\w*)")
+                              "([\d\.]+)MiB:([\d\.]+)MiB:(\w*):(.*):(.*);")
 
 CONFIGDRIVE_LABEL = "config-2"
 MAX_CONFIG_DRIVE_SIZE_MB = 64
@@ -84,7 +84,8 @@ def list_partitions(device):
 
     :param device: The device path.
     :returns: list of dictionaries (one per partition) with keys:
-              number, start, end, size (in MiB), filesystem, flags
+              number, start, end, size (in MiB), filesystem, partition_name,
+              flags
     """
     output = utils.execute(
         'parted', '-s', '-m', device, 'unit', 'MiB', 'print',
@@ -93,7 +94,8 @@ def list_partitions(device):
         output = output.decode("utf-8")
     lines = [line for line in output.split('\n') if line.strip()][2:]
     # Example of line: 1:1.00MiB:501MiB:500MiB:ext4::boot
-    fields = ('number', 'start', 'end', 'size', 'filesystem', 'flags')
+    fields = ('number', 'start', 'end', 'size', 'filesystem', 'partition_name',
+              'flags')
     result = []
     for line in lines:
         match = _PARTED_PRINT_RE.match(line)
diff --git a/ironic_lib/tests/test_disk_utils.py b/ironic_lib/tests/test_disk_utils.py
index e138601e..c531d300 100644
--- a/ironic_lib/tests/test_disk_utils.py
+++ b/ironic_lib/tests/test_disk_utils.py
@@ -47,9 +47,9 @@ BYT;
 """
         expected = [
             {'number': 1, 'start': 1, 'end': 501, 'size': 500,
-             'filesystem': 'ext4', 'flags': 'boot'},
+             'filesystem': 'ext4', 'partition_name': '', 'flags': 'boot'},
             {'number': 2, 'start': 501, 'end': 476940, 'size': 476439,
-             'filesystem': '', 'flags': ''},
+             'filesystem': '', 'partition_name': '', 'flags': ''},
         ]
         execute_mock.return_value = (output, '')
         result = disk_utils.list_partitions('/dev/fake')
@@ -70,6 +70,46 @@ BYT;
         self.assertEqual(1, log_mock.call_count)
 
 
+@mock.patch.object(utils, 'execute', autospec=True)
+class ListPartitionsGPTTestCase(base.IronicLibTestCase):
+
+    def test_correct(self, execute_mock):
+        output = """
+BYT;
+/dev/vda:40960MiB:virtblk:512:512:gpt:Virtio Block Device:;
+2:1.00MiB:2.00MiB:1.00MiB::Bios partition:bios_grub;
+1:4.00MiB:5407MiB:5403MiB:ext4:Root partition:;
+3:5407MiB:5507MiB:100MiB:fat16:Boot partition:boot, esp;
+"""
+        expected = [
+            {'end': 2, 'number': 2, 'start': 1, 'flags': 'bios_grub',
+             'filesystem': '', 'partition_name': 'Bios partition', 'size': 1},
+            {'end': 5407, 'number': 1, 'start': 4, 'flags': '',
+             'filesystem': 'ext4', 'partition_name': 'Root partition',
+             'size': 5403},
+            {'end': 5507, 'number': 3, 'start': 5407,
+             'flags': 'boot, esp', 'filesystem': 'fat16',
+             'partition_name': 'Boot partition', 'size': 100},
+        ]
+        execute_mock.return_value = (output, '')
+        result = disk_utils.list_partitions('/dev/fake')
+        self.assertEqual(expected, result)
+        execute_mock.assert_called_once_with(
+            'parted', '-s', '-m', '/dev/fake', 'unit', 'MiB', 'print',
+            use_standard_locale=True, run_as_root=True)
+
+    @mock.patch.object(disk_utils.LOG, 'warning', autospec=True)
+    def test_incorrect(self, log_mock, execute_mock):
+        output = """
+BYT;
+/dev/vda:40960MiB:virtblk:512:512:gpt:Virtio Block Device:;
+2:XX1.00MiB:---:1.00MiB::primary:bios_grub;
+"""
+        execute_mock.return_value = (output, '')
+        self.assertEqual([], disk_utils.list_partitions('/dev/fake'))
+        self.assertEqual(1, log_mock.call_count)
+
+
 @mock.patch.object(disk_partitioner.DiskPartitioner, 'commit', lambda _: None)
 class WorkOnDiskTestCase(base.IronicLibTestCase):
 
diff --git a/releasenotes/notes/extend-list-partitions-b71f81c77f6ecfdb.yaml b/releasenotes/notes/extend-list-partitions-b71f81c77f6ecfdb.yaml
new file mode 100644
index 00000000..d892a48b
--- /dev/null
+++ b/releasenotes/notes/extend-list-partitions-b71f81c77f6ecfdb.yaml
@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Fixes an issues when parsing GPT partitions with names or multiple flags.
+    See `story 2005322 <https://storyboard.openstack.org/#!/story/2005322>`_
+    for details.