NSX policy: Integration with new backend version

Previous APIs were marked obsolete by NSX Policy.
New version makes a few changes in the model:

1. Communication profiles (rule sets in gbp language) are removed
2. Services (classifiers in gbp language) are consumed directly
by communication maps.

Change-Id: I25dd63d44801449b925d3f5430d4ae9482e79e05

gbpservice/neutron/services/grouppolicy/drivers/vmware/nsx_policy/nsx_policy_mapping.py

@@ -72,6 +72,11 @@ class UpdateClassifierProtocolNotSupported(gpexc.GroupPolicyBadRequest):
                "with %s" % DRIVER_NAME)
 
 
+class UpdateClassifierDirectionNotSupported(gpexc.GroupPolicyBadRequest):
+    message = ("Update operation on classifier direction is not supported "
+               "with %s" % DRIVER_NAME)
+
+
 class ProtocolNotSupported(gpexc.GroupPolicyBadRequest):
     message = ("Unsupported classifier protocol. Only icmp, tcp and udp are "
                "supported with %s" % DRIVER_NAME)
@@ -203,19 +208,6 @@ class NsxPolicyMappingDriver(api.ResourceMappingDriver):
             LOG.warning('Domain %s was not found on backend',
                         project_id)
 
-    def _create_or_update_communication_profile(self, profile_id, name,
-                                                description, rules,
-                                                update_flow=False):
-
-        services = [rule['policy_classifier_id']
-                    for rule in rules]
-
-        self.nsx_policy.comm_profile.create_or_overwrite(
-                name=generate_nsx_name(profile_id, name),
-                profile_id=profile_id,
-                description=description,
-                services=services)
-
     def _split_rules_by_direction(self, context, rules):
         in_dir = [g_const.GP_DIRECTION_BI, g_const.GP_DIRECTION_IN]
         out_dir = [g_const.GP_DIRECTION_BI, g_const.GP_DIRECTION_OUT]
@@ -236,44 +228,23 @@ class NsxPolicyMappingDriver(api.ResourceMappingDriver):
 
         return in_rules, out_rules
 
-    def _delete_comm_profile(self, comm_profile_id):
+    def _get_services_from_rule_set(self, context, rule_set_id):
-        try:
-            self.nsx_policy.comm_profile.delete(comm_profile_id)
-        except nsxlib_exc.ManagerError:
-            LOG.error('Communication profile %s not found on backend',
-                      comm_profile_id)
-
-    def _create_or_update_policy_rule_set(self, context, update_flow=False):
-
-        rule_set_id = context.current['id']
 
+        ruleset = self.gbp_plugin.get_policy_rule_set(
+                context._plugin_context, rule_set_id)
         rules = self.gbp_plugin.get_policy_rules(
                 context._plugin_context,
-                {'id': context.current['policy_rules']})
+                {'id': ruleset['policy_rules']})
 
         in_rules, out_rules = self._split_rules_by_direction(context, rules)
+        in_services = set()
+        for rule in in_rules:
+            in_services.add(rule['policy_classifier_id'])
+        out_services = set()
+        for rule in out_rules:
+            out_services.add(rule['policy_classifier_id'])
 
-        if in_rules:
+        return sorted(list(in_services)), sorted(list(out_services))
-            self._create_or_update_communication_profile(
-                append_in_dir(rule_set_id),
-                generate_nsx_name(rule_set_id,
-                                  context.current['name'],
-                                  '_IN'),
-                context.current['description'] + '(ingress)',
-                in_rules)
-        elif update_flow:
-            self._delete_comm_profile(append_in_dir(rule_set_id))
-
-        if out_rules:
-            self._create_or_update_communication_profile(
-                append_out_dir(rule_set_id),
-                generate_nsx_name(rule_set_id,
-                                  context.current['name'],
-                                  '_OUT'),
-                context.current['description'] + '(egress)',
-                out_rules)
-        elif update_flow:
-            self._delete_comm_profile(append_out_dir(rule_set_id))
 
     def _filter_ptgs_by_ruleset(self, ptgs, ruleset_id):
         providing_ptgs = [ptg['id'] for ptg in ptgs
@@ -282,8 +253,15 @@ class NsxPolicyMappingDriver(api.ResourceMappingDriver):
                           if ruleset_id in ptg['consumed_policy_rule_sets']]
         return providing_ptgs, consuming_ptgs
 
-    def _map_rule_set(self, ptgs, profiles, project_id,
+    def _map_rule_set(self, context, ptgs, project_id,
-                      group_id, ruleset_id, delete_flow):
+                      ruleset_id, delete_flow):
+
+        def delete_map_if_exists(ruleset):
+            try:
+                self.nsx_policy.comm_map.delete(project_id, ruleset)
+            except nsxlib_exc.ManagerError:
+                # TODO(annak) - narrow this exception down
+                pass
 
         providing_ptgs, consuming_ptgs = self._filter_ptgs_by_ruleset(
             ptgs, ruleset_id)
@@ -298,53 +276,54 @@ class NsxPolicyMappingDriver(api.ResourceMappingDriver):
 
             # we need to delete map entry if exists
             for ruleset in (ruleset_in, ruleset_out):
-                if ruleset in profiles:
+                delete_map_if_exists(ruleset)
-                    try:
+
-                        self.nsx_policy.comm_map.delete(project_id, ruleset)
-                    except nsxlib_exc.ManagerError:
-                        pass
             return
 
