Minor changes to neutron security groups code

This is a follow-on patch to 3197e44c04.
It cleans up a bit of the code and addresses the nits (changes a
LOG.exception to LOG.error and adds a unit test).

Change-Id: I02b6346d9a2abff858c9dd6083fd29f393c63e97
Partial-bug: #1594242

ironic/common/neutron.py

@@ -69,13 +69,21 @@ def get_client(token=None):
 
 
 def _verify_security_groups(security_groups, client):
+    """Verify that the security groups exist.
+
+    :param security_groups: a list of security group UUIDs; may be None or
+        empty
+    :param client: Neutron client
+    :raises: NetworkError
+    """
+
     if not security_groups:
         return
     try:
         neutron_sec_groups = (
-            client.list_security_groups().get('security_groups') or [])
+            client.list_security_groups().get('security_groups', []))
     except neutron_exceptions.NeutronClientException as e:
-        msg = (_("Could not retrieve neutron security groups %(exc)s") %
+        msg = (_("Could not retrieve security groups from neutron: %(exc)s") %
                {'exc': e})
         LOG.exception(msg)
         raise exception.NetworkError(msg)
@@ -83,10 +91,10 @@ def _verify_security_groups(security_groups, client):
     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 = (_('Security Groups specified in Ironic config '
+        msg = (_('Could not find these security groups (specified via ironic '
-                 '%(ir-sg)s are not found') %
+                 'config) in neutron: %(ir-sg)s')
-               {'ir-sg': list(missing_sec_groups)})
+               % {'ir-sg': list(missing_sec_groups)})
-        LOG.exception(msg)
+        LOG.error(msg)
         raise exception.NetworkError(msg)
 
 

ironic/tests/unit/common/test_neutron.py

@@ -208,6 +208,7 @@ class TestNeutronNetworkActions(db_base.DbTestCase):
 
         self.assertIsNone(
             neutron._verify_security_groups(sg_ids, client))
+        client.list_security_groups.assert_called_once_with()
 
     def test_verify_sec_groups_less_than_configured(self):
         sg_ids = []
@@ -223,6 +224,7 @@ class TestNeutronNetworkActions(db_base.DbTestCase):
 
         self.assertIsNone(
             neutron._verify_security_groups(sg_ids[:1], client))
+        client.list_security_groups.assert_called_once_with()
 
     def test_verify_sec_groups_more_than_configured(self):
         sg_ids = []
@@ -236,6 +238,20 @@ class TestNeutronNetworkActions(db_base.DbTestCase):
         self.assertRaises(
             exception.NetworkError,
             neutron._verify_security_groups, sg_ids, client)
+        client.list_security_groups.assert_called_once_with()
+
+    def test_verify_sec_groups_no_sg_from_neutron(self):
+        sg_ids = []
+        for i in range(1):
+            sg_ids.append(uuidutils.generate_uuid())
+
+        client = mock.MagicMock()
+        client.list_security_groups.return_value = {}
+
+        self.assertRaises(
+            exception.NetworkError,
+            neutron._verify_security_groups, sg_ids, client)
+        client.list_security_groups.assert_called_once_with()
 
     def test_verify_sec_groups_exception_by_neutronclient(self):
         sg_ids = []
@@ -248,8 +264,9 @@ class TestNeutronNetworkActions(db_base.DbTestCase):
 
         self.assertRaisesRegex(
             exception.NetworkError,
-            "Could not retrieve neutron security groups",
+            "Could not retrieve security groups",
             neutron._verify_security_groups, sg_ids, client)
+        client.list_security_groups.assert_called_once_with()
 
     def test_add_ports_with_client_id_to_vlan_network(self):
         self._test_add_ports_to_vlan_network(is_client_id=True)