From ed4abbd51929c8c847181af357e912c3ffe83d56 Mon Sep 17 00:00:00 2001
From: Julia Kreger <juliaashleykreger@gmail.com>
Date: Thu, 12 Nov 2020 07:48:18 -0800
Subject: [PATCH] Fix disk label to account for UEFI

Previously disk labels would not be populated if not explicitly
set by an API user, which lead to a dangerous possible case,
which sometimes could work, but was ultimately wrong to
setup a UEFI booting machine with a BIOS MBR partition table.

Not all systems support this, but UEFI systems are supposed to
support GPT partition tables.

We now fallback if no explicit override is set and assume GPT
if the machine is set to UEFI mode.

Change-Id: I001d8c6ee3b1d6c466c71ea5179bdbca9bdd692d
---
 ironic/drivers/modules/deploy_utils.py             |  8 +++++++-
 .../unit/drivers/modules/test_deploy_utils.py      | 14 ++++++++++++++
 ...me-gpt-for-uefi-boot-mode-8f9c77721394459a.yaml |  7 +++++++
 3 files changed, 28 insertions(+), 1 deletion(-)
 create mode 100644 releasenotes/notes/assume-gpt-for-uefi-boot-mode-8f9c77721394459a.yaml

diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py
index ae576c6081..3c5465e3ee 100644
--- a/ironic/drivers/modules/deploy_utils.py
+++ b/ironic/drivers/modules/deploy_utils.py
@@ -356,7 +356,13 @@ def get_disk_label(node):
     :returns: the disk label or None if no disk label was specified.
     """
     capabilities = utils.parse_instance_info_capabilities(node)
-    return capabilities.get('disk_label')
+    label = capabilities.get('disk_label')
+    # NOTE(TheJulia): If the node is UEFI based, we should likely just default
+    # the table type to gpt as otherwise we rely upon the user to supply the
+    # right information, and for UEFI mode bios may work, but is wrong.
+    if label is None and boot_mode_utils.get_boot_mode(node) == 'uefi':
+        label = 'gpt'
+    return label
 
 
 def get_pxe_boot_file(node):
diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py
index f0b0e96216..41cb502ee9 100644
--- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py
+++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py
@@ -1039,6 +1039,20 @@ class ParseInstanceInfoCapabilitiesTestCase(tests_base.TestCase):
         result = utils.get_disk_label(self.node)
         self.assertEqual('gpt', result)
 
+    def test_get_disk_label_nothing_set(self):
+        inst_info = {'capabilities': {'cat': 'meows'}}
+        self.node.instance_info = inst_info
+        result = utils.get_disk_label(self.node)
+        self.assertIsNone(result)
+
+    def test_get_disk_label_uefi_mode(self):
+        inst_info = {'capabilities': {'cat': 'meows'}}
+        properties = {'capabilities': 'boot_mode:uefi'}
+        self.node.instance_info = inst_info
+        self.node.properties = properties
+        result = utils.get_disk_label(self.node)
+        self.assertEqual('gpt', result)
+
 
 class TrySetBootDeviceTestCase(db_base.DbTestCase):
 
diff --git a/releasenotes/notes/assume-gpt-for-uefi-boot-mode-8f9c77721394459a.yaml b/releasenotes/notes/assume-gpt-for-uefi-boot-mode-8f9c77721394459a.yaml
new file mode 100644
index 0000000000..4e29eca90f
--- /dev/null
+++ b/releasenotes/notes/assume-gpt-for-uefi-boot-mode-8f9c77721394459a.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Fixes the logic which determines the partition table type to utilize with
+    partition images account for the boot mode of the machine. If no value is
+    set by the API user, Ironic now correctly defaults to GPT if the node
+    has been set in UEFI mode.