Merge "Fix handling of 'timeout' parameter to power methods"

6 files changed, 224 insertions, 14 deletions

diff --git a/ironic/tests/unit/drivers/modules/cimc/test_power.py b/ironic/tests/unit/drivers/modules/cimc/test_power.py
index 4617c7d5c..6ffaf6733 100644
--- a/ironic/tests/unit/drivers/modules/cimc/test_power.py
+++ b/ironic/tests/unit/drivers/modules/cimc/test_power.py
@@ -190,7 +190,8 @@ class PowerTestCase(test_common.CIMCBaseTestCase):
                 handle.get_imc_managedobject.assert_called_with(
                     None, None, params={"Dn": "sys/rack-unit-1"})
 
-    def test_set_power_state_on_ok(self, mock_handle):
+    @mock.patch.object(power.LOG, 'warning')
+    def test_set_power_state_on_ok(self, mock_log, mock_handle):
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
             with mock_handle(task) as handle:
@@ -213,6 +214,7 @@ class PowerTestCase(test_common.CIMCBaseTestCase):
 
                 handle.get_imc_managedobject.assert_called_with(
                     None, None, params={"Dn": "sys/rack-unit-1"})
+                self.assertFalse(mock_log.called)
 
     def test_set_power_state_on_fail(self, mock_handle):
         with task_manager.acquire(self.context, self.node.uuid,
@@ -236,6 +238,33 @@ class PowerTestCase(test_common.CIMCBaseTestCase):
                 handle.get_imc_managedobject.assert_called_with(
                     None, None, params={"Dn": "sys/rack-unit-1"})
 
+    @mock.patch.object(power.LOG, 'warning')
+    def test_set_power_state_on_timeout(self, mock_log, mock_handle):
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            with mock_handle(task) as handle:
+                mock_rack_unit = mock.MagicMock()
+                mock_rack_unit.get_attr.side_effect = [
+                    imcsdk.ComputeRackUnit.CONST_OPER_POWER_OFF,
+                    imcsdk.ComputeRackUnit.CONST_OPER_POWER_ON
+                ]
+                handle.get_imc_managedobject.return_value = [mock_rack_unit]
+
+                task.driver.power.set_power_state(task, states.POWER_ON,
+                                                  timeout=10)
+
+                handle.set_imc_managedobject.assert_called_once_with(
+                    None, class_id="ComputeRackUnit",
+                    params={
+                        imcsdk.ComputeRackUnit.ADMIN_POWER:
+                            imcsdk.ComputeRackUnit.CONST_ADMIN_POWER_UP,
+                        imcsdk.ComputeRackUnit.DN: "sys/rack-unit-1"
+                    })
+
+                handle.get_imc_managedobject.assert_called_with(
+                    None, None, params={"Dn": "sys/rack-unit-1"})
+                self.assertTrue(mock_log.called)
+
     def test_set_power_state_off_ok(self, mock_handle):
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
@@ -282,14 +311,17 @@ class PowerTestCase(test_common.CIMCBaseTestCase):
                 handle.get_imc_managedobject.assert_called_with(
                     None, None, params={"Dn": "sys/rack-unit-1"})
 
+    @mock.patch.object(power.LOG, 'warning')
     @mock.patch.object(power.Power, "set_power_state", autospec=True)
     @mock.patch.object(power.Power, "get_power_state", autospec=True)
-    def test_reboot_on(self, mock_get_state, mock_set_state, mock_handle):
+    def test_reboot_on(self, mock_get_state, mock_set_state, mock_log,
+                       mock_handle):
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
             mock_get_state.return_value = states.POWER_ON
             task.driver.power.reboot(task)
             mock_set_state.assert_called_with(mock.ANY, task, states.REBOOT)
+            self.assertFalse(mock_log.called)
 
     @mock.patch.object(power.Power, "set_power_state", autospec=True)
     @mock.patch.object(power.Power, "get_power_state", autospec=True)
@@ -299,3 +331,15 @@ class PowerTestCase(test_common.CIMCBaseTestCase):
             mock_get_state.return_value = states.POWER_OFF
             task.driver.power.reboot(task)
             mock_set_state.assert_called_with(mock.ANY, task, states.POWER_ON)
+
+    @mock.patch.object(power.LOG, 'warning')
+    @mock.patch.object(power.Power, "set_power_state", autospec=True)
+    @mock.patch.object(power.Power, "get_power_state", autospec=True)
+    def test_reboot_on_timeout(self, mock_get_state, mock_set_state, mock_log,
+                               mock_handle):
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            mock_get_state.return_value = states.POWER_ON
+            task.driver.power.reboot(task, timeout=30)
+            mock_set_state.assert_called_with(mock.ANY, task, states.REBOOT)
+            self.assertTrue(mock_log.called)
diff --git a/ironic/tests/unit/drivers/modules/drac/test_power.py b/ironic/tests/unit/drivers/modules/drac/test_power.py
index a507f1b28..d1edf36d8 100644
--- a/ironic/tests/unit/drivers/modules/drac/test_power.py
+++ b/ironic/tests/unit/drivers/modules/drac/test_power.py
@@ -71,7 +71,8 @@ class DracPowerTestCase(db_base.DbTestCase):
 
         mock_client.get_power_state.assert_called_once_with()
 
-    def test_set_power_state(self, mock_get_drac_client):
+    @mock.patch.object(drac_power.LOG, 'warning')
+    def test_set_power_state(self, mock_log, mock_get_drac_client):
         mock_client = mock_get_drac_client.return_value
 
         with task_manager.acquire(self.context, self.node.uuid,
@@ -80,6 +81,7 @@ class DracPowerTestCase(db_base.DbTestCase):
 
         drac_power_state = drac_power.REVERSE_POWER_STATES[states.POWER_OFF]
         mock_client.set_power_state.assert_called_once_with(drac_power_state)
+        self.assertFalse(mock_log.called)
 
     def test_set_power_state_fail(self, mock_get_drac_client):
         mock_client = mock_get_drac_client.return_value
@@ -95,7 +97,21 @@ class DracPowerTestCase(db_base.DbTestCase):
         drac_power_state = drac_power.REVERSE_POWER_STATES[states.POWER_OFF]
         mock_client.set_power_state.assert_called_once_with(drac_power_state)
 
-    def test_reboot_while_powered_on(self, mock_get_drac_client):
+    @mock.patch.object(drac_power.LOG, 'warning')
+    def test_set_power_state_timeout(self, mock_log, mock_get_drac_client):
+        mock_client = mock_get_drac_client.return_value
+
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            task.driver.power.set_power_state(task, states.POWER_OFF,
+                                              timeout=11)
+
+        drac_power_state = drac_power.REVERSE_POWER_STATES[states.POWER_OFF]
+        mock_client.set_power_state.assert_called_once_with(drac_power_state)
+        self.assertTrue(mock_log.called)
+
+    @mock.patch.object(drac_power.LOG, 'warning')
+    def test_reboot_while_powered_on(self, mock_log, mock_get_drac_client):
         mock_client = mock_get_drac_client.return_value
         mock_client.get_power_state.return_value = drac_constants.POWER_ON
 
@@ -105,6 +121,21 @@ class DracPowerTestCase(db_base.DbTestCase):
 
         drac_power_state = drac_power.REVERSE_POWER_STATES[states.REBOOT]
         mock_client.set_power_state.assert_called_once_with(drac_power_state)
+        self.assertFalse(mock_log.called)
+
+    @mock.patch.object(drac_power.LOG, 'warning')
+    def test_reboot_while_powered_on_timeout(self, mock_log,
+                                             mock_get_drac_client):
+        mock_client = mock_get_drac_client.return_value
+        mock_client.get_power_state.return_value = drac_constants.POWER_ON
+
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            task.driver.power.reboot(task, timeout=42)
+
+        drac_power_state = drac_power.REVERSE_POWER_STATES[states.REBOOT]
+        mock_client.set_power_state.assert_called_once_with(drac_power_state)
+        self.assertTrue(mock_log.called)
 
     def test_reboot_while_powered_off(self, mock_get_drac_client):
         mock_client = mock_get_drac_client.return_value
diff --git a/ironic/tests/unit/drivers/modules/ilo/test_power.py b/ironic/tests/unit/drivers/modules/ilo/test_power.py
index 392a30cea..52216db76 100644
--- a/ironic/tests/unit/drivers/modules/ilo/test_power.py
+++ b/ironic/tests/unit/drivers/modules/ilo/test_power.py
@@ -209,20 +209,35 @@ class IloPowerTestCase(db_base.DbTestCase):
                              task.driver.power.get_power_state(task))
             mock_get_power.assert_called_once_with(task.node)
 
+    @mock.patch.object(ilo_power.LOG, 'warning')
     @mock.patch.object(ilo_power, '_set_power_state', spec_set=True,
                        autospec=True)
-    def test_set_power_state(self, mock_set_power):
+    def test_set_power_state(self, mock_set_power, mock_log):
         mock_set_power.return_value = states.POWER_ON
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
             task.driver.power.set_power_state(task, states.POWER_ON)
         mock_set_power.assert_called_once_with(task, states.POWER_ON)
+        self.assertFalse(mock_log.called)
 
+    @mock.patch.object(ilo_power.LOG, 'warning')
+    @mock.patch.object(ilo_power, '_set_power_state', spec_set=True,
+                       autospec=True)
+    def test_set_power_state_timeout(self, mock_set_power, mock_log):
+        mock_set_power.return_value = states.POWER_ON
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            task.driver.power.set_power_state(task, states.POWER_ON,
+                                              timeout=13)
+        mock_set_power.assert_called_once_with(task, states.POWER_ON)
+        self.assertTrue(mock_log.called)
+
+    @mock.patch.object(ilo_power.LOG, 'warning')
     @mock.patch.object(ilo_power, '_set_power_state', spec_set=True,
                        autospec=True)
     @mock.patch.object(ilo_power, '_get_power_state', spec_set=True,
                        autospec=True)
-    def test_reboot(self, mock_get_power, mock_set_power):
+    def test_reboot(self, mock_get_power, mock_set_power, mock_log):
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
             mock_get_power.return_value = states.POWER_ON
@@ -230,3 +245,19 @@ class IloPowerTestCase(db_base.DbTestCase):
             task.driver.power.reboot(task)
             mock_get_power.assert_called_once_with(task.node)
             mock_set_power.assert_called_once_with(task, states.REBOOT)
+            self.assertFalse(mock_log.called)
+
+    @mock.patch.object(ilo_power.LOG, 'warning')
+    @mock.patch.object(ilo_power, '_set_power_state', spec_set=True,
+                       autospec=True)
+    @mock.patch.object(ilo_power, '_get_power_state', spec_set=True,
+                       autospec=True)
+    def test_reboot_timeout(self, mock_get_power, mock_set_power, mock_log):
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            mock_get_power.return_value = states.POWER_ON
+            mock_set_power.return_value = states.POWER_ON
+            task.driver.power.reboot(task, timeout=123)
+            mock_get_power.assert_called_once_with(task.node)
+            mock_set_power.assert_called_once_with(task, states.REBOOT)
+            self.assertTrue(mock_log.called)
diff --git a/ironic/tests/unit/drivers/modules/test_snmp.py b/ironic/tests/unit/drivers/modules/test_snmp.py
index 0a127601f..349268a40 100644
--- a/ironic/tests/unit/drivers/modules/test_snmp.py
+++ b/ironic/tests/unit/drivers/modules/test_snmp.py
@@ -1267,12 +1267,24 @@ class SNMPDriverTestCase(db_base.DbTestCase):
                               task.driver.power.get_power_state, task)
         mock_driver.power_state.assert_called_once_with()
 