-        if ruleset_in in profiles:
+        services_in, services_out = self._get_services_from_rule_set(
+            context, ruleset_id)
+
+        if services_in:
             self.nsx_policy.comm_map.create_or_overwrite(
                 name=ruleset_in,
                 domain_id=project_id,
                 map_id=ruleset_in,
                 description="GBP ruleset ingress",
-                    profile_id=ruleset_in,
+                service_ids=services_in,
                 source_groups=consuming_ptgs,
                 dest_groups=providing_ptgs)
+        else:
+            delete_map_if_exists(ruleset_in)
 
-        if ruleset_out in profiles:
+        if services_out:
             self.nsx_policy.comm_map.create_or_overwrite(
                 name=ruleset_out,
                 domain_id=project_id,
                 map_id=ruleset_out,
                 description="GBP ruleset egress",
-                    profile_id=ruleset_out,
+                service_ids=services_out,
                 source_groups=providing_ptgs,
                 dest_groups=consuming_ptgs)
+        else:
+            delete_map_if_exists(ruleset_out)
 
-    def _map_group_rule_sets(self, context, group_id,
+    def _map_group_rule_sets(self, context,
                              provided_policy_rule_sets,
                              consumed_policy_rule_sets,
                              delete_flow=False):
 
         project_id = context.current['project_id']
 
-        profiles = self.nsx_policy.comm_profile.list()
-        profiles = [p['id'] for p in profiles]
-
         # create communication maps
         ptgs = context._plugin.get_policy_target_groups(
             context._plugin_context)
         for ruleset in provided_policy_rule_sets:
-            self._map_rule_set(ptgs, profiles, project_id,
+            self._map_rule_set(context, ptgs, project_id,
-                               group_id, ruleset, delete_flow)
+                               ruleset, delete_flow)
 
         for ruleset in consumed_policy_rule_sets:
-            self._map_rule_set(ptgs, profiles, project_id,
+            self._map_rule_set(context, ptgs, project_id,
-                               group_id, ruleset, delete_flow)
+                               ruleset, delete_flow)
 
     # overrides base class, called from base group_create_postcommit
     # REVISIT(annak): Suggest a better design for driver-specific callbacks,
@@ -365,10 +344,8 @@ class NsxPolicyMappingDriver(api.ResourceMappingDriver):
                            provided_policy_rule_sets,
                            consumed_policy_rule_sets, op):
 
-        group_id = context.current['id']
-
         self._map_group_rule_sets(
-             context, group_id,
+             context,
              provided_policy_rule_sets,
              consumed_policy_rule_sets,
              delete_flow=(op == "DISASSOCIATE"))
@@ -402,7 +379,7 @@ class NsxPolicyMappingDriver(api.ResourceMappingDriver):
             ports = [str(p) for p in range(lower, upper)]
 
         # service entry in nsx policy has single direction
