diff --git a/os_win/exceptions.py b/os_win/exceptions.py index 5047f4a..d47c3a7 100644 --- a/os_win/exceptions.py +++ b/os_win/exceptions.py @@ -193,6 +193,19 @@ class DNSZoneAlreadyExists(DNSException): msg_fmt = _("DNS Zone already exists: %(zone_name)s") +class WMIJobFailed(HyperVException): + msg_fmt = _("WMI job failed with status %(job_state)s. " + "Error summary description: %(error_summ_desc)s. " + "Error description: %(error_desc)s " + "Error code: %(error_code)s.") + + def __init__(self, message=None, **kwargs): + self.error_code = kwargs.get('error_code', None) + self.job_state = kwargs.get('job_state', None) + + super(WMIJobFailed, self).__init__(message, **kwargs) + + class JobTerminateFailed(HyperVException): msg_fmt = _("Could not terminate the requested job(s).") diff --git a/os_win/tests/unit/utils/test_jobutils.py b/os_win/tests/unit/utils/test_jobutils.py index 9d5dbdf..3e3469b 100644 --- a/os_win/tests/unit/utils/test_jobutils.py +++ b/os_win/tests/unit/utils/test_jobutils.py @@ -32,7 +32,6 @@ class JobUtilsTestCase(test_base.OsWinBaseTestCase): _FAKE_JOB_PATH = 'fake_job_path' _FAKE_ERROR = "fake_error" _FAKE_ELAPSED_TIME = 0 - _CONCRETE_JOB = "Msvm_ConcreteJob" def setUp(self): super(JobUtilsTestCase, self).setUp() @@ -52,41 +51,33 @@ class JobUtilsTestCase(test_base.OsWinBaseTestCase): self.assertFalse(mock_wait_for_job.called) def test_check_ret_val_exception(self): - self.assertRaises(exceptions.HyperVException, + self.assertRaises(exceptions.WMIJobFailed, self.jobutils.check_ret_val, mock.sentinel.ret_val_bad, mock.sentinel.job_path) - def test_wait_for_job_exception_concrete_job(self): - mock_job = self._prepare_wait_for_job() - mock_job.path.return_value.Class = self._CONCRETE_JOB - self.assertRaises(exceptions.HyperVException, - self.jobutils._wait_for_job, - self._FAKE_JOB_PATH) - - def test_wait_for_job_exception_with_error(self): - mock_job = self._prepare_wait_for_job() - mock_job.GetError.return_value = (self._FAKE_ERROR, self._FAKE_RET_VAL) - self.assertRaises(exceptions.HyperVException, - self.jobutils._wait_for_job, - self._FAKE_JOB_PATH) - mock_job.GetError.assert_called_once_with() - - def test_wait_for_job_exception_no_error_details(self): - mock_job = self._prepare_wait_for_job() - mock_job.GetError.return_value = (None, None) - self.assertRaises(exceptions.HyperVException, - self.jobutils._wait_for_job, - self._FAKE_JOB_PATH) - def test_wait_for_job_ok(self): mock_job = self._prepare_wait_for_job( - constants.WMI_JOB_STATE_COMPLETED) + constants.JOB_STATE_COMPLETED_WITH_WARNINGS) job = self.jobutils._wait_for_job(self._FAKE_JOB_PATH) self.assertEqual(mock_job, job) - @ddt.data(True, False) - def test_get_pending_jobs(self, ignore_error_state): + def test_wait_for_job_error_state(self): + self._prepare_wait_for_job( + constants.JOB_STATE_TERMINATED) + self.assertRaises(exceptions.WMIJobFailed, + self.jobutils._wait_for_job, + self._FAKE_JOB_PATH) + + def test_wait_for_job_error_code(self): + self._prepare_wait_for_job( + constants.JOB_STATE_COMPLETED_WITH_WARNINGS, + error_code=1) + self.assertRaises(exceptions.WMIJobFailed, + self.jobutils._wait_for_job, + self._FAKE_JOB_PATH) + + def test_get_pending_jobs(self): mock_killed_job = mock.Mock(JobState=constants.JOB_STATE_KILLED) mock_running_job = mock.Mock(JobState=constants.WMI_JOB_STATE_RUNNING) mock_error_st_job = mock.Mock(JobState=constants.JOB_STATE_EXCEPTION) @@ -99,12 +90,8 @@ class JobUtilsTestCase(test_base.OsWinBaseTestCase): mock_affected_element = mock.Mock() expected_pending_jobs = [mock_running_job] - if not ignore_error_state: - expected_pending_jobs.append(mock_error_st_job) - pending_jobs = self.jobutils._get_pending_jobs_affecting_element( - mock_affected_element, - ignore_error_state=ignore_error_state) + mock_affected_element) self.assertEqual(expected_pending_jobs, pending_jobs) self.jobutils._conn.Msvm_AffectedJobElement.assert_called_once_with( @@ -161,8 +148,7 @@ class JobUtilsTestCase(test_base.OsWinBaseTestCase): mock.sentinel.vm) mock_get_pending_jobs.assert_has_calls( - [mock.call(mock.sentinel.vm, ignore_error_state=False), - mock.call(mock.sentinel.vm)]) + [mock.call(mock.sentinel.vm)] * 2) mock_job1.RequestStateChange.assert_called_once_with( self.jobutils._KILL_JOB_STATE_CHANGE_REQUEST) @@ -186,9 +172,11 @@ class JobUtilsTestCase(test_base.OsWinBaseTestCase): self.assertFalse(self.jobutils._is_job_completed(job)) - def _prepare_wait_for_job(self, state=_FAKE_JOB_STATUS_BAD): + def _prepare_wait_for_job(self, state=_FAKE_JOB_STATUS_BAD, + error_code=0): mock_job = mock.MagicMock() mock_job.JobState = state + mock_job.ErrorCode = error_code mock_job.Description = self._FAKE_JOB_DESCRIPTION mock_job.ElapsedTime = self._FAKE_ELAPSED_TIME diff --git a/os_win/utils/jobutils.py b/os_win/utils/jobutils.py index f497951..e54a3a1 100644 --- a/os_win/utils/jobutils.py +++ b/os_win/utils/jobutils.py @@ -22,7 +22,6 @@ import time from oslo_log import log as logging -from os_win._i18n import _ from os_win import _utils from os_win import constants from os_win import exceptions @@ -41,7 +40,10 @@ class JobUtils(baseutils.BaseUtilsVirt): _completed_job_states = [constants.JOB_STATE_COMPLETED, constants.JOB_STATE_TERMINATED, constants.JOB_STATE_KILLED, - constants.JOB_STATE_COMPLETED_WITH_WARNINGS] + constants.JOB_STATE_COMPLETED_WITH_WARNINGS, + constants.JOB_STATE_EXCEPTION] + _successful_job_states = [constants.JOB_STATE_COMPLETED, + constants.JOB_STATE_COMPLETED_WITH_WARNINGS] def check_ret_val(self, ret_val, job_path, success_values=[0]): """Checks that the job represented by the given arguments succeeded. @@ -58,18 +60,20 @@ class JobUtils(baseutils.BaseUtilsVirt): :param success_values: list of return values that can be considered successful. WMI_JOB_STATUS_STARTED and WMI_JOB_STATE_RUNNING values are ignored. - :raises exceptions.HyperVException: if the given ret_val is + :raises exceptions.WMIJobFailed: if the given ret_val is WMI_JOB_STATUS_STARTED or WMI_JOB_STATE_RUNNING and the state of job represented by the given job_path is not - WMI_JOB_STATE_COMPLETED, or if the given ret_val is not in the - list of given success_values. + WMI_JOB_STATE_COMPLETED or JOB_STATE_COMPLETED_WITH_WARNINGS, or + if the given ret_val is not in the list of given success_values. """ if ret_val in [constants.WMI_JOB_STATUS_STARTED, constants.WMI_JOB_STATE_RUNNING]: return self._wait_for_job(job_path) elif ret_val not in success_values: - raise exceptions.HyperVException( - _('Operation failed with return value: %s') % ret_val) + raise exceptions.WMIJobFailed(error_code=ret_val, + job_state=None, + error_summ_desc=None, + error_desc=None) def _wait_for_job(self, job_path): """Poll WMI job state and wait for completion.""" @@ -77,44 +81,35 @@ class JobUtils(baseutils.BaseUtilsVirt): job_wmi_path = job_path.replace('\\', '/') job = self._get_wmi_obj(job_wmi_path) - while job.JobState == constants.WMI_JOB_STATE_RUNNING: + while not self._is_job_completed(job): time.sleep(0.1) job = self._get_wmi_obj(job_wmi_path) - if job.JobState != constants.WMI_JOB_STATE_COMPLETED: - job_state = job.JobState - if job.path().Class == "Msvm_ConcreteJob": - err_sum_desc = job.ErrorSummaryDescription - err_desc = job.ErrorDescription - err_code = job.ErrorCode - data = {'job_state': job_state, - 'err_sum_desc': err_sum_desc, - 'err_desc': err_desc, - 'err_code': err_code} - raise exceptions.HyperVException( - _("WMI job failed with status %(job_state)d. " - "Error details: %(err_sum_desc)s - %(err_desc)s - " - "Error code: %(err_code)d") % data) - else: - (error, ret_val) = job.GetError() - if not ret_val and error: - data = {'job_state': job_state, - 'error': error} - raise exceptions.HyperVException( - _("WMI job failed with status %(job_state)d. " - "Error details: %(error)s") % data) - else: - raise exceptions.HyperVException( - _("WMI job failed with status %d. No error " - "description available") % job_state) + job_state = job.JobState + err_code = job.ErrorCode + + # We'll raise an exception for killed jobs. + job_failed = job_state not in self._successful_job_states or err_code + if job_failed: + err_sum_desc = getattr(job, 'ErrorSummaryDescription', None) + err_desc = job.ErrorDescription + + raise exceptions.WMIJobFailed(job_state=job_state, + error_code=err_code, + error_summ_desc=err_sum_desc, + error_desc=err_desc) + + if job_state == constants.JOB_STATE_COMPLETED_WITH_WARNINGS: + LOG.warning("WMI job completed with warnings. For detailed " + "information, please check the Windows event logs.") + desc = job.Description elap = job.ElapsedTime LOG.debug("WMI job succeeded: %(desc)s, Elapsed=%(elap)s", {'desc': desc, 'elap': elap}) return job - def _get_pending_jobs_affecting_element(self, element, - ignore_error_state=True): + def _get_pending_jobs_affecting_element(self, element): # Msvm_AffectedJobElement is in fact an association between # the affected element and the affecting job. mappings = self._conn.Msvm_AffectedJobElement( @@ -123,7 +118,7 @@ class JobUtils(baseutils.BaseUtilsVirt): for mapping in mappings: try: if mapping.AffectingElement and not self._is_job_completed( - mapping.AffectingElement, ignore_error_state): + mapping.AffectingElement): pending_jobs.append(mapping.AffectingElement) except exceptions.x_wmi as ex: @@ -134,16 +129,13 @@ class JobUtils(baseutils.BaseUtilsVirt): return pending_jobs def _stop_jobs(self, element): - pending_jobs = self._get_pending_jobs_affecting_element( - element, ignore_error_state=False) + pending_jobs = self._get_pending_jobs_affecting_element(element) for job in pending_jobs: try: if not job.Cancellable: LOG.debug("Got request to terminate " "non-cancelable job.") continue - elif job.JobState == constants.JOB_STATE_EXCEPTION: - LOG.debug("Attempting to terminate exception state job.") job.RequestStateChange( self._KILL_JOB_STATE_CHANGE_REQUEST) @@ -162,10 +154,8 @@ class JobUtils(baseutils.BaseUtilsVirt): pending_count=len(pending_jobs))) raise exceptions.JobTerminateFailed() - def _is_job_completed(self, job, ignore_error_state=True): - return (job.JobState in self._completed_job_states or - (job.JobState == constants.JOB_STATE_EXCEPTION and - ignore_error_state)) + def _is_job_completed(self, job): + return job.JobState in self._completed_job_states def stop_jobs(self, element, timeout=_DEFAULT_JOB_TERMINATE_TIMEOUT): """Stops the Hyper-V jobs associated with the given resource.