diff --git a/nova/tests/unit/virt/ironic/test_driver.py b/nova/tests/unit/virt/ironic/test_driver.py index 9408b7a2341f..540b7c87368a 100644 --- a/nova/tests/unit/virt/ironic/test_driver.py +++ b/nova/tests/unit/virt/ironic/test_driver.py @@ -731,9 +731,11 @@ class IronicDriverTestCase(test.NoDBTestCase): expected_uuids = [n['uuid'] for n in node_dicts if n['expected']] self.assertEqual(sorted(expected_uuids), sorted(available_nodes)) + @mock.patch.object(ironic_driver.IronicDriver, + '_node_resources_unavailable', return_value=False) @mock.patch.object(ironic_driver.IronicDriver, '_node_resource') @mock.patch.object(ironic_driver.IronicDriver, '_node_from_cache') - def test_get_inventory_no_rc(self, mock_nfc, mock_nr): + def test_get_inventory_no_rc(self, mock_nfc, mock_nr, mock_res_unavail): """Ensure that when node.resource_class is missing, that we return the legacy VCPU, MEMORY_MB and DISK_GB resources for inventory. """ @@ -777,11 +779,14 @@ class IronicDriverTestCase(test.NoDBTestCase): } mock_nfc.assert_called_once_with(mock.sentinel.nodename) mock_nr.assert_called_once_with(mock_nfc.return_value) + mock_res_unavail.assert_called_once_with(mock_nfc.return_value) self.assertEqual(expected, result) + @mock.patch.object(ironic_driver.IronicDriver, + '_node_resources_unavailable', return_value=False) @mock.patch.object(ironic_driver.IronicDriver, '_node_resource') @mock.patch.object(ironic_driver.IronicDriver, '_node_from_cache') - def test_get_inventory_with_rc(self, mock_nfc, mock_nr): + def test_get_inventory_with_rc(self, mock_nfc, mock_nr, mock_res_unavail): """Ensure that when node.resource_class is present, that we return the legacy VCPU, MEMORY_MB and DISK_GB resources for inventory in addition to the custom resource class inventory record. @@ -834,11 +839,51 @@ class IronicDriverTestCase(test.NoDBTestCase): } mock_nfc.assert_called_once_with(mock.sentinel.nodename) mock_nr.assert_called_once_with(mock_nfc.return_value) + mock_res_unavail.assert_called_once_with(mock_nfc.return_value) self.assertEqual(expected, result) + @mock.patch.object(ironic_driver.IronicDriver, + '_node_resources_unavailable', return_value=False) @mock.patch.object(ironic_driver.IronicDriver, '_node_resource') @mock.patch.object(ironic_driver.IronicDriver, '_node_from_cache') - def test_get_inventory_with_rc_occupied(self, mock_nfc, mock_nr): + def test_get_inventory_only_rc(self, mock_nfc, mock_nr, mock_res_unavail): + """Ensure that when node.resource_class is present, that we return the + legacy VCPU, MEMORY_MB and DISK_GB resources for inventory in addition + to the custom resource class inventory record. + """ + mock_nr.return_value = { + 'vcpus': 0, + 'vcpus_used': 0, + 'memory_mb': 0, + 'memory_mb_used': 0, + 'local_gb': 0, + 'local_gb_used': 0, + 'resource_class': 'iron-nfv', + } + + result = self.driver.get_inventory(mock.sentinel.nodename) + + expected = { + 'CUSTOM_IRON_NFV': { + 'total': 1, + 'reserved': 0, + 'min_unit': 1, + 'max_unit': 1, + 'step_size': 1, + 'allocation_ratio': 1.0, + }, + } + mock_nfc.assert_called_once_with(mock.sentinel.nodename) + mock_nr.assert_called_once_with(mock_nfc.return_value) + mock_res_unavail.assert_called_once_with(mock_nfc.return_value) + self.assertEqual(expected, result) + + @mock.patch.object(ironic_driver.IronicDriver, + '_node_resources_unavailable', return_value=False) + @mock.patch.object(ironic_driver.IronicDriver, '_node_resource') + @mock.patch.object(ironic_driver.IronicDriver, '_node_from_cache') + def test_get_inventory_with_rc_occupied(self, mock_nfc, mock_nr, + mock_res_unavail): """Ensure that when a node is used, we report the inventory matching the consumed resources. """ @@ -890,27 +935,19 @@ class IronicDriverTestCase(test.NoDBTestCase): } mock_nfc.assert_called_once_with(mock.sentinel.nodename) mock_nr.assert_called_once_with(mock_nfc.return_value) + mock_res_unavail.assert_called_once_with(mock_nfc.return_value) self.assertEqual(expected, result) - @mock.patch.object(ironic_driver.IronicDriver, '_node_resource') + @mock.patch.object(ironic_driver.IronicDriver, + '_node_resources_unavailable', return_value=True) @mock.patch.object(ironic_driver.IronicDriver, '_node_from_cache') - def test_get_inventory_disabled_node(self, mock_nfc, mock_nr): - """Ensure that when vcpus == 0 (which happens when a node is disabled), - that get_inventory() returns an empty dict. + def test_get_inventory_disabled_node(self, mock_nfc, mock_res_unavail): + """Ensure that when a node is disabled, that get_inventory() returns + an empty dict. """ - mock_nr.return_value = { - 'vcpus': 0, - 'vcpus_used': 0, - 'memory_mb': 0, - 'memory_mb_used': 0, - 'local_gb': 0, - 'local_gb_used': 0, - 'resource_class': None, - } - result = self.driver.get_inventory(mock.sentinel.nodename) mock_nfc.assert_called_once_with(mock.sentinel.nodename) - mock_nr.assert_called_once_with(mock_nfc.return_value) + mock_res_unavail.assert_called_once_with(mock_nfc.return_value) self.assertEqual({}, result) @mock.patch.object(FAKE_CLIENT.node, 'get') diff --git a/nova/virt/ironic/driver.py b/nova/virt/ironic/driver.py index edee96979244..234f1a116f84 100644 --- a/nova/virt/ironic/driver.py +++ b/nova/virt/ironic/driver.py @@ -754,16 +754,13 @@ class IronicDriver(virt_driver.ComputeDriver): """ # nodename is the ironic node's UUID. node = self._node_from_cache(nodename) - info = self._node_resource(node) # TODO(jaypipes): Completely remove the reporting of VCPU, MEMORY_MB, # and DISK_GB resource classes in early Queens when Ironic nodes will # *always* return the custom resource class that represents the # baremetal node class in an atomic, singular unit. - if info['vcpus'] == 0: - # NOTE(jaypipes): The driver can return 0-valued vcpus when the - # node is "disabled". In the future, we should detach inventory - # accounting from the concept of a node being disabled or not. The - # two things don't really have anything to do with each other. + if self._node_resources_unavailable(node): + # TODO(dtantsur): report resources as reserved instead of reporting + # an empty inventory LOG.debug('Node %(node)s is not ready for a deployment, ' 'reporting an empty inventory for it. Node\'s ' 'provision state is %(prov)s, power state is ' @@ -772,32 +769,23 @@ class IronicDriver(virt_driver.ComputeDriver): 'power': node.power_state, 'maint': node.maintenance}) return {} - result = { - obj_fields.ResourceClass.VCPU: { - 'total': info['vcpus'], - 'reserved': 0, - 'min_unit': 1, - 'max_unit': info['vcpus'], - 'step_size': 1, - 'allocation_ratio': 1.0, - }, - obj_fields.ResourceClass.MEMORY_MB: { - 'total': info['memory_mb'], - 'reserved': 0, - 'min_unit': 1, - 'max_unit': info['memory_mb'], - 'step_size': 1, - 'allocation_ratio': 1.0, - }, - obj_fields.ResourceClass.DISK_GB: { - 'total': info['local_gb'], - 'reserved': 0, - 'min_unit': 1, - 'max_unit': info['local_gb'], - 'step_size': 1, - 'allocation_ratio': 1.0, - }, - } + info = self._node_resource(node) + result = {} + for rc, field in [(obj_fields.ResourceClass.VCPU, 'vcpus'), + (obj_fields.ResourceClass.MEMORY_MB, 'memory_mb'), + (obj_fields.ResourceClass.DISK_GB, 'local_gb')]: + # NOTE(dtantsur): any of these fields can be zero starting with + # the Pike release. + if info[field]: + result[rc] = { + 'total': info[field], + 'reserved': 0, + 'min_unit': 1, + 'max_unit': info[field], + 'step_size': 1, + 'allocation_ratio': 1.0, + } + rc_name = info.get('resource_class') if rc_name is not None: # TODO(jaypipes): Raise an exception in Queens if Ironic doesn't diff --git a/releasenotes/notes/ironic-empty-vcpus-66b4e1500ef8a34e.yaml b/releasenotes/notes/ironic-empty-vcpus-66b4e1500ef8a34e.yaml new file mode 100644 index 000000000000..2dbb0b5c9155 --- /dev/null +++ b/releasenotes/notes/ironic-empty-vcpus-66b4e1500ef8a34e.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixes a bug preventing ironic nodes without VCPUs, memory or disk in their + properties from being picked by nova.