-        # directions will be enforced on communication profile level
+        # directions will be enforced on communication map level
         self.nsx_policy.service.create_or_overwrite(
             name=generate_nsx_name(classifier['id'], classifier['name']),
             service_id=classifier['id'],
@@ -421,7 +398,7 @@ class NsxPolicyMappingDriver(api.ResourceMappingDriver):
             raise HierarchicalContractsNotSupported()
 
     def create_policy_rule_set_postcommit(self, context):
-        self._create_or_update_policy_rule_set(context)
+        pass
 
     def create_policy_target_precommit(self, context):
         super(NsxPolicyMappingDriver,
@@ -515,17 +492,7 @@ class NsxPolicyMappingDriver(api.ResourceMappingDriver):
         pass
 
     def delete_policy_rule_set_postcommit(self, context):
-        ruleset_id = context.current['id']
+        pass
-        rules = self.gbp_plugin.get_policy_rules(
-                context._plugin_context,
-                {'id': context.current['policy_rules']})
-
-        in_rules, out_rules = self._split_rules_by_direction(context, rules)
-        if in_rules:
-            self._delete_comm_profile(append_in_dir(ruleset_id))
-
-        if out_rules:
-            self._delete_comm_profile(append_out_dir(ruleset_id))
 
     def delete_policy_target_postcommit(self, context):
         # This is inherited behavior without:
@@ -541,7 +508,9 @@ class NsxPolicyMappingDriver(api.ResourceMappingDriver):
         self._reject_shared(context.current, 'policy_rule_set')
 
     def update_policy_rule_set_postcommit(self, context):
-        self._create_or_update_policy_rule_set(context, update_flow=True)
+        if (context.current['policy_rules'] !=
+            context.original['policy_rules']):
+            self._on_policy_rule_set_updated(context, context.current)
 
     def update_policy_target_precommit(self, context):
         # Parent call verifies change of PTG is not supported
@@ -554,10 +523,27 @@ class NsxPolicyMappingDriver(api.ResourceMappingDriver):
               self).update_policy_target_postcommit(context)
 
     def update_policy_rule_precommit(self, context):
-        raise UpdateOperationNotSupported()
+        super(NsxPolicyMappingDriver,
+              self).update_policy_rule_precommit(context)
+
+    def _on_policy_rule_set_updated(self, context, prs):
+        ptgs = context._plugin.get_policy_target_groups(
+            context._plugin_context)
+
+        self._map_rule_set(context, ptgs, prs['project_id'],
+                           prs['id'], False)
 
     def update_policy_rule_postcommit(self, context):
-        pass
+        if (context.current['policy_classifier_id'] !=
+            context.original['policy_classifier_id']):
+            # All groups using this rule need to be updated
+            prs_ids = (
+                context._plugin._get_policy_rule_policy_rule_sets(
+                    context._plugin_context, context.current['id']))
+            policy_rule_sets = context._plugin.get_policy_rule_sets(
+                context._plugin_context, filters={'id': prs_ids})
+            for prs in policy_rule_sets:
+                self._on_policy_rule_set_updated(context, prs)
 
     def update_policy_action_precommit(self, context):
         raise UpdateOperationNotSupported()
@@ -566,6 +552,10 @@ class NsxPolicyMappingDriver(api.ResourceMappingDriver):
         if context.current['protocol'] != context.original['protocol']:
             raise UpdateClassifierProtocolNotSupported()
 
+        # TODO(annak): support this
+        if context.current['direction'] != context.original['direction']:
+            raise UpdateClassifierDirectionNotSupported()
+
     def update_policy_classifier_postcommit(self, context):
         self.create_policy_classifier_postcommit(context)
 

gbpservice/neutron/tests/unit/services/grouppolicy/test_nsx_policy_mapping_driver.py

@@ -23,11 +23,9 @@ from gbpservice.neutron.services.grouppolicy.drivers.vmware.nsx_policy import (
     nsx_policy_mapping as driver)
 from gbpservice.neutron.tests.unit.services.grouppolicy import (
     test_resource_mapping as test_rmd)
-import unittest2
 
 
 TEST_PROJECT = 'test-project'
-TEMPORARY_SKIP = 'skipping temporarily while adjusting to next backend version'
 
 
 class NsxPolicyMappingTestCase(test_rmd.ResourceMappingTestCase):
@@ -85,30 +83,6 @@ class NsxPolicyMappingTestCase(test_rmd.ResourceMappingTestCase):
     def _mock_icmp_service_delete(self):
         return mock.patch.object(self.nsx_policy.icmp_service, 'delete')
 
-    def _mock_profile_create(self):
-        return mock.patch.object(self.nsx_policy.comm_profile,
-                                 'create_or_overwrite')
-
-    def _mock_nth_profile_create_fails(self, n=2):
-        self.call_count = 1
-
-        def raise_on_nth_call(**kwargs):
-            if self.call_count == n:
-                raise nsxlib_exc.ManagerError
-            else:
-                self.call_count += 1
-        return mock.patch.object(self.nsx_policy.comm_profile,
-                                 'create_or_overwrite',
-                                 side_effect=raise_on_nth_call)
-
-    def _mock_profile_delete(self):
-        return mock.patch.object(self.nsx_policy.comm_profile, 'delete')
-
-    def _mock_profile_list(self, profile_ids):
-        return mock.patch.object(self.nsx_policy.comm_profile, 'list',
-                                 return_value=[{'id': p}
-                                               for p in profile_ids])
-
     def _mock_group_create(self):
         return mock.patch.object(self.nsx_policy.group, 'create_or_overwrite')
 
@@ -162,7 +136,6 @@ class NsxPolicyMappingTestCase(test_rmd.ResourceMappingTestCase):
 
 class TestPolicyClassifier(NsxPolicyMappingTestCase):
 
-    @unittest2.skip(TEMPORARY_SKIP)
     def test_l4_lifecycle(self):
         with self._mock_service_create() as service_create_call, \
             self._mock_service_delete() as service_delete_call:
@@ -187,8 +160,7 @@ class TestPolicyClassifier(NsxPolicyMappingTestCase):
             # classifier update
             cl = self.update_policy_classifier(
                     cl['id'],
-                    port_range='443',
+                    port_range='443')['policy_classifier']
-                    direction='in')['policy_classifier']
 
             service_create_call.assert_called_with(
                 name=mock.ANY,
@@ -202,7 +174,6 @@ class TestPolicyClassifier(NsxPolicyMappingTestCase):
 
             service_delete_call.assert_called_with(cl['id'])
 
-    @unittest2.skip(TEMPORARY_SKIP)
     def test_create_port_range(self):
         with self._mock_service_create() as service_create_call:
 
@@ -219,7 +190,6 @@ class TestPolicyClassifier(NsxPolicyMappingTestCase):
                 dest_ports=port_list,
                 service_id=mock.ANY)
 
-    @unittest2.skip(TEMPORARY_SKIP)
     def test_create_without_ports(self):
         with self._mock_service_create() as service_create_call:
 
@@ -234,7 +204,6 @@ class TestPolicyClassifier(NsxPolicyMappingTestCase):
                 dest_ports=[],
                 service_id=mock.ANY)
 
-    @unittest2.skip(TEMPORARY_SKIP)
     def test_icmp_lifecycle(self):
         with self._mock_icmp_service_create() as service_create_call, \
             self._mock_icmp_service_delete() as service_delete_call:
@@ -251,7 +220,6 @@ class TestPolicyClassifier(NsxPolicyMappingTestCase):
 
             service_delete_call.assert_called_with(cl['id'])
 
-    @unittest2.skip(TEMPORARY_SKIP)
     def test_update_protocol_fails(self):
         with self._mock_icmp_service_create():
 
@@ -266,7 +234,6 @@ class TestPolicyClassifier(NsxPolicyMappingTestCase):
                               protocol='tcp',
                               dest_ports=['80'])
 
