From 59787768cbf1d757611be27406777a088fb16659 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Harald=20Jens=C3=A5s?= Date: Fri, 19 Jul 2019 12:08:27 +0200 Subject: [PATCH] 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 --- ironic/common/neutron.py | 21 +++++++++++-------- ironic/tests/unit/common/test_neutron.py | 19 ++++++++++------- ...st-add-query-filters-f72cfcefa1e093d2.yaml | 9 ++++++++ 3 files changed, 32 insertions(+), 17 deletions(-) create mode 100644 releasenotes/notes/fix-security-group-list-add-query-filters-f72cfcefa1e093d2.yaml diff --git a/ironic/common/neutron.py b/ironic/common/neutron.py index f74e80440c..0f12a24bd6 100644 --- a/ironic/common/neutron.py +++ b/ironic/common/neutron.py @@ -184,21 +184,24 @@ def _verify_security_groups(security_groups, client): return try: 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: msg = (_("Could not retrieve security groups from neutron: %(exc)s") % {'exc': e}) LOG.exception(msg) raise exception.NetworkError(msg) - existing_sec_groups = [sec_group['id'] for sec_group in neutron_sec_groups] - missing_sec_groups = set(security_groups) - set(existing_sec_groups) - if missing_sec_groups: - msg = (_('Could not find these security groups (specified via ironic ' - 'config) in neutron: %(ir-sg)s') - % {'ir-sg': list(missing_sec_groups)}) - LOG.error(msg) - raise exception.NetworkError(msg) + if set(security_groups).issubset(x['id'] for x in neutron_sec_groups): + return + + missing_sec_groups = set(security_groups).difference( + x['id'] for x in neutron_sec_groups) + msg = (_('Could not find these security groups (specified via ironic ' + 'config) in neutron: %(ir-sg)s') + % {'ir-sg': list(missing_sec_groups)}) + LOG.error(msg) + raise exception.NetworkError(msg) def add_ports_to_network(task, network_uuid, security_groups=None): diff --git a/ironic/tests/unit/common/test_neutron.py b/ironic/tests/unit/common/test_neutron.py index 1813659280..b21a8ed9df 100644 --- a/ironic/tests/unit/common/test_neutron.py +++ b/ironic/tests/unit/common/test_neutron.py @@ -270,23 +270,23 @@ class TestNeutronNetworkActions(db_base.DbTestCase): self.assertIsNone( 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): sg_ids = [] for i in range(2): sg_ids.append(uuidutils.generate_uuid()) - expected_vals = {'security_groups': []} - for sg in sg_ids: - expected_vals['security_groups'].append({'id': sg}) + expected_vals = {'security_groups': [{'id': sg_ids[0]}]} client = mock.MagicMock() client.list_security_groups.return_value = expected_vals self.assertIsNone( 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): sg_ids = [] @@ -300,7 +300,8 @@ class TestNeutronNetworkActions(db_base.DbTestCase): self.assertRaises( exception.NetworkError, 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): sg_ids = [] @@ -313,7 +314,8 @@ class TestNeutronNetworkActions(db_base.DbTestCase): self.assertRaises( exception.NetworkError, 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): sg_ids = [] @@ -328,7 +330,8 @@ class TestNeutronNetworkActions(db_base.DbTestCase): exception.NetworkError, "Could not retrieve security groups", 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): self._test_add_ports_to_network(is_client_id=True) diff --git a/releasenotes/notes/fix-security-group-list-add-query-filters-f72cfcefa1e093d2.yaml b/releasenotes/notes/fix-security-group-list-add-query-filters-f72cfcefa1e093d2.yaml new file mode 100644 index 0000000000..ae5a980fef --- /dev/null +++ b/releasenotes/notes/fix-security-group-list-add-query-filters-f72cfcefa1e093d2.yaml @@ -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 `_.) +