openstack
/
ironic
Code Issues Proposed changes

Decouple state inspection and availability check 

The state of a node should only be considered when calculating
resources, not when determining availability.  This makes the driver
consider any node that exists in the node inventory as available, and
take into account its state when determining available resources. All nodes
will be presented to Nova, but those that are in maintenence mode, have no
power state or have an error power state will present zero resources.

Closes-Bug: #1309048
Change-Id: Ia78294ac5d39955dac99d3d773751e8f7538a74d
This commit is contained in:
Adam Gandelman 2014-04-17 17:47:26 -07:00
parent 2291251b3d
commit 1f73e37cba
2 changed files with 102 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

117
ironic/nova/tests/virt/ironic/test_driver.py
View File

@ -223,6 +223,7 @@ class IronicDriverTestCase(test.NoDBTestCase):
'local_gb': disk, 'cpu_arch': arch}
node = ironic_utils.get_test_node(uuid=node_uuid,
instance_uuid=None,
power_state=ironic_states.POWER_OFF,
properties=properties)
result = self.driver._node_resource(node)
@ -238,6 +239,34 @@ class IronicDriverTestCase(test.NoDBTestCase):
'"test_spec": "test_value"}',
result['stats'])
@mock.patch.object(ironic_driver.IronicDriver,
'_node_resources_unavailable')
def test__node_resource_unavailable_node_res(self, mock_res_unavail):
mock_res_unavail.return_value = True
node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
cpus = 2
mem = 512
disk = 10
arch = 'x86_64'
properties = {'cpus': cpus, 'memory_mb': mem,
'local_gb': disk, 'cpu_arch': arch}
node = ironic_utils.get_test_node(uuid=node_uuid,
instance_uuid=None,
properties=properties)
result = self.driver._node_resource(node)
self.assertEqual(0, result['vcpus'])
self.assertEqual(0, result['vcpus_used'])
self.assertEqual(0, result['memory_mb'])
self.assertEqual(0, result['memory_mb_used'])
self.assertEqual(0, result['local_gb'])
self.assertEqual(0, result['local_gb_used'])
self.assertEqual(node_uuid, result['hypervisor_hostname'])
self.assertEqual('{"cpu_arch": "x86_64", "ironic_driver": "'
'ironic.nova.virt.ironic.driver.IronicDriver", '
'"test_spec": "test_value"}',
result['stats'])
def test__start_firewall(self):
func_list = ['setup_basic_filtering',
'prepare_instance_filter',
@ -284,51 +313,56 @@ class IronicDriverTestCase(test.NoDBTestCase):
@mock.patch.object(FAKE_CLIENT.node, 'get')
def test_node_is_available(self, mock_get):
no_guid = None
any_guid = uuidutils.generate_uuid()
in_maintenance = True
is_available = True
not_in_maintenance = False
not_available = False
power_off = ironic_states.POWER_OFF
not_power_off = ironic_states.POWER_ON
node = ironic_utils.get_test_node()
mock_get.return_value = node
self.assertTrue(self.driver.node_is_available(node.uuid))
mock_get.assert_called_with(node.uuid)
testing_set = {(no_guid, not_in_maintenance, power_off):is_available,
(no_guid, not_in_maintenance, not_power_off):not_available,
(no_guid, in_maintenance, power_off):not_available,
(no_guid, in_maintenance, not_power_off):not_available,
(any_guid, not_in_maintenance, power_off):not_available,
(any_guid, not_in_maintenance, not_power_off):not_available,
(any_guid, in_maintenance, power_off):not_available,
(any_guid, in_maintenance, not_power_off):not_available}
mock_get.side_effect = ironic_exception.HTTPNotFound
self.assertFalse(self.driver.node_is_available(node.uuid))
for key in testing_set.keys():
node = ironic_utils.get_test_node(instance_uuid=key[0],
maintenance=key[1],
power_state=key[2])
mock_get.return_value = node
expected = testing_set[key]
observed = self.driver.node_is_available("dummy_nodename")
self.assertEqual(expected, observed)
def test__node_resources_unavailable(self):
node_dicts = [
# a node in maintenance /w no instance and power OFF
{'uuid': uuidutils.generate_uuid(),
'maintenance': True,
'power_state': ironic_states.POWER_OFF},
# a node in maintenance /w no instance and ERROR power state
{'uuid': uuidutils.generate_uuid(),
'maintenance': True,
'power_state': ironic_states.ERROR},
# a node not in maintenance /w no instance and bad power state
{'uuid': uuidutils.generate_uuid(),
'power_state': ironic_states.NOSTATE},
]
for n in node_dicts:
node = ironic_utils.get_test_node(**n)
self.assertTrue(self.driver._node_resources_unavailable(node))
def test_get_available_nodes(self):
num_nodes = 2
nodes = []
for n in range(num_nodes):
nodes.append(ironic_utils.get_test_node(
uuid=uuidutils.generate_uuid(),
power_state=ironic_states.POWER_OFF))
# append a node w/o power_state which shouldn't be listed
nodes.append(ironic_utils.get_test_node(power_state=None))
avail_node = ironic_utils.get_test_node(
power_state=ironic_states.POWER_OFF)
self.assertFalse(self.driver._node_resources_unavailable(avail_node))
with mock.patch.object(cw.IronicClientWrapper, 'call') as mock_list:
mock_list.return_value = nodes
expected = [n.uuid for n in nodes if n.power_state]
available_nodes = self.driver.get_available_nodes()
mock_list.assert_called_with("node.list")
self.assertEqual(sorted(expected), sorted(available_nodes))
self.assertEqual(num_nodes, len(available_nodes))
@mock.patch.object(FAKE_CLIENT.node, 'list')
def test_get_available_nodes(self, mock_list):
node_dicts = [
# a node in maintenance /w no instance and power OFF
{'uuid': uuidutils.generate_uuid(),
'maintenance': True,
'power_state': ironic_states.POWER_OFF},
# a node /w instance and power ON
{'uuid': uuidutils.generate_uuid(),
'instance_uuid': uuidutils.generate_uuid(),
'power_state': ironic_states.POWER_ON},
# a node not in maintenance /w no instance and bad power state
{'uuid': uuidutils.generate_uuid(),
'power_state': ironic_states.ERROR},
]
nodes = [ironic_utils.get_test_node(**n) for n in node_dicts]
mock_list.return_value = nodes
available_nodes = self.driver.get_available_nodes()
expected_uuids = [n['uuid'] for n in node_dicts]
self.assertEqual(sorted(expected_uuids), sorted(available_nodes))
def test_get_available_resource(self):
node = ironic_utils.get_test_node()
@ -340,6 +374,7 @@ class IronicDriverTestCase(test.NoDBTestCase):
with mock.patch.object(self.driver, '_node_resource') as mock_nr:
mock_nr.return_value = fake_resource
result = self.driver.get_available_resource(node.uuid)
self.assertEqual(fake_resource, result)
mock_nr.assert_called_once_with(node)

44
ironic/nova/virt/ironic/driver.py
View File

@ -21,7 +21,6 @@ A driver wrapping the Ironic API, such that Nova may provision
bare metal resources.
"""
from ironicclient import client as ironic_client
from ironicclient import exc as ironic_exception
from oslo.config import cfg
@ -33,7 +32,6 @@ from nova import exception
from nova.objects import flavor as flavor_obj
from nova.openstack.common import excutils
from nova.openstack.common.gettextutils import _
from nova.openstack.common import importutils
from nova.openstack.common import jsonutils
from nova.openstack.common import log as logging
from nova.openstack.common import loopingcall
@ -160,9 +158,16 @@ class IronicDriver(virt_driver.ComputeDriver):
self.extra_specs = extra_specs
def _node_resources_unavailable(self, node_obj):
"""Determines whether the node's resources should be presented
to Nova for use based on the current power and maintenance state.
"""
bad_states = [ironic_states.ERROR, ironic_states.NOSTATE]
return (node_obj.maintenance or
node_obj.power_state in bad_states)
def _node_resource(self, node):
"""Helper method to create resource dict from node stats."""
vcpus = int(node.properties.get('cpus', 0))
memory_mb = int(node.properties.get('memory_mb', 0))
local_gb = int(node.properties.get('local_gb', 0))
@ -170,14 +175,21 @@ class IronicDriver(virt_driver.ComputeDriver):
nodes_extra_specs = self.extra_specs
nodes_extra_specs['cpu_arch'] = cpu_arch
vcpus_used = 0
memory_mb_used = 0
local_gb_used = 0
if node.instance_uuid:
# Node has an instance, report all resource as unavailable
vcpus_used = vcpus
memory_mb_used = memory_mb
local_gb_used = local_gb
else:
vcpus_used = 0
memory_mb_used = 0
local_gb_used = 0
elif self._node_resources_unavailable(node):
# The node's current state is such that it should not present any
# of its resources to Nova
vcpus = 0
memory_mb = 0
local_gb = 0
dic = {'node': str(node.uuid),
'hypervisor_hostname': str(node.uuid),
@ -270,22 +282,18 @@ class IronicDriver(virt_driver.ComputeDriver):
return instances
def node_is_available(self, nodename):
"""Confirms a Nova hypervisor node exists in the Ironic inventory."""
icli = client_wrapper.IronicClientWrapper()
node = icli.call("node.get", nodename)
return not node.instance_uuid and not node.maintenance \
and node.power_state == ironic_states.POWER_OFF
try:
icli.call("node.get", nodename)
return True
except ironic_exception.HTTPNotFound:
return False
def get_available_nodes(self, refresh=False):
nodes = []
icli = client_wrapper.IronicClientWrapper()
node_list = icli.call("node.list")
for n in node_list:
# for now we'll use the nodes power state. if power_state is None
# we'll assume it is not ready to be presented to Nova.
if n.power_state:
nodes.append(n.uuid)
nodes = [n.uuid for n in node_list]
LOG.debug("Returning Nodes: %s" % nodes)
return nodes