-    @unittest2.skip(TEMPORARY_SKIP)
     def test_icmpv6_protocol_fails(self):
         self.assertRaises(webob.exc.HTTPClientError,
                           self.create_policy_classifier,
@@ -277,13 +244,18 @@ class TestPolicyClassifier(NsxPolicyMappingTestCase):
 
 class TestPolicyTargetGroup(NsxPolicyMappingTestCase):
 
-    def _prepare_rule_set(self, name='test'):
+    def _prepare_rule_set(self, name='test_rule',
-        with self._mock_service_create(),\
+                          direction='bi', rule_count=1):
-             self._mock_profile_create():
+        self.rules = []
+        with self._mock_service_create():
 
-            rule = self._create_simple_policy_rule()
+            for i in range(rule_count):
-            return self.create_policy_rule_set(
+                self.rules.append(
-                    name=name, policy_rules=[rule['id']])['policy_rule_set']
+                    self._create_simple_policy_rule(direction=direction))
+
+            rule_ids = [rule['id'] for rule in self.rules]
+            return self.create_policy_rule_set(name=name,
+                policy_rules=rule_ids)['policy_rule_set']
 
     def assert_neutron_resources(self, net_count, subnet_count, port_count):
         networks = self._plugin.get_networks(self._context)
@@ -305,25 +277,27 @@ class TestPolicyTargetGroup(NsxPolicyMappingTestCase):
                     cond_val=group_id,
                     group_id=group_id)
 
-    def ingress_map_call(self, prs_id, provider_ids, consumer_ids):
+    def ingress_map_call(self, prs_id, provider_ids,
+                         consumer_ids, service_ids):
         return call(domain_id=TEST_PROJECT,
-                    profile_id=driver.append_in_dir(prs_id),
                     map_id=mock.ANY,
                     name=driver.append_in_dir(prs_id),
+                    service_ids=service_ids,
                     description=mock.ANY,
                     source_groups=consumer_ids,
-                    dest_groups=provider_ids)
+                    dest_groups=provider_ids,
+                    )
 
-    def egress_map_call(self, prs_id, provider_ids, consumer_ids):
+    def egress_map_call(self, prs_id, provider_ids,
+                        consumer_ids, service_ids):
         return call(domain_id=TEST_PROJECT,
-                    profile_id=driver.append_out_dir(prs_id),
                     map_id=mock.ANY,
+                    service_ids=service_ids,
                     name=driver.append_out_dir(prs_id),
                     description=mock.ANY,
                     source_groups=provider_ids,
                     dest_groups=consumer_ids)
 
-    @unittest2.skip(TEMPORARY_SKIP)
     def test_create_first_ptg_for_project(self):
         '''Create first ptg for tenant and verify domain creation'''
 
@@ -340,27 +314,27 @@ class TestPolicyTargetGroup(NsxPolicyMappingTestCase):
             group_create.assert_has_calls([self.group_call('test', ptg['id'])])
             map_create.assert_not_called()
 
-    def _test_ptg_pair_with_single_rule(self,
+    def _test_ptg_pair_with_rule_set(self,
                                      direction_in=True,
-                                        direction_out=True):
+                                     direction_out=True,
+                                     rule_count=1):
         '''Test consumer and producer group pair with single rule lifecycle.
 
         Verify backend group and rule creation calls.
         Verify spawned neutron resources.
         '''
 
-        policy_rule_set = self._prepare_rule_set()
+        direction = 'bi'
-        profile_in = driver.append_in_dir(policy_rule_set['id'])
+        if not direction_in:
-        profile_out = driver.append_out_dir(policy_rule_set['id'])
+            direction = 'out'
-        profile_ids = []
+        if not direction_out:
-        if direction_in:
+            direction = 'in'
-            profile_ids.append(profile_in)
+
-        if direction_out:
+        policy_rule_set = self._prepare_rule_set(direction=direction,
-            profile_ids.append(profile_out)
+                                                 rule_count=rule_count)
 
         # Create group pair
         with self._mock_group_create() as group_create,\
-            self._mock_profile_list(profile_ids),\
             self._mock_map_create() as map_create,\
             self._mock_domain_create():
 
@@ -374,14 +348,20 @@ class TestPolicyTargetGroup(NsxPolicyMappingTestCase):
 
             # validate communication map creation on backend
             calls = []
+            classifier_ids = sorted([rule['policy_classifier_id']
+                                     for rule in self.rules])
             if direction_in:
-                calls.append(self.ingress_map_call(policy_rule_set['id'],
+                calls.append(self.ingress_map_call(
+                    policy_rule_set['id'],
                     [provider_ptg],
-                                                   [consumer_ptg]))
+                    [consumer_ptg],
+                    classifier_ids))
             if direction_out:
-                calls.append(self.egress_map_call(policy_rule_set['id'],
+                calls.append(self.egress_map_call(
+                    policy_rule_set['id'],
                     [provider_ptg],
-                                                  [consumer_ptg]))
+                    [consumer_ptg],
+                    classifier_ids))
             map_create.assert_has_calls(calls)
 
             # validate neutron resources
@@ -389,7 +369,6 @@ class TestPolicyTargetGroup(NsxPolicyMappingTestCase):
 
         # Delete producer
         with self._mock_map_delete() as map_delete,\
-            self._mock_profile_list(profile_ids),\
             self._mock_group_delete() as group_delete,\
             self._mock_domain_delete() as domain_delete:
 
@@ -416,7 +395,6 @@ class TestPolicyTargetGroup(NsxPolicyMappingTestCase):
 
         # Delete consumer
         with self._mock_map_delete() as map_delete,\
-            self._mock_profile_list(profile_ids),\
             self._mock_group_delete() as group_delete,\
             self._mock_domain_delete() as domain_delete:
 
@@ -431,19 +409,24 @@ class TestPolicyTargetGroup(NsxPolicyMappingTestCase):
             # last group is deleted, domain should go as well
             domain_delete.assert_called_with(TEST_PROJECT)
 
