SEBA-117 Allow FabricCrossconnectServiceInstance to specify westbound fields directly

Change-Id: Ie96fe912ecf68f5c8274c9fcbb2ee339bbe7f19d
diff --git a/samples/vm_crossconnect.yaml b/samples/vm_crossconnect.yaml
new file mode 100644
index 0000000..c07ccfb
--- /dev/null
+++ b/samples/vm_crossconnect.yaml
@@ -0,0 +1,40 @@
+# Copyright 2017-present Open Networking Foundation
+#
+# 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.
+
+# curl -H "xos-username: admin@opencord.org" -H "xos-password: letmein" -X POST --data-binary @pon_port.yaml http://192.168.99.100:30007/run
+
+tosca_definitions_version: tosca_simple_yaml_1_0
+imports:
+  - custom_types/fabriccrossconnectservice.yaml
+  - custom_types/fabriccrossconnectserviceinstance.yaml
+description: Create a FabricCrossconnectServiceInstance
+topology_template:
+  node_templates:
+    service#fabric-crossconnect:
+      type: tosca.nodes.FabricCrossconnectService
+      properties:
+        name: fabric-crossconnect
+        must-exist: true
+
+    fcsi:
+      type: tosca.nodes.FabricCrossconnectServiceInstance
+      properties:
+        name: "custom_vm_crossconnect"
+        s_tag: 123
+        source_port: 3
+        switch_datapath_id: "of:0000000000000201"
+      requirements:
+        - owner:
+            node: service#fabric-crossconnect
+            relationship: tosca.relationships.BelongsToOne
diff --git a/xos/synchronizer/model_policies/model_policy_fabriccrossconnectserviceinstance.py b/xos/synchronizer/model_policies/model_policy_fabriccrossconnectserviceinstance.py
index ea591dd..db439a3 100644
--- a/xos/synchronizer/model_policies/model_policy_fabriccrossconnectserviceinstance.py
+++ b/xos/synchronizer/model_policies/model_policy_fabriccrossconnectserviceinstance.py
@@ -14,7 +14,7 @@
 # limitations under the License.
 
 
-from synchronizers.new_base.modelaccessor import FabricCrossconnectServiceInstance, model_accessor
+from synchronizers.new_base.modelaccessor import FabricCrossconnectServiceInstance, ServiceInstance, model_accessor
 from synchronizers.new_base.policy import Policy
 from synchronizers.new_base.exceptions import *
 
@@ -39,6 +39,48 @@
                 service_instance.delete()
             return
 
+        # If there is a westbound link, then make sure the SerivceInstance is consistent with the
+        # westbound fields.
+
+        if service_instance.provided_links.exists():
+            updated_fields = []
+
+            si = ServiceInstance.objects.get(id=service_instance.id)
+            s_tag = si.get_westbound_service_instance_properties("s_tag")
+            switch_datapath_id = si.get_westbound_service_instance_properties("switch_datapath_id")
+            source_port = si.get_westbound_service_instance_properties("switch_port")
+
+            if (s_tag is None):
+                raise Exception("Westbound ServiceInstance s-tag is None on fcsi %s" % service_instance.id)
+
+            if (not switch_datapath_id):
+                raise Exception("Westbound ServiceInstance switch_datapath_id is unset on fcsi %s" % service_instance.id)
+
+            if (source_port is None):
+                raise Exception("Westbound ServiceInstance switch_port is None on fcsi %s" % service_instance.id)
+
+            s_tag = int(s_tag)
+            source_port = int(source_port)
+
+            if (s_tag != service_instance.s_tag):
+                if service_instance.s_tag is not None:
+                    raise Exception("Westbound ServiceInstance changing s-tag is not currently permitted")
+                service_instance.s_tag = s_tag
+                updated_fields.append("s_tag")
+            if (switch_datapath_id != service_instance.switch_datapath_id):
+                if service_instance.switch_datapath_id:
+                    raise Exception("Westbound ServiceInstance changing switch_datapath_id is not currently permitted")
+                service_instance.switch_datapath_id = switch_datapath_id
+                updated_fields.append("switch_datapath_id")
+            if (source_port != service_instance.source_port):
+                if service_instance.source_port is not None:
+                    raise Exception("Westbound ServiceInstance changing source_port is not currently permitted")
+                service_instance.source_port = source_port
+                updated_fields.append("source_port")
+
+            if updated_fields:
+                service_instance.save(update_fields = updated_fields)
+
     def handle_delete(self, service_instance):
         log.info("Handle_delete Fabric-Crossconnect Service Instance", service_instance=service_instance)
 
