From 4be3fcef26e63aafcefa49c8e8611bb405d0e372 Mon Sep 17 00:00:00 2001
From: Pradip Kadam <pradip.kadam001@gmail.com>
Date: Thu, 19 Sep 2019 09:39:40 -0400
Subject: [PATCH] DRAC: Fix a bug for clear_job_queue clean step with non-BIOS
pending job
If we create pending non-bios config job(E.g create/delete virtual disk)
with "At next reboot" option and execute "clear_job_queue" clean step,
then that job gets deleted after job execution instead of getting
deleted before job execution.
Change-Id: Ibeae18798a25e33fa070f1120ce0cc0981f66850
Story: #2006580
Task: #36695
---
ironic/drivers/modules/drac/management.py | 29 ++--
.../drivers/modules/drac/test_management.py | 139 ++++++++++++++++--
...n_bios_job_execution-4b22e168ac915f4f.yaml | 10 ++
3 files changed, 156 insertions(+), 22 deletions(-)
create mode 100644 releasenotes/notes/fix_pending_non_bios_job_execution-4b22e168ac915f4f.yaml
diff --git a/ironic/drivers/modules/drac/management.py b/ironic/drivers/modules/drac/management.py
index 37f9ca846c..68b09ecabb 100644
--- a/ironic/drivers/modules/drac/management.py
+++ b/ironic/drivers/modules/drac/management.py
@@ -81,8 +81,8 @@ _NON_PERSISTENT_BOOT_MODE = 'OneTime'
# Clear job id's constant
_CLEAR_JOB_IDS = 'JID_CLEARALL_FORCE'
-# BIOS pending job constant
-_BIOS_JOB_NAME = 'BIOS.Setup.1-1'
+# Clean steps constant
+_CLEAR_JOBS_CLEAN_STEPS = ['clear_job_queue', 'known_good_state']
def _get_boot_device(node, drac_boot_devices=None):
@@ -190,19 +190,24 @@ def set_boot_device(node, device, persistent=False):
Default: False.
:raises: DracOperationError on an error from python-dracclient.
"""
-
client = drac_common.get_drac_client(node)
- # If pending BIOS job found in job queue, we need to clear that job
- # before executing cleaning step of management interface.
+ # If pending BIOS job or pending non-BIOS job found in job queue,
+ # we need to clear that jobs before executing clear_job_queue or
+ # known_good_state clean step of management interface.
# Otherwise, pending BIOS config job can cause creating new config jobs
- # to fail.
- unfinished_jobs = drac_job.list_unfinished_jobs(node)
- if unfinished_jobs:
- unfinished_bios_jobs = [job.id for job in unfinished_jobs if
- _BIOS_JOB_NAME in job.name]
- if unfinished_bios_jobs:
- client.delete_jobs(job_ids=unfinished_bios_jobs)
+ # to fail and pending non-BIOS job can execute on reboot the node.
+ validate_job_queue = True
+ if node.driver_internal_info.get("clean_steps"):
+ if node.driver_internal_info.get("clean_steps")[0].get(
+ 'step') in _CLEAR_JOBS_CLEAN_STEPS:
+ unfinished_jobs = drac_job.list_unfinished_jobs(node)
+ if unfinished_jobs:
+ validate_job_queue = False
+ client.delete_jobs(job_ids=[job.id for job in unfinished_jobs])
+
+ if validate_job_queue:
+ drac_job.validate_job_queue(node)
try:
drac_boot_devices = client.list_boot_devices()
diff --git a/ironic/tests/unit/drivers/modules/drac/test_management.py b/ironic/tests/unit/drivers/modules/drac/test_management.py
index c16662e3c4..74f2ea3728 100644
--- a/ironic/tests/unit/drivers/modules/drac/test_management.py
+++ b/ironic/tests/unit/drivers/modules/drac/test_management.py
@@ -250,7 +250,10 @@ class DracManagementInternalMethodsTestCase(test_utils.BaseDracTest):
autospec=True)
@mock.patch.object(drac_job, 'list_unfinished_jobs', spec_set=True,
autospec=True)
- def test_set_boot_device(self, mock_list_unfinished_jobs,
+ @mock.patch.object(drac_job, 'validate_job_queue', spec_set=True,
+ autospec=True)
+ def test_set_boot_device(self, mock_validate_job_queue,
+ mock_list_unfinished_jobs,
mock__get_boot_device,
mock__get_next_persistent_boot_mode,
mock_get_drac_client):
@@ -267,11 +270,14 @@ class DracManagementInternalMethodsTestCase(test_utils.BaseDracTest):
'persistent': True}
mock__get_boot_device.return_value = boot_device
mock__get_next_persistent_boot_mode.return_value = 'IPL'
+ self.node.driver_internal_info['clean_steps'] = []
boot_device = drac_mgmt.set_boot_device(
self.node, ironic.common.boot_devices.PXE, persistent=False)
- mock_list_unfinished_jobs.assert_called_once_with(self.node)
+ self.assertEqual(0, mock_list_unfinished_jobs.call_count)
+ self.assertEqual(0, mock_client.delete_jobs.call_count)
+ mock_validate_job_queue.assert_called_once_with(self.node)
mock_client.change_boot_device_order.assert_called_once_with(
'OneTime', 'BIOS.Setup.1-1#BootSeq#NIC.Embedded.1-1-1')
self.assertEqual(0, mock_client.set_bios_settings.call_count)
@@ -495,15 +501,18 @@ class DracManagementInternalMethodsTestCase(test_utils.BaseDracTest):
self.assertEqual(0, mock_client.set_bios_settings.call_count)
self.assertEqual(0, mock_client.commit_pending_bios_changes.call_count)
+ @mock.patch.object(drac_job, 'validate_job_queue', spec_set=True,
+ autospec=True)
@mock.patch.object(drac_job, 'list_unfinished_jobs', spec_set=True,
autospec=True)
@mock.patch.object(drac_mgmt, '_get_boot_device', spec_set=True,
autospec=True)
@mock.patch.object(drac_mgmt, '_get_next_persistent_boot_mode',
spec_set=True, autospec=True)
- def test_set_boot_device_with_list_unfinished_jobs(
+ def test_set_boot_device_with_list_unfinished_jobs_without_clean_step(
self, mock__get_next_persistent_boot_mode, mock__get_boot_device,
- mock_list_unfinished_jobs, mock_get_drac_client):
+ mock_list_unfinished_jobs, mock_validate_job_queue,
+ mock_get_drac_client):
mock_client = mock.Mock()
mock_get_drac_client.return_value = mock_client
@@ -525,21 +534,27 @@ class DracManagementInternalMethodsTestCase(test_utils.BaseDracTest):
mock__get_boot_device.return_value = boot_device
mock__get_next_persistent_boot_mode.return_value = 'IPL'
+ self.node.driver_internal_info['clean_steps'] = []
+
drac_mgmt.set_boot_device(self.node, ironic.common.boot_devices.DISK,
persistent=True)
- mock_list_unfinished_jobs.assert_called_once_with(self.node)
- mock_client.delete_jobs.assert_called_once_with(
- job_ids=['JID_602553293345'])
+ self.assertEqual(0, mock_list_unfinished_jobs.call_count)
+ self.assertEqual(0, mock_client.delete_jobs.call_count)
+ mock_validate_job_queue.assert_called_once_with(self.node)
+
+ @mock.patch.object(drac_job, 'validate_job_queue', spec_set=True,
+ autospec=True)
@mock.patch.object(drac_job, 'list_unfinished_jobs', spec_set=True,
autospec=True)
@mock.patch.object(drac_mgmt, '_get_boot_device', spec_set=True,
autospec=True)
@mock.patch.object(drac_mgmt, '_get_next_persistent_boot_mode',
spec_set=True, autospec=True)
- def test_set_boot_device_with_multiple_unfinished_jobs(
+ def test_set_boot_device_with_multiple_unfinished_jobs_without_clean_step(
self, mock__get_next_persistent_boot_mode, mock__get_boot_device,
- mock_list_unfinished_jobs, mock_get_drac_client):
+ mock_list_unfinished_jobs, mock_validate_job_queue,
+ mock_get_drac_client):
mock_client = mock.Mock()
mock_get_drac_client.return_value = mock_client
@@ -571,11 +586,115 @@ class DracManagementInternalMethodsTestCase(test_utils.BaseDracTest):
mock__get_boot_device.return_value = boot_device
mock__get_next_persistent_boot_mode.return_value = 'IPL'
+ self.node.driver_internal_info['clean_steps'] = []
+ drac_mgmt.set_boot_device(self.node, ironic.common.boot_devices.DISK,
+ persistent=True)
+ self.assertEqual(0, mock_list_unfinished_jobs.call_count)
+ self.assertEqual(0, mock_client.delete_jobs.call_count)
+
+ mock_validate_job_queue.assert_called_once_with(self.node)
+
+ @mock.patch.object(drac_mgmt, '_get_next_persistent_boot_mode',
+ spec_set=True, autospec=True)
+ @mock.patch.object(drac_mgmt, '_get_boot_device', spec_set=True,
+ autospec=True)
+ @mock.patch.object(drac_job, 'list_unfinished_jobs', spec_set=True,
+ autospec=True)
+ @mock.patch.object(drac_job, 'validate_job_queue', spec_set=True,
+ autospec=True)
+ def test_set_boot_device_with_list_unfinished_jobs_with_clean_step(
+ self, mock_validate_job_queue,
+ mock_list_unfinished_jobs,
+ mock__get_boot_device,
+ mock__get_next_persistent_boot_mode,
+ mock_get_drac_client):
+ mock_client = mock.Mock()
+ mock_get_drac_client.return_value = mock_client
+ mock_client.list_boot_devices.return_value = self.boot_devices['IPL']
+
+ boot_device = {'boot_device': ironic.common.boot_devices.DISK,
+ 'persistent': True}
+ mock__get_boot_device.return_value = boot_device
+ mock__get_next_persistent_boot_mode.return_value = 'IPL'
+
+ mock_job = mock.Mock()
+ mock_job.status = "Scheduled"
+ mock_client.get_job.return_value = mock_job
+
+ bios_job_dict = {
+ 'id': 'JID_602553293345',
+ 'name': 'ConfigBIOS:BIOS.Setup.1-1',
+ 'start_time': 'TIME_NOW',
+ 'until_time': 'TIME_NA',
+ 'message': 'Task successfully scheduled.',
+ 'status': 'Scheduled',
+ 'percent_complete': 0}
+ bios_job = test_utils.make_job(bios_job_dict)
+ mock_list_unfinished_jobs.return_value = [bios_job]
+
+ self.node.driver_internal_info['clean_steps'] = [{
+ u'interface': u'management', u'step': u'clear_job_queue'}]
+ boot_device = drac_mgmt.set_boot_device(
+ self.node, ironic.common.boot_devices.PXE, persistent=False)
+ mock_list_unfinished_jobs.assert_called_once_with(self.node)
+ mock_client.delete_jobs.assert_called_once_with(
+ job_ids=['JID_602553293345'])
+
+ self.assertEqual(0, mock_validate_job_queue.call_count)
+
+ @mock.patch.object(drac_job, 'validate_job_queue', spec_set=True,
+ autospec=True)
+ @mock.patch.object(drac_job, 'list_unfinished_jobs', spec_set=True,
+ autospec=True)
+ @mock.patch.object(drac_mgmt, '_get_boot_device', spec_set=True,
+ autospec=True)
+ @mock.patch.object(drac_mgmt, '_get_next_persistent_boot_mode',
+ spec_set=True, autospec=True)
+ def test_set_boot_device_with_multiple_unfinished_jobs_with_clean_step(
+ self, mock__get_next_persistent_boot_mode, mock__get_boot_device,
+ mock_list_unfinished_jobs, mock_validate_job_queue,
+ mock_get_drac_client):
+ mock_client = mock.Mock()
+ mock_get_drac_client.return_value = mock_client
+
+ job_dict = {
+ 'id': 'JID_602553293345',
+ 'name': 'Config:RAID:RAID.Integrated.1-1',
+ 'start_time': 'TIME_NOW',
+ 'until_time': 'TIME_NA',
+ 'message': 'Task successfully scheduled.',
+ 'status': 'Scheduled',
+ 'percent_complete': 0}
+ job = test_utils.make_job(job_dict)
+
+ bios_job_dict = {
+ 'id': 'JID_602553293346',
+ 'name': 'ConfigBIOS:BIOS.Setup.1-1',
+ 'start_time': 'TIME_NOW',
+ 'until_time': 'TIME_NA',
+ 'message': 'Task successfully scheduled.',
+ 'status': 'Scheduled',
+ 'percent_complete': 0}
+ bios_job = test_utils.make_job(bios_job_dict)
+
+ mock_list_unfinished_jobs.return_value = [job, bios_job]
+ mock_client.list_boot_devices.return_value = self.boot_devices['IPL']
+ boot_device = {'boot_device': ironic.common.boot_devices.DISK,
+ 'persistent': True}
+
+ mock__get_boot_device.return_value = boot_device
+ mock__get_next_persistent_boot_mode.return_value = 'IPL'
+
+ self.node.driver_internal_info['clean_steps'] = [{
+ u'interface': u'management', u'step': u'clear_job_queue'}]
+
drac_mgmt.set_boot_device(self.node, ironic.common.boot_devices.DISK,
persistent=True)
mock_list_unfinished_jobs.assert_called_once_with(self.node)
mock_client.delete_jobs.assert_called_once_with(
- job_ids=['JID_602553293346'])
+ job_ids=['JID_602553293345', 'JID_602553293346'])
+
+ self.assertEqual(0, mock_validate_job_queue.call_count)
@mock.patch.object(drac_common, 'get_drac_client', spec_set=True,
diff --git a/releasenotes/notes/fix_pending_non_bios_job_execution-4b22e168ac915f4f.yaml b/releasenotes/notes/fix_pending_non_bios_job_execution-4b22e168ac915f4f.yaml
new file mode 100644
index 0000000000..1722c263e6
--- /dev/null
+++ b/releasenotes/notes/fix_pending_non_bios_job_execution-4b22e168ac915f4f.yaml
@@ -0,0 +1,10 @@
+---
+fixes:
+ - |
+ Fixes a bug in the ``idrac`` hardware type where executing
+ the ``clear_job_queue`` clean step, pending non-BIOS config jobs
+ (E.g. create/delete virtual disk) were not being deleted before
+ job execution.
+
+ See bug `2006580 https://storyboard.openstack.org/#!/story/2006580`
+ for details