-    @unittest2.skip(TEMPORARY_SKIP)
+    def test_ptg_pair_with_single_rule_in(self):
-    def test_create_ptg_pair_with_single_rule_in(self):
+        self._test_ptg_pair_with_rule_set(True, False)
-        self._test_ptg_pair_with_single_rule(True, False)
 
-    @unittest2.skip(TEMPORARY_SKIP)
+    def test_ptg_pair_with_single_rule_out(self):
-    def test_create_ptg_pair_with_single_rule_out(self):
+        self._test_ptg_pair_with_rule_set(False, True)
-        self._test_ptg_pair_with_single_rule(False, True)
 
-    @unittest2.skip(TEMPORARY_SKIP)
+    def test_ptg_pair_with_single_rule_bi(self):
-    def test_create_ptg_pair_with_single_rule_bi(self):
+        self._test_ptg_pair_with_rule_set(True, True)
-        self._test_ptg_pair_with_single_rule(True, True)
+
+    def test_ptg_pair_with_multi_rule_in(self):
+        self._test_ptg_pair_with_rule_set(True, False, 2)
+
+    def test_ptg_pair_with_multi_rule_out(self):
+        self._test_ptg_pair_with_rule_set(False, True, 3)
+
+    def test_ptg_pair_with_multi_rule_bi(self):
+        self._test_ptg_pair_with_rule_set(True, True, 5)
 
-    @unittest2.skip(TEMPORARY_SKIP)
     def test_create_fail_isolated(self):
         '''Verify integrity when backend fails on isolated group creation.
 
@@ -470,7 +453,6 @@ class TestPolicyTargetGroup(NsxPolicyMappingTestCase):
 
             self.assert_neutron_rollback()
 
-    @unittest2.skip(TEMPORARY_SKIP)
     def test_create_fail_connected(self):
         '''Verify integrity when backend fails on connectivity map creation
 
@@ -481,11 +463,8 @@ class TestPolicyTargetGroup(NsxPolicyMappingTestCase):
         '''
 
         policy_rule_set = self._prepare_rule_set()
-        profile_ids = [driver.append_in_dir(policy_rule_set['id']),
-                       driver.append_out_dir(policy_rule_set['id'])]
 
         with self._mock_group_create(),\
-            self._mock_profile_list(profile_ids),\
             self._mock_map_create_fails(),\
             self._mock_group_delete() as group_delete:
 
@@ -497,7 +476,6 @@ class TestPolicyTargetGroup(NsxPolicyMappingTestCase):
 
             self.assert_neutron_resources(1, 1, 1)
 
-    @unittest2.skip(TEMPORARY_SKIP)
     def test_create_fail_multi_connected(self):
         '''Verify integrity when backend fails on connectivity map creation
 
@@ -509,15 +487,14 @@ class TestPolicyTargetGroup(NsxPolicyMappingTestCase):
         '''
 
         prs1 = self._prepare_rule_set()['id']
+        service1 = self.rules[0]['policy_classifier_id']
         prs2 = self._prepare_rule_set()['id']
+        service2 = self.rules[0]['policy_classifier_id']
         prs3 = self._prepare_rule_set()['id']
-        profile_ids = [driver.append_in_dir(prs1), driver.append_out_dir(prs1),
+        service3 = self.rules[0]['policy_classifier_id']
-                       driver.append_in_dir(prs2), driver.append_out_dir(prs2),
-                       driver.append_in_dir(prs3), driver.append_out_dir(prs3)]
 
         # Create a and c
         with self._mock_group_create(),\
-            self._mock_profile_list(profile_ids),\
             self._mock_map_create():
 
             ab_dict = {prs1: None}
@@ -530,7 +507,6 @@ class TestPolicyTargetGroup(NsxPolicyMappingTestCase):
                 consumed_policy_rule_sets=bc_dict)['policy_target_group']['id']
 
         with self._mock_group_create(),\
-            self._mock_profile_list(profile_ids),\
             self._mock_nth_map_create_fails(n=6) as map_create,\
             self._mock_map_delete() as map_delete,\
             self._mock_group_delete() as group_delete:
@@ -542,12 +518,13 @@ class TestPolicyTargetGroup(NsxPolicyMappingTestCase):
                               provided_policy_rule_sets=bc_dict)
 
             b = mock.ANY
-            map_create_calls = [self.ingress_map_call(prs1, [a], [b]),
+            map_create_calls = [
-                                self.egress_map_call(prs1, [a], [b]),
+                self.ingress_map_call(prs1, [a], [b], [service1]),
-                                self.ingress_map_call(prs2, [b], [c]),
+                self.egress_map_call(prs1, [a], [b], [service1]),
-                                self.egress_map_call(prs2, [b], [c]),
+                self.ingress_map_call(prs2, [b], [c], [service2]),
-                                self.ingress_map_call(prs3, [b], [c]),
+                self.egress_map_call(prs2, [b], [c], [service2]),
-                                self.egress_map_call(prs3, [b], [c])]
+                self.ingress_map_call(prs3, [b], [c], [service3]),
+                self.egress_map_call(prs3, [b], [c], [service3])]
 
             map_create.assert_has_calls(map_create_calls, any_order=True)
 
@@ -564,7 +541,6 @@ class TestPolicyTargetGroup(NsxPolicyMappingTestCase):
 
             self.assert_neutron_resources(2, 2, 2)
 
-    @unittest2.skip(TEMPORARY_SKIP)
     def test_create_ptg_pair_multi_rule_set(self):
         '''Create ptg pair based on 3 rule sets
 
