Merge "Add Redfish inspect interface follow up"
This commit is contained in:
Zuul
Gerrit Code Review
commit
8105ba2660
@ -110,16 +110,15 @@ Out-Of-Band inspection
|
||||
^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
The ``redfish`` hardware type can inspect the bare metal node by querying
|
||||
Redfish BMC. This process if quick and reliable compared to the way
|
||||
how the ``inspector`` hardware type works e.g. booting bare metal node into
|
||||
the introspection ramdisk.
|
||||
Redfish compatible BMC. This process is quick and reliable compared to the
|
||||
way the ``inspector`` hardware type works i.e. booting bare metal node
|
||||
into the introspection ramdisk.
|
||||
|
||||
.. note::
|
||||
|
||||
The ``redfish`` inspect interface largely relies on the optional parts
|
||||
of the Redfish specification. Not all Redfish-compliant BMCs might serve
|
||||
the required information, in which case bare metal node inspection would
|
||||
fail.
|
||||
The ``redfish`` inspect interface relies on the optional parts of the
|
||||
Redfish specification. Not all Redfish-compliant BMCs might serve the
|
||||
required information, in which case bare metal node inspection will fail.
|
||||
|
||||
.. _Redfish: http://redfish.dmtf.org/
|
||||
.. _Sushy: https://git.openstack.org/cgit/openstack/sushy
|
||||
|
||||
@ -579,34 +579,6 @@ def get_single_nic_with_vif_port_id(task):
|
||||
return port.address
|
||||
|
||||
|
||||
def create_ports_if_not_exist(task, macs):
|
||||
"""Create ironic ports for the mac addresses.
|
||||
|
||||
Creates ironic ports for the mac addresses returned with inspection
|
||||
or as requested by operator.
|
||||
|
||||
:param task: a TaskManager instance.
|
||||
:param macs: A dictionary of port numbers to mac addresses
|
||||
returned by node inspection.
|
||||
|
||||
"""
|
||||
node = task.node
|
||||
for port_num, mac in macs.items():
|
||||
port_dict = {'address': mac, 'node_id': node.id}
|
||||
port = objects.Port(task.context, **port_dict)
|
||||
|
||||
try:
|
||||
port.create()
|
||||
LOG.info(_("Port %(port_num)s created for MAC address %(address)s "
|
||||
"for node %(node)s"),
|
||||
{'address': mac, 'node': node.uuid, 'port_num': port_num})
|
||||
except exception.MACAlreadyExists:
|
||||
LOG.warning(_("Port %(port_num)s already exists for MAC address"
|
||||
"%(address)s for node %(node)s"),
|
||||
{'address': mac, 'node': node.uuid,
|
||||
'port_num': port_num})
|
||||
|
||||
|
||||
def agent_get_clean_steps(task, interface=None, override_priorities=None):
|
||||
"""Get the list of cached clean steps from the agent.
|
||||
|
||||
|
||||
@ -22,8 +22,8 @@ from ironic.common import states
|
||||
from ironic.common import utils
|
||||
from ironic.conductor import utils as conductor_utils
|
||||
from ironic.drivers import base
|
||||
from ironic.drivers.modules import deploy_utils
|
||||
from ironic.drivers.modules.ilo import common as ilo_common
|
||||
from ironic.drivers.modules import inspect_utils
|
||||
|
||||
ilo_error = importutils.try_import('proliantutils.exception')
|
||||
|
||||
@ -244,7 +244,7 @@ class IloInspect(base.InspectInterface):
|
||||
task.node.save()
|
||||
|
||||
# Create ports for the nics detected.
|
||||
deploy_utils.create_ports_if_not_exist(task, result['macs'])
|
||||
inspect_utils.create_ports_if_not_exist(task, result['macs'])
|
||||
|
||||
LOG.debug("Node properties for %(node)s are updated as "
|
||||
"%(properties)s",
|
||||
|
||||
51
ironic/drivers/modules/inspect_utils.py
Normal file
51
ironic/drivers/modules/inspect_utils.py
Normal file
@ -0,0 +1,51 @@
|
||||
# Copyright 2018 Red Hat, Inc.
|
||||
# All Rights Reserved.
|
||||
#
|
||||
# Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||
# not use this file except in compliance with the License. You may obtain
|
||||
# a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
from oslo_log import log as logging
|
||||
|
||||
from ironic.common import exception
|
||||
from ironic import objects
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def create_ports_if_not_exist(task, macs):
|
||||
"""Create ironic ports for the mac addresses.
|
||||
|
||||
Creates ironic ports for the mac addresses returned with inspection
|
||||
or as requested by operator.
|
||||
|
||||
:param task: A TaskManager instance.
|
||||
:param macs: A dictionary of port numbers to mac addresses
|
||||
returned by node inspection.
|
||||
|
||||
"""
|
||||
node = task.node
|
||||
for port_num, mac in macs.items():
|
||||
# TODO(etingof): detect --pxe-enabled flag
|
||||
port_dict = {'address': mac, 'node_id': node.id}
|
||||
port = objects.Port(task.context, **port_dict)
|
||||
|
||||
try:
|
||||
port.create()
|
||||
LOG.info("Port %(port_num)s created for MAC address %(address)s "
|
||||
"for node %(node)s", {'address': mac, 'node': node.uuid,
|
||||
'port_num': port_num})
|
||||
except exception.MACAlreadyExists:
|
||||
LOG.warning("Port %(port_num)s already exists for "
|
||||
"MAC address %(address)s for node "
|
||||
"%(node)s", {'address': mac,
|
||||
'node': node.uuid,
|
||||
'port_num': port_num})
|
||||
@ -21,7 +21,7 @@ from ironic.common import exception
|
||||
from ironic.common.i18n import _
|
||||
from ironic.common import states
|
||||
from ironic.drivers import base
|
||||
from ironic.drivers.modules import deploy_utils
|
||||
from ironic.drivers.modules import inspect_utils
|
||||
from ironic.drivers.modules.redfish import utils as redfish_utils
|
||||
|
||||
LOG = log.getLogger(__name__)
|
||||
@ -108,10 +108,9 @@ class RedfishInspect(base.InspectInterface):
|
||||
inspected_properties['cpu_arch'] = CPU_ARCH_MAP[arch]
|
||||
|
||||
except KeyError:
|
||||
LOG.warning(
|
||||
_("Unknown CPU arch %(arch)s discovered "
|
||||
"for Node %(node)s"), {'node': task.node.uuid,
|
||||
'arch': arch})
|
||||
LOG.warning("Unknown CPU arch %(arch)s discovered "
|
||||
"for node %(node)s", {'node': task.node.uuid,
|
||||
'arch': arch})
|
||||
|
||||
simple_storage_size = 0
|
||||
|
||||
@ -125,10 +124,10 @@ class RedfishInspect(base.InspectInterface):
|
||||
|
||||
simple_storage_size = simple_storage_size[0]
|
||||
|
||||
except sushy.SushyError:
|
||||
LOG.info(
|
||||
_("No simple storage information discovered "
|
||||
"for Node %(node)s"), {'node': task.node.uuid})
|
||||
except sushy.SushyError as ex:
|
||||
LOG.debug("No simple storage information discovered "
|
||||
"for node %(node)s: %(err)s", {'node': task.node.uuid,
|
||||
'err': ex})
|
||||
|
||||
storage_size = 0
|
||||
|
||||
@ -141,23 +140,31 @@ class RedfishInspect(base.InspectInterface):
|
||||
|
||||
storage_size = storage_size[0]
|
||||
|
||||
except sushy.SushyError:
|
||||
LOG.info(_("No storage volume information discovered "
|
||||
"for Node %(node)s"), {'node': task.node.uuid})
|
||||
except sushy.SushyError as ex:
|
||||
LOG.debug("No storage volume information discovered "
|
||||
"for node %(node)s: %(err)s", {'node': task.node.uuid,
|
||||
'err': ex})
|
||||
|
||||
local_gb = max(simple_storage_size, storage_size)
|
||||
# NOTE(etingof): pick the smallest disk larger than 4G among available
|
||||
if simple_storage_size and storage_size:
|
||||
local_gb = min(simple_storage_size, storage_size)
|
||||
|
||||
else:
|
||||
local_gb = max(simple_storage_size, storage_size)
|
||||
|
||||
# Note(deray): Convert the received size to GiB and reduce the
|
||||
# value by 1 GB as consumers like Ironic requires the ``local_gb``
|
||||
# to be returned 1 less than actual size.
|
||||
local_gb = max(0, int(local_gb / units.Gi - 1))
|
||||
|
||||
# TODO(etingof): should we respect root device hints here?
|
||||
|
||||
if local_gb:
|
||||
inspected_properties['local_gb'] = str(local_gb)
|
||||
|
||||
else:
|
||||
LOG.warning(_("Could not provide a valid storage size configured "
|
||||
"for Node %(node)s"), {'node': task.node.uuid})
|
||||
LOG.warning("Could not provide a valid storage size configured "
|
||||
"for node %(node)s", {'node': task.node.uuid})
|
||||
|
||||
valid_keys = self.ESSENTIAL_PROPERTIES
|
||||
missing_keys = valid_keys - set(inspected_properties)
|
||||
@ -171,22 +178,19 @@ class RedfishInspect(base.InspectInterface):
|
||||
task.node.properties = inspected_properties
|
||||
task.node.save()
|
||||
|
||||
LOG.debug(_("Node properties for %(node)s are updated as "
|
||||
"%(properties)s"),
|
||||
{'properties': inspected_properties,
|
||||
'node': task.node.uuid})
|
||||
LOG.debug("Node properties for %(node)s are updated as "
|
||||
"%(properties)s", {'properties': inspected_properties,
|
||||
'node': task.node.uuid})
|
||||
|
||||
if (system.ethernet_interfaces and
|
||||
system.ethernet_interfaces.eth_summary):
|
||||
macs = system.ethernet_interfaces.eth_summary
|
||||
|
||||
# Create ports for the nics detected.
|
||||
deploy_utils.create_ports_if_not_exist(task, macs)
|
||||
inspect_utils.create_ports_if_not_exist(task, macs)
|
||||
|
||||
else:
|
||||
LOG.info(_("No NIC information discovered "
|
||||
"for Node %(node)s"), {'node': task.node.uuid})
|
||||
|
||||
LOG.info(_("Node %(node)s inspected."), {'node': task.node.uuid})
|
||||
LOG.warning("No NIC information discovered "
|
||||
"for node %(node)s", {'node': task.node.uuid})
|
||||
|
||||
return states.MANAGEABLE
|
||||
|
||||
@ -23,10 +23,10 @@ from ironic.common import states
|
||||
from ironic.common import utils
|
||||
from ironic.conductor import task_manager
|
||||
from ironic.conductor import utils as conductor_utils
|
||||
from ironic.drivers.modules import deploy_utils
|
||||
from ironic.drivers.modules.ilo import common as ilo_common
|
||||
from ironic.drivers.modules.ilo import inspect as ilo_inspect
|
||||
from ironic.drivers.modules.ilo import power as ilo_power
|
||||
from ironic.drivers.modules import inspect_utils
|
||||
from ironic.tests.unit.drivers.modules.ilo import test_common
|
||||
|
||||
|
||||
@ -51,7 +51,7 @@ class IloInspectTestCase(test_common.BaseIloTest):
|
||||
|
||||
@mock.patch.object(ilo_inspect, '_get_capabilities', spec_set=True,
|
||||
autospec=True)
|
||||
@mock.patch.object(deploy_utils, 'create_ports_if_not_exist',
|
||||
@mock.patch.object(inspect_utils, 'create_ports_if_not_exist',
|
||||
spec_set=True, autospec=True)
|
||||
@mock.patch.object(ilo_inspect, '_get_essential_properties', spec_set=True,
|
||||
autospec=True)
|
||||
@ -88,7 +88,7 @@ class IloInspectTestCase(test_common.BaseIloTest):
|
||||
spec_set=True, autospec=True)
|
||||
@mock.patch.object(ilo_inspect, '_get_capabilities', spec_set=True,
|
||||
autospec=True)
|
||||
@mock.patch.object(deploy_utils, 'create_ports_if_not_exist',
|
||||
@mock.patch.object(inspect_utils, 'create_ports_if_not_exist',
|
||||
spec_set=True, autospec=True)
|
||||
@mock.patch.object(ilo_inspect, '_get_essential_properties', spec_set=True,
|
||||
autospec=True)
|
||||
@ -131,7 +131,7 @@ class IloInspectTestCase(test_common.BaseIloTest):
|
||||
|
||||
@mock.patch.object(ilo_inspect, '_get_capabilities', spec_set=True,
|
||||
autospec=True)
|
||||
@mock.patch.object(deploy_utils, 'create_ports_if_not_exist',
|
||||
@mock.patch.object(inspect_utils, 'create_ports_if_not_exist',
|
||||
spec_set=True, autospec=True)
|
||||
@mock.patch.object(ilo_inspect, '_get_essential_properties', spec_set=True,
|
||||
autospec=True)
|
||||
@ -170,7 +170,7 @@ class IloInspectTestCase(test_common.BaseIloTest):
|
||||
|
||||
@mock.patch.object(ilo_inspect, '_get_capabilities', spec_set=True,
|
||||
autospec=True)
|
||||
@mock.patch.object(deploy_utils, 'create_ports_if_not_exist',
|
||||
@mock.patch.object(inspect_utils, 'create_ports_if_not_exist',
|
||||
spec_set=True, autospec=True)
|
||||
@mock.patch.object(ilo_inspect, '_get_essential_properties', spec_set=True,
|
||||
autospec=True)
|
||||
@ -209,7 +209,7 @@ class IloInspectTestCase(test_common.BaseIloTest):
|
||||
|
||||
@mock.patch.object(ilo_inspect, '_get_capabilities', spec_set=True,
|
||||
autospec=True)
|
||||
@mock.patch.object(deploy_utils, 'create_ports_if_not_exist',
|
||||
@mock.patch.object(inspect_utils, 'create_ports_if_not_exist',
|
||||
spec_set=True, autospec=True)
|
||||
@mock.patch.object(ilo_inspect, '_get_essential_properties', spec_set=True,
|
||||
autospec=True)
|
||||
|
||||
@ -19,7 +19,7 @@ from oslo_utils import units
|
||||
|
||||
from ironic.common import exception
|
||||
from ironic.conductor import task_manager
|
||||
from ironic.drivers.modules import deploy_utils
|
||||
from ironic.drivers.modules import inspect_utils
|
||||
from ironic.drivers.modules.redfish import utils as redfish_utils
|
||||
from ironic.tests.unit.db import base as db_base
|
||||
from ironic.tests.unit.db import utils as db_utils
|
||||
@ -79,13 +79,13 @@ class RedfishInspectTestCase(db_base.DbTestCase):
|
||||
mock_parse_driver_info.assert_called_once_with(task.node)
|
||||
|
||||
@mock.patch.object(redfish_utils, 'get_system', autospec=True)
|
||||
@mock.patch.object(deploy_utils, 'create_ports_if_not_exist',
|
||||
@mock.patch.object(inspect_utils, 'create_ports_if_not_exist',
|
||||
autospec=True)
|
||||
def test_inspect_hardware_ok(self, mock_create_ports_if_not_exist,
|
||||
mock_get_system):
|
||||
expected_properties = {
|
||||
'cpu_arch': 'mips', 'cpus': '8',
|
||||
'local_gb': '4', 'memory_mb': '2048'
|
||||
'local_gb': '3', 'memory_mb': '2048'
|
||||
}
|
||||
|
||||
system = self.init_system_mock(mock_get_system.return_value)
|
||||
@ -118,7 +118,7 @@ class RedfishInspectTestCase(db_base.DbTestCase):
|
||||
shared=True) as task:
|
||||
expected_properties = {
|
||||
'cpu_arch': 'x86_64', 'cpus': '8',
|
||||
'local_gb': '4', 'memory_mb': '2048'
|
||||
'local_gb': '3', 'memory_mb': '2048'
|
||||
}
|
||||
task.driver.inspect.inspect_hardware(task)
|
||||
self.assertEqual(expected_properties, task.node.properties)
|
||||
@ -170,13 +170,13 @@ class RedfishInspectTestCase(db_base.DbTestCase):
|
||||
shared=True) as task:
|
||||
expected_properties = {
|
||||
'cpu_arch': 'mips', 'cpus': '8',
|
||||
'local_gb': '4', 'memory_mb': '4096'
|
||||
'local_gb': '3', 'memory_mb': '4096'
|
||||
}
|
||||
task.driver.inspect.inspect_hardware(task)
|
||||
self.assertEqual(expected_properties, task.node.properties)
|
||||
|
||||
@mock.patch.object(redfish_utils, 'get_system', autospec=True)
|
||||
@mock.patch.object(deploy_utils, 'create_ports_if_not_exist',
|
||||
@mock.patch.object(inspect_utils, 'create_ports_if_not_exist',
|
||||
autospec=True)
|
||||
def test_inspect_hardware_ignore_missing_nics(
|
||||
self, mock_create_ports_if_not_exist, mock_get_system):
|
||||
|
||||
@ -44,7 +44,6 @@ from ironic.drivers.modules import image_cache
|
||||
from ironic.drivers.modules import pxe
|
||||
from ironic.drivers.modules.storage import cinder
|
||||
from ironic.drivers import utils as driver_utils
|
||||
from ironic import objects
|
||||
from ironic.tests import base as tests_base
|
||||
from ironic.tests.unit.db import base as db_base
|
||||
from ironic.tests.unit.db import utils as db_utils
|
||||
@ -1308,38 +1307,6 @@ class OtherFunctionTestCase(db_base.DbTestCase):
|
||||
self.assertRaises(exception.InvalidParameterValue,
|
||||
utils.get_ironic_api_url)
|
||||
|
||||
@mock.patch.object(utils.LOG, 'info', spec_set=True, autospec=True)
|
||||
@mock.patch.object(objects, 'Port', spec_set=True, autospec=True)
|
||||
def test_create_ports_if_not_exist(self, port_mock, log_mock):
|
||||
macs = {'Port 1': 'aa:aa:aa:aa:aa:aa', 'Port 2': 'bb:bb:bb:bb:bb:bb'}
|
||||
node_id = self.node.id
|
||||
port_dict1 = {'address': 'aa:aa:aa:aa:aa:aa', 'node_id': node_id}
|
||||
port_dict2 = {'address': 'bb:bb:bb:bb:bb:bb', 'node_id': node_id}
|
||||
port_obj1, port_obj2 = mock.MagicMock(), mock.MagicMock()
|
||||
port_mock.side_effect = [port_obj1, port_obj2]
|
||||
with task_manager.acquire(self.context, self.node.uuid,
|
||||
shared=False) as task:
|
||||
utils.create_ports_if_not_exist(task, macs)
|
||||
self.assertTrue(log_mock.called)
|
||||
expected_calls = [mock.call(task.context, **port_dict1),
|
||||
mock.call(task.context, **port_dict2)]
|
||||
port_mock.assert_has_calls(expected_calls, any_order=True)
|
||||
port_obj1.create.assert_called_once_with()
|
||||
port_obj2.create.assert_called_once_with()
|
||||
|
||||
@mock.patch.object(utils.LOG, 'warning',
|
||||
spec_set=True, autospec=True)
|
||||
@mock.patch.object(objects.Port, 'create', spec_set=True, autospec=True)
|
||||
def test_create_ports_if_not_exist_mac_exception(self,
|
||||
create_mock,
|
||||
log_mock):
|
||||
create_mock.side_effect = exception.MACAlreadyExists('f')
|
||||
macs = {'Port 1': 'aa:aa:aa:aa:aa:aa', 'Port 2': 'bb:bb:bb:bb:bb:bb'}
|
||||
with task_manager.acquire(self.context, self.node.uuid,
|
||||
shared=False) as task:
|
||||
utils.create_ports_if_not_exist(task, macs)
|
||||
self.assertEqual(2, log_mock.call_count)
|
||||
|
||||
|
||||
class GetSingleNicTestCase(db_base.DbTestCase):
|
||||
|
||||
|
||||
65
ironic/tests/unit/drivers/modules/test_inspect_utils.py
Normal file
65
ironic/tests/unit/drivers/modules/test_inspect_utils.py
Normal file
@ -0,0 +1,65 @@
|
||||
# Copyright 2018 Red Hat, Inc.
|
||||
# All Rights Reserved.
|
||||
#
|
||||
# Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||
# not use this file except in compliance with the License. You may obtain
|
||||
# a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
|
||||
import mock
|
||||
|
||||
from ironic.common import exception
|
||||
from ironic.conductor import task_manager
|
||||
from ironic.drivers.modules import inspect_utils as utils
|
||||
from ironic import objects
|
||||
from ironic.tests.unit.db import base as db_base
|
||||
from ironic.tests.unit.objects import utils as obj_utils
|
||||
|
||||
|
||||
@mock.patch('time.sleep', lambda sec: None)
|
||||
class InspectFunctionTestCase(db_base.DbTestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(InspectFunctionTestCase, self).setUp()
|
||||
self.node = obj_utils.create_test_node(self.context,
|
||||
boot_interface='pxe')
|
||||
|
||||
@mock.patch.object(utils.LOG, 'info', spec_set=True, autospec=True)
|
||||
@mock.patch.object(objects, 'Port', spec_set=True, autospec=True)
|
||||
def test_create_ports_if_not_exist(self, port_mock, log_mock):
|
||||
macs = {'Port 1': 'aa:aa:aa:aa:aa:aa', 'Port 2': 'bb:bb:bb:bb:bb:bb'}
|
||||
node_id = self.node.id
|
||||
port_dict1 = {'address': 'aa:aa:aa:aa:aa:aa', 'node_id': node_id}
|
||||
port_dict2 = {'address': 'bb:bb:bb:bb:bb:bb', 'node_id': node_id}
|
||||
port_obj1, port_obj2 = mock.MagicMock(), mock.MagicMock()
|
||||
port_mock.side_effect = [port_obj1, port_obj2]
|
||||
with task_manager.acquire(self.context, self.node.uuid,
|
||||
shared=False) as task:
|
||||
utils.create_ports_if_not_exist(task, macs)
|
||||
self.assertTrue(log_mock.called)
|
||||
expected_calls = [mock.call(task.context, **port_dict1),
|
||||
mock.call(task.context, **port_dict2)]
|
||||
port_mock.assert_has_calls(expected_calls, any_order=True)
|
||||
port_obj1.create.assert_called_once_with()
|
||||
port_obj2.create.assert_called_once_with()
|
||||
|
||||
@mock.patch.object(utils.LOG, 'warning',
|
||||
spec_set=True, autospec=True)
|
||||
@mock.patch.object(objects.Port, 'create', spec_set=True, autospec=True)
|
||||
def test_create_ports_if_not_exist_mac_exception(self,
|
||||
create_mock,
|
||||
log_mock):
|
||||
create_mock.side_effect = exception.MACAlreadyExists('f')
|
||||
macs = {'Port 1': 'aa:aa:aa:aa:aa:aa', 'Port 2': 'bb:bb:bb:bb:bb:bb'}
|
||||
with task_manager.acquire(self.context, self.node.uuid,
|
||||
shared=False) as task:
|
||||
utils.create_ports_if_not_exist(task, macs)
|
||||
self.assertEqual(2, log_mock.call_count)
|
||||
Loading…
Reference in New Issue
Block a user