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

@@ -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 "
+        LOG.warning("Performing an insecure connection with OneView, the CA "
-                  "certificate file: %s will be ignored.", ssl_certificate)
+                    "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"),

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 = None
-    onetime_boot = boot.get('BootSourceOverrideEnabled')
+    boot = response.dict.get('Boot', {})
+    if boot:
+        onetime_boot = boot.get('BootSourceOverrideEnabled')
     return onetime_boot == 'Once'
 
 

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:

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(
+    @mock.patch.object(ov_common, 'validate_oneview_resources_compatibility',
-        oneview_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(
+    @mock.patch.object(ov_common, 'validate_oneview_resources_compatibility',
-        oneview_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:

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.

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
+    Configuration option ``[oneview]max_polling_attempts`` is removed since
-    ``hpOneView`` doesn't support this option.
+    the ``hpOneView`` library doesn't support it.