SEBA-406 OLT Admin_state as declarative instead of feedback state

Change-Id: I5ae35f02bfc23d966049b39db5141d52311da297
diff --git a/VERSION b/VERSION
index 2f1a5aa..d302656 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-2.1.17
+2.1.18
diff --git a/xos/synchronizer/models/volt.xproto b/xos/synchronizer/models/volt.xproto
index a1c5837..4155ceb 100644
--- a/xos/synchronizer/models/volt.xproto
+++ b/xos/synchronizer/models/volt.xproto
@@ -30,7 +30,7 @@
 
     optional string serial_number = 9 [help_text = "Serial Number", db_index = False];
     optional string device_id = 10 [help_text = "Voltha Device ID", db_index = False, feedback_state = True];
-    optional string admin_state = 11 [help_text = "admin state, whether OLT should be enabled", db_index = False, feedback_state = True];
+    optional string admin_state = 11 [help_text = "admin state, whether OLT should be enabled", choices = "(('DISABLED', 'DISABLED'), ('ENABLED', 'ENABLED'))", default="ENABLED", db_index = False];
     optional string oper_status = 12 [help_text = "operational status, whether OLT is active", db_index = False, feedback_state = True];
     optional string of_id = 13 [help_text = "Logical device openflow id", db_index = False, feedback_state = True];
     optional string dp_id = 14 [help_text = "Logical device datapath id", db_index = False];
diff --git a/xos/synchronizer/pull_steps/pull_olts.py b/xos/synchronizer/pull_steps/pull_olts.py
index 30afbd4..574a527 100644
--- a/xos/synchronizer/pull_steps/pull_olts.py
+++ b/xos/synchronizer/pull_steps/pull_olts.py
@@ -49,9 +49,12 @@
             if ld["root_device_id"] == o.device_id:
                 o.of_id = ld["id"]
                 o.dp_id = "of:" + Helpers.datapath_id_to_hex(ld["datapath_id"])  # convert to hex
-                return o
 
-        raise Exception("Can't find a logical device for device id: %s" % o.device_id)
+        # Note: If the device is administratively disabled, then it's likely we won't find a logical device for
+        # it. Only throw the exception for OLTs that are enabled.
+
+        if not o.of_id and not o.dp_id and o.admin_state == "ENABLED":
+            raise Exception("Can't find a logical device for device id: %s" % o.device_id)
 
     def pull_records(self):
         log.debug("[OLT pull step] pulling OLT devices from VOLTHA")
@@ -138,11 +141,13 @@
                 # NNI ports when that situation arises.
                 model.uplink = str(nni_ports[0]["port_no"])
 
+                # Initial admin_state
+                model.admin_state = olt["admin_state"]
+
                 log.debug("[OLT pull step] OLTDevice is new, creating it", device_type=olt["type"], host=host, port=port)
 
             # Adding feedback state to the device
             model.device_id = olt["id"]
-            model.admin_state = olt["admin_state"]
             model.oper_status = olt["oper_status"]
             model.serial_number = olt['serial_number']
 
diff --git a/xos/synchronizer/pull_steps/test_pull_olts.py b/xos/synchronizer/pull_steps/test_pull_olts.py
index a06d3ff..755f4fb 100644
--- a/xos/synchronizer/pull_steps/test_pull_olts.py
+++ b/xos/synchronizer/pull_steps/test_pull_olts.py
@@ -162,6 +162,7 @@
     def test_pull_existing(self, m):
 
         existing_olt = Mock()
+        existing_olt.admin_state = "ENABLED"
         existing_olt.enacted = 2
         existing_olt.updated = 1
 
diff --git a/xos/synchronizer/steps/sync_olt_device.py b/xos/synchronizer/steps/sync_olt_device.py
index 40f884e..e1c1c4a 100644
--- a/xos/synchronizer/steps/sync_olt_device.py
+++ b/xos/synchronizer/steps/sync_olt_device.py
@@ -80,15 +80,17 @@
 
         log.info("Add device json res", res=res)
 
+        # TODO(smbaker): Potential partial failure. If device is created in Voltha but synchronizer crashes before the
+        # model is saved, then synchronizer will continue to try to preprovision and fail due to preexisting
+        # device.
+
         if not res['id']:
             raise Exception(
                 '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']
-
-
-        return model
+            model.save()
 
     def activate_olt(self, model):
 
@@ -113,8 +115,6 @@
             request = requests.get("%s:%d/api/v1/devices/%s" % (voltha['url'], voltha['port'], model.device_id)).json()
             attempted = attempted + 1
 
-
-        model.admin_state = request['admin_state']
         model.oper_status = request['oper_status']
         model.serial_number = request['serial_number']
 
