SEBA-746 do not overwrite OLTDevice serial number with empty string;
Ensure OLTs with an incorrect serial number don't work
Change-Id: Idd1665ce427351195a8a848d8404aeeb4a997b98
diff --git a/xos/synchronizer/pull_steps/pull_olts.py b/xos/synchronizer/pull_steps/pull_olts.py
index 7c2cadf..8e33642 100644
--- a/xos/synchronizer/pull_steps/pull_olts.py
+++ b/xos/synchronizer/pull_steps/pull_olts.py
@@ -114,11 +114,10 @@
try:
if "host_and_port" in olt:
model = OLTDevice.objects.filter(device_type=olt["type"], host=host, port=port)[0]
- log.debug("[OLT pull step] OLTDevice already exists, updating it", device_type=olt["type"], host=host, port=port)
+ log.debug("[OLT pull step] OLTDevice already exists, updating it", device_type=olt["type"], id=model.id, host=host, port=port)
elif "mac_address" in olt:
model = OLTDevice.objects.filter(device_type=olt["type"], mac_address=mac_address)[0]
- log.debug("[OLT pull step] OLTDevice already exists, updating it", device_type=olt["type"], mac_address=mac_address)
-
+ log.debug("[OLT pull step] OLTDevice already exists, updating it", device_type=olt["type"], id=model.id, mac_address=mac_address)
if model.enacted < model.updated:
log.debug("[OLT pull step] Skipping pull on OLTDevice %s as enacted < updated" % model.name, name=model.name, id=model.id, enacted=model.enacted, updated=model.updated)
@@ -136,6 +135,7 @@
if olt["type"] == "simulated_olt":
model.host = "172.17.0.1"
model.port = 50060
+ log.debug("[OLT pull step] OLTDevice is new, creating it. Simulated")
elif "host_and_port" in olt:
[host, port] = olt["host_and_port"].split(":")
model.host = host
@@ -151,6 +151,8 @@
nni_ports = [p for p in olt_ports if "ETHERNET_NNI" in p["type"]]
if not nni_ports:
log.warning("[OLT pull step] No NNI ports, so no way to determine uplink. Skipping.", device_type=olt["type"], host=host, port=port)
+ model.device_id = olt["id"] # device_id must be populated for delete_olts
+ updated_olts.append(model)
continue
# Exctract uplink from the first NNI port. This decision is arbitrary, we will worry about multiple
@@ -160,10 +162,44 @@
# Initial admin_state
model.admin_state = olt["admin_state"]
+ # Check to see if Voltha's serial_number field is populated. During Activation it's possible that
+ # Voltha's serial_number field may be blank. We want to avoid overwriting a populated data model
+ # serial number with an unpopulated Voltha serial number. IF this happened, then there would be
+ # a window of vulnerability where sadis requests will fail.
+
+ if olt['serial_number'] and model.serial_number != olt['serial_number']:
+ # Check to see if data model serial number is already populated. If the serial number was
+ # already populated, and what we just learned from Voltha differs, then this is an error
+ # that should be made visible to the operator, so the operator may correct it.
+ if model.serial_number:
+ log.error("Serial number mismatch when pulling olt. Aborting OLT Update.",
+ model_serial_number=model.serial_number,
+ voltha_serial_number=olt['serial_number'],
+ olt_id=model.id)
+ model.backend_status = "Incorrect serial number"
+ model.backend_code = 2
+ model.save_changed_fields()
+ # Have to include this in the result, or delete_olts() will delete it
+ updated_olts.append(model)
+ # Stop processing this OLT
+ continue
+
+ # Preserve existing behavior.
+ # Data model serial number is unpopulated, so populate it.
+
+ # TODO(smbaker): Consider making serial_number a required field, then do away with this.
+ # Deferred until after SEBA-2.0 release.
+
+ log.info("Pull step learned olt serial number from voltha",
+ model_serial_number=model.serial_number,
+ voltha_serial_number=olt['serial_number'],
+ olt_id=model.id)
+
+ model.serial_number = olt['serial_number']
+
# Adding feedback state to the device
model.device_id = olt["id"]
model.oper_status = olt["oper_status"]
- model.serial_number = olt['serial_number']
model.volt_service = self.volt_service
model.volt_service_id = self.volt_service.id
@@ -275,8 +311,8 @@
if model.enacted < model.updated:
# DO NOT delete a model that is being processed
log.debug("[OLT pull step] device is not present in VOLTHA, skipping deletion as sync is in progress", device_id=o.device_id,
- name=o.name)
+ name=o.name)
continue
- log.debug("[OLT pull step] deleting device as it's not present in VOLTHA", device_id=o.device_id, name=o.name)
+ log.debug("[OLT pull step] deleting device as it's not present in VOLTHA", device_id=o.device_id, name=o.name, id=o.id)
model.delete()
diff --git a/xos/synchronizer/pull_steps/test_pull_olts.py b/xos/synchronizer/pull_steps/test_pull_olts.py
index 5894f5e..de2b967 100644
--- a/xos/synchronizer/pull_steps/test_pull_olts.py
+++ b/xos/synchronizer/pull_steps/test_pull_olts.py
@@ -202,6 +202,7 @@
existing_olt.admin_state = "ENABLED"
existing_olt.enacted = 2
existing_olt.updated = 1
+ existing_olt.serial_number = ""
with patch.object(VOLTService.objects, "all") as olt_service_mock, \
patch.object(OLTDevice.objects, "filter") as mock_get, \
@@ -223,12 +224,79 @@
self.assertEqual(existing_olt.device_id, "test_id")
self.assertEqual(existing_olt.of_id, "of_id")
self.assertEqual(existing_olt.dp_id, "of:0000000ce2314000")
+ self.assertEqual(existing_olt.serial_number, "serial_number")
# mock_olt_save.assert_called()
mock_pon_save.assert_called()
mock_nni_save.assert_called()
@requests_mock.Mocker()
+ def test_pull_existing_empty_voltha_serial(self, m):
+
+ existing_olt = Mock()
+ existing_olt.admin_state = "ENABLED"
+ existing_olt.enacted = 2
+ existing_olt.updated = 1
+ existing_olt.serial_number = "orig_serial"
+
+ self.devices["items"][0]["serial_number"] = ""
+
+ with patch.object(VOLTService.objects, "all") as olt_service_mock, \
+ patch.object(OLTDevice.objects, "filter") as mock_get, \
+ patch.object(PONPort, "save") as mock_pon_save, \
+ patch.object(NNIPort, "save") as mock_nni_save, \
+ patch.object(existing_olt, "save") as mock_olt_save:
+ olt_service_mock.return_value = [self.volt_service]
+ mock_get.return_value = [existing_olt]
+
+ m.get("http://voltha_url:1234/api/v1/devices", status_code=200, json=self.devices)
+ m.get("http://voltha_url:1234/api/v1/devices/test_id/ports", status_code=200, json=self.ports)
+ m.get("http://voltha_url:1234/api/v1/logical_devices", status_code=200, json=self.logical_devices)
+
+ self.sync_step(model_accessor=self.model_accessor).pull_records()
+
+ self.assertEqual(existing_olt.admin_state, "ENABLED")
+ self.assertEqual(existing_olt.oper_status, "ACTIVE")
+ self.assertEqual(existing_olt.volt_service_id, "volt_service_id")
+ self.assertEqual(existing_olt.device_id, "test_id")
+ self.assertEqual(existing_olt.of_id, "of_id")
+ self.assertEqual(existing_olt.dp_id, "of:0000000ce2314000")
+ self.assertEqual(existing_olt.serial_number, "orig_serial")
+
+ # mock_olt_save.assert_called()
+ mock_pon_save.assert_called()
+ mock_nni_save.assert_called()
+
+ @requests_mock.Mocker()
+ def test_pull_existing_incorrect_voltha_serial(self, m):
+
+ existing_olt = Mock()
+ existing_olt.admin_state = "ENABLED"
+ existing_olt.enacted = 2
+ existing_olt.updated = 1
+ existing_olt.serial_number = "orig_serial"
+
+ self.devices["items"][0]["serial_number"] = "wrong_serial"
+
+ with patch.object(VOLTService.objects, "all") as olt_service_mock, \
+ patch.object(OLTDevice.objects, "filter") as mock_get, \
+ patch.object(PONPort, "save") as mock_pon_save, \
+ patch.object(NNIPort, "save") as mock_nni_save, \
+ patch.object(existing_olt, "save") as mock_olt_save:
+ olt_service_mock.return_value = [self.volt_service]
+ mock_get.return_value = [existing_olt]
+
+ m.get("http://voltha_url:1234/api/v1/devices", status_code=200, json=self.devices)
+ m.get("http://voltha_url:1234/api/v1/devices/test_id/ports", status_code=200, json=self.ports)
+ m.get("http://voltha_url:1234/api/v1/logical_devices", status_code=200, json=self.logical_devices)
+
+ self.sync_step(model_accessor=self.model_accessor).pull_records()
+
+ self.assertEqual(existing_olt.backend_code, 2)
+ self.assertEqual(existing_olt.backend_status, "Incorrect serial number")
+ self.assertEqual(existing_olt.serial_number, "orig_serial")
+
+ @requests_mock.Mocker()
def test_pull_existing_do_not_sync(self, m):
existing_olt = Mock()
existing_olt.enacted = 1
diff --git a/xos/synchronizer/steps/sync_olt_device.py b/xos/synchronizer/steps/sync_olt_device.py
index 469e879..6d4dae1 100644
--- a/xos/synchronizer/steps/sync_olt_device.py
+++ b/xos/synchronizer/steps/sync_olt_device.py
@@ -89,7 +89,16 @@
'VOLTHA Device Id is empty. This probably means that the OLT device is already provisioned in VOLTHA')
else:
model.device_id = res['id']
- model.serial_number = res['serial_number']
+
+ # Only update the serial number if it is not already populated. See comments in similar code in the
+ # pull step. Let the pull step handle emitting any error message if the serial numbers differ.
+ if res['serial_number'] and (not model.serial_number):
+ log.info("Sync step learned olt serial number from voltha",
+ model_serial_number=model.serial_number,
+ voltha_serial_number=res['serial_number'],
+ olt_id=model.id)
+ model.serial_number = res['serial_number']
+
model.save_changed_fields()
def activate_olt(self, model):
@@ -116,7 +125,15 @@
attempted = attempted + 1
model.oper_status = request['oper_status']
- model.serial_number = request['serial_number']
+
+ # Only update the serial number if it is not already populated. See comments in similar code in the
+ # pull step. Let the pull step handle emitting any error message if the serial numbers differ.
+ if request['serial_number'] and (not model.serial_number):
+ log.info("Sync step learned olt serial number from voltha",
+ model_serial_number=model.serial_number,
+ voltha_serial_number=request['serial_number'],
+ olt_id=model.id)
+ model.serial_number = request['serial_number']
if model.oper_status != "ACTIVE":
raise Exception("It was not possible to activate OLTDevice with id %s" % model.id)
diff --git a/xos/synchronizer/steps/test_sync_olt_device.py b/xos/synchronizer/steps/test_sync_olt_device.py
index 57330d2..40f13d8 100644
--- a/xos/synchronizer/steps/test_sync_olt_device.py
+++ b/xos/synchronizer/steps/test_sync_olt_device.py
@@ -88,6 +88,7 @@
# feedback state
o.device_id = None
o.oper_status = None
+ o.serial_number= None
o.of_id = None
o.id = 1