@@ -572,17 +548,14 @@ class TestPolicyTargetGroup(NsxPolicyMappingTestCase):
         second - only egress connectivity, and third - both
         '''
         prs1 = self._prepare_rule_set()['id']
+        service1 = self.rules[0]['policy_classifier_id']
         prs2 = self._prepare_rule_set()['id']
+        service2 = self.rules[0]['policy_classifier_id']
         prs3 = self._prepare_rule_set()['id']
-
+        service3 = self.rules[0]['policy_classifier_id']
-        profile_ids = [driver.append_in_dir(prs1),
-                       driver.append_out_dir(prs2),
-                       driver.append_in_dir(prs3),
-                       driver.append_out_dir(prs3)]
 
         with self._mock_domain_create(),\
             self._mock_group_create() as group_create,\
-            self._mock_profile_list(profile_ids),\
             self._mock_map_create() as map_create:
 
             rule_set_dict = {prs1: None, prs2: None, prs3: None}
@@ -598,25 +571,27 @@ class TestPolicyTargetGroup(NsxPolicyMappingTestCase):
                  self.group_call('ptg2', consumer_id)])
 
             map_calls = [
-                self.ingress_map_call(prs1, [provider_id], [consumer_id]),
+                self.ingress_map_call(prs1, [provider_id],
-                self.egress_map_call(prs2, [provider_id], [consumer_id]),
+                                      [consumer_id], [service1]),
-                self.ingress_map_call(prs3, [provider_id], [consumer_id]),
+                self.egress_map_call(prs2, [provider_id],
-                self.egress_map_call(prs3, [provider_id], [consumer_id])]
+                                     [consumer_id], [service2]),
+                self.ingress_map_call(prs3, [provider_id],
+                                      [consumer_id], [service3]),
+                self.egress_map_call(prs3, [provider_id],
+                                     [consumer_id], [service3])]
             map_create.assert_has_calls(map_calls, any_order=True)
 
-    @unittest2.skip(TEMPORARY_SKIP)
     def test_create_ptg_ring(self):
         ring_size = 10
 
         prs_ids = []
+        services = []
         for i in range(0, ring_size):
             prs_ids.append(self._prepare_rule_set()['id'])
-
+            services.append(self.rules[0]['policy_classifier_id'])
-        profile_ids = [driver.append_in_dir(prs_id) for prs_id in prs_ids]
 
         # Create ring topology
         with self._mock_domain_create(),\
-            self._mock_profile_list(profile_ids),\
             self._mock_group_create() as group_create,\
             self._mock_map_create() as map_create:
 
@@ -641,11 +616,13 @@ class TestPolicyTargetGroup(NsxPolicyMappingTestCase):
                     map_calls.append(self.ingress_map_call(
                         prs_ids[i],
                         [ptg_id],
-                        [ptg_ids[i - 1]]))
+                        [ptg_ids[i - 1]],
+                        [services[i]]))
 
             map_calls.append(self.ingress_map_call(prs_ids[0],
                                                    [ptg_ids[0]],
-                                                   [ptg_id]))
+                                                   [ptg_id],
+                                                   [services[0]]))
 
             group_create.assert_has_calls(group_calls)
             map_create.assert_has_calls(map_calls, any_order=True)
@@ -655,14 +632,15 @@ class TestPolicyTargetGroup(NsxPolicyMappingTestCase):
         # Delete single group and verify connectors are deleted
         with self._mock_map_delete() as map_delete,\
             self._mock_map_create() as map_create,\
-            self._mock_profile_list(profile_ids),\
             self._mock_group_delete() as group_delete:
 
             ptg_id = ptg_ids[2]
             self.delete_policy_target_group(ptg_id)
 
             map_calls = [call(TEST_PROJECT, driver.append_in_dir(prs_ids[2])),
-                         call(TEST_PROJECT, driver.append_in_dir(prs_ids[3]))]
+                         call(TEST_PROJECT, driver.append_out_dir(prs_ids[2])),
+                         call(TEST_PROJECT, driver.append_in_dir(prs_ids[3])),
+                         call(TEST_PROJECT, driver.append_out_dir(prs_ids[3]))]
 
             map_delete.assert_has_calls(map_calls)
             map_create.assert_not_called()
@@ -671,7 +649,6 @@ class TestPolicyTargetGroup(NsxPolicyMappingTestCase):
         # Remove connectors from single group
         with self._mock_map_delete() as map_delete,\
             self._mock_map_create() as map_create,\
-            self._mock_profile_list(profile_ids),\
             self._mock_group_delete() as group_delete:
 
             ptg_id = ptg_ids[5]
@@ -683,18 +660,16 @@ class TestPolicyTargetGroup(NsxPolicyMappingTestCase):
             map_create.assert_not_called()
             group_delete.assert_not_called()
 
-    @unittest2.skip(TEMPORARY_SKIP)
     def test_create_ptg_star(self):
         '''Star-like topology (single producer and N consumers) lifecycle'''
 
         star_size = 10
         policy_rule_set = self._prepare_rule_set()
+        service = self.rules[0]['policy_classifier_id']
         prs_id = policy_rule_set['id']
-        profile_ids = [driver.append_in_dir(prs_id)]
 
         # Create topology
         with self._mock_domain_create(),\
-            self._mock_profile_list(profile_ids),\
             self._mock_group_create() as group_create,\
             self._mock_map_create() as map_create:
 
@@ -722,7 +697,14 @@ class TestPolicyTargetGroup(NsxPolicyMappingTestCase):
                 map_calls.append(self.ingress_map_call(
                     prs_id,
                     [provider_id],
-                    consumer_ids[:]))
+                    consumer_ids[:],
+                    [service]))
+
+                map_calls.append(self.egress_map_call(
+                    prs_id,
+                    [provider_id],
+                    consumer_ids[:],
+                    [service]))
 
             group_create.assert_has_calls(group_calls)
             map_create.assert_has_calls(map_calls)
@@ -733,7 +715,6 @@ class TestPolicyTargetGroup(NsxPolicyMappingTestCase):
         # Delete one consumer group
         with self._mock_map_delete() as map_delete,\
             self._mock_map_create() as map_create,\
-            self._mock_profile_list(profile_ids),\
             self._mock_group_delete() as group_delete:
 
             consumer_id = consumer_ids.pop(0)
@@ -742,7 +723,8 @@ class TestPolicyTargetGroup(NsxPolicyMappingTestCase):
             map_create.assert_has_calls(
                 [self.ingress_map_call(prs_id,
                                       [provider_id],
-                                      consumer_ids)])
+                                      consumer_ids,
+                                      [service])])
 
             map_delete.assert_not_called()
 
@@ -754,164 +736,104 @@ class TestPolicyTargetGroup(NsxPolicyMappingTestCase):
         # Delete provider group
         with self._mock_map_delete() as map_delete,\
             self._mock_map_create() as map_create,\
-            self._mock_profile_list(profile_ids),\
             self._mock_group_delete() as group_delete:
 
             self.delete_policy_target_group(provider_id)
 
             map_create.assert_not_called()
-            map_delete.assert_called_with(TEST_PROJECT,
+            map_delete.assert_has_calls(
-                                          driver.append_in_dir(prs_id))
+                [call(TEST_PROJECT, driver.append_in_dir(prs_id)),
+                 call(TEST_PROJECT, driver.append_out_dir(prs_id))])
 
             star_size -= 1
             group_delete.assert_called_with(TEST_PROJECT, provider_id)
 
+    def test_update_rule_set(self):
+        '''Under consumer and producer group pair with single rule lifecycle,
+        Update rule set and verify backend gets updated
+        '''
+        rule_set = self._prepare_rule_set(direction='in')
+
+        # Create group pair
+        with self._mock_group_create(), \
+            self._mock_map_create(), \
+            self._mock_domain_create():
+
+            provider_ptg, consumer_ptg = self._create_provider_consumer_ptgs(
+                    rule_set['id'])
+
+        # Add another rule to the rule set
+        with self._mock_service_create(),\
+            self._mock_map_create() as map_update:
+
+            rule = self._create_simple_policy_rule(direction='in')
+            self.rules.append(rule)
+
+            rules = [rule['id'] for rule in self.rules]
+            self.update_policy_rule_set(rule_set['id'], policy_rules=rules)
+            classifier_ids = sorted([rule['policy_classifier_id']
+                                     for rule in self.rules])
+
+            map_update.assert_has_calls([self.ingress_map_call(
+                rule_set['id'],
+                [provider_ptg],
+                [consumer_ptg],
+                classifier_ids)])
+
+        # Delete all rules from rule set, leaving the rule set
+        with self._mock_map_delete() as map_delete:
+            self.update_policy_rule_set(rule_set['id'], policy_rules=[])
+
+            # verify communication map delete on backend
+            map_delete.assert_any_call(
+                TEST_PROJECT, driver.append_in_dir(rule_set['id']))
+
+    def test_update_rule_classifier(self):
+        '''Under consumer and producer group pair with single rule lifecycle,
+        Update rule set and verify backend gets updated
+        '''
+        rule_set = self._prepare_rule_set(direction='in')
+
+        # Create group pair
+        with self._mock_group_create(), \
+            self._mock_map_create(), \
+            self._mock_domain_create():
+
+            provider_ptg, consumer_ptg = self._create_provider_consumer_ptgs(
+                    rule_set['id'])
+
+        # Update classifier in rule
+        with self._mock_service_create(),\
+            self._mock_map_create() as map_update:
+
+            cl = self.create_policy_classifier(
+                name='test',
+                protocol='UDP',
+                port_range='512',
+                direction='bi')['policy_classifier']
+
+            self.update_policy_rule(self.rules[0]['id'],
+                                    policy_classifier_id=cl['id'])
+
+            map_update.assert_has_calls([self.ingress_map_call(
+                rule_set['id'],
+                [provider_ptg],
+                [consumer_ptg],
+                [cl['id']])])
+
 
 class TestPolicyRuleSet(NsxPolicyMappingTestCase):
 
-    @unittest2.skip(TEMPORARY_SKIP)
+    def test_basic(self):
-    def test_bidirectional(self):
+        ''' Create and delete unused rule set'''
-        ''' Create and delete bidirectional rule set'''
-
-        with self._mock_profile_create() as profile_create,\
-            self._mock_profile_delete() as profile_delete:
 
         rule = self._create_simple_policy_rule()
-            rule_set = self.create_policy_rule_set(
+        rule_set = self.create_policy_rule_set(name='test',
-                name='test', policy_rules=[rule['id']])['policy_rule_set']
+            policy_rules=[rule['id']])['policy_rule_set']
-
-            calls = [call(name=mock.ANY,
-                          description=mock.ANY,
-                          profile_id=driver.append_in_dir(rule_set['id']),
-                          services=[rule['policy_classifier_id']]),
-                     call(name=mock.ANY,
-                          description=mock.ANY,
-                          profile_id=driver.append_out_dir(rule_set['id']),
-                          services=[rule['policy_classifier_id']])]
-
-            profile_create.assert_has_calls(calls)
 
+        self.update_policy_rule_set(rule_set['id'], name='test1')
         self.delete_policy_rule_set(rule_set['id'])
 
-            calls = [call(driver.append_in_dir(rule_set['id'])),
-                     call(driver.append_out_dir(rule_set['id']))]
-            profile_delete.assert_has_calls(calls)
-
-    @unittest2.skip(TEMPORARY_SKIP)
-    def test_empty(self):
-        ''' Create and delete empty rule set and verify no backend calls'''
-        rule = self._create_simple_policy_rule()
-        rule_set = self.create_policy_rule_set(
-            name='test', policy_rules=[rule['id']])['policy_rule_set']
-
-        self.delete_policy_rule_set(rule_set['id'])
-
-    @unittest2.skip(TEMPORARY_SKIP)
-    def test_create_fails(self):
-        ''' Create bidirectional rule set and fail second API call'''
-
-        with self._mock_nth_profile_create_fails() as profile_create,\
-            self._mock_profile_delete() as profile_delete:
-
-            rule = self._create_simple_policy_rule()
-            self.assertRaises(webob.exc.HTTPClientError,
-                              self.create_policy_rule_set,
-                              name='test',
-                              policy_rules=[rule['id']])
-
-            # Two create calls expected
-            calls = [call(name=mock.ANY,
-                          description=mock.ANY,
-                          profile_id=mock.ANY,
-                          services=[rule['policy_classifier_id']]),
-                     call(name=mock.ANY,
-                          description=mock.ANY,
-                          profile_id=mock.ANY,
-                          services=[rule['policy_classifier_id']])]
-
-            profile_create.assert_has_calls(calls)
-
-            # Rollback - two delete calls expected
-            calls = [call(mock.ANY), call(mock.ANY)]
-            profile_delete.assert_has_calls(calls)
-
-    @unittest2.skip(TEMPORARY_SKIP)
-    def _assert_profile_call(self, mock_calls,
-                             name, profile_id, services):
-        '''Asserts service list in any order'''
-
-        services_set = set(services)
-        for mock_call in mock_calls.call_args_list:
-            if isinstance(mock_call, dict):
-                if (mock_call.get('name') == name and
-                    mock_call.get('profile_id') == profile_id and
-                    set(mock_call.get('services')) == services_set):
-
-                    return True
-
-    @unittest2.skip(TEMPORARY_SKIP)
-    def test_multi_set(self):
-        '''Test lifecycle of set with 3 rules having different dirs'''
-
-        # Create rule set with 3 rules
-        with self._mock_profile_create() as profile_create:
-
-            rule1 = self._create_simple_policy_rule('in', 'tcp', '7887')
-            rule2 = self._create_simple_policy_rule('out', 'udp', '8778')
-            rule3 = self._create_simple_policy_rule('bi', 'tcp', '5060')
-
-            rule_set = self.create_policy_rule_set(
-                name='test', policy_rules=[rule1['id'],
-                                           rule2['id'],
-                                           rule3['id']])['policy_rule_set']
-
-            self.assertEqual(2, profile_create.call_count)
-            profile_create._assert_profile_call(
-                driver.append_in_dir('test'),
-                driver.append_in_dir(rule_set['id']),
-                [rule1['policy_classifier_id'], rule3['policy_classifier_id']])
-            profile_create._assert_profile_call(
-                driver.append_out_dir('test'),
-                driver.append_out_dir(rule_set['id']),
-                [rule2['policy_classifier_id'], rule3['policy_classifier_id']])
-
-        # Replace rule3 with rule4
-        with self._mock_profile_create() as profile_update:
-            rule4 = self._create_simple_policy_rule('out', 'tcp', '555:777')
-
-            rule_set1 = self.update_policy_rule_set(
-                rule_set['id'], policy_rules=[rule1['id'],
-                                              rule2['id'],
-                                              rule4['id']])['policy_rule_set']
-
-            self.assertEqual(rule_set['id'], rule_set1['id'])
-            self.assertEqual(2, profile_create.call_count)
-            profile_update._assert_profile_call(
-                driver.append_in_dir('test'),
-                driver.append_in_dir(rule_set['id']),
-                [rule1['policy_classifier_id']])
-            profile_update._assert_profile_call(
-                driver.append_out_dir('test'),
-                driver.append_out_dir(rule_set['id']),
-                [rule2['policy_classifier_id'], rule4['policy_classifier_id']])
-
-        # Delete rule1 from the rule set and verify ingress profile is
-        # is deleted on backend
-        with self._mock_profile_delete() as profile_delete:
-            self.update_policy_rule_set(rule_set['id'],
-                                        policy_rules=[rule2['id'],
-                                                      rule4['id']])
-
-            profile_delete.assert_called_once_with(
-                driver.append_in_dir(rule_set['id']))
-
-        # Delete the rule set and verify egress profile is deleted
-        with self._mock_profile_delete() as profile_delete:
-            self.delete_policy_rule_set(rule_set['id'])
-
-            profile_delete.assert_called_once_with(
-                driver.append_out_dir(rule_set['id']))
-
 
 class TestPolicyTargetTag(NsxPolicyMappingTestCase):
 
@@ -920,7 +842,6 @@ class TestPolicyTargetTag(NsxPolicyMappingTestCase):
             return self.create_policy_target_group(
                 name='test')['policy_target_group']
 
-    @unittest2.skip(TEMPORARY_SKIP)
     def test_target_lifecycle(self):
         self._mock_nsx_db()