@@ -122,10 +122,17 @@
             raise Exception("It was not possible to activate OLTDevice with id %s" % model.id)
 
         # Find the of_id of the device
-        model = self.get_ids_from_logical_device(model)
+        self.get_ids_from_logical_device(model)
         model.save()
 
-        return model
+    def deactivate_olt(self, model):
+        voltha = Helpers.get_voltha_info(model.volt_service)
+
+        # Disable device
+        request = requests.post("%s:%d/api/v1/devices/%s/disable" % (voltha['url'], voltha['port'], model.device_id))
+
+        if request.status_code != 200:
+            raise Exception("Failed to disable OLT device: %s" % request.text)
 
     def configure_onos(self, model):
 
@@ -158,22 +165,36 @@
                 print request.json()
             except Exception:
                 print request.text
-        return model
 
     def sync_record(self, model):
         log.info("Synching device", object=str(model), **model.tologdict())
 
+        if model.admin_state not in ["ENABLED", "DISABLED"]:
+            raise Exception("OLT Device %s admin_state has invalid value %s" % (model.id, model.admin_state))
+
         # If the device has feedback_state is already present in voltha
-        if not model.device_id and not model.admin_state and not model.oper_status and not model.of_id:
+        if not model.device_id and not model.oper_status and not model.of_id:
             log.info("Pushing OLT device to VOLTHA", object=str(model), **model.tologdict())
-            model = self.pre_provision_olt_device(model)
-            self.activate_olt(model)
-        elif model.oper_status != "ACTIVE":
-            raise Exception("It was not possible to activate OLTDevice with id %s" % model.id)
+            self.pre_provision_olt_device(model)
+            model.oper_status = "UNKNOWN" # fall-though to activate OLT
         else:
             log.info("OLT device already exists in VOLTHA", object=str(model), **model.tologdict())
 
-        self.configure_onos(model)
+        # Reconcile admin_state and oper_status, activating or deactivating the OLT as necessary.
+
+        if model.oper_status != "ACTIVE" and model.admin_state == "ENABLED":
+            self.activate_olt(model)
+        elif model.oper_status == "ACTIVE" and model.admin_state == "DISABLED":
+            self.deactivate_olt(model)
+
+        if model.admin_state == "ENABLED":
+            # If we were not able to reconcile ENABLE/ACTIVE, then throw an exception and do not proceed to onos
+            # configuration.
+            if model.oper_status != "ACTIVE":
+                raise Exception("It was not possible to activate OLTDevice with id %s" % model.id)
+
+            # At this point OLT is enabled and active. Configure ONOS.
+            self.configure_onos(model)
 
     def delete_record(self, model):
         log.info("Deleting OLT device", object=str(model), **model.tologdict())
diff --git a/xos/synchronizer/steps/test_sync_olt_device.py b/xos/synchronizer/steps/test_sync_olt_device.py
index 8d01579..50302d5 100644
--- a/xos/synchronizer/steps/test_sync_olt_device.py
+++ b/xos/synchronizer/steps/test_sync_olt_device.py
@@ -96,10 +96,10 @@
         o.uplink = "129"
         o.driver = "voltha"
         o.name = "Test Device"
+        o.admin_state = "ENABLED"
 
         # feedback state
         o.device_id = None
-        o.admin_state = None
         o.oper_status = None
         o.of_id = None
         o.id = 1
@@ -211,7 +211,11 @@
         self.assertEqual(self.o.oper_status, "ACTIVE")
         self.assertEqual(self.o.serial_number, "foobar")
         self.assertEqual(self.o.of_id, "0001000ce2314000")
-        self.assertEqual(self.o.save.call_count, 2) # we're updating the backend_status when activating and then adding logical device ids
+
+        # One save during preprovision
+        # One save during activation to set backend_status to "Waiting for device to activate"
+        # One save after activation has succeeded
+        self.assertEqual(self.o.save.call_count, 3)
 
     @requests_mock.Mocker()
     def test_sync_record_success_mac_address(self, m):
@@ -256,12 +260,19 @@
         self.assertEqual(self.o.admin_state, "ENABLED")
         self.assertEqual(self.o.oper_status, "ACTIVE")
         self.assertEqual(self.o.of_id, "0001000ce2314000")
