From 320d642db171d7c463700de0e8445164ed16e5bf Mon Sep 17 00:00:00 2001
From: Nikolay Stanchev <ns17@it-innovation.soton.ac.uk>
Date: Tue, 16 Apr 2019 12:46:32 +0100
Subject: [PATCH] Adds a GET status API endpoint for a graph monitoring
 pipeline script

---
 docs/clmc-service.md                      | 24 ++++++++-
 src/service/clmcservice/graphapi/tests.py | 60 +++++++++++++++++++----
 src/service/clmcservice/graphapi/views.py | 45 ++++++++++++++---
 src/service/setup.py                      |  1 +
 4 files changed, 112 insertions(+), 18 deletions(-)

diff --git a/docs/clmc-service.md b/docs/clmc-service.md
index 7a435b7..bd6d537 100644
--- a/docs/clmc-service.md
+++ b/docs/clmc-service.md
@@ -339,8 +339,8 @@ with **/clmc-service** so that the nginx reverse proxy server (listening on port
 
 * **DELETE** ***/graph/monitor/{request_id}*** 
 
-    This API methods instructs the CLMC service to stop running a graph monitoring pipeline script associated with the request identifier in the URL.
-    (retrieved from the response of a POST request for /graph/monitor), e.g. request sent to */graph/monitor/75df6f8d-3829-4fd8-a3e6-b3e917010141*
+    This API method instructs the CLMC service to stop running a graph monitoring pipeline script associated with the request identifier in the URL.
+    (could be retrieved from the response of a POST request for /graph/monitor), e.g. request sent to */graph/monitor/75df6f8d-3829-4fd8-a3e6-b3e917010141*
 
     * Response:
 
@@ -356,6 +356,26 @@ with **/clmc-service** so that the nginx reverse proxy server (listening on port
         }
         ```
 
+* **GET** ***/graph/monitor/{request_id}*** 
+
+    This API method fetches the status of a graph monitoring pipeline script associated with the request identifier in the URL.
+    (could be retrieved from the response of a POST request for /graph/monitor), e.g. request sent to */graph/monitor/75df6f8d-3829-4fd8-a3e6-b3e917010141*
+
+    * Response:
+    
+        The response of this request is a JSON content, which contains a single output message along with the status of the monitoring pipeline.
+
+        Returns a 404 Not Found error if the request ID is not associated with any graph monitoring process.
+    
+    * Response Body Example:
+    
+        ```json
+        {
+          "status": "sleeping",
+          "msg": "Successfully fetched status of graph pipeline process."
+        }
+        ```
+
 * **POST** ***/graph/temporal***
 
     This API method sends a request to the CLMC service to build a graph snapshot in the time range between the *from* and *to* timestamps.
diff --git a/src/service/clmcservice/graphapi/tests.py b/src/service/clmcservice/graphapi/tests.py
index 4a8bd58..871dd5e 100644
--- a/src/service/clmcservice/graphapi/tests.py
+++ b/src/service/clmcservice/graphapi/tests.py
@@ -23,7 +23,7 @@
 """
 
 from json import dumps, loads
-from signal import SIGKILL
+from psutil import NoSuchProcess, STATUS_SLEEPING
 from unittest.mock import patch, Mock, MagicMock, PropertyMock
 import pytest
 from pyramid import testing
@@ -770,12 +770,52 @@ class TestGraphAPI(object):
         assert len(popen_mock.call_args_list) == 2, "No subprocess should be started if the UE nodes list is empty (network topology not built)"
         nodes_matcher_mock.assert_called_with("UserEquipment")  # assert that the graph nodes match function has been called with "UserEquipment" as argument
 
-    @patch('clmcservice.graphapi.views.kill')
-    def test_stop_graph_pipeline(self, mock_kill):
+    @patch('clmcservice.graphapi.views.Process')
+    def test_get_graph_pipeline_status(self, mock_process):
         """
-        Tests the funcitonality to stop a graph monitoring script.
+        Tests the functionality to fetch the status of a graph monitoring script.
 
-        :param mock_kill: mock object to mimic the behavior of the os.kill functionality
+        :param mock_process: mock object to mimic the behavior of the psutil.Process functionality
+        """
+
+        # mock a monitoring process
+        pid = 111
+        reqid = "test_request_id"
+        MonitoringProcess.add({"request_id": reqid, "process_id": pid})
+
+        # test behaviour with not-existing request UUID
+        request = testing.DummyRequest()
+        request.matchdict["request_id"] = "unknown-request-uuid"
+        error_raised = False
+        try:
+            GraphAPI(request).get_graph_pipeline_status()
+        except HTTPNotFound:
+            error_raised = True
+        assert error_raised, "Error must have been raised for unrecognised request UUID."
+
+        # test the behaviour when the PID doesn't exist or another OSError is thrown
+        mock_process.side_effect = NoSuchProcess("error")
+        request = testing.DummyRequest()
+        request.matchdict["request_id"] = reqid
+        response = GraphAPI(request).get_graph_pipeline_status()
+        assert response == {"msg": "Monitoring process has been stopped or killed or terminated before this request was executed."}
+
+        # test behaviour with existing request UUID and existing PID
+        assert MonitoringProcess.exists(reqid)
+        mock_process.side_effect = None
+        mock_process.return_value.status = Mock(return_value=STATUS_SLEEPING)
+        request = testing.DummyRequest()
+        request.matchdict["request_id"] = reqid
+        response = GraphAPI(request).get_graph_pipeline_status()
+        assert response == {"status": STATUS_SLEEPING, "msg": "Successfully fetched status of graph pipeline process."}
+        mock_process.return_value.status.assert_called_with()
+
+    @patch('clmcservice.graphapi.views.Process')
+    def test_stop_graph_pipeline(self, mock_process):
+        """
+        Tests the functionality to stop a graph monitoring script.
+
+        :param mock_process: mock object to mimic the behavior of the psutil.Process functionality
         """
 
         # mock a monitoring process
@@ -794,21 +834,23 @@ class TestGraphAPI(object):
         assert error_raised, "Error must have been raised for unrecognised request UUID."
 
         # test the behaviour when the PID doesn't exist or another OSError is thrown
-        mock_kill.side_effect = OSError("error")
+        mock_process.side_effect = NoSuchProcess("error")
         request = testing.DummyRequest()
         request.matchdict["request_id"] = reqid
         response = GraphAPI(request).stop_graph_pipeline()
-        assert response == {"msg": "Monitoring process has been stopped before this request was executed."}
+        assert response == {"msg": "Monitoring process has been stopped or killed or terminated before this request was executed."}
 
         # test behaviour with existing request UUID and existing PID
         MonitoringProcess.add({"request_id": reqid, "process_id": pid})
         assert MonitoringProcess.exists(reqid)
-        mock_kill.side_effect = None
+        mock_process.side_effect = None
+        mock_process.return_value = Mock()
         request = testing.DummyRequest()
         request.matchdict["request_id"] = reqid
         response = GraphAPI(request).stop_graph_pipeline()
         assert response == {"msg": "Monitoring process has been successfully stopped."}
-        mock_kill.assert_called_with(pid, SIGKILL)  # assert that os.kill was called with termination signal
+        mock_process.return_value.terminate.assert_called_with()
+        mock_process.return_value.wait.assert_called_with(timeout=3)
         assert not MonitoringProcess.exists(reqid), "Request ID must be removed when the process is killed."
 
     @staticmethod
diff --git a/src/service/clmcservice/graphapi/views.py b/src/service/clmcservice/graphapi/views.py
index f7f2515..9232e81 100644
--- a/src/service/clmcservice/graphapi/views.py
+++ b/src/service/clmcservice/graphapi/views.py
@@ -34,8 +34,7 @@ from requests import exceptions, get
 from uuid import uuid4
 from json import load, dumps
 from subprocess import Popen
-from os import kill
-from signal import SIGKILL
+from psutil import Process, NoSuchProcess, TimeoutExpired
 from logging import getLogger
 
 
@@ -437,6 +436,32 @@ class GraphAPI(object):
             log.warning("Graph pipeline process for SFC {0} with PID {1} has finished executing unexpectedly with return code {2}".format(sfc, process_pid, process_return_code))
             raise HTTPInternalServerError("An unexpected error occurred while trying to start monitoring graph measurements for service function chain {0}".format(sfc))
 
+    @view_config(route_name='graph_manage_pipeline', request_method='GET')
+    def get_graph_pipeline_status(self):
+        """
+        An API endpoint to get the status of a monitoring graph pipeline script.
+
+        :return: A JSON response with the status of the background process.
+        """
+
+        request_id = self.request.matchdict['request_id']  # get the UUID of the request from the URL
+        process_id = MonitoringProcess.get(request_id)
+
+        if process_id is None:
+            raise HTTPNotFound("A monitoring process with ID {0} couldn't be found.".format(request_id))
+
+        # create a process management class instance
+        try:
+            process_obj = Process(process_id)
+            status = process_obj.status()
+            log.info("Fetching process status with request ID {0} and process ID {1}, status - {2}".format(request_id, process_id, status))
+            response = {"status": status, "msg": "Successfully fetched status of graph pipeline process."}
+        except NoSuchProcess as e:
+            log.warning("Unexpected error occurred when trying to get a process that is registered in the CLMC service, but doesn't exist on the OS - {0}".format(e))
+            response = {"msg": "Monitoring process has been stopped or killed or terminated before this request was executed."}
+
+        return response
+
     @view_config(route_name='graph_manage_pipeline', request_method='DELETE')
     def stop_graph_pipeline(self):
         """
@@ -451,13 +476,19 @@ class GraphAPI(object):
         if process_id is None:
             raise HTTPNotFound("A monitoring process with ID {0} couldn't be found.".format(request_id))
 
+        # create a process management class instance and terminate the process
         try:
-            kill(process_id, SIGKILL)
-            log.info("Successfully stopped process with request ID {0} and process ID {1}".format(request_id, process_id))
+            process_obj = Process(process_id)
+            log.info("Terminating process with request ID {0} and process ID {1}, status before termination - {2}".format(request_id, process_id, process_obj.status()))
+            process_obj.terminate()
+            process_obj.wait(timeout=3)
             response = {"msg": "Monitoring process has been successfully stopped."}
-        except OSError as e:
-            log.warning("Couldn't stop monitoring process with request ID {0} and process ID {1} due to error {2}".format(request_id, process_id, e))
-            response = {"msg": "Monitoring process has been stopped before this request was executed."}
+        except TimeoutExpired as e:
+            log.error("Unexpected error occurred while waiting for a graph pipeline process to terminate - {0}".format(e))
+            raise HTTPInternalServerError("Termination of monitoring process couldn't be completed.")
+        except NoSuchProcess as e:
+            log.warning("Unexpected error occurred when trying to get a process that is registered in the CLMC service, but doesn't exist on the OS - {0}".format(e))
+            response = {"msg": "Monitoring process has been stopped or killed or terminated before this request was executed."}
 
         MonitoringProcess.delete(request_id)
 
diff --git a/src/service/setup.py b/src/service/setup.py
index d1e8ccf..10eef05 100644
--- a/src/service/setup.py
+++ b/src/service/setup.py
@@ -66,6 +66,7 @@ requires = [
     'tosca-parser==1.1.0',
     'schema==0.6.8',
     'requests==2.21.0',
+    'psutil==5.6.1',
     'pytest==3.8.1'
 ]
 
-- 
GitLab