-    def test_set_power_state_on(self, mock_get_driver):
+    @mock.patch.object(snmp.LOG, 'warning')
+    def test_set_power_state_on(self, mock_log, mock_get_driver):
         mock_driver = mock_get_driver.return_value
         mock_driver.power_on.return_value = states.POWER_ON
         with task_manager.acquire(self.context, self.node.uuid) as task:
             task.driver.power.set_power_state(task, states.POWER_ON)
         mock_driver.power_on.assert_called_once_with()
+        self.assertFalse(mock_log.called)
+
+    @mock.patch.object(snmp.LOG, 'warning')
+    def test_set_power_state_on_timeout(self, mock_log, mock_get_driver):
+        mock_driver = mock_get_driver.return_value
+        mock_driver.power_on.return_value = states.POWER_ON
+        with task_manager.acquire(self.context, self.node.uuid) as task:
+            task.driver.power.set_power_state(task, states.POWER_ON,
+                                              timeout=222)
+        mock_driver.power_on.assert_called_once_with()
+        self.assertTrue(mock_log.called)
 
     def test_set_power_state_off(self, mock_get_driver):
         mock_driver = mock_get_driver.return_value
@@ -1305,7 +1317,7 @@ class SNMPDriverTestCase(db_base.DbTestCase):
                               task, states.POWER_OFF)
         mock_driver.power_off.assert_called_once_with()
 