diff --git a/xos/synchronizer/model_policies/test_model_policy_fabriccrossconnectserviceinstance.py b/xos/synchronizer/model_policies/test_model_policy_fabriccrossconnectserviceinstance.py
new file mode 100644
index 0000000..0099881
--- /dev/null
+++ b/xos/synchronizer/model_policies/test_model_policy_fabriccrossconnectserviceinstance.py
@@ -0,0 +1,255 @@
+# Copyright 2017-present Open Networking Foundation
+#
+# 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.
+
+import unittest
+
+import functools
+from mock import patch, call, Mock, PropertyMock, MagicMock
+import requests_mock
+import multistructlog
+from multistructlog import create_logger
+
+import os, sys
+
+# Hack to load synchronizer framework
+test_path=os.path.abspath(os.path.dirname(os.path.realpath(__file__)))
+xos_dir=os.path.join(test_path, "../../..")
+if not os.path.exists(os.path.join(test_path, "new_base")):
+    xos_dir=os.path.join(test_path, "../../../../../../orchestration/xos/xos")
+    services_dir = os.path.join(xos_dir, "../../xos_services")
+sys.path.append(xos_dir)
+sys.path.append(os.path.join(xos_dir, 'synchronizers', 'new_base'))
+# END Hack to load synchronizer framework
+
+# generate model from xproto
+def get_models_fn(service_name, xproto_name):
+    name = os.path.join(service_name, "xos", xproto_name)
+    if os.path.exists(os.path.join(services_dir, name)):
+        return name
+    else:
+        name = os.path.join(service_name, "xos", "synchronizer", "models", xproto_name)
+        if os.path.exists(os.path.join(services_dir, name)):
+            return name
+    raise Exception("Unable to find service=%s xproto=%s" % (service_name, xproto_name))
+# END generate model from xproto
+
+def mock_get_westbound_service_instance_properties(props, prop):
+    return props[prop]
+
+def match_json(desired, req):
+    if desired!=req.json():
+        raise Exception("Got request %s, but body is not matching" % req.url)
+        return False
+    return True
+
+class TestPolicyFabricCrossconnectServiceInstance(unittest.TestCase):
+
+    def setUp(self):
+        global DeferredException
+
+        self.sys_path_save = sys.path
+        sys.path.append(xos_dir)
+        sys.path.append(os.path.join(xos_dir, 'synchronizers', 'new_base'))
+
+        # Setting up the config module
+        from xosconfig import Config
+        config = os.path.join(test_path, "../test_fabric_crossconnect_config.yaml")
+        Config.clear()
+        Config.init(config, "synchronizer-config-schema.yaml")
+        # END Setting up the config module
+
+        from synchronizers.new_base.mock_modelaccessor_build import build_mock_modelaccessor
+        build_mock_modelaccessor(xos_dir, services_dir, [get_models_fn("fabric-crossconnect", "fabric-crossconnect.xproto")])
+        import synchronizers.new_base.modelaccessor
+
+        from mock_modelaccessor import MockObjectList
+        self.MockObjectList = MockObjectList
+
+        from model_policy_fabriccrossconnectserviceinstance import FabricCrossconnectServiceInstancePolicy, \
+            model_accessor
+
+        # import all class names to globals
+        for (k, v) in model_accessor.all_model_classes.items():
+            globals()[k] = v
+
+        self.policy_step = FabricCrossconnectServiceInstancePolicy
+        self.policy_step.log = Mock()
+
+        # mock onos-fabric
+        self.onos_fabric = Service(name = "onos-fabric",
+                              rest_hostname = "onos-fabric",
+                              rest_port = "8181",
+                              rest_username = "onos",
+                              rest_password = "rocks")
+
+        self.service = FabricCrossconnectService(name = "fcservice",
+                                                 provider_services = [self.onos_fabric])
+
+    def mock_westbound(self, fsi, s_tag, switch_datapath_id, switch_port):
+        # Mock out a ServiceInstance so the syncstep can call get_westbound_service_instance_properties on it
+        si = ServiceInstance(id=fsi.id)
+        si.get_westbound_service_instance_properties = functools.partial(
+            mock_get_westbound_service_instance_properties,
+            {"s_tag": s_tag,
+             "switch_datapath_id": switch_datapath_id,
+             "switch_port": switch_port})
+
+        fsi.provided_links=Mock(exists=Mock(return_value=True))
+
+        return si
+
+    def test_handle_update(self):
+        with patch.object(ServiceInstance.objects, "get_items") as serviceinstance_objects, \
+            patch.object(FabricCrossconnectServiceInstance, "save") as fcsi_save:
+
+            fsi = FabricCrossconnectServiceInstance(id=7777, owner=self.service, s_tag=None, source_port=None,
+                                                    switch_datapath_id=None)
+
+            serviceinstance_objects.return_value = [fsi]
+
+            si = self.mock_westbound(fsi, s_tag=111, switch_datapath_id = "of:0000000000000201", switch_port = 3)
+            serviceinstance_objects.return_value = [si]
+
+            self.policy_step().handle_update(fsi)
+
+            self.assertEqual(fsi.s_tag, 111)
+            self.assertEqual(fsi.switch_datapath_id, "of:0000000000000201")
+            self.assertEqual(fsi.source_port, 3)
+
+            fcsi_save.assert_called_with(update_fields=["s_tag", "switch_datapath_id", "source_port"])
+
+    def test_handle_update_west_si_bad_s_tag(self):
+        with patch.object(ServiceInstance.objects, "get_items") as serviceinstance_objects, \
+            patch.object(FabricCrossconnectServiceInstance, "save") as fcsi_save:
+
+            fsi = FabricCrossconnectServiceInstance(id=7777, owner=self.service, s_tag=None, source_port=None,
+                                                    switch_datapath_id=None)
+
+            serviceinstance_objects.return_value = [fsi]
+
+            si = self.mock_westbound(fsi, s_tag=None, switch_datapath_id = "of:0000000000000201", switch_port = 3)
+            serviceinstance_objects.return_value = [si]
+
+            with self.assertRaises(Exception) as e:
+                self.policy_step().handle_update(fsi)
+
+            self.assertEqual(e.exception.message, "Westbound ServiceInstance s-tag is None on fcsi 7777")
+
+    def test_handle_update_west_si_bad_port(self):
+        with patch.object(ServiceInstance.objects, "get_items") as serviceinstance_objects, \
+            patch.object(FabricCrossconnectServiceInstance, "save") as fcsi_save:
+
+            fsi = FabricCrossconnectServiceInstance(id=7777, owner=self.service, s_tag=None, source_port=None,
+                                                    switch_datapath_id=None)
+
+            serviceinstance_objects.return_value = [fsi]
+
+            si = self.mock_westbound(fsi, s_tag=111, switch_datapath_id = "of:0000000000000201", switch_port = None)
+            serviceinstance_objects.return_value = [si]
+
+            with self.assertRaises(Exception) as e:
+                self.policy_step().handle_update(fsi)
+
+            self.assertEqual(e.exception.message, "Westbound ServiceInstance switch_port is None on fcsi 7777")
+
+    def test_handle_update_west_si_bad_switch_datapath_id(self):
+        with patch.object(ServiceInstance.objects, "get_items") as serviceinstance_objects, \
+            patch.object(FabricCrossconnectServiceInstance, "save") as fcsi_save:
+
+            fsi = FabricCrossconnectServiceInstance(id=7777, owner=self.service, s_tag=None, source_port=None,
+                                                    switch_datapath_id=None)
+
+            serviceinstance_objects.return_value = [fsi]
+
+            si = self.mock_westbound(fsi, s_tag=111, switch_datapath_id = None, switch_port = 3)
+            serviceinstance_objects.return_value = [si]
+
+            with self.assertRaises(Exception) as e:
+                self.policy_step().handle_update(fsi)
+
+            self.assertEqual(e.exception.message, "Westbound ServiceInstance switch_datapath_id is unset on fcsi 7777")
+
+    def test_handle_update_no_links(self):
+        with patch.object(ServiceInstance.objects, "get_items") as serviceinstance_objects, \
+            patch.object(FabricCrossconnectServiceInstance, "save") as fcsi_save:
+
+            fsi = FabricCrossconnectServiceInstance(id=7777, owner=self.service, s_tag=None, source_port=None,
+                                                    switch_datapath_id=None)
+
+            fsi.provided_links = Mock(exists=Mock(return_value=False))
+
+            serviceinstance_objects.return_value = [fsi]
+
+            self.policy_step().handle_update(fsi)
+
+            fcsi_save.assert_not_called()
+
+    def test_handle_update_change_s_tag(self):
+        with patch.object(ServiceInstance.objects, "get_items") as serviceinstance_objects, \
+            patch.object(FabricCrossconnectServiceInstance, "save") as fcsi_save:
+
+            fsi = FabricCrossconnectServiceInstance(id=7777, owner=self.service, s_tag=100, source_port=None,
+                                                    switch_datapath_id=None)
+
+            serviceinstance_objects.return_value = [fsi]
+
+            si = self.mock_westbound(fsi, s_tag=111, switch_datapath_id = "of:0000000000000201", switch_port = 3)
+            serviceinstance_objects.return_value = [si]
+
+            with self.assertRaises(Exception) as e:
+                self.policy_step().handle_update(fsi)
+
+            self.assertEqual(e.exception.message, "Westbound ServiceInstance changing s-tag is not currently permitted")
+
+    def test_handle_update_change_switch_datapath_id(self):
+        with patch.object(ServiceInstance.objects, "get_items") as serviceinstance_objects, \
+            patch.object(FabricCrossconnectServiceInstance, "save") as fcsi_save:
+
+            fsi = FabricCrossconnectServiceInstance(id=7777, owner=self.service, s_tag=None, source_port=None,
+                                                    switch_datapath_id="foo")
+
+            serviceinstance_objects.return_value = [fsi]
+
+            si = self.mock_westbound(fsi, s_tag=111, switch_datapath_id = "of:0000000000000201", switch_port = 3)
+            serviceinstance_objects.return_value = [si]
+
+            with self.assertRaises(Exception) as e:
+                self.policy_step().handle_update(fsi)
+
+            self.assertEqual(e.exception.message, "Westbound ServiceInstance changing switch_datapath_id is not currently permitted")
+
+    def test_handle_update_change_source_port(self):
+        with patch.object(ServiceInstance.objects, "get_items") as serviceinstance_objects, \
+            patch.object(FabricCrossconnectServiceInstance, "save") as fcsi_save:
+
+            fsi = FabricCrossconnectServiceInstance(id=7777, owner=self.service, s_tag=None, source_port=5,
+                                                    switch_datapath_id=None)
+
+            serviceinstance_objects.return_value = [fsi]
+
+            si = self.mock_westbound(fsi, s_tag=111, switch_datapath_id = "of:0000000000000201", switch_port = 3)
+            serviceinstance_objects.return_value = [si]
+
+            with self.assertRaises(Exception) as e:
+                self.policy_step().handle_update(fsi)
+
+            self.assertEqual(e.exception.message, "Westbound ServiceInstance changing source_port is not currently permitted")
+
+
+    def tearDown(self):
+        self.o = None
+        sys.path = self.sys_path_save
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/xos/synchronizer/models/fabric-crossconnect.xproto b/xos/synchronizer/models/fabric-crossconnect.xproto
old mode 100755
new mode 100644
index 989ebc1..d0d78e4
--- a/xos/synchronizer/models/fabric-crossconnect.xproto
+++ b/xos/synchronizer/models/fabric-crossconnect.xproto
@@ -10,6 +10,10 @@
 message FabricCrossconnectServiceInstance (ServiceInstance){
     option verbose_name = "Fabric Crossconnect Service Instance";
     option owner_class_name="FabricCrossconnectService";
+
+    optional int32 s_tag = 1 [help_text = "s-tag"];
+    optional string switch_datapath_id = 2 [help_text = "switch datapath id"];
+    optional int32 source_port = 3 [help_text = "source port of fabric crossconnect"];
 }
 
 message BNGPortMapping (XOSBase) {
diff --git a/xos/synchronizer/models/models.py b/xos/synchronizer/models/models.py
old mode 100755
new mode 100644
index 75ebe1d..8c75a2e
--- a/xos/synchronizer/models/models.py
+++ b/xos/synchronizer/models/models.py
@@ -48,5 +48,6 @@
 
     def save(self, *args, **kwargs):
         self.validate_range(self.s_tag)
+
         super(BNGPortMapping, self).save(*args, **kwargs)
 
diff --git a/xos/synchronizer/steps/sync_fabric_crossconnect_service_instance.py b/xos/synchronizer/steps/sync_fabric_crossconnect_service_instance.py
old mode 100755
new mode 100644
index ee067de..165eda9
--- a/xos/synchronizer/steps/sync_fabric_crossconnect_service_instance.py
+++ b/xos/synchronizer/steps/sync_fabric_crossconnect_service_instance.py
@@ -55,16 +55,16 @@
             'pass': fabric_onos.rest_password
         }
 