-        self.assertEqual(self.o.save.call_count, 2)
+
+        # One save during preprovision
+        # One save during activation to set backend_status to "Waiting for device to activate"
+        # One save after activation has succeeded
+        self.assertEqual(self.o.save.call_count, 3)
 
     @requests_mock.Mocker()
     def test_sync_record_enable_timeout(self, m):
         """
-        If device.enable fails we need to tell the suer
+        If device activation fails we need to tell the user.
+
+        OLT will be preprovisioned.
+        OLT will return "ERROR" for oper_status during activate and will eventually exceed retries.s
         """
 
         expected_conf = {
@@ -274,7 +285,7 @@
         m.post("http://voltha_url:1234/api/v1/devices/123/enable", status_code=200)
         m.get("http://voltha_url:1234/api/v1/devices/123", [
                   {"json": {"oper_status": "ACTIVATING", "admin_state": "ENABLED", "serial_number": "foobar"}, "status_code": 200},
-                  {"json": {"oper_status": "ERROR", "admin_state": "FAILED", "serial_number": "foobar"}, "status_code": 200}
+                  {"json": {"oper_status": "ERROR", "admin_state": "ENABLED", "serial_number": "foobar"}, "status_code": 200}
               ])
 
         logical_devices = {
@@ -290,11 +301,20 @@
 
         self.assertEqual(e.exception.message, "It was not possible to activate OLTDevice with id 1")
         self.assertEqual(self.o.oper_status, "ERROR")
-        self.assertEqual(self.o.admin_state, "FAILED")
-        self.assertEqual(self.o.save.call_count, 1)
+        self.assertEqual(self.o.admin_state, "ENABLED")
+        self.assertEqual(self.o.device_id, "123")
+        self.assertEqual(self.o.serial_number, "foobar")
+
+        # One save from preprovision to set device_id, serial_number
+        # One save from activate to set backend_status to "Waiting for device to be activated"
+        self.assertEqual(self.o.save.call_count, 2)
 
     @requests_mock.Mocker()
     def test_sync_record_already_existing_in_voltha(self, m):
+        """
+        If device.admin_state == "ENABLED" and oper_status == "ACTIVE", then the OLT should not be reactivated.
+        """
+
         # mock device feedback state
         self.o.device_id = "123"
         self.o.admin_state = "ENABLED"
@@ -318,6 +338,62 @@
         self.o.save.assert_not_called()
 
     @requests_mock.Mocker()
+    def test_sync_record_deactivate(self, m):
+        """
+        If device.admin_state == "DISABLED" and oper_status == "ACTIVE", then OLT should be deactivated.
+        """
+
+        expected_conf = {
+            "type": self.o.device_type,
+            "host_and_port": "%s:%s" % (self.o.host, self.o.port)
+        }
+
+        # Make it look like we have an active OLT that we are deactivating.
+        self.o.admin_state = "DISABLED"
+        self.o.oper_status = "ACTIVE"
+        self.o.serial_number = "foobar"
+        self.o.device_id = "123"
+        self.o.of_id = "0001000ce2314000"
+
+        m.post("http://voltha_url:1234/api/v1/devices", status_code=200, json=self.voltha_devices_response, additional_matcher=functools.partial(match_json, expected_conf))
+        m.post("http://voltha_url:1234/api/v1/devices/123/disable", status_code=200)
+
+        self.sync_step().sync_record(self.o)
+
+        # No saves as state has not changed (will eventually be saved by synchronizer framework to update backend_status)
+        self.assertEqual(self.o.save.call_count, 0)
+
+        # Make sure disable was called
+        urls = [x.url for x in m.request_history]
+        self.assertIn("http://voltha_url:1234/api/v1/devices/123/disable", urls)
+
+    @requests_mock.Mocker()
+    def test_sync_record_deactivate_already_inactive(self, m):
+        """
+        If device.admin_state == "DISABLED" and device.oper_status == "UNKNOWN", then the device is already deactivated
+        and VOLTHA should not be called.
+        """
+
+        expected_conf = {
+            "type": self.o.device_type,
+            "host_and_port": "%s:%s" % (self.o.host, self.o.port)
+        }
+
+        # Make it look like we have an active OLT that we are deactivating.
+        self.o.admin_state = "DISABLED"
+        self.o.oper_status = "UNKNOWN"
+        self.o.serial_number = "foobar"
+        self.o.device_id = "123"
+        self.o.of_id = "0001000ce2314000"
+
+        m.post("http://voltha_url:1234/api/v1/devices", status_code=200, json=self.voltha_devices_response, additional_matcher=functools.partial(match_json, expected_conf))
+
+        self.sync_step().sync_record(self.o)
+
+        # No saves as state has not changed (will eventually be saved by synchronizer framework to update backend_status)
+        self.assertEqual(self.o.save.call_count, 0)
+
+    @requests_mock.Mocker()
     def test_delete_record(self, m):
         self.o.of_id = "0001000ce2314000"
         self.o.device_id = "123"