Filter security group list on the ID's we expect

Filter the list of security groups based on the security
group IDs we plan on using for the network. Also only
get the id field as this is all we need to compare.

Fixes an issue where deployment fail on systems with a
high number of security groups.

Story: 2006256
Task: 35871
Change-Id: I83bbd3c77f13aaab0912354c3ec9cdd5e1123d0f
(cherry picked from commit 59787768cb)
This commit is contained in:
Harald Jensås 2019-07-19 12:08:27 +02:00 committed by Dmitry Tantsur
parent 246b866157
commit 1939909563
3 changed files with 32 additions and 17 deletions

View File

@ -203,21 +203,24 @@ def _verify_security_groups(security_groups, client):
return return
try: try:
neutron_sec_groups = ( neutron_sec_groups = (
client.list_security_groups().get('security_groups', [])) client.list_security_groups(id=security_groups, fields='id').get(
'security_groups', []))
except neutron_exceptions.NeutronClientException as e: except neutron_exceptions.NeutronClientException as e:
msg = (_("Could not retrieve security groups from neutron: %(exc)s") % msg = (_("Could not retrieve security groups from neutron: %(exc)s") %
{'exc': e}) {'exc': e})
LOG.exception(msg) LOG.exception(msg)
raise exception.NetworkError(msg) raise exception.NetworkError(msg)
existing_sec_groups = [sec_group['id'] for sec_group in neutron_sec_groups] if set(security_groups).issubset(x['id'] for x in neutron_sec_groups):
missing_sec_groups = set(security_groups) - set(existing_sec_groups) return
if missing_sec_groups:
msg = (_('Could not find these security groups (specified via ironic ' missing_sec_groups = set(security_groups).difference(
'config) in neutron: %(ir-sg)s') x['id'] for x in neutron_sec_groups)
% {'ir-sg': list(missing_sec_groups)}) msg = (_('Could not find these security groups (specified via ironic '
LOG.error(msg) 'config) in neutron: %(ir-sg)s')
raise exception.NetworkError(msg) % {'ir-sg': list(missing_sec_groups)})
LOG.error(msg)
raise exception.NetworkError(msg)
def add_ports_to_network(task, network_uuid, security_groups=None): def add_ports_to_network(task, network_uuid, security_groups=None):

View File

@ -266,23 +266,23 @@ class TestNeutronNetworkActions(db_base.DbTestCase):
self.assertIsNone( self.assertIsNone(
neutron._verify_security_groups(sg_ids, client)) neutron._verify_security_groups(sg_ids, client))
client.list_security_groups.assert_called_once_with() client.list_security_groups.assert_called_once_with(
fields='id', id=sg_ids)
def test_verify_sec_groups_less_than_configured(self): def test_verify_sec_groups_less_than_configured(self):
sg_ids = [] sg_ids = []
for i in range(2): for i in range(2):
sg_ids.append(uuidutils.generate_uuid()) sg_ids.append(uuidutils.generate_uuid())
expected_vals = {'security_groups': []} expected_vals = {'security_groups': [{'id': sg_ids[0]}]}
for sg in sg_ids:
expected_vals['security_groups'].append({'id': sg})
client = mock.MagicMock() client = mock.MagicMock()
client.list_security_groups.return_value = expected_vals client.list_security_groups.return_value = expected_vals
self.assertIsNone( self.assertIsNone(
neutron._verify_security_groups(sg_ids[:1], client)) neutron._verify_security_groups(sg_ids[:1], client))
client.list_security_groups.assert_called_once_with() client.list_security_groups.assert_called_once_with(
fields='id', id=sg_ids[:1])
def test_verify_sec_groups_more_than_configured(self): def test_verify_sec_groups_more_than_configured(self):
sg_ids = [] sg_ids = []
@ -296,7 +296,8 @@ class TestNeutronNetworkActions(db_base.DbTestCase):
self.assertRaises( self.assertRaises(
exception.NetworkError, exception.NetworkError,
neutron._verify_security_groups, sg_ids, client) neutron._verify_security_groups, sg_ids, client)
client.list_security_groups.assert_called_once_with() client.list_security_groups.assert_called_once_with(
fields='id', id=sg_ids)
def test_verify_sec_groups_no_sg_from_neutron(self): def test_verify_sec_groups_no_sg_from_neutron(self):
sg_ids = [] sg_ids = []
@ -309,7 +310,8 @@ class TestNeutronNetworkActions(db_base.DbTestCase):
self.assertRaises( self.assertRaises(
exception.NetworkError, exception.NetworkError,
neutron._verify_security_groups, sg_ids, client) neutron._verify_security_groups, sg_ids, client)
client.list_security_groups.assert_called_once_with() client.list_security_groups.assert_called_once_with(
fields='id', id=sg_ids)
def test_verify_sec_groups_exception_by_neutronclient(self): def test_verify_sec_groups_exception_by_neutronclient(self):
sg_ids = [] sg_ids = []
@ -324,7 +326,8 @@ class TestNeutronNetworkActions(db_base.DbTestCase):
exception.NetworkError, exception.NetworkError,
"Could not retrieve security groups", "Could not retrieve security groups",
neutron._verify_security_groups, sg_ids, client) neutron._verify_security_groups, sg_ids, client)
client.list_security_groups.assert_called_once_with() client.list_security_groups.assert_called_once_with(
fields='id', id=sg_ids)
def test_add_ports_with_client_id_to_network(self): def test_add_ports_with_client_id_to_network(self):
self._test_add_ports_to_network(is_client_id=True) self._test_add_ports_to_network(is_client_id=True)

View File

@ -0,0 +1,9 @@
---
fixes:
- |
Fixes an issue where baremetal node deployment would fail on clouds
with a high number of security groups. Listing the security groups
took too long. Instead of listing all security groups, a query filter
was added to list only the security groups to be used for the network.
(See bug `2006256 <https://storyboard.openstack.org/#!/story/2006256>`_.)