From f06240f7dd2b0314068f3f9dc6a82667d658bc28 Mon Sep 17 00:00:00 2001
From: Dmitry Tantsur <dtantsur@protonmail.com>
Date: Fri, 26 Jul 2019 16:06:50 +0200
Subject: [PATCH] Allow configuring global deploy and rescue kernel/ramdisk

The devstack plugin was updated to configure basic ops before
ironic starts, so that we can put links to deploy images
in the ironic.conf.

Change-Id: I305fc3712b1ac0cf2fe64569729e236c7b614bb4
Story: #2006175
Task: #35699
---
 devstack/lib/ironic                           | 15 +++++------
 devstack/plugin.sh                            |  3 ++-
 ironic/common/pxe_utils.py                    |  7 ++++-
 ironic/conf/conductor.py                      | 16 +++++++++++
 ironic/tests/unit/common/test_pxe_utils.py    | 27 +++++++++++++++++++
 .../conf-deploy-image-5adb6c1963b149ae.yaml   |  6 +++++
 6 files changed, 63 insertions(+), 11 deletions(-)
 create mode 100644 releasenotes/notes/conf-deploy-image-5adb6c1963b149ae.yaml

diff --git a/devstack/lib/ironic b/devstack/lib/ironic
index 37a29683d7..24acaa7eab 100644
--- a/devstack/lib/ironic
+++ b/devstack/lib/ironic
@@ -1999,12 +1999,6 @@ function enroll_nodes {
 
     local interface_info
 
-    common_node_options="\
-        --driver-info deploy_kernel=$IRONIC_DEPLOY_KERNEL_ID \
-        --driver-info deploy_ramdisk=$IRONIC_DEPLOY_RAMDISK_ID \
-        --driver-info rescue_kernel=$IRONIC_DEPLOY_KERNEL_ID \
-        --driver-info rescue_ramdisk=$IRONIC_DEPLOY_RAMDISK_ID"
-
     if [[ "$IRONIC_IS_HARDWARE" == "False" ]]; then
         local ironic_node_cpu=$IRONIC_VM_SPECS_CPU
         local ironic_node_ram=$IRONIC_VM_SPECS_RAM
@@ -2031,7 +2025,6 @@ function enroll_nodes {
                 --driver-info redfish_username=admin \
                 --driver-info redfish_password=password"
         fi
-        node_options+=" $common_node_options"
 
     else
         local ironic_node_cpu=$IRONIC_HW_NODE_CPU
@@ -2112,7 +2105,7 @@ function enroll_nodes {
             bmc_username=$(echo $hardware_info |awk '{print $3}')
             local bmc_passwd
             bmc_passwd=$(echo $hardware_info |awk '{print $4}')
-            local node_options="$common_node_options"
+            local node_options=""
 
             if is_deployed_by_ipmi; then
                 node_options+=" --driver-info ipmi_address=$bmc_address \
@@ -2591,6 +2584,11 @@ function upload_baremetal_ironic_deploy {
         IRONIC_DEPLOY_KERNEL_ID=$(openstack image show $ironic_deploy_kernel_name -f value -c id)
         IRONIC_DEPLOY_RAMDISK_ID=$(openstack image show $ironic_deploy_ramdisk_name -f value -c id)
     fi
+
+    iniset $IRONIC_CONF_FILE conductor deploy_kernel $IRONIC_DEPLOY_KERNEL_ID
+    iniset $IRONIC_CONF_FILE conductor deploy_ramdisk $IRONIC_DEPLOY_RAMDISK_ID
+    iniset $IRONIC_CONF_FILE conductor rescue_kernel $IRONIC_DEPLOY_KERNEL_ID
+    iniset $IRONIC_CONF_FILE conductor rescue_ramdisk $IRONIC_DEPLOY_RAMDISK_ID
 }
 
 function prepare_baremetal_basic_ops {
@@ -2611,7 +2609,6 @@ function prepare_baremetal_basic_ops {
     upload_baremetal_ironic_deploy
     configure_tftpd
     configure_iptables
-    enroll_nodes
 }
 
 function cleanup_baremetal_basic_ops {
diff --git a/devstack/plugin.sh b/devstack/plugin.sh
index 54d50ce251..4e42242d42 100644
--- a/devstack/plugin.sh
+++ b/devstack/plugin.sh
@@ -65,9 +65,10 @@ if is_service_enabled ir-api ir-cond; then
             fi
 
             # Start the ironic API and ironic taskmgr components
+            prepare_baremetal_basic_ops
             echo_summary "Starting Ironic"
             start_ironic
-            prepare_baremetal_basic_ops
+            enroll_nodes
 
         elif [[ "$2" == "test-config" ]]; then
         # stack/test-config - Called at the end of devstack used to configure tempest
diff --git a/ironic/common/pxe_utils.py b/ironic/common/pxe_utils.py
index cc163a6a62..53dfbc8ad9 100644
--- a/ironic/common/pxe_utils.py
+++ b/ironic/common/pxe_utils.py
@@ -570,8 +570,13 @@ def parse_driver_info(node, mode='deploy'):
     params_to_check = KERNEL_RAMDISK_LABELS[mode]
 
     d_info = {k: info.get(k) for k in params_to_check}
+    if not any(d_info.values()):
+        # NOTE(dtantsur): avoid situation when e.g. deploy_kernel comes from
+        # driver_info but deploy_ramdisk comes from configuration, since it's
+        # a sign of a potential operator's mistake.
+        d_info = {k: getattr(CONF.conductor, k) for k in params_to_check}
     error_msg = _("Cannot validate PXE bootloader. Some parameters were"
-                  " missing in node's driver_info")
+                  " missing in node's driver_info and configuration")
     deploy_utils.check_for_missing_params(d_info, error_msg)
     return d_info
 
diff --git a/ironic/conf/conductor.py b/ironic/conf/conductor.py
index 9f142a3b70..03c235c769 100644
--- a/ironic/conf/conductor.py
+++ b/ironic/conf/conductor.py
@@ -224,6 +224,22 @@ opts = [
     cfg.BoolOpt('enable_mdns', default=False,
                 help=_('Whether to enable publishing the baremetal API '
                        'endpoint via multicast DNS.')),
+    cfg.StrOpt('deploy_kernel',
+               mutable=True,
+               help=_('Glance ID, http:// or file:// URL of the kernel of '
+                      'the default deploy image.')),
+    cfg.StrOpt('deploy_ramdisk',
+               mutable=True,
+               help=_('Glance ID, http:// or file:// URL of the initramfs of '
+                      'the default deploy image.')),
+    cfg.StrOpt('rescue_kernel',
+               mutable=True,
+               help=_('Glance ID, http:// or file:// URL of the kernel of '
+                      'the default rescue image.')),
+    cfg.StrOpt('rescue_ramdisk',
+               mutable=True,
+               help=_('Glance ID, http:// or file:// URL of the initramfs of '
+                      'the default rescue image.')),
 ]
 
 
diff --git a/ironic/tests/unit/common/test_pxe_utils.py b/ironic/tests/unit/common/test_pxe_utils.py
index f7f469558f..1085a978b3 100644
--- a/ironic/tests/unit/common/test_pxe_utils.py
+++ b/ironic/tests/unit/common/test_pxe_utils.py
@@ -1030,6 +1030,33 @@ class PXEInterfacesTestCase(db_base.DbTestCase):
     def test_parse_driver_info_rescue(self):
         self._test_parse_driver_info(mode='rescue')
 
+    def _test_parse_driver_info_from_conf(self, mode='deploy'):
+        del self.node.driver_info['%s_kernel' % mode]
+        del self.node.driver_info['%s_ramdisk' % mode]
+        exp_info = {'%s_ramdisk' % mode: 'glance://%s_ramdisk_uuid' % mode,
+                    '%s_kernel' % mode: 'glance://%s_kernel_uuid' % mode}
+        self.config(group='conductor', **exp_info)
+        image_info = pxe_utils.parse_driver_info(self.node, mode=mode)
+        self.assertEqual(exp_info, image_info)
+
+    def test_parse_driver_info_from_conf_deploy(self):
+        self._test_parse_driver_info_from_conf()
+
+    def test_parse_driver_info_from_conf_rescue(self):
+        self._test_parse_driver_info_from_conf(mode='rescue')
+
+    def test_parse_driver_info_mixed_source_deploy(self):
+        self.config(deploy_kernel='file:///image',
+                    deploy_ramdisk='file:///image',
+                    group='conductor')
+        self._test_parse_driver_info_missing_ramdisk()
+
+    def test_parse_driver_info_mixed_source_rescue(self):
+        self.config(rescue_kernel='file:///image',
+                    rescue_ramdisk='file:///image',
+                    group='conductor')
+        self._test_parse_driver_info_missing_ramdisk(mode='rescue')
+
     def test__get_deploy_image_info(self):
         expected_info = {'deploy_ramdisk':
                          (DRV_INFO_DICT['deploy_ramdisk'],
diff --git a/releasenotes/notes/conf-deploy-image-5adb6c1963b149ae.yaml b/releasenotes/notes/conf-deploy-image-5adb6c1963b149ae.yaml
new file mode 100644
index 0000000000..bae8b7f90d
--- /dev/null
+++ b/releasenotes/notes/conf-deploy-image-5adb6c1963b149ae.yaml
@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    The deploy and/or rescue kernel and ramdisk can now be configured via
+    the new configuration options ``deploy_kernel``, ``deploy_ramdisk``,
+    ``rescue_kernel`` and ``rescue_ramdisk`` respectively.