Merge "Fix nodes count check on stack-update with custom roles"

tripleoclient/tests/test_utils.py

@@ -248,7 +248,7 @@ class TestWaitForStackUtil(TestCase):
         self.assertRaises(ValueError, utils.file_checksum, '/dev/zero')
 
 
-class TestCheckNodesCount(TestCase):
+class TestCheckNodesCountCreate(TestCase):
 
     def setUp(self):
         self.baremetal = mock.Mock()
@@ -259,7 +259,7 @@ class TestCheckNodesCount(TestCase):
             'BlockStorageCount': 0,
             'CephStorageCount': 0,
         }
-        self.stack = mock.Mock(parameters=self.defaults)
+        self.stack = None
 
         def ironic_node_list(*args, **kwargs):
             if kwargs.get('associated') is True:
@@ -293,13 +293,72 @@ class TestCheckNodesCount(TestCase):
                          utils.check_nodes_count(self.baremetal, self.stack,
                                                  user_params, self.defaults))
 
+
+class TestCheckNodesCountUpdate(TestCheckNodesCountCreate):
+
+    def setUp(self):
+        super(TestCheckNodesCountUpdate, self).setUp()
+        self.stack = mock.Mock(parameters=self.defaults)
+
     def test_check_default_param_not_in_stack(self):
+        user_params = {'ControllerCount': 3}
         missing_param = 'CephStorageCount'
         self.stack.parameters = self.defaults.copy()
         del self.stack.parameters[missing_param]
 
-        self.assertRaises(ValueError, utils.check_nodes_count,
+        self.assertEqual((False, 4, 3),
-                          self.baremetal, self.stack, dict(), self.defaults)
+                         utils.check_nodes_count(self.baremetal, self.stack,
+                                                 user_params, self.defaults))
+
+
+class TestCheckNodesCountCustomRolesCreate(TestCase):
+
+    def setUp(self):
+        self.baremetal = mock.Mock()
+        self.custom_roles_defaults = {
+            'ControllerApiCount': 3,
+            'ControllerPacemakerCount': 3,
+            'ComputeDvrCount': 3
+        }
+        self.stack = None
+
+        def ironic_node_list(*args, **kwargs):
+            if kwargs.get('associated') is True:
+                nodes = range(2)
+            elif kwargs.get('maintenance') is False:
+                nodes = range(9)
+            return nodes
+        self.baremetal.node.list.side_effect = ironic_node_list
+
+    def test_check_nodes_count_custom_roles_scale_enough_nodes(self):
+        user_params = {
+            'ControllerApiCount': 3,
+            'ControllerPacemakerCount': 3,
+            'ComputeDvrCount': 3
+        }
+        self.assertEqual((True, 9, 11),
+                         utils.check_nodes_count(self.baremetal, self.stack,
+                                                 user_params,
+                                                 self.custom_roles_defaults))
+
+    def test_check_nodes_count_custom_roles_scale_too_much(self):
+        user_params = {
+            'ControllerApiCount': 3,
+            'ControllerPacemakerCount': 3,
+            'ComputeDvrCount': 6
+        }
+        self.assertEqual((False, 12, 11),
+                         utils.check_nodes_count(self.baremetal, self.stack,
+                                                 user_params,
+                                                 self.custom_roles_defaults))
+
+
+class TestCheckNodesCountCustomRolesUpdate(
+        TestCheckNodesCountCustomRolesCreate):
+
+    def setUp(self):
+        super(TestCheckNodesCountCustomRolesUpdate, self).setUp()
+        self.stack = mock.Mock(parameters=self.custom_roles_defaults)
 
 
 class TestEnsureRunAsNormalUser(TestCase):

tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py

@@ -1107,6 +1107,8 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
     @mock.patch('tripleoclient.utils.check_nodes_count')
     @mock.patch('tripleoclient.utils.check_hypervisor_stats')
     @mock.patch('tripleoclient.utils.assign_and_verify_profiles')
+    @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.'
+                '_get_default_role_counts')
     @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.'
                 '_check_ironic_boot_configuration')
     @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.'
@@ -1114,6 +1116,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
     def test_predeploy_verify_capabilities_hypervisor_stats(
             self, mock_collect_flavors,
             mock_check_ironic_boot_configuration,
+            mock_get_default_role_counts,
             mock_assign_and_verify_profiles,
             mock_check_hypervisor_stats,
             mock_check_nodes_count):

tripleoclient/utils.py

@@ -515,16 +515,17 @@ def file_checksum(filepath):
 def check_nodes_count(baremetal_client, stack, parameters, defaults):
     """Check if there are enough available nodes for creating/scaling stack"""
     count = 0
-    if stack:
+
-        for param in defaults:
+    for param, default in defaults.items():
+        if stack:
             try:
                 current = int(stack.parameters[param])
             except KeyError:
-                raise ValueError(
+                # We could be adding a new role on stack-update, so there's no
-                    "Parameter '%s' was not found in existing stack" % param)
+                # assumption the parameter exists in the stack.
+                current = parameters.get(param, default)
             count += parameters.get(param, current)
-    else:
+        else:
-        for param, default in defaults.items():
             count += parameters.get(param, default)
 
     # We get number of nodes usable for the stack by getting already

tripleoclient/v1/overcloud_deploy.py

@@ -709,6 +709,28 @@ class DeployOvercloud(command.Command):
                                           "specified when Neutron tunnel "
                                           "types is specified")
 
+    def _get_default_role_counts(self, parsed_args):
+
+        if parsed_args.roles_file:
+            roles_data = yaml.safe_load(open(parsed_args.roles_file).read())
+        else:
+            # Assume default role counts
+            return {
+                'ControllerCount': 1,
+                'ComputeCount': 1,
+                'ObjectStorageCount': 0,
+                'BlockStorageCount': 0,
+                'CephStorageCount': 0
+            }
+
+        default_role_counts = {}
+        for r in roles_data:
+            default_role_counts.setdefault(
+                "%sCount" % r['name'],
+                r['CountDefault'])
+
+        return default_role_counts
+
     def _predeploy_verify_capabilities(self, stack, parameters, parsed_args):
         self.predeploy_errors = 0
         self.predeploy_warnings = 0
@@ -738,17 +760,12 @@ class DeployOvercloud(command.Command):
             self.predeploy_errors += 1
 
         self.log.debug("Checking nodes count")
+        default_role_counts = self._get_default_role_counts(parsed_args)
         enough_nodes, count, ironic_nodes_count = utils.check_nodes_count(
             bm_client,
             stack,
             parameters,
-            {
+            default_role_counts
-                'ControllerCount': 1,
-                'ComputeCount': 1,
-                'ObjectStorageCount': 0,
-                'BlockStorageCount': 0,
-                'CephStorageCount': 0,
-            }
         )
         if not enough_nodes:
             self.log.error(