-    def make_handle(self, s_tag, west_dpid):
+    def make_handle(self, s_tag, switch_datapath_id):
         # Generate a backend_handle that uniquely identifies the cross connect. ONOS doesn't provide us a handle, so
         # we make up our own. This helps us to detect other FabricCrossconnectServiceInstance using the same
         # entry, as well as to be able to extract the necessary information to delete the entry later.
-        return "%d/%s" % (s_tag, west_dpid)
+        return "%d/%s" % (s_tag, switch_datapath_id)
 
     def extract_handle(self, backend_handle):
-        (s_tag, dpid) = backend_handle.split("/",1)
+        (s_tag, switch_datapath_id) = backend_handle.split("/",1)
         s_tag = int(s_tag)
-        return (s_tag, dpid)
+        return (s_tag, switch_datapath_id)
 
     def range_matches(self, value, pattern):
         value=int(value)
@@ -102,22 +102,30 @@
     def sync_record(self, o):
         self.log.info("Sync'ing Fabric Crossconnect Service Instance", service_instance=o)
 
+        if (o.policed is None) or (o.policed < o.updated):
+            raise DeferredException("Waiting for model_policy to run on fcsi %s" % o.id)
+
         onos = self.get_fabric_onos_info(o)
 
         si = ServiceInstance.objects.get(id=o.id)
 