-    def test_set_power_state_on_timeout(self, mock_get_driver):
+    def test_set_power_state_on_error(self, mock_get_driver):
         mock_driver = mock_get_driver.return_value
         mock_driver.power_on.return_value = states.ERROR
         with task_manager.acquire(self.context, self.node.uuid) as task:
@@ -1314,7 +1326,7 @@ class SNMPDriverTestCase(db_base.DbTestCase):
                               task, states.POWER_ON)
         mock_driver.power_on.assert_called_once_with()
 
-    def test_set_power_state_off_timeout(self, mock_get_driver):
+    def test_set_power_state_off_error(self, mock_get_driver):
         mock_driver = mock_get_driver.return_value
         mock_driver.power_off.return_value = states.ERROR
         with task_manager.acquire(self.context, self.node.uuid) as task:
@@ -1323,12 +1335,23 @@ class SNMPDriverTestCase(db_base.DbTestCase):
                               task, states.POWER_OFF)
         mock_driver.power_off.assert_called_once_with()
 
-    def test_reboot(self, mock_get_driver):
+    @mock.patch.object(snmp.LOG, 'warning')
+    def test_reboot(self, mock_log, mock_get_driver):
         mock_driver = mock_get_driver.return_value
         mock_driver.power_reset.return_value = states.POWER_ON
         with task_manager.acquire(self.context, self.node.uuid) as task:
             task.driver.power.reboot(task)
         mock_driver.power_reset.assert_called_once_with()
