Security group deletion

1. What is the problem?

A security group(sg) may map to several local sg in local neutron.
During the deletion of sg local neutron may recreate the sg if it
recieve a get-request of sg.

2. What is the solution to the problem?

The solution to resource deleting is covered in
specs/queens/resource deleting.rst

3. What the features to be implemented in the Tricircle to realize the solution?
No new features.

Change-Id: I37feacf93b1bde1459bfd8513c7860fe77c113a7

tricircle/common/resource_handle.py

@@ -94,7 +94,7 @@ class NeutronResourceHandle(ResourceHandle):
         'subnet': LIST | CREATE | DELETE | GET | UPDATE,
         'port': LIST | CREATE | DELETE | GET | UPDATE,
         'router': LIST | CREATE | DELETE | ACTION | GET | UPDATE,
-        'security_group': LIST | CREATE | GET,
+        'security_group': LIST | CREATE | GET | DELETE,
         'security_group_rule': LIST | CREATE | DELETE,
         'floatingip': LIST | CREATE | UPDATE | DELETE,
         'trunk': LIST | CREATE | UPDATE | GET | DELETE | ACTION,

tricircle/network/central_plugin.py

@@ -81,7 +81,7 @@ from tricircle.network import helper
 from tricircle.network import managers
 from tricircle.network import qos_driver
 from tricircle.network import security_groups