-        s_tag = si.get_westbound_service_instance_properties("s_tag")
-        dpid = si.get_westbound_service_instance_properties("switch_datapath_id")
-        west_port = si.get_westbound_service_instance_properties("switch_port")
+        if (o.s_tag is None):
+            raise Exception("Cannot sync FabricCrossconnectServiceInstance if s_tag is None on fcsi %s" % o.id)
 
-        bng_mapping = self.find_bng(s_tag = s_tag)
+        if (o.source_port is None):
+            raise Exception("Cannot sync FabricCrossconnectServiceInstance if source_port is None on fcsi %s" % o.id)
+
+        if (not o.switch_datapath_id):
+            raise Exception("Cannot sync FabricCrossconnectServiceInstance if switch_datapath_id is unset on fcsi %s" % o.id)
+
+        bng_mapping = self.find_bng(s_tag = o.s_tag)
         if not bng_mapping:
-            raise Exception("Unable to determine BNG port for s_tag %s" % s_tag)
+            raise Exception("Unable to determine BNG port for s_tag %s" % o.s_tag)
         east_port = bng_mapping.switch_port
 
-        data = { "deviceId": dpid,
-                 "vlanId": s_tag,
-                 "ports": [ int(west_port), int(east_port) ] } 
+        data = { "deviceId": o.switch_datapath_id,
+                 "vlanId": o.s_tag,
+                 "ports": [ int(o.source_port), int(east_port) ] }
 
         url = onos['url'] + '/onos/segmentrouting/xconnect'
 
