SEBA-108 acquire_service_instance and validate_links methods
Change-Id: Ie63de0fd7beb0aa50d4f482f7c1c9000ebb2cb34
diff --git a/Dockerfile.synchronizer b/Dockerfile.synchronizer
index 636f554..1f03dd2 100644
--- a/Dockerfile.synchronizer
+++ b/Dockerfile.synchronizer
@@ -16,7 +16,7 @@
# xosproject/fabric-crossconnect-synchronizer
-FROM xosproject/xos-synchronizer-base:2.1.4
+FROM xosproject/xos-synchronizer-base:2.1.8
COPY xos/synchronizer /opt/xos/synchronizers/fabric-crossconnect
COPY VERSION /opt/xos/synchronizers/fabric-crossconnect/
diff --git a/VERSION b/VERSION
index 6d7de6e..9084fa2 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-1.0.2
+1.1.0
diff --git a/xos/synchronizer/model_policies/model_policy_fabriccrossconnectserviceinstance.py b/xos/synchronizer/model_policies/model_policy_fabriccrossconnectserviceinstance.py
index db439a3..5a61506 100644
--- a/xos/synchronizer/model_policies/model_policy_fabriccrossconnectserviceinstance.py
+++ b/xos/synchronizer/model_policies/model_policy_fabriccrossconnectserviceinstance.py
@@ -39,48 +39,6 @@
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
index 0099881..bccdfdc 100644
--- a/xos/synchronizer/model_policies/test_model_policy_fabriccrossconnectserviceinstance.py
+++ b/xos/synchronizer/model_policies/test_model_policy_fabriccrossconnectserviceinstance.py
@@ -123,130 +123,6 @@
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
diff --git a/xos/synchronizer/models/convenience/fabric_crossconnect_service.py b/xos/synchronizer/models/convenience/fabric_crossconnect_service.py
new file mode 100644
index 0000000..c2bb8bf
--- /dev/null
+++ b/xos/synchronizer/models/convenience/fabric_crossconnect_service.py
@@ -0,0 +1,111 @@
+
+# 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.
+
+
+from xosapi.orm import ORMWrapper, register_convenience_wrapper
+from xosapi.convenience.service import ORMWrapperService
+
+class ORMWrapperFabricCrossconnectService(ORMWrapperService):
+
+ """ Calling convention. Assume the subscribing service does approximately (needs some checks to see
+ if the methods exist before calling them) the following in its model_policy:
+
+ if not eastbound_service.validate_links(self):
+ eastbound_service.acquire_service_instance(self)
+ """
+
+ def acquire_service_instance(self, subscriber_service_instance):
+ """ Given a subscriber_service_instance:
+ 1) If there is an eligible provider_service_instance that can be used, then link to it
+ 2) Otherwise, create a new provider_service_instance and link to it.
+ """
+ (s_tag, switch_datapath_id, source_port) = self._get_west_fields(subscriber_service_instance)
+
+ FabricCrossconnectServiceInstance = self.stub.FabricCrossconnectServiceInstance
+ ServiceInstanceLink = self.stub.ServiceInstanceLink
+
+ candidates = FabricCrossconnectServiceInstance.objects.filter(owner_id=self.id,
+ s_tag=s_tag,
+ switch_datapath_id=switch_datapath_id,
+ source_port=source_port)
+
+ if candidates:
+ provider_service_instance = candidates[0]
+ else:
+ provider_service_instance = FabricCrossconnectServiceInstance(owner=self,
+ s_tag=s_tag,
+ switch_datapath_id=switch_datapath_id,
+ source_port=source_port)
+ provider_service_instance.save()
+
+ # NOTE: Lack-of-atomicity vulnerability -- provider_service_instance could be deleted before we created the
+ # link.
+
+ link = ServiceInstanceLink(provider_service_instance=provider_service_instance,
+ subscriber_service_instance=subscriber_service_instance)
+ link.save()
+
+ return provider_service_instance
+
+ def validate_links(self, subscriber_service_instance):
+ """ Validate existing links between the provider and subscriber service instances. If a valid link exists,
+ then return it. Return [] otherwise.
+
+ As a side-effect, delete any invalid links.
+ """
+
+ # Short-cut -- if there are no subscriber links then we can skip getting all the properties.
+ if not subscriber_service_instance.subscribed_links.exists():
+ return None
+
+ (s_tag, switch_datapath_id, source_port) = self._get_west_fields(subscriber_service_instance)
+
+ matched = []
+ for link in subscriber_service_instance.subscribed_links.all():
+ if link.provider_service_instance.owner.id == self.id:
+ fcsi = link.provider_service_instance.leaf_model
+ if (fcsi.s_tag == s_tag) and (fcsi.switch_datapath_id == switch_datapath_id) and \
+ (fcsi.source_port == source_port):
+ matched.append(fcsi)
+ else:
+ link.delete()
+ return matched
+
+ def _get_west_fields(self, subscriber_si):
+ """ _get_west_fields()
+
+ Helper function to inspect westbound service instance for fields that will be used inside of
+ FabricCrossconnectServiceInstance.
+ """
+
+ s_tag = subscriber_si.get_westbound_service_instance_properties("s_tag", include_self=True)
+ switch_datapath_id = subscriber_si.get_westbound_service_instance_properties("switch_datapath_id", include_self=True)
+ source_port = subscriber_si.get_westbound_service_instance_properties("switch_port", include_self=True)
+
+ if (s_tag is None):
+ raise Exception("Subscriber ServiceInstance %s s-tag is None" % subscriber_si.id)
+
+ if (not switch_datapath_id):
+ raise Exception("Subscriber ServiceInstance %s switch_datapath_id is unset" % subscriber_si.id)
+
+ if (source_port is None):
+ raise Exception("Subscriber ServiceInstance %s switch_port is None" % subscriber_si.id)
+
+ s_tag = int(s_tag)
+ source_port = int(source_port)
+
+ return (s_tag, switch_datapath_id, source_port)
+
+register_convenience_wrapper("FabricCrossconnectService", ORMWrapperFabricCrossconnectService)
diff --git a/xos/synchronizer/models/fabric-crossconnect.xproto b/xos/synchronizer/models/fabric-crossconnect.xproto
index d0d78e4..bbcfa66 100644
--- a/xos/synchronizer/models/fabric-crossconnect.xproto
+++ b/xos/synchronizer/models/fabric-crossconnect.xproto
@@ -11,9 +11,9 @@
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"];
+ required int32 s_tag = 1 [help_text = "s-tag"];
+ required string switch_datapath_id = 2 [help_text = "switch datapath id"];
+ required int32 source_port = 3 [help_text = "source port of fabric crossconnect"];
}
message BNGPortMapping (XOSBase) {
diff --git a/xos/synchronizer/steps/sync_fabric_crossconnect_service_instance.py b/xos/synchronizer/steps/sync_fabric_crossconnect_service_instance.py
index 165eda9..5299fc1 100644
--- a/xos/synchronizer/steps/sync_fabric_crossconnect_service_instance.py
+++ b/xos/synchronizer/steps/sync_fabric_crossconnect_service_instance.py
@@ -152,13 +152,6 @@
if o.backend_handle:
onos = self.get_fabric_onos_info(o)
- # If some other subscriber is using the same entry, then we shouldn't delete it
- other_subscribers = FabricCrossconnectServiceInstance.objects.filter(backend_handle=o.backend_handle)
- other_subscribers = [x for x in other_subscribers if x.id != o.id]
- if other_subscribers:
- self.log.info("Other subscribers exist using same fabric crossconnect entry. Not deleting.")
- return
-
# backend_handle has everything we need in it to delete this entry.
(s_tag, switch_datapath_id) = self.extract_handle(o.backend_handle)
diff --git a/xos/synchronizer/steps/test_sync_fabric_crossconnect_service_instance.py b/xos/synchronizer/steps/test_sync_fabric_crossconnect_service_instance.py
index 8e4c5a8..9f07687 100644
--- a/xos/synchronizer/steps/test_sync_fabric_crossconnect_service_instance.py
+++ b/xos/synchronizer/steps/test_sync_fabric_crossconnect_service_instance.py
@@ -292,31 +292,6 @@
self.sync_step().delete_record(fsi)
self.assertTrue(m.called)
- @requests_mock.Mocker()
- def test_delete_in_use(self, m):
- with patch.object(FabricCrossconnectServiceInstance.objects, "get_items") as fcsi_objects:
- # The subscriber we want to delete
- fsi = FabricCrossconnectServiceInstance(id=7777, owner=self.service,
- backend_handle="111/of:0000000000000201",
- enacted=True)
-
- # 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)
-
- fcsi_objects.return_value=[fsi, fsi2]
-
- desired_data = {"deviceId": "of:0000000000000201",
- "vlanId": 111}
-
- m.delete("http://onos-fabric:8181/onos/segmentrouting/xconnect",
- status_code=204,
- additional_matcher=functools.partial(match_json, desired_data))
-
- self.sync_step().delete_record(fsi)
- self.assertFalse(m.called)
-
def tearDown(self):
self.o = None
sys.path = self.sys_path_save