From c924a374c4467ab347858c17563ac7a6d3767271 Mon Sep 17 00:00:00 2001 From: Hugo Nicodemos Date: Tue, 30 Jan 2018 12:15:10 -0300 Subject: [PATCH] Follow-up for Switch OneView driver to hpOneView and ilorest libraries This patch addresses comments for patchs related to bug #1693788 Change-Id: I1fbab4674050bfbd94882c65686881867b100191 Related-Bug: 1693788 --- ironic/drivers/modules/oneview/common.py | 6 +++--- ironic/drivers/modules/oneview/management.py | 7 ++++--- ironic/drivers/modules/oneview/power.py | 4 ++-- .../unit/drivers/modules/oneview/test_inspect.py | 16 +++++++++------- .../oneview-timeout-power-db5125e05831d925.yaml | 2 +- ...ve-python-oneviewclient-b1d345ef861e156e.yaml | 6 +++--- 6 files changed, 22 insertions(+), 19 deletions(-) diff --git a/ironic/drivers/modules/oneview/common.py b/ironic/drivers/modules/oneview/common.py index 9bb048516f..811109edae 100644 --- a/ironic/drivers/modules/oneview/common.py +++ b/ironic/drivers/modules/oneview/common.py @@ -98,8 +98,8 @@ def get_hponeview_client(): # NOTE(nicodemos) Ignore the CA certificate if it's an insecure connection if insecure and ssl_certificate: - LOG.debug("Doing an insecure connection with OneView, the CA " - "certificate file: %s will be ignored.", ssl_certificate) + LOG.warning("Performing an insecure connection with OneView, the CA " + "certificate file: %s will be ignored.", ssl_certificate) ssl_certificate = None config = { @@ -322,7 +322,7 @@ def _get_server_hardware_mac_from_ilo(server_hardware): LOG.error("Failed in JSON object getting path: %s", ilo_path) raise exception.OneViewError(error=exc) except (ValueError, TypeError, IndexError) as exc: - LOG.error( + LOG.exception( "Failed to get mac from server hardware %(server_hardware)s " "via iLO. Error: %(message)s", { "server_hardware": server_hardware.get("uri"), diff --git a/ironic/drivers/modules/oneview/management.py b/ironic/drivers/modules/oneview/management.py index 1812cea630..85dcf667e2 100644 --- a/ironic/drivers/modules/oneview/management.py +++ b/ironic/drivers/modules/oneview/management.py @@ -100,14 +100,15 @@ def _is_onetime_boot(task): :param task: a task from TaskManager. :returns: Boolean value. True if onetime boot is 'Once' False otherwise. - :raises: AttributeError if Boot is None. """ server_hardware = task.node.driver_info.get('server_hardware_uri') ilo_client = common.get_ilorest_client(server_hardware) response = ilo_client.get(path=ILO_SYSTEM_PATH, headers=ILO_REQUEST_HEADERS) - boot = response.dict.get('Boot') - onetime_boot = boot.get('BootSourceOverrideEnabled') + onetime_boot = None + boot = response.dict.get('Boot', {}) + if boot: + onetime_boot = boot.get('BootSourceOverrideEnabled') return onetime_boot == 'Once' diff --git a/ironic/drivers/modules/oneview/power.py b/ironic/drivers/modules/oneview/power.py index 2100159a86..388ef88064 100644 --- a/ironic/drivers/modules/oneview/power.py +++ b/ironic/drivers/modules/oneview/power.py @@ -127,7 +127,7 @@ class OneViewPower(base.PowerInterface): :param power_state: The desired power state POWER_ON, POWER_OFF or REBOOT from :mod:`ironic.common.states`. :param timeout: timeout (in seconds) positive integer (> 0) for any - power state. ``None`` indicates to use default timeout. + power state. ``None`` indicates the default timeout. :raises: InvalidParameterValue if an invalid power state was specified. :raises: PowerStateFailure if the power couldn't be set to power_state. :raises: OneViewError if OneView fails setting the power state. @@ -154,7 +154,7 @@ class OneViewPower(base.PowerInterface): {'node_uuid': task.node.uuid, 'power_state': power_state}) server_hardware = task.node.driver_info.get('server_hardware_uri') - timeout = (-1 if timeout is None else timeout) + timeout = -1 if timeout is None else timeout try: if power_state == states.POWER_ON: diff --git a/ironic/tests/unit/drivers/modules/oneview/test_inspect.py b/ironic/tests/unit/drivers/modules/oneview/test_inspect.py index 0849b3d217..bcc1a195fa 100644 --- a/ironic/tests/unit/drivers/modules/oneview/test_inspect.py +++ b/ironic/tests/unit/drivers/modules/oneview/test_inspect.py @@ -16,7 +16,7 @@ import mock from ironic.conductor import task_manager -from ironic.drivers.modules.oneview import common as oneview_common +from ironic.drivers.modules.oneview import common as ov_common from ironic.drivers.modules.oneview import deploy_utils from ironic.tests.unit.conductor import mgr_utils from ironic.tests.unit.db import base as db_base @@ -43,8 +43,8 @@ class AgentPXEOneViewInspectTestCase(db_base.DbTestCase): shared=True) as task: self.assertEqual(expected, task.driver.inspect.get_properties()) - @mock.patch.object( - oneview_common, 'validate_oneview_resources_compatibility') + @mock.patch.object(ov_common, 'validate_oneview_resources_compatibility', + autospect=True) def test_validate(self, mock_validate): self.config(enabled=False, group='inspector') with task_manager.acquire(self.context, self.node.uuid, @@ -52,7 +52,8 @@ class AgentPXEOneViewInspectTestCase(db_base.DbTestCase): task.driver.inspect.validate(task) self.assertTrue(mock_validate.called) - @mock.patch.object(deploy_utils, 'allocate_server_hardware_to_ironic') + @mock.patch.object(deploy_utils, 'allocate_server_hardware_to_ironic', + autospect=True) def test_inspect_hardware(self, mock_allocate_server_hardware_to_ironic): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: @@ -79,8 +80,8 @@ class ISCSIPXEOneViewInspectTestCase(db_base.DbTestCase): shared=True) as task: self.assertEqual(expected, task.driver.inspect.get_properties()) - @mock.patch.object( - oneview_common, 'validate_oneview_resources_compatibility') + @mock.patch.object(ov_common, 'validate_oneview_resources_compatibility', + autospect=True) def test_validate(self, mock_validate): self.config(enabled=False, group='inspector') with task_manager.acquire(self.context, self.node.uuid, @@ -88,7 +89,8 @@ class ISCSIPXEOneViewInspectTestCase(db_base.DbTestCase): task.driver.inspect.validate(task) self.assertTrue(mock_validate.called) - @mock.patch.object(deploy_utils, 'allocate_server_hardware_to_ironic') + @mock.patch.object(deploy_utils, 'allocate_server_hardware_to_ironic', + autospect=True) def test_inspect_hardware(self, mock_allocate_server_hardware_to_ironic): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: diff --git a/releasenotes/notes/oneview-timeout-power-db5125e05831d925.yaml b/releasenotes/notes/oneview-timeout-power-db5125e05831d925.yaml index 1b968cdc5d..1c858a96c2 100644 --- a/releasenotes/notes/oneview-timeout-power-db5125e05831d925.yaml +++ b/releasenotes/notes/oneview-timeout-power-db5125e05831d925.yaml @@ -1,5 +1,5 @@ --- features: - | - Adds support for ``timeout`` parameter when powering on/off and reboot + Adds support for ``timeout`` parameter when powering on/off or rebooting a bare metal node managed by the ``oneview`` hardware type. diff --git a/releasenotes/notes/remove-python-oneviewclient-b1d345ef861e156e.yaml b/releasenotes/notes/remove-python-oneviewclient-b1d345ef861e156e.yaml index 771dd665ec..367e2ce19e 100644 --- a/releasenotes/notes/remove-python-oneviewclient-b1d345ef861e156e.yaml +++ b/releasenotes/notes/remove-python-oneviewclient-b1d345ef861e156e.yaml @@ -7,10 +7,10 @@ issues: hardware type is in use. upgrade: - | - The ``oneview`` hardware type now use ``hpOneView`` and + The ``oneview`` hardware type now uses ``hpOneView`` and ``python-ilorest-library`` libraries to communicate with OneView appliances. The ``python-oneviewclient`` library is no longer used. upgrade: - | - Configuration ``[oneview]max_polling_attempts`` is removed since - ``hpOneView`` doesn't support this option. + Configuration option ``[oneview]max_polling_attempts`` is removed since + the ``hpOneView`` library doesn't support it.