+        self.assertFalse(mock_log.called)
+
+    @mock.patch.object(snmp.LOG, 'warning')
+    def test_reboot_timeout(self, mock_log, mock_get_driver):
+        mock_driver = mock_get_driver.return_value
+        mock_driver.power_reset.return_value = states.POWER_ON
+        with task_manager.acquire(self.context, self.node.uuid) as task:
+            task.driver.power.reboot(task, timeout=1)
+        mock_driver.power_reset.assert_called_once_with()
+        self.assertTrue(mock_log.called)
 
     def test_reboot_snmp_failure(self, mock_get_driver):
         mock_driver = mock_get_driver.return_value
@@ -1338,7 +1361,7 @@ class SNMPDriverTestCase(db_base.DbTestCase):
                               task.driver.power.reboot, task)
         mock_driver.power_reset.assert_called_once_with()
 
-    def test_reboot_timeout(self, mock_get_driver):
+    def test_reboot_error(self, mock_get_driver):
         mock_driver = mock_get_driver.return_value
         mock_driver.power_reset.return_value = states.ERROR
         with task_manager.acquire(self.context, self.node.uuid) as task:
diff --git a/ironic/tests/unit/drivers/modules/ucs/test_power.py b/ironic/tests/unit/drivers/modules/ucs/test_power.py
index 3c16847f4..9e76604cf 100644
--- a/ironic/tests/unit/drivers/modules/ucs/test_power.py
+++ b/ironic/tests/unit/drivers/modules/ucs/test_power.py
@@ -137,13 +137,15 @@ class UcsPowerTestCase(db_base.DbTestCase):
                               task)
         power.get_power_state.assert_called_with()
 
