Make CONF.debug also reflect on IPA

Prior to this patch, setting the CONF.debug to True in the ironic.conf
doesn't enable the debug logs in IPA. The user had to also append
"ipa-debug=1" to the CONF.pxe.pxe_append_params configuration option.

This patch is changing this behavior by making the CONF.debug option
reflect on the IPA logs as well. As I understand, IPA is just an
extension of the Ironic code that runs in-band on the node allowing
Ironic to access the local hardware. This way, in order to enable debug
as a whole the user only needs to set one configuration option.

The code in this patch checks if CONF.debug is set to True and also
check if "ipa-debug" is not explicit set in CONF.pxe.pxe_append_params
configuration - to not overwrite the user configuration - and only then
will append "ipa-debug=1" to it.

The "ipa-debug=1" kernel parameter was also removed from the devstack
lib because CONF.debug is always set to True there.

Change-Id: I675fb11248d3789d45cf5905b7c15368f026b345
Closes-Bug: #1649561

devstack/lib/ironic

@@ -832,7 +832,7 @@ function configure_ironic_conductor {
     fi
 
     local pxe_params="nofb nomodeset vga=normal console=${IRONIC_TTY_DEV}"
-    pxe_params+=" systemd.journald.forward_to_console=yes ipa-debug=1"
+    pxe_params+=" systemd.journald.forward_to_console=yes"
 
     pxe_params+=" $IRONIC_EXTRA_PXE_PARAMS"
 

ironic/drivers/modules/pxe.py

@@ -200,8 +200,14 @@ def _build_pxe_config_options(task, pxe_info):
     pxe_options.setdefault('aki_path', 'no_kernel')
     pxe_options.setdefault('ari_path', 'no_ramdisk')
 
+    # Enable debug in IPA according to CONF.debug if it was not
+    # specified yet
+    pxe_append_params = CONF.pxe.pxe_append_params
+    if CONF.debug and 'ipa-debug' not in pxe_append_params:
+        pxe_append_params += ' ipa-debug=1'
+
     pxe_options.update({
-        'pxe_append_params': CONF.pxe.pxe_append_params,
+        'pxe_append_params': pxe_append_params,
         'tftp_server': CONF.pxe.tftp_server,
         'ipxe_timeout': CONF.pxe.ipxe_timeout * 1000
     })

ironic/tests/unit/drivers/modules/test_pxe.py

@@ -164,7 +164,9 @@ class PXEPrivateMethodsTestCase(db_base.DbTestCase):
 
     @mock.patch('ironic.common.utils.render_template', autospec=True)
     def _test_build_pxe_config_options_pxe(self, render_mock,
-                                           whle_dsk_img=False):
+                                           whle_dsk_img=False,
+                                           debug=False):
+        self.config(debug=debug)
         self.config(pxe_append_params='test_param', group='pxe')
         # NOTE: right '/' should be removed from url string
         self.config(api_url='http://192.168.122.184:6385', group='conductor')
@@ -209,10 +211,14 @@ class PXEPrivateMethodsTestCase(db_base.DbTestCase):
                                          'ramdisk'))
             })
 
+        expected_pxe_params = 'test_param'
+        if debug:
+            expected_pxe_params += ' ipa-debug=1'
+
         expected_options = {
             'ari_path': ramdisk,
             'deployment_ari_path': deploy_ramdisk,
-            'pxe_append_params': 'test_param',
+            'pxe_append_params': expected_pxe_params,
             'aki_path': kernel,
             'deployment_aki_path': deploy_kernel,
             'tftp_server': tftp_server,
@@ -227,6 +233,9 @@ class PXEPrivateMethodsTestCase(db_base.DbTestCase):
     def test__build_pxe_config_options_pxe(self):
         self._test_build_pxe_config_options_pxe(whle_dsk_img=True)
 
+    def test__build_pxe_config_options_pxe_ipa_debug(self):
+        self._test_build_pxe_config_options_pxe(debug=True)
+
     def test__build_pxe_config_options_pxe_local_boot(self):
         del self.node.driver_internal_info['is_whole_disk_image']
         i_info = self.node.instance_info
@@ -243,8 +252,9 @@ class PXEPrivateMethodsTestCase(db_base.DbTestCase):
     def test__build_pxe_config_options_pxe_no_kernel_no_ramdisk(self):
         del self.node.driver_internal_info['is_whole_disk_image']
         self.node.save()
+        pxe_params = 'my-pxe-append-params ipa-debug=0'
         self.config(group='pxe', tftp_server='my-tftp-server')
-        self.config(group='pxe', pxe_append_params='my-pxe-append-params')
+        self.config(group='pxe', pxe_append_params=pxe_params)
         self.config(group='pxe', tftp_root='/tftp-path/')
         image_info = {
             'deploy_kernel': ('deploy_kernel',
@@ -261,7 +271,7 @@ class PXEPrivateMethodsTestCase(db_base.DbTestCase):
         expected_options = {
             'deployment_aki_path': 'path-to-deploy_kernel',
             'deployment_ari_path': 'path-to-deploy_ramdisk',
-            'pxe_append_params': 'my-pxe-append-params',
+            'pxe_append_params': pxe_params,
             'tftp_server': 'my-tftp-server',
             'aki_path': 'no_kernel',
             'ari_path': 'no_ramdisk',
@@ -274,7 +284,9 @@ class PXEPrivateMethodsTestCase(db_base.DbTestCase):
     def _test_build_pxe_config_options_ipxe(self, render_mock, glance_mock,
                                             whle_dsk_img=False,
                                             ipxe_timeout=0,
-                                            ipxe_use_swift=False):
+                                            ipxe_use_swift=False,
+                                            debug=False):
+        self.config(debug=debug)
         self.config(pxe_append_params='test_param', group='pxe')
         # NOTE: right '/' should be removed from url string
         self.config(api_url='http://192.168.122.184:6385', group='conductor')
@@ -344,10 +356,14 @@ class PXEPrivateMethodsTestCase(db_base.DbTestCase):
 
         ipxe_timeout_in_ms = ipxe_timeout * 1000
 
+        expected_pxe_params = 'test_param'
+        if debug:
+            expected_pxe_params += ' ipa-debug=1'
+
         expected_options = {
             'ari_path': ramdisk,
             'deployment_ari_path': deploy_ramdisk,
-            'pxe_append_params': 'test_param',
+            'pxe_append_params': expected_pxe_params,
             'aki_path': kernel,
             'deployment_aki_path': deploy_kernel,
             'tftp_server': tftp_server,
@@ -362,6 +378,9 @@ class PXEPrivateMethodsTestCase(db_base.DbTestCase):
     def test__build_pxe_config_options_ipxe(self):
         self._test_build_pxe_config_options_ipxe(whle_dsk_img=True)
 
+    def test__build_pxe_config_options_ipxe_ipa_debug(self):
+        self._test_build_pxe_config_options_ipxe(debug=True)
+
     def test__build_pxe_config_options_ipxe_local_boot(self):
         del self.node.driver_internal_info['is_whole_disk_image']
         i_info = self.node.instance_info

releasenotes/notes/conf-debug-ipa-1d75e2283ca83395.yaml

@@ -0,0 +1,6 @@
+---
+upgrade:
+  - The ``[DEFAULT]/debug`` configuration option now also enables debug
+    logs for the ``ironic-python-agent`` ramdisk. If the ``ipa-debug``
+    kernel option is already present in the ``[pxe]/pxe_append_params``
+    configuration option Ironic will not overwrite it.