Move create_ports to NodeInfo
Makes code more consistent and fixes bug with ironicclient error reporting. Also make NodeInfo.ports() return a dict to be more useful for callers (like process.py). Change-Id: I6ba587a31e839aba511c16fbf93047c7fdc54cc2 Closes-Bug: #1464184
This commit is contained in:
parent
4fcd8f4758
commit
ea73ac3d49
|
@ -113,7 +113,7 @@ def introspect(uuid, new_ipmi_credentials=None):
|
|||
|
||||
def _background_introspect(ironic, node_info):
|
||||
# TODO(dtantsur): pagination
|
||||
macs = [p.address for p in node_info.ports(ironic)]
|
||||
macs = list(node_info.ports(ironic))
|
||||
if macs:
|
||||
node_info.add_attribute(node_cache.MACS_ATTRIBUTE, macs)
|
||||
LOG.info(_LI('Whitelisting MAC\'s %(macs)s for node %(node)s on the'
|
||||
|
|
|
@ -21,9 +21,10 @@ import sqlite3
|
|||
import sys
|
||||
import time
|
||||
|
||||
from ironicclient import exceptions
|
||||
from oslo_config import cfg
|
||||
|
||||
from ironic_inspector.common.i18n import _, _LC, _LE
|
||||
from ironic_inspector.common.i18n import _, _LC, _LE, _LW
|
||||
from ironic_inspector import utils
|
||||
|
||||
CONF = cfg.CONF
|
||||
|
@ -61,6 +62,8 @@ class NodeInfo(object):
|
|||
self.error = error
|
||||
self.invalidate_cache()
|
||||
self._node = node
|
||||
if ports is not None and not isinstance(ports, dict):
|
||||
ports = {p.address: p for p in ports}
|
||||
self._ports = ports
|
||||
|
||||
@property
|
||||
|
@ -142,13 +145,44 @@ class NodeInfo(object):
|
|||
self._node = ironic.node.get(self.uuid)
|
||||
return self._node
|
||||
|
||||
def create_ports(self, macs, ironic=None):
|
||||
"""Create one or several ports for this node.
|
||||
|
||||
A warning is issued if port already exists on a node.
|
||||
"""
|
||||
ironic = utils.get_client() if ironic is None else ironic
|
||||
for mac in macs:
|
||||
if mac not in self.ports():
|
||||
self._create_port(mac, ironic)
|
||||
else:
|
||||
LOG.warn(_LW('Port %(mac)s already exists for node %(uuid)s, '
|
||||
'skipping'), {'mac': mac, 'uuid': self.uuid})
|
||||
|
||||
def ports(self, ironic=None):
|
||||
"""Get Ironic port objects associated with the cached node record."""
|
||||
"""Get Ironic port objects associated with the cached node record.
|
||||
|
||||
This value is cached as well, use invalidate_cache() to clean.
|
||||
|
||||
:return: dict MAC -> port object
|
||||
"""
|
||||
if self._ports is None:
|
||||
ironic = utils.get_client() if ironic is None else ironic
|
||||
self._ports = ironic.node.list_ports(self.uuid, limit=0)
|
||||
self._ports = {p.address: p
|
||||
for p in ironic.node.list_ports(self.uuid, limit=0)}
|
||||
return self._ports
|
||||
|
||||
def _create_port(self, mac, ironic):
|
||||
try:
|
||||
port = ironic.port.create(node_uuid=self.uuid, address=mac)
|
||||
except exceptions.Conflict:
|
||||
LOG.warn(_LW('Port %(mac)s already exists for node %(uuid)s, '
|
||||
'skipping'), {'mac': mac, 'uuid': self.uuid})
|
||||
# NOTE(dtantsur): we didn't get port object back, so we have to
|
||||
# reload ports on next access
|
||||
self._ports = None
|
||||
else:
|
||||
self._ports[mac] = port
|
||||
|
||||
|
||||
def init():
|
||||
"""Initialize the database."""
|
||||
|
|
|
@ -143,7 +143,7 @@ class ValidateInterfacesHook(base.ProcessingHook):
|
|||
return
|
||||
|
||||
ironic = utils.get_client()
|
||||
for port in node_info.ports():
|
||||
for port in node_info.ports(ironic).values():
|
||||
if port.address not in expected_macs:
|
||||
LOG.info(_LI("Deleting port %(port)s as its MAC %(mac)s is "
|
||||
"not in expected MAC list %(expected)s for node "
|
||||
|
|
|
@ -18,7 +18,7 @@ import logging
|
|||
import eventlet
|
||||
from ironicclient import exceptions
|
||||
|
||||
from ironic_inspector.common.i18n import _, _LE, _LI, _LW
|
||||
from ironic_inspector.common.i18n import _, _LE, _LI
|
||||
from ironic_inspector import firewall
|
||||
from ironic_inspector import node_cache
|
||||
from ironic_inspector.plugins import base as plugins_base
|
||||
|
@ -114,7 +114,7 @@ def _run_post_hooks(node_info, introspection_data):
|
|||
|
||||
node_patches = [p for p in node_patches if p]
|
||||
port_patches = {mac: patch for (mac, patch) in port_patches.items()
|
||||
if patch}
|
||||
if patch and mac in node_info.ports()}
|
||||
return node_patches, port_patches
|
||||
|
||||
|
||||
|
@ -122,25 +122,15 @@ def _process_node(ironic, node, introspection_data, node_info):
|
|||
# NOTE(dtantsur): repeat the check in case something changed
|
||||
utils.check_provision_state(node)
|
||||
|
||||
ports = {}
|
||||
for mac in (introspection_data.get('macs') or ()):
|
||||
try:
|
||||
port = ironic.port.create(node_uuid=node.uuid, address=mac)
|
||||
ports[mac] = port
|
||||
except exceptions.Conflict:
|
||||
LOG.warning(_LW('MAC %(mac)s appeared in introspection data for '
|
||||
'node %(node)s, but already exists in '
|
||||
'database - skipping') %
|
||||
{'mac': mac, 'node': node.uuid})
|
||||
node_info.create_ports(introspection_data.get('macs') or (), ironic=ironic)
|
||||
|
||||
# NOTE(dtanstur): make sure plugins get the latest information
|
||||
node_info.invalidate_cache()
|
||||
node_patches, port_patches = _run_post_hooks(node_info,
|
||||
introspection_data)
|
||||
|
||||
node = utils.retry_on_conflict(ironic.node.update, node.uuid, node_patches)
|
||||
for mac, patches in port_patches.items():
|
||||
utils.retry_on_conflict(ironic.port.update, ports[mac].uuid, patches)
|
||||
port = node_info.ports(ironic)[mac]
|
||||
utils.retry_on_conflict(ironic.port.update, port.uuid, patches)
|
||||
|
||||
LOG.debug('Node %s was updated with data from introspection process, '
|
||||
'patches %s, port patches %s',
|
||||
|
|
|
@ -11,6 +11,8 @@
|
|||
# See the License for the specific language governing permissions and
|
||||
# limitations under the License.
|
||||
|
||||
import collections
|
||||
|
||||
import eventlet
|
||||
from ironicclient import exceptions
|
||||
import mock
|
||||
|
@ -37,8 +39,10 @@ class BaseTest(test_base.NodeTest):
|
|||
power_state='power on',
|
||||
provision_state='foobar')
|
||||
self.ports = [mock.Mock(address=m) for m in self.macs]
|
||||
self.ports_dict = collections.OrderedDict((p.address, p)
|
||||
for p in self.ports)
|
||||
self.node_info = mock.Mock(uuid=self.uuid, options={})
|
||||
self.node_info.ports.return_value = self.ports
|
||||
self.node_info.ports.return_value = self.ports_dict
|
||||
|
||||
def _prepare(self, client_mock):
|
||||
cli = client_mock.return_value
|
||||
|
@ -154,7 +158,8 @@ class TestIntrospect(BaseTest):
|
|||
cli.node.validate.return_value = mock.Mock(power={'result': True})
|
||||
add_mock.return_value = mock.Mock(uuid=self.node_compat.uuid,
|
||||
options={})
|
||||
add_mock.return_value.ports.return_value = self.ports
|
||||
add_mock.return_value.ports.return_value = collections.OrderedDict(
|
||||
(p.address, p) for p in self.ports)
|
||||
|
||||
introspect.introspect(self.node_compat.uuid)
|
||||
|
||||
|
@ -175,7 +180,7 @@ class TestIntrospect(BaseTest):
|
|||
|
||||
def test_no_macs(self, client_mock, add_mock, filters_mock):
|
||||
cli = self._prepare(client_mock)
|
||||
self.ports[:] = []
|
||||
self.node_info.ports.return_value = []
|
||||
add_mock.return_value = self.node_info
|
||||
|
||||
introspect.introspect(self.node.uuid)
|
||||
|
|
|
@ -325,6 +325,11 @@ class TestNodeInfoOptions(test_base.NodeTest):
|
|||
|
||||
@mock.patch.object(utils, 'get_client')
|
||||
class TestNodeCacheIronicObjects(unittest.TestCase):
|
||||
def setUp(self):
|
||||
super(TestNodeCacheIronicObjects, self).setUp()
|
||||
self.ports = {'mac1': mock.Mock(address='mac1', spec=['address']),
|
||||
'mac2': mock.Mock(address='mac2', spec=['address'])}
|
||||
|
||||
def test_node_provided(self, mock_ironic):
|
||||
node_info = node_cache.NodeInfo(uuid='uuid', started_at=0,
|
||||
node=mock.sentinel.node)
|
||||
|
@ -355,17 +360,24 @@ class TestNodeCacheIronicObjects(unittest.TestCase):
|
|||
|
||||
def test_ports_provided(self, mock_ironic):
|
||||
node_info = node_cache.NodeInfo(uuid='uuid', started_at=0,
|
||||
ports=mock.sentinel.ports)
|
||||
self.assertIs(mock.sentinel.ports, node_info.ports())
|
||||
self.assertIs(mock.sentinel.ports, node_info.ports(ironic='ironic'))
|
||||
ports=self.ports)
|
||||
self.assertIs(self.ports, node_info.ports())
|
||||
self.assertIs(self.ports, node_info.ports(ironic='ironic'))
|
||||
self.assertFalse(mock_ironic.called)
|
||||
|
||||
def test_ports_provided_list(self, mock_ironic):
|
||||
node_info = node_cache.NodeInfo(uuid='uuid', started_at=0,
|
||||
ports=list(self.ports.values()))
|
||||
self.assertEqual(self.ports, node_info.ports())
|
||||
self.assertEqual(self.ports, node_info.ports(ironic='ironic'))
|
||||
self.assertFalse(mock_ironic.called)
|
||||
|
||||
def test_ports_not_provided(self, mock_ironic):
|
||||
mock_ironic.return_value.node.list_ports.return_value = (
|
||||
mock.sentinel.ports)
|
||||
mock_ironic.return_value.node.list_ports.return_value = list(
|
||||
self.ports.values())
|
||||
node_info = node_cache.NodeInfo(uuid='uuid', started_at=0)
|
||||
|
||||
self.assertIs(mock.sentinel.ports, node_info.ports())
|
||||
self.assertEqual(self.ports, node_info.ports())
|
||||
self.assertIs(node_info.ports(), node_info.ports())
|
||||
|
||||
mock_ironic.assert_called_once_with()
|
||||
|
@ -374,10 +386,10 @@ class TestNodeCacheIronicObjects(unittest.TestCase):
|
|||
|
||||
def test_ports_ironic_arg(self, mock_ironic):
|
||||
ironic2 = mock.Mock()
|
||||
ironic2.node.list_ports.return_value = mock.sentinel.ports
|
||||
ironic2.node.list_ports.return_value = list(self.ports.values())
|
||||
node_info = node_cache.NodeInfo(uuid='uuid', started_at=0)
|
||||
|
||||
self.assertIs(mock.sentinel.ports, node_info.ports(ironic=ironic2))
|
||||
self.assertEqual(self.ports, node_info.ports(ironic=ironic2))
|
||||
self.assertIs(node_info.ports(), node_info.ports(ironic=ironic2))
|
||||
|
||||
self.assertFalse(mock_ironic.called)
|
||||
|
|
|
@ -305,7 +305,6 @@ class TestProcess(BaseTest):
|
|||
self.assertFalse(pop_mock.return_value.finished.called)
|
||||
|
||||
|
||||
@mock.patch.object(node_cache.NodeInfo, 'invalidate_cache', lambda self: None)
|
||||
@mock.patch.object(utils, 'spawn_n',
|
||||
lambda f, *a: f(*a) and None)
|
||||
@mock.patch.object(eventlet.greenthread, 'sleep', lambda _: None)
|
||||
|
@ -344,6 +343,7 @@ class TestProcessNode(BaseTest):
|
|||
[RuntimeError()] * self.validate_attempts + [None])
|
||||
self.cli.port.create.side_effect = self.ports
|
||||
self.cli.node.update.return_value = self.node
|
||||
self.cli.node.list_ports.return_value = []
|
||||
|
||||
@mock.patch.object(utils, 'get_client')
|
||||
def call(self, mock_cli):
|
||||
|
@ -424,7 +424,8 @@ class TestProcessNode(BaseTest):
|
|||
self.assertEqual(2, self.cli.node.set_power_state.call_count)
|
||||
|
||||
def test_port_failed(self, filters_mock, post_hook_mock):
|
||||
self.ports[0] = exceptions.Conflict()
|
||||
self.cli.port.create.side_effect = (
|
||||
[exceptions.Conflict()] + self.ports[1:])
|
||||
|
||||
self.call()
|
||||
|
||||
|
|
Loading…
Reference in New Issue