+    @mock.patch.object(ucs_power.LOG, 'warning')
     @mock.patch('ironic.drivers.modules.ucs.helper.ucs_helper',
                 spec_set=True, autospec=True)
     @mock.patch('ironic.drivers.modules.ucs.power._wait_for_state_change',
                 spec_set=True, autospec=True)
     @mock.patch('ironic.drivers.modules.ucs.power.ucs_power.UcsPower',
                 spec_set=True, autospec=True)
-    def test_set_power_state(self, mock_power_helper, mock__wait, mock_helper):
+    def test_set_power_state(self, mock_power_helper, mock__wait, mock_helper,
+                             mock_log):
         target_state = states.POWER_ON
         mock_power = mock_power_helper.return_value
         mock_power.get_power_state.side_effect = ['down', 'up']
@@ -157,6 +159,32 @@ class UcsPowerTestCase(db_base.DbTestCase):
         mock_power.set_power_state.assert_called_once_with('up')
         mock_power.get_power_state.assert_called_once_with()
         mock__wait.assert_called_once_with(target_state, mock_power)
+        self.assertFalse(mock_log.called)
+
+    @mock.patch.object(ucs_power.LOG, 'warning')
+    @mock.patch('ironic.drivers.modules.ucs.helper.ucs_helper',
+                spec_set=True, autospec=True)
+    @mock.patch('ironic.drivers.modules.ucs.power._wait_for_state_change',
+                spec_set=True, autospec=True)
+    @mock.patch('ironic.drivers.modules.ucs.power.ucs_power.UcsPower',
+                spec_set=True, autospec=True)
+    def test_set_power_state_timeout(self, mock_power_helper, mock__wait,
+                                     mock_helper, mock_log):
+        target_state = states.POWER_ON
+        mock_power = mock_power_helper.return_value
+        mock_power.get_power_state.side_effect = ['down', 'up']
+        mock_helper.generate_ucsm_handle.return_value = (True, mock.Mock())
+        mock__wait.return_value = target_state
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            self.assertIsNone(self.interface.set_power_state(task,
+                                                             target_state,
+                                                             timeout=23))
+
+        mock_power.set_power_state.assert_called_once_with('up')
+        mock_power.get_power_state.assert_called_once_with()
+        mock__wait.assert_called_once_with(target_state, mock_power)
+        self.assertTrue(mock_log.called)
 
     @mock.patch('ironic.drivers.modules.ucs.helper.ucs_helper',
                 spec_set=True, autospec=True)
@@ -249,13 +277,15 @@ class UcsPowerTestCase(db_base.DbTestCase):
         mock_power.get_power_state.assert_called_once_with()
         mock__wait.assert_called_once_with(target_state, mock_power)
 
+    @mock.patch.object(ucs_power.LOG, 'warning')
     @mock.patch('ironic.drivers.modules.ucs.helper.ucs_helper',
                 spec_set=True, autospec=True)
     @mock.patch('ironic.drivers.modules.ucs.power._wait_for_state_change',
                 spec_set=True, autospec=True)
     @mock.patch('ironic.drivers.modules.ucs.power.ucs_power.UcsPower',
                 spec_set=True, autospec=True)
-    def test_reboot(self, mock_power_helper, mock__wait, mock_helper):
+    def test_reboot(self, mock_power_helper, mock__wait, mock_helper,
+                    mock_log):
         mock_helper.generate_ucsm_handle.return_value = (True, mock.Mock())
         mock_power = mock_power_helper.return_value
         mock__wait.return_value = states.POWER_ON
@@ -263,6 +293,25 @@ class UcsPowerTestCase(db_base.DbTestCase):
                                   shared=False) as task:
             self.assertIsNone(self.interface.reboot(task))
             mock_power.reboot.assert_called_once_with()