-
+from tricircle.network import utils as nt_utils
 
 tricircle_opts = [
     cfg.ListOpt('type_drivers',
@@ -354,77 +354,17 @@ class TricirclePlugin(db_base_plugin_v2.NeutronDbPluginV2,
         res['tags'] = []
         return res
 
-    def check_resource_not_in_deleting(self, context, dict_para):
-        t_ctx = t_context.get_context_from_neutron_context(context)
-        with t_ctx.session.begin():
-            resource_filters = []
-            for key in dict_para.keys():
-                resource_filters.append({'key': key,
-                                         'comparator': 'eq',
-                                         'value': dict_para[key]})
-
-            deleting_resource = core.query_resource(t_ctx,
-                                                    models.DeletingResources,
-                                                    resource_filters, [])
-
-            if len(deleting_resource):
-                if not hasattr(context, "USER_AGENT") or \
-                        context.USER_AGENT == t_constants.USER_AGENT:
-                    raise t_exceptions.ResourceIsInDeleting()
-                elif context.USER_AGENT == t_constants.LOCAL:
-                    raise t_exceptions.ResourceNotFound(
-                        models.DeletingResources, dict_para['resource_id'])
-
-    def _check_network_not_in_use(self, context, t_ctx, network_id):
-        # use a different name to avoid override _ensure_entwork_not_in_use
-        subnets = self._get_subnets_by_network(context, network_id)
-        auto_delete_port_names = []
-
-        for subnet in subnets:
-            subnet_id = subnet['id']
-            region_names = [e[0] for e in t_ctx.session.query(
-                sql.distinct(models.Pod.region_name)).join(
-                models.ResourceRouting,
-                models.Pod.pod_id == models.ResourceRouting.pod_id).filter(
-                models.ResourceRouting.top_id == subnet_id)]
-            auto_delete_port_names.extend([t_constants.interface_port_name % (
-                region_name, subnet_id) for region_name in region_names])
-            dhcp_port_name = t_constants.dhcp_port_name % subnet_id
-            snat_port_name = t_constants.snat_port_name % subnet_id
-            auto_delete_port_names.append(dhcp_port_name)
-            auto_delete_port_names.append(snat_port_name)
-
-        if not auto_delete_port_names:
-            # pre-created port not found, any ports left need to be deleted
-            # before deleting network
-            non_auto_delete_ports = context.session.query(
-                models_v2.Port.id).filter_by(network_id=network_id)
-            if non_auto_delete_ports.count():
-                raise exceptions.NetworkInUse(net_id=network_id)
-            return
-
-        t_pod = db_api.get_top_pod(t_ctx)
-        auto_delete_port_ids = [e[0] for e in t_ctx.session.query(
-            models.ResourceRouting.bottom_id).filter_by(
-            pod_id=t_pod['pod_id'], resource_type=t_constants.RT_PORT).filter(
-            models.ResourceRouting.top_id.in_(auto_delete_port_names))]
-
-        non_auto_delete_ports = context.session.query(
-            models_v2.Port.id).filter_by(network_id=network_id).filter(
-            ~models_v2.Port.id.in_(auto_delete_port_ids))
-        if non_auto_delete_ports.count():
-            raise exceptions.NetworkInUse(net_id=network_id)
-
     def delete_network(self, context, network_id):
         t_ctx = t_context.get_context_from_neutron_context(context)
         dict_para = {'resource_id': network_id, 'resource_type': 'network'}
-        self.check_resource_not_in_deleting(context, dict_para)
-        self._check_network_not_in_use(context, t_ctx, network_id)
+        nt_utils.check_resource_not_in_deleting(context, dict_para)
+        nt_utils.check_network_not_in_use(self, context, t_ctx, network_id)
         dict_para['deleted_at'] = datetime.datetime.utcnow()
-        with t_ctx.session.begin():
-            core.create_resource(t_ctx, models.DeletingResources, dict_para)
 
         try:
+            with t_ctx.session.begin():
+                core.create_resource(
+                    t_ctx, models.DeletingResources, dict_para)
             for pod, bottom_network_id in (
                     self.helper.get_real_shadow_resource_iterator(
                         t_ctx, t_constants.RT_NETWORK, network_id)):
@@ -442,26 +382,28 @@ class TricirclePlugin(db_base_plugin_v2.NeutronDbPluginV2,
                                   'value': network_id},
                                  {'key': 'pod_id', 'comparator': 'eq',
                                   'value': pod['pod_id']}])
+
+            with t_ctx.session.begin():
+                core.delete_resources(t_ctx, models.ResourceRouting,
+                                      filters=[{'key': 'top_id',
+                                                'comparator': 'eq',
+                                                'value': network_id}])
+
+            subnets = self._get_subnets_by_network(context, network_id)
+            for subnet in subnets:
+                self.delete_subnet(context, subnet['id'])
+            with context.session.begin(subtransactions=True):
+                self.type_manager.release_network_segments(context, network_id)
+                super(TricirclePlugin, self).delete_network(
+                    context, network_id)
         except Exception:
             raise
-        with t_ctx.session.begin():
-            core.delete_resources(t_ctx, models.ResourceRouting,
-                                  filters=[{'key': 'top_id',
-                                            'comparator': 'eq',
-                                            'value': network_id}])
-
-        subnets = self._get_subnets_by_network(context, network_id)
-        for subnet in subnets:
-            self.delete_subnet(context, subnet['id'])
-        with context.session.begin(subtransactions=True):
-            self.type_manager.release_network_segments(context, network_id)
-            super(TricirclePlugin, self).delete_network(context, network_id)
-
-        with t_ctx.session.begin():
-            core.delete_resources(t_ctx, models.DeletingResources,
-                                  filters=[{'key': 'resource_id',
-                                            'comparator': 'eq',
-                                            'value': network_id}])
+        finally:
+            with t_ctx.session.begin():
+                core.delete_resources(t_ctx, models.DeletingResources,
+                                      filters=[{'key': 'resource_id',
+                                                'comparator': 'eq',
+                                                'value': network_id}])
 
     def _raise_if_updates_external_attribute(self, attrs):
         """Raise exception if external attributes are present.
@@ -562,8 +504,8 @@ class TricirclePlugin(db_base_plugin_v2.NeutronDbPluginV2,
 
         dict_para = {'resource_id': network_id, 'resource_type': 'network'}
         try:
-            self.check_resource_not_in_deleting(context, dict_para)
-        except t_exceptions.ResourceIsInDeleting():
+            nt_utils.check_resource_not_in_deleting(context, dict_para)
+        except t_exceptions.ResourceIsInDeleting:
             return network_id
         except t_exceptions.ResourceNotFound:
             raise exceptions.NotFound()
@@ -1588,7 +1530,9 @@ class TricirclePlugin(db_base_plugin_v2.NeutronDbPluginV2,
                 'cidr': constants.ATTR_NOT_SPECIFIED,
                 'subnetpool_id': pool_id,
                 'enable_dhcp': False,
-                'tenant_id': project_id
+                'tenant_id': project_id,
+                'ipv6_ra_mode': None,
+                'ipv6_address_mode': None
             }
         }
         _, subnet_id = self._prepare_top_element(

tricircle/network/helper.py

@@ -971,11 +971,14 @@ class NetworkHelper(object):
 
     @staticmethod
     def get_real_shadow_resource_iterator(t_ctx, res_type, res_id):
-        shadow_res_type = t_constants.REAL_SHADOW_TYPE_MAP[res_type]
+        shadow_res_type = None
+        if res_type in t_constants.REAL_SHADOW_TYPE_MAP:
+            shadow_res_type = t_constants.REAL_SHADOW_TYPE_MAP[res_type]
         mappings = db_api.get_bottom_mappings_by_top_id(
             t_ctx, res_id, res_type)
-        mappings.extend(db_api.get_bottom_mappings_by_top_id(
-            t_ctx, res_id, shadow_res_type))
+        if shadow_res_type:
+            mappings.extend(db_api.get_bottom_mappings_by_top_id(
+                t_ctx, res_id, shadow_res_type))
 
         processed_pod_set = set()
         for pod, bottom_res_id in mappings:

tricircle/network/security_groups.py

@@ -13,11 +13,22 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from oslo_log import log
+
 from neutron.db import securitygroups_db
 
+import tricircle.common.client as t_client
+import tricircle.common.constants as t_constants
 from tricircle.common import context
+import tricircle.common.context as t_context
+import tricircle.common.exceptions as t_exceptions
 from tricircle.common import xrpcapi
+from tricircle.db import core
+from tricircle.db import models
 import tricircle.network.exceptions as n_exceptions
+from tricircle.network import utils as nt_utils
+
+LOG = log.getLogger(__name__)
 
 
 class TricircleSecurityGroupMixin(securitygroups_db.SecurityGroupDbMixin):
@@ -25,6 +36,7 @@ class TricircleSecurityGroupMixin(securitygroups_db.SecurityGroupDbMixin):
     def __init__(self):
         super(TricircleSecurityGroupMixin, self).__init__()
         self.xjob_handler = xrpcapi.XJobAPI()
+        self.clients = {}
 
     @staticmethod
     def _compare_rule(rule1, rule2):
@@ -34,6 +46,11 @@ class TricircleSecurityGroupMixin(securitygroups_db.SecurityGroupDbMixin):
                 return False
         return True
 
+    def _get_client(self, region_name):
+        if region_name not in self.clients:
+            self.clients[region_name] = t_client.Client(region_name)
+        return self.clients[region_name]
+
     def create_security_group_rule(self, q_context, security_group_rule):
         rule = security_group_rule['security_group_rule']
         if rule['remote_group_id']:
@@ -80,3 +97,58 @@ class TricircleSecurityGroupMixin(securitygroups_db.SecurityGroupDbMixin):
         except Exception:
             raise n_exceptions.BottomPodOperationFailure(
                 resource='security group rule', region_name='')
+
+    def get_security_group(self, context, sg_id, fields=None, tenant_id=None):
+        dict_param = {'resource_id': sg_id, 'resource_type': t_constants.RT_SG}
+        security_group_list = None
+        try:
+            security_group_list = nt_utils.check_resource_not_in_deleting(
+                context, dict_param)
+        except t_exceptions.ResourceNotFound:
+            raise
+
+        if security_group_list:
+            return security_group_list
+        else:
+            return super(TricircleSecurityGroupMixin, self).\
+                get_security_group(context, sg_id)
+
+    def delete_security_group(self, context, sg_id):
+        LOG.debug("lyman--enter delete security group")
+        t_ctx = t_context.get_context_from_neutron_context(context)
+        # check the sg whether in security group
+        super(TricircleSecurityGroupMixin, self).\
+            get_security_group(context, sg_id)
+        # check the sg whether in deleting
+        dict_para = {'resource_id': sg_id, 'resource_type': t_constants.RT_SG}
+
+        nt_utils.check_resource_not_in_deleting(context, dict_para)
+        try:
+            with t_ctx.session.begin():
+                core.create_resource(
+                    t_ctx, models.DeletingResources, dict_para)
+            for pod, bottom_security_group_id in (
+                    self.helper.get_real_shadow_resource_iterator(
+                        t_ctx, t_constants.RT_SG, sg_id)):
+                self._get_client(pod['region_name']). \
+                    delete_security_groups(t_ctx, bottom_security_group_id)
+                with t_ctx.session.begin():
+                    core.delete_resources(
+                        t_ctx, models.ResourceRouting,
+                        filters=[{'key': 'top_id', 'comparator': 'eq',
+                                  'value': sg_id},
+                                 {'key': 'pod_id', 'comparator': 'eq',
+                                  'value': pod['pod_id']}])
+
+            with t_ctx.session.begin():
+                super(TricircleSecurityGroupMixin, self). \
+                    delete_security_group(context, sg_id)
+        except Exception:
+            raise
+        finally:
+            with t_ctx.session.begin():
+                core.delete_resources(
+                    t_ctx, models.DeletingResources,
+                    filters=[{
+                        'key': 'resource_id', 'comparator': 'eq',
+                        'value': sg_id}])

tricircle/network/utils.py

@@ -0,0 +1,88 @@
+# Copyright 2015 Huawei Technologies Co., Ltd.
+# 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 neutron.db import models_v2
+from neutron_lib import exceptions
+from sqlalchemy import sql
+
+import tricircle.common.constants as t_constants
+import tricircle.common.context as t_context
+import tricircle.common.exceptions as t_exceptions
+import tricircle.db.api as db_api
+from tricircle.db import core
+from tricircle.db import models
+
+
+def check_resource_not_in_deleting(context, dict_para):
+    t_ctx = t_context.get_context_from_neutron_context(context)
+    with t_ctx.session.begin():
+        resource_filters = []
+        for key in dict_para.keys():
+            resource_filters.append({'key': key,
+                                     'comparator': 'eq',
+                                     'value': dict_para[key]})
+
+        deleting_resource = core.query_resource(t_ctx,
+                                                models.DeletingResources,
+                                                resource_filters, [])
+
+        if len(deleting_resource):
+            if hasattr(context, "USER_AGENT") and \
+                    context.USER_AGENT == t_constants.LOCAL:
+                raise t_exceptions.ResourceNotFound(
+                    models.DeletingResources, dict_para['resource_id'])
+            else:
+                raise t_exceptions.ResourceIsInDeleting()
+
+
+def check_network_not_in_use(self, context, t_ctx, network_id):
+    # use a different name to avoid override _ensure_entwork_not_in_use
+    subnets = self._get_subnets_by_network(context, network_id)
+    auto_delete_port_names = []
+
+    for subnet in subnets:
+        subnet_id = subnet['id']
+        region_names = [e[0] for e in t_ctx.session.query(
+            sql.distinct(models.Pod.region_name)).join(
+            models.ResourceRouting,
+            models.Pod.pod_id == models.ResourceRouting.pod_id).filter(
+            models.ResourceRouting.top_id == subnet_id)]
+        auto_delete_port_names.extend([t_constants.interface_port_name % (
+            region_name, subnet_id) for region_name in region_names])
+        dhcp_port_name = t_constants.dhcp_port_name % subnet_id
+        snat_port_name = t_constants.snat_port_name % subnet_id
+        auto_delete_port_names.append(dhcp_port_name)
+        auto_delete_port_names.append(snat_port_name)
+
+    if not auto_delete_port_names:
+        # pre-created port not found, any ports left need to be deleted
+        # before deleting network
+        non_auto_delete_ports = context.session.query(
+            models_v2.Port.id).filter_by(network_id=network_id)
+        if non_auto_delete_ports.count():
+            raise exceptions.NetworkInUse(net_id=network_id)
+        return
+
+    t_pod = db_api.get_top_pod(t_ctx)
+    auto_delete_port_ids = [e[0] for e in t_ctx.session.query(
+        models.ResourceRouting.bottom_id).filter_by(
+        pod_id=t_pod['pod_id'], resource_type=t_constants.RT_PORT).filter(
+        models.ResourceRouting.top_id.in_(auto_delete_port_names))]
+
+    non_auto_delete_ports = context.session.query(
+        models_v2.Port.id).filter_by(network_id=network_id).filter(
+        ~models_v2.Port.id.in_(auto_delete_port_ids))
+    if non_auto_delete_ports.count():
+        raise exceptions.NetworkInUse(net_id=network_id)

tricircle/tests/unit/network/test_central_plugin.py

@@ -499,6 +499,12 @@ class FakeClient(test_utils.FakeClient):
     def get_security_group(self, ctx, _id, fields=None, tenant_id=None):
         pass
 
+    def delete_security_groups(self, ctx, sg_id):
+        res_list = self._res_map[self.region_name]['security_group']
+        for sg in res_list:
+            if sg['id'] == sg_id:
+                res_list.remove(sg)
+
     def get_qos_policies(self, ctx, policy_id):
         rules = {'rules': []}
         rule_list = \
@@ -865,6 +871,9 @@ class FakePlugin(plugin.TricirclePlugin,
     def _make_security_group_dict(self, security_group, fields=None):
         return security_group
 
+    def _get_port_security_group_bindings(self, context, filters):
+        return None
+
 
 def fake_get_context_from_neutron_context(q_context):
     return context.get_db_context()
@@ -3532,6 +3541,30 @@ class PluginTest(unittest.TestCase,
                                      'pod_id_1', TOP_SGS,
                                      TOP_SG_RULES, BOTTOM1_SGS)
 
+    @patch.object(context, 'get_context_from_neutron_context')
+    def test_get_security_group(self, mock_context):
+        self._basic_pod_route_setup()
+
+        fake_plugin = FakePlugin()
+        q_ctx = FakeNeutronContext()
+        t_ctx = context.get_db_context()
+        mock_context.return_value = t_ctx
+
+        self._test_get_security_group(fake_plugin, q_ctx, t_ctx,
+                                      'pod_id_1', TOP_SGS, BOTTOM1_SGS)
+
+    @patch.object(context, 'get_context_from_neutron_context')
+    def test_delete_security_group(self, mock_context):
+        self._basic_pod_route_setup()
+
+        fake_plugin = FakePlugin()
+        q_ctx = FakeNeutronContext()
+        t_ctx = context.get_db_context()
+        mock_context.return_value = t_ctx
+
+        self._test_delete_security_group(fake_plugin, q_ctx, t_ctx,
+                                         'pod_id_1', TOP_SGS, BOTTOM1_SGS)
+
     @patch.object(context, 'get_context_from_neutron_context')
     def test_create_policy(self, mock_context):
         fake_plugin = FakePlugin()

tricircle/tests/unit/network/test_security_groups.py

@@ -13,9 +13,12 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from neutron.extensions import securitygroup as ext_sg
 from oslo_utils import uuidutils
 
 from tricircle.common import constants
+import tricircle.common.constants as t_constants
+import tricircle.common.exceptions as t_exceptions
 from tricircle.db import core
 from tricircle.db import models
 from tricircle.network import exceptions
@@ -260,3 +263,95 @@ class TricircleSecurityGroupTestMixin(object):
             self.assertTrue(self._compare_rule(
                 bottom1_sgs[0]['security_group_rules'][i],
                 top_sgs[0]['security_group_rules'][i]))
+
+    def _test_get_security_group(self, plugin, q_ctx, t_ctx,
+                                 pod_id, top_sgs, bottom1_sgs):
+        t_sg_id = uuidutils.generate_uuid()
+        t_rule1_id = uuidutils.generate_uuid()
+        t_rule2_id = uuidutils.generate_uuid()
+        b_sg_id = uuidutils.generate_uuid()
+        project_id = 'test_prject_id'
+        t_rule1 = self._build_test_rule(
+            t_rule1_id, t_sg_id, project_id, '10.0.0.0/24')
+        t_rule2 = self._build_test_rule(
+            t_rule2_id, t_sg_id, project_id, '192.168.56.0/24')
+        t_sg = {'id': t_sg_id, 'name': 'top_sg', 'description': '',
+                'tenant_id': project_id,
+                'security_group_rules': [t_rule1, t_rule2]}
+        b_sg = {'id': b_sg_id, 'name': 'bottom_sg', 'description': '',
+                'tenant_id': project_id,
+                'security_group_rules': [t_rule1, t_rule2]}
+        top_sgs.append(t_sg)
+        bottom1_sgs.append(b_sg)
+
+        route1 = {
+            'top_id': t_sg_id,
+            'pod_id': pod_id,
+            'bottom_id': b_sg_id,
+            'resource_type': constants.RT_SG}
+        with t_ctx.session.begin():
+            core.create_resource(t_ctx, models.ResourceRouting, route1)
+
+        # test get_sg for normal situation
+        res = plugin.get_security_group(q_ctx, t_sg_id)
+        self.assertTrue(res['id'] == t_sg_id and res['name'] == 'top_sg')
+
+        # test get_sg when the top_sg is under deleting
+        dict_para = {'resource_id': t_sg_id,
+                     'resource_type': t_constants.RT_SG}
+        with t_ctx.session.begin():
+            core.create_resource(t_ctx, models.DeletingResources,
+                                 dict_para)
+
+        q_ctx.USER_AGENT = t_constants.LOCAL
+        self.assertRaises(t_exceptions.ResourceNotFound,
+                          plugin.get_security_group,
+                          q_ctx, t_sg_id)
+
+        # test get_sg when the request is from user_agent
+        q_ctx.USER_AGENT = t_constants.USER_AGENT
+        self.assertRaises(t_exceptions.ResourceIsInDeleting,
+                          plugin.get_security_group,
+                          q_ctx, t_sg_id)
+
+    def _test_delete_security_group(self, plugin, q_ctx, t_ctx,
+                                    pod_id, top_sgs, bottom1_sgs):
+        t_sg_id = uuidutils.generate_uuid()
+        t_rule1_id = uuidutils.generate_uuid()
+        t_rule2_id = uuidutils.generate_uuid()
+        b_sg_id = uuidutils.generate_uuid()
+        project_id = 'test_prject_id'
+        t_rule1 = self._build_test_rule(
+            t_rule1_id, t_sg_id, project_id, '10.0.0.0/24')
+        t_rule2 = self._build_test_rule(
+            t_rule2_id, t_sg_id, project_id, '192.168.56.0/24')
+        t_sg = {'id': t_sg_id, 'name': 'top_sg', 'description': '',
+                'tenant_id': project_id,
+                'security_group_rules': [t_rule1, t_rule2]}
+        b_sg = {'id': b_sg_id, 'name': 'bottom_sg', 'description': '',
+                'tenant_id': project_id,
+                'security_group_rules': [t_rule1, t_rule2]}
+        top_sgs.append(t_sg)
+        bottom1_sgs.append(b_sg)
+
+        route1 = {
+            'top_id': t_sg_id,
+            'pod_id': pod_id,
+            'bottom_id': b_sg_id,
+            'resource_type': constants.RT_SG}
+        with t_ctx.session.begin():
+            core.create_resource(t_ctx, models.ResourceRouting, route1)
+
+        # test delete_sg when sg is not exit
+        rand_id = uuidutils.generate_uuid()
+        self.assertRaises(ext_sg.SecurityGroupNotFound,
+                          plugin.delete_security_group, q_ctx, rand_id)
+        # when sg is under deleting from Local
+        dict_para = {'resource_id': t_sg_id,
+                     'resource_type': t_constants.RT_SG}
+        q_ctx.USER_AGENT = t_constants.LOCAL
+        with t_ctx.session.begin():
+            core.create_resource(t_ctx, models.DeletingResources,
+                                 dict_para)
+        self.assertRaises(t_exceptions.ResourceNotFound,
+                          plugin.delete_security_group, q_ctx, t_sg_id)