@@ -128,7 +136,12 @@
         if r.status_code != 200:
             raise Exception("Failed to create fabric crossconnect in ONOS: %s" % r.text)
 
-        o.backend_handle = self.make_handle(s_tag, dpid)
+        # TODO(smbaker): If the o.backend_handle changed, then someone must have changed the
+        #   FabricCrossconnectServiceInstance. If so, then we potentially need to clean up the old
+        #   entry in ONOS. Furthermore, we might want to also save the two port numbers that we used,
+        #   to detect someone changing those.
+
+        o.backend_handle = self.make_handle(o.s_tag, o.switch_datapath_id)
         o.save(update_fields=["backend_handle"])
 
         self.log.info("ONOS response", res=r.text)
@@ -147,9 +160,9 @@
                 return
 
             # backend_handle has everything we need in it to delete this entry.
-            (s_tag, dpid) = self.extract_handle(o.backend_handle)
+            (s_tag, switch_datapath_id) = self.extract_handle(o.backend_handle)
 
-            data = { "deviceId": dpid,
+            data = { "deviceId": switch_datapath_id,
                      "vlanId": s_tag }
 
             url = onos['url'] + '/onos/segmentrouting/xconnect'
diff --git a/xos/synchronizer/steps/test_sync_fabric_crossconnect_service_instance.py b/xos/synchronizer/steps/test_sync_fabric_crossconnect_service_instance.py
old mode 100755
new mode 100644
index a1b3e23..8e4c5a8
--- a/xos/synchronizer/steps/test_sync_fabric_crossconnect_service_instance.py
+++ b/xos/synchronizer/steps/test_sync_fabric_crossconnect_service_instance.py
@@ -111,10 +111,10 @@
 
     def test_make_handle_extract_handle(self):
         h = self.sync_step().make_handle(222, "of:0000000000000201")
-        (s_tag, dpid) = self.sync_step().extract_handle(h)
+        (s_tag, switch_datapath_id) = self.sync_step().extract_handle(h)
 
         self.assertEqual(s_tag, 222)
-        self.assertEqual(dpid, "of:0000000000000201")
+        self.assertEqual(switch_datapath_id, "of:0000000000000201")
 
     def test_get_fabric_onos_init(self):
         fsi = FabricCrossconnectServiceInstance(id=7777, owner=self.service)
@@ -180,10 +180,10 @@
             patch.object(BNGPortMapping.objects, "get_items") as bng_objects, \
             patch.object(FabricCrossconnectServiceInstance, "save") as fcsi_save:
 
-            fsi = FabricCrossconnectServiceInstance(id=7777, owner=self.service)
+            fsi = FabricCrossconnectServiceInstance(id=7777, owner=self.service, s_tag=111, source_port=3,
+                                                    switch_datapath_id="of:0000000000000201", updated=1, policed=2)
 
-            si = self.mock_westbound(fsi, s_tag=111, switch_datapath_id = "of:0000000000000201", switch_port = 3)
-            serviceinstance_objects.return_value = [si]
+            serviceinstance_objects.return_value = [fsi]
 
             bngmapping = BNGPortMapping(s_tag="111", switch_port=4)
             bng_objects.return_value = [bngmapping]
@@ -206,16 +206,72 @@
         with patch.object(ServiceInstance.objects, "get_items") as serviceinstance_objects, \
             patch.object(FabricCrossconnectServiceInstance, "save") as fcsi_save:
 
-            fsi = FabricCrossconnectServiceInstance(id=7777, owner=self.service)
+            fsi = FabricCrossconnectServiceInstance(id=7777, owner=self.service, s_tag=111, source_port=3,
+                                                    switch_datapath_id="of:0000000000000201", updated=1, policed=2)
 
-            si = self.mock_westbound(fsi, s_tag="111", switch_datapath_id = "of:0000000000000201", switch_port = 3)
-            serviceinstance_objects.return_value = [si]
+            serviceinstance_objects.return_value = [fsi]
 
             with self.assertRaises(Exception) as e:
                 self.sync_step().sync_record(fsi)
 
             self.assertEqual(e.exception.message, "Unable to determine BNG port for s_tag 111")
 
+    def test_sync_not_policed(self):
+        with patch.object(ServiceInstance.objects, "get_items") as serviceinstance_objects, \
+            patch.object(FabricCrossconnectServiceInstance, "save") as fcsi_save:
+
+            fsi = FabricCrossconnectServiceInstance(id=7777, owner=self.service, source_port=3,
+                                                    switch_datapath_id="of:0000000000000201", updated=1, policed=0)
+
+            serviceinstance_objects.return_value = [fsi]
+
+            with self.assertRaises(Exception) as e:
+                self.sync_step().sync_record(fsi)
+
+            self.assertEqual(e.exception.message, "Waiting for model_policy to run on fcsi 7777")
+
+    def test_sync_no_s_tag(self):
+        with patch.object(ServiceInstance.objects, "get_items") as serviceinstance_objects, \
+            patch.object(FabricCrossconnectServiceInstance, "save") as fcsi_save:
+
+            fsi = FabricCrossconnectServiceInstance(id=7777, owner=self.service, source_port=3,
+                                                    switch_datapath_id="of:0000000000000201", updated=1, policed=2)
+
+            serviceinstance_objects.return_value = [fsi]
+
+            with self.assertRaises(Exception) as e:
+                self.sync_step().sync_record(fsi)
+
+            self.assertEqual(e.exception.message, "Cannot sync FabricCrossconnectServiceInstance if s_tag is None on fcsi 7777")
+
+    def test_sync_no_switch_datapath_id(self):
+        with patch.object(ServiceInstance.objects, "get_items") as serviceinstance_objects, \
+            patch.object(FabricCrossconnectServiceInstance, "save") as fcsi_save:
+
+            fsi = FabricCrossconnectServiceInstance(id=7777, owner=self.service, source_port=3, s_tag=111,
+                                                    updated=1, policed=2)
+
+            serviceinstance_objects.return_value = [fsi]
+
+            with self.assertRaises(Exception) as e:
+                self.sync_step().sync_record(fsi)
+
+            self.assertEqual(e.exception.message, "Cannot sync FabricCrossconnectServiceInstance if switch_datapath_id is unset on fcsi 7777")
+
+    def test_sync_no_source_port(self):
+        with patch.object(ServiceInstance.objects, "get_items") as serviceinstance_objects, \
+            patch.object(FabricCrossconnectServiceInstance, "save") as fcsi_save:
+
+            fsi = FabricCrossconnectServiceInstance(id=7777, owner=self.service, s_tag=111,
+                                                    switch_datapath_id="of:0000000000000201", updated=1, policed=2)
+
+            serviceinstance_objects.return_value = [fsi]
+
+            with self.assertRaises(Exception) as e:
+                self.sync_step().sync_record(fsi)
+
+            self.assertEqual(e.exception.message, "Cannot sync FabricCrossconnectServiceInstance if source_port is None on fcsi 7777")
+
     @requests_mock.Mocker()
     def test_delete(self, m):
         with patch.object(FabricCrossconnectServiceInstance.objects, "get_items") as fcsi_objects, \
@@ -244,7 +300,7 @@
                                                     backend_handle="111/of:0000000000000201",
                                                     enacted=True)
 
-            # Another subscriber using the same (s_tag, dpid) pair
+            # Another subscriber using the same (s_tag, switch_datapath_id) pair
             fsi2 = FabricCrossconnectServiceInstance(id=7778, owner=self.service,
                                                      backend_handle="111/of:0000000000000201",
                                                      enacted=True)