From f13f1ed654042daa405d8e92f03ddfca40d18d08 Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Tue, 13 Dec 2016 11:08:49 +0000 Subject: [PATCH] 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 | 2 +- ironic/drivers/modules/pxe.py | 8 ++++- ironic/tests/unit/drivers/modules/test_pxe.py | 31 +++++++++++++++---- .../conf-debug-ipa-1d75e2283ca83395.yaml | 6 ++++ 4 files changed, 39 insertions(+), 8 deletions(-) create mode 100644 releasenotes/notes/conf-debug-ipa-1d75e2283ca83395.yaml diff --git a/devstack/lib/ironic b/devstack/lib/ironic index 2bba3e6c25..f80430567c 100644 --- a/devstack/lib/ironic +++ b/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" diff --git a/ironic/drivers/modules/pxe.py b/ironic/drivers/modules/pxe.py index 659de2d3ab..59f5cb582e 100644 --- a/ironic/drivers/modules/pxe.py +++ b/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 }) diff --git a/ironic/tests/unit/drivers/modules/test_pxe.py b/ironic/tests/unit/drivers/modules/test_pxe.py index 2b7e4f42e5..0f92b52c60 100644 --- a/ironic/tests/unit/drivers/modules/test_pxe.py +++ b/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 diff --git a/releasenotes/notes/conf-debug-ipa-1d75e2283ca83395.yaml b/releasenotes/notes/conf-debug-ipa-1d75e2283ca83395.yaml new file mode 100644 index 0000000000..b34da3bd42 --- /dev/null +++ b/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.