From 2688d636303a729f97d39b913b153d5413653e5d Mon Sep 17 00:00:00 2001 From: Xavier Date: Thu, 18 Aug 2016 16:27:35 -0300 Subject: [PATCH] Fix for check if dynamic allocation model is enabled This patch address the case of adding dynamic allocation as True into a given node using Ironic CLI. By default Ironic CLI uses string type for representing even boolean values in the node. To workaround that and allow operators migrate nodes from pre-allocation model to dynamic allocation using the Ironic CLI, this patch changes the way of checking the flag. Change-Id: I940904fbbd44b595759883d944a54dd37fac5c35 Closes-Bug: 1614608 --- ironic/drivers/modules/oneview/common.py | 15 +++-- .../drivers/modules/oneview/test_common.py | 56 ++++++++++++++++++- ...c-allocation-enabled-e94f3b8963b114d0.yaml | 5 ++ 3 files changed, 65 insertions(+), 11 deletions(-) create mode 100644 releasenotes/notes/check-dynamic-allocation-enabled-e94f3b8963b114d0.yaml diff --git a/ironic/drivers/modules/oneview/common.py b/ironic/drivers/modules/oneview/common.py index b3d40b58ab..1c6754696f 100644 --- a/ironic/drivers/modules/oneview/common.py +++ b/ironic/drivers/modules/oneview/common.py @@ -15,6 +15,7 @@ from oslo_log import log as logging from oslo_utils import importutils +from oslo_utils import strutils from ironic.common import exception from ironic.common.i18n import _, _LE @@ -269,12 +270,10 @@ def node_has_server_profile(func): def is_dynamic_allocation_enabled(node): flag = node.driver_info.get('dynamic_allocation') if flag: - if isinstance(flag, bool): - return flag is True - else: - msg = (_LE("Invalid dynamic_allocation parameter value in " - "node's %(node_uuid)s driver_info. Valid values " - "are booleans true or false.") % - {"node_uuid": node.uuid}) + try: + return strutils.bool_from_string(flag, strict=True) + except ValueError: + msg = (_LE("Invalid dynamic_allocation parameter value " + "'%(flag)s' in node's %(node_uuid)s driver_info.") % + {"flag": flag, "node_uuid": node.uuid}) raise exception.InvalidParameterValue(msg) - return False diff --git a/ironic/tests/unit/drivers/modules/oneview/test_common.py b/ironic/tests/unit/drivers/modules/oneview/test_common.py index bbb50b9c89..1330a70988 100644 --- a/ironic/tests/unit/drivers/modules/oneview/test_common.py +++ b/ironic/tests/unit/drivers/modules/oneview/test_common.py @@ -302,7 +302,7 @@ class OneViewCommonTestCase(db_base.DbTestCase): oneview_client. is_node_port_mac_compatible_with_server_profile.called) - def test_is_dynamic_allocation_enabled(self): + def test_is_dynamic_allocation_enabled_boolean(self): """Ensure Dynamic Allocation is enabled when flag is True. 1) Set 'dynamic_allocation' flag as True on node's driver_info @@ -318,7 +318,23 @@ class OneViewCommonTestCase(db_base.DbTestCase): common.is_dynamic_allocation_enabled(task.node) ) - def test_is_dynamic_allocation_enabled_false(self): + def test_is_dynamic_allocation_enabled_string(self): + """Ensure Dynamic Allocation is enabled when flag is 'True'. + + 1) Set 'dynamic_allocation' flag as True on node's driver_info + 2) Check Dynamic Allocation is enabled for the given node + + """ + with task_manager.acquire(self.context, self.node.uuid) as task: + driver_info = task.node.driver_info + driver_info['dynamic_allocation'] = 'True' + task.node.driver_info = driver_info + + self.assertTrue( + common.is_dynamic_allocation_enabled(task.node) + ) + + def test_is_dynamic_allocation_enabled_false_boolean(self): """Ensure Dynamic Allocation is disabled when flag is False. 1) Set 'dynamic_allocation' flag as False on node's driver_info @@ -334,7 +350,23 @@ class OneViewCommonTestCase(db_base.DbTestCase): common.is_dynamic_allocation_enabled(task.node) ) - def test_is_dynamic_allocation_enabled_none(self): + def test_is_dynamic_allocation_enabled_false_string(self): + """Ensure Dynamic Allocation is disabled when flag is 'False'. + + 1) Set 'dynamic_allocation' flag as False on node's driver_info + 2) Check Dynamic Allocation is disabled for the given node + + """ + with task_manager.acquire(self.context, self.node.uuid) as task: + driver_info = task.node.driver_info + driver_info['dynamic_allocation'] = 'False' + task.node.driver_info = driver_info + + self.assertFalse( + common.is_dynamic_allocation_enabled(task.node) + ) + + def test_is_dynamic_allocation_enabled_none_object(self): """Ensure Dynamic Allocation is disabled when flag is None. 1) Set 'dynamic_allocation' flag as None on node's driver_info @@ -361,3 +393,21 @@ class OneViewCommonTestCase(db_base.DbTestCase): self.assertFalse( common.is_dynamic_allocation_enabled(task.node) ) + + def test_is_dynamic_allocation_enabled_with_invalid_value_for_flag(self): + """Ensure raises an InvalidParameterValue when flag is invalid. + + 1) Create a node with an invalid value for 'dynamic_allocation' flag + 2) Check if method raises an InvalidParameterValue for the given node + + """ + with task_manager.acquire(self.context, self.node.uuid) as task: + driver_info = task.node.driver_info + driver_info['dynamic_allocation'] = 'invalid flag' + task.node.driver_info = driver_info + + self.assertRaises( + exception.InvalidParameterValue, + common.is_dynamic_allocation_enabled, + task.node + ) diff --git a/releasenotes/notes/check-dynamic-allocation-enabled-e94f3b8963b114d0.yaml b/releasenotes/notes/check-dynamic-allocation-enabled-e94f3b8963b114d0.yaml new file mode 100644 index 0000000000..0ed4179e05 --- /dev/null +++ b/releasenotes/notes/check-dynamic-allocation-enabled-e94f3b8963b114d0.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - dynamic_allocation flag in node's driver_info can now accept all + the values that can be converted to booleans by bool_from_string + function from oslo_utils.