+            self.assertFalse(mock_log.called)
+
+    @mock.patch.object(ucs_power.LOG, 'warning')
+    @mock.patch('ironic.drivers.modules.ucs.helper.ucs_helper',
+                spec_set=True, autospec=True)
+    @mock.patch('ironic.drivers.modules.ucs.power._wait_for_state_change',
+                spec_set=True, autospec=True)
+    @mock.patch('ironic.drivers.modules.ucs.power.ucs_power.UcsPower',
+                spec_set=True, autospec=True)
+    def test_reboot_timeout(self, mock_power_helper, mock__wait, mock_helper,
+                            mock_log):
+        mock_helper.generate_ucsm_handle.return_value = (True, mock.Mock())
+        mock_power = mock_power_helper.return_value
+        mock__wait.return_value = states.POWER_ON
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            self.assertIsNone(self.interface.reboot(task, timeout=88))
+            mock_power.reboot.assert_called_once_with()
+            self.assertTrue(mock_log.called)
 
     @mock.patch('ironic.drivers.modules.ucs.helper.ucs_helper',
                 spec_set=True, autospec=True)
diff --git a/ironic/tests/unit/drivers/modules/xclarity/test_power.py b/ironic/tests/unit/drivers/modules/xclarity/test_power.py
index f695c4c29..6cec8cd31 100644
--- a/ironic/tests/unit/drivers/modules/xclarity/test_power.py
+++ b/ironic/tests/unit/drivers/modules/xclarity/test_power.py
@@ -90,13 +90,28 @@ class XClarityPowerDriverTestCase(db_base.DbTestCase):
                               task.driver.power.get_power_state,
                               task)
 
+    @mock.patch.object(power.LOG, 'warning')
     @mock.patch.object(power.XClarityPower, 'get_power_state',
                        return_value=states.POWER_ON)
-    def test_set_power(self, mock_set_power_state, mock_get_xc_client):
+    def test_set_power(self, mock_set_power_state, mock_log,
+                       mock_get_xc_client):
         with task_manager.acquire(self.context, self.node.uuid) as task:
             task.driver.power.set_power_state(task, states.POWER_ON)
             expected = task.driver.power.get_power_state(task)
         self.assertEqual(expected, states.POWER_ON)
+        self.assertFalse(mock_log.called)
+
+    @mock.patch.object(power.LOG, 'warning')
+    @mock.patch.object(power.XClarityPower, 'get_power_state',
+                       return_value=states.POWER_ON)
+    def test_set_power_timeout(self, mock_set_power_state, mock_log,
+                               mock_get_xc_client):
+        with task_manager.acquire(self.context, self.node.uuid) as task:
+            task.driver.power.set_power_state(task, states.POWER_ON,
+                                              timeout=21)
+            expected = task.driver.power.get_power_state(task)
+        self.assertEqual(expected, states.POWER_ON)
+        self.assertTrue(mock_log.called)
 
     def test_set_power_fail(self, mock_xc_client):
         with task_manager.acquire(self.context, self.node.uuid) as task:
@@ -111,3 +126,20 @@ class XClarityPowerDriverTestCase(db_base.DbTestCase):
             self.assertRaises(common.XClarityError,
                               task.driver.power.set_power_state,
                               task, states.POWER_OFF)
+
+    @mock.patch.object(power.LOG, 'warning')
+    @mock.patch.object(power.XClarityPower, 'set_power_state')
+    def test_reboot(self, mock_set_power_state, mock_log, mock_get_xc_client):
+        with task_manager.acquire(self.context, self.node.uuid) as task:
+            task.driver.power.reboot(task)
+            mock_set_power_state.assert_called_with(task, states.REBOOT)
+            self.assertFalse(mock_log.called)
+
+    @mock.patch.object(power.LOG, 'warning')
+    @mock.patch.object(power.XClarityPower, 'set_power_state')
+    def test_reboot_timeout(self, mock_set_power_state, mock_log,
+                            mock_get_xc_client):
+        with task_manager.acquire(self.context, self.node.uuid) as task:
+            task.driver.power.reboot(task, timeout=55)
+            mock_set_power_state.assert_called_with(task, states.REBOOT)
+            self.assertTrue(mock_log.called)