Dynamic service: Do not save state for GUEST users (ITISFoundation#2348)

Author: sanderegg

api/specs/common/schemas/running_service.yaml

@@ -6,23 +6,23 @@ components:
         - data
       properties:
         data:
-          $ref: '#/components/schemas/RunningServicesArray'
+          $ref: "#/components/schemas/RunningServicesArray"
         error:
           nullable: true
           default: null
 
     RunningServicesArray:
       type: array
       items:
-        $ref: '#/components/schemas/RunningServiceType'
+        $ref: "#/components/schemas/RunningServiceType"
 
     RunningServiceEnveloped:
       type: object
       required:
         - data
       properties:
         data:
-          $ref: '#/components/schemas/RunningServiceType'
+          $ref: "#/components/schemas/RunningServiceType"
         error:
           nullable: true
           default: null
@@ -37,6 +37,7 @@ components:
         - service_host
         - service_port
         - service_state
+        - user_id
       properties:
         published_port:
           description: The ports where the service provides its interface
@@ -80,8 +81,8 @@ components:
         service_basepath:
           description: different base path where current service is mounted otherwise defaults to root
           type: string
-          example: '/x/E1O2E-LAH'
-          default: ''
+          example: "/x/E1O2E-LAH"
+          default: ""
         service_state:
           description: >
             the service state
@@ -104,3 +105,8 @@ components:
             the service message
           type: string
           example: no suitable node (insufficient resources on 1 node)
+        user_id:
+          description: >-
+            the user that started the service
+          type: string
+          example: "123"

api/specs/director/openapi.yaml

@@ -286,6 +286,7 @@ paths:
       operationId: running_interactive_services_delete
       parameters:
         - $ref: "#/components/parameters/ServiceUuid"
+        - $ref: "#/components/parameters/SaveState"
       responses:
         "204":
           description: Succesfully stopped and removed the service from the oSparc platform
@@ -425,6 +426,15 @@ components:
           - 1.0.0
           - 0.0.1
 
+    SaveState:
+      in: query
+      name: save_state
+      description: Save the state prior to removing the service
+      required: false
+      schema:
+        type: boolean
+        default: true
+
   schemas:
     ErrorEnveloped:
       $ref: "../common/schemas/error.yaml#/components/schemas/ErrorEnveloped"

services/director/requirements/_test.in

@@ -12,6 +12,7 @@
 #
 
 # testing
+aioresponses
 coverage==4.5.1 # TODO: Downgraded because of a bug https://github.com/nedbat/coveragepy/issues/716
 pytest
 pytest-aiohttp  # incompatible with pytest-asyncio. See https://github.com/pytest-dev/pytest-asyncio/issues/76

services/director/requirements/_test.txt

@@ -6,6 +6,7 @@
 #
 aiodebug==1.1.2           # via -r requirements/_base.txt, simcore-service-library
 aiodocker==0.14.0         # via -r requirements/_base.txt
+aioresponses==0.7.2       # manually added. needed for mocking http calls and extensive testing.
 git+https://github.com/ITISFoundation/aiohttp_apiset.git@5c8a61ceb6de7ed9e09db5b4609b458a0d3773df  # via -r requirements/_base.txt
 aiohttp==3.3.2            # via -r requirements/_base.txt, aiodocker, aiohttp-apiset, aiozipkin, pytest-aiohttp, simcore-service-library
 aiopg==1.0.0              # via -r requirements/_base.txt, simcore-service-library

services/director/src/simcore_service_director/api/v0/openapi.yaml

@@ -1123,6 +1123,7 @@ paths:
                         - service_host
                         - service_port
                         - service_state
+                        - user_id
                       properties:
                         published_port:
                           description: The ports where the service provides its interface
@@ -1181,6 +1182,10 @@ paths:
                           description: the service message
                           type: string
                           example: no suitable node (insufficient resources on 1 node)
+                        user_id:
+                          description: the user that started the service
+                          type: string
+                          example: '123'
                   error:
                     nullable: true
                     default: null
@@ -1295,6 +1300,7 @@ paths:
                       - service_host
                       - service_port
                       - service_state
+                      - user_id
                     properties:
                       published_port:
                         description: The ports where the service provides its interface
@@ -1353,6 +1359,10 @@ paths:
                         description: the service message
                         type: string
                         example: no suitable node (insufficient resources on 1 node)
+                      user_id:
+                        description: the user that started the service
+                        type: string
+                        example: '123'
                   error:
                     nullable: true
                     default: null
@@ -1561,6 +1571,7 @@ paths:
                       - service_host
                       - service_port
                       - service_state
+                      - user_id
                     properties:
                       published_port:
                         description: The ports where the service provides its interface
@@ -1619,6 +1630,10 @@ paths:
                         description: the service message
                         type: string
                         example: no suitable node (insufficient resources on 1 node)
+                      user_id:
+                        description: the user that started the service
+                        type: string
+                        example: '123'
                   error:
                     nullable: true
                     default: null
@@ -1738,6 +1753,13 @@ paths:
           schema:
             type: string
             example: 123e4567-e89b-12d3-a456-426655440000
+        - in: query
+          name: save_state
+          description: Save the state prior to removing the service
+          required: false
+          schema:
+            type: boolean
+            default: true
       responses:
         '204':
           description: Succesfully stopped and removed the service from the oSparc platform
@@ -1948,6 +1970,14 @@ components:
         example:
           - 1.0.0
           - 0.0.1
+    SaveState:
+      in: query
+      name: save_state
+      description: Save the state prior to removing the service
+      required: false
+      schema:
+        type: boolean
+        default: true
   schemas:
     ErrorEnveloped:
       type: object
@@ -1994,6 +2024,7 @@ components:
             - service_host
             - service_port
             - service_state
+            - user_id
           properties:
             published_port:
               description: The ports where the service provides its interface
@@ -2052,6 +2083,10 @@ components:
               description: the service message
               type: string
               example: no suitable node (insufficient resources on 1 node)
+            user_id:
+              description: the user that started the service
+              type: string
+              example: '123'
         error:
           nullable: true
           default: null
@@ -2072,6 +2107,7 @@ components:
               - service_host
               - service_port
               - service_state
+              - user_id
             properties:
               published_port:
                 description: The ports where the service provides its interface
@@ -2130,6 +2166,10 @@ components:
                 description: the service message
                 type: string
                 example: no suitable node (insufficient resources on 1 node)
+              user_id:
+                description: the user that started the service
+                type: string
+                example: '123'
         error:
           nullable: true
           default: null

services/director/src/simcore_service_director/producer.py

@@ -728,6 +728,7 @@ async def _start_docker_service(
             "service_basepath": node_base_path,
             "service_state": service_state.value,
             "service_message": service_msg,
+            "user_id": user_id,
         }
         return container_meta_data
 
@@ -743,7 +744,7 @@ async def _start_docker_service(
 
 async def _silent_service_cleanup(app: web.Application, node_uuid: str) -> None:
     try:
-        await stop_service(app, node_uuid)
+        await stop_service(app, node_uuid, False)
     except exceptions.DirectorException:
         pass
 
@@ -887,6 +888,7 @@ async def _get_node_details(
     service_state, service_msg = results[2]
     service_name = service["Spec"]["Name"]
     service_uuid = service["Spec"]["Labels"]["uuid"]
+    user_id = service["Spec"]["Labels"]["user_id"]
 
     # get the published port
     published_port, target_port = await _get_docker_image_port_mapping(service)
@@ -901,6 +903,7 @@ async def _get_node_details(
         "service_basepath": service_basepath,
         "service_state": service_state.value,
         "service_message": service_msg,
+        "user_id": user_id,
     }
     return node_details
 
@@ -968,7 +971,7 @@ async def get_service_details(app: web.Application, node_uuid: str) -> Dict:
 
 
 @run_sequentially_in_context(target_args=["node_uuid"])
-async def stop_service(app: web.Application, node_uuid: str) -> None:
+async def stop_service(app: web.Application, node_uuid: str, save_state: bool) -> None:
     log.debug("stopping service with uuid %s", node_uuid)
     # get the docker client
     async with docker_utils.docker_client() as client:  # pylint: disable=not-async-context-manager
@@ -1004,27 +1007,29 @@ async def stop_service(app: web.Application, node_uuid: str) -> None:
             else "",
         )
         log.debug("saving state of service %s...", service_host_name)
-        try:
-            session = app[APP_CLIENT_SESSION_KEY]
-            service_url = "http://" + service_host_name + "/" + "state"
-            async with session.post(
-                service_url,
-                timeout=ServicesCommonSettings().director_dynamic_service_save_timeout,
-            ) as response:
-                if 199 < response.status < 300:
-                    log.debug(
-                        "service %s successfully saved its state", service_host_name
-                    )
-                else:
-                    log.warning(
-                        "service %s does not allow saving state, answered %s",
-                        service_host_name,
-                        await response.text(),
-                    )
-        except ClientConnectionError:
-            log.warning(
-                "service %s could not be contacted, state not saved", service_host_name
-            )
+        if save_state:
+            try:
+                session = app[APP_CLIENT_SESSION_KEY]
+                service_url = "http://" + service_host_name + "/" + "state"
+                async with session.post(
+                    service_url,
+                    timeout=ServicesCommonSettings().director_dynamic_service_save_timeout,
+                ) as response:
+                    if 199 < response.status < 300:
+                        log.debug(
+                            "service %s successfully saved its state", service_host_name
+                        )
+                    else:
+                        log.warning(
+                            "service %s does not allow saving state, answered %s",
+                            service_host_name,
+                            await response.text(),
+                        )
+            except ClientConnectionError:
+                log.warning(
+                    "service %s could not be contacted, state not saved",
+                    service_host_name,
+                )
 
         # remove the services
         try:

services/director/src/simcore_service_director/rest/generated_code/models/inline_response2003_data.py

@@ -15,7 +15,7 @@ class InlineResponse2003Data(Model):
     Do not edit the class manually.
     """
 
-    def __init__(self, published_port: int=None, entry_point: str=None, service_uuid: str=None, service_key: str=None, service_version: str=None, service_host: str=None, service_port: int=None, service_basepath: str='', service_state: str=None, service_message: str=None):
+    def __init__(self, published_port: int=None, entry_point: str=None, service_uuid: str=None, service_key: str=None, service_version: str=None, service_host: str=None, service_port: int=None, service_basepath: str='', service_state: str=None, service_message: str=None, user_id: str=None):
         """InlineResponse2003Data - a model defined in OpenAPI
 
         :param published_port: The published_port of this InlineResponse2003Data.
@@ -28,6 +28,7 @@ def __init__(self, published_port: int=None, entry_point: str=None, service_uuid
         :param service_basepath: The service_basepath of this InlineResponse2003Data.
         :param service_state: The service_state of this InlineResponse2003Data.
         :param service_message: The service_message of this InlineResponse2003Data.
+        :param user_id: The user_id of this InlineResponse2003Data.
         """
         self.openapi_types = {
             'published_port': int,
@@ -39,7 +40,8 @@ def __init__(self, published_port: int=None, entry_point: str=None, service_uuid
             'service_port': int,
             'service_basepath': str,
             'service_state': str,
-            'service_message': str
+            'service_message': str,
+            'user_id': str
         }
 
         self.attribute_map = {
@@ -52,7 +54,8 @@ def __init__(self, published_port: int=None, entry_point: str=None, service_uuid
             'service_port': 'service_port',
             'service_basepath': 'service_basepath',
             'service_state': 'service_state',
-            'service_message': 'service_message'
+            'service_message': 'service_message',
+            'user_id': 'user_id'
         }
 
         self._published_port = published_port
@@ -65,6 +68,7 @@ def __init__(self, published_port: int=None, entry_point: str=None, service_uuid
         self._service_basepath = service_basepath
         self._service_state = service_state
         self._service_message = service_message
+        self._user_id = user_id
 
     @classmethod
     def from_dict(cls, dikt: dict) -> 'InlineResponse2003Data':
@@ -172,8 +176,8 @@ def service_key(self, service_key):
         """
         if service_key is None:
             raise ValueError("Invalid value for `service_key`, must not be `None`")
-        if service_key is not None and not re.search(r'^(simcore)\/(services)\/(comp|dynamic)(\/[^\s\/]+)+$', service_key):
-            raise ValueError("Invalid value for `service_key`, must be a follow pattern or equal to `/^(simcore)\/(services)\/(comp|dynamic)(\/[^\s\/]+)+$/`")
+        if service_key is not None and not re.search(r'^(simcore)\/(services)\/(comp|dynamic)(\/[\w\/-]+)+$', service_key):
+            raise ValueError("Invalid value for `service_key`, must be a follow pattern or equal to `/^(simcore)\/(services)\/(comp|dynamic)(\/[\w\/-]+)+$/`")
 
         self._service_key = service_key
 
@@ -330,3 +334,28 @@ def service_message(self, service_message):
         """
 
         self._service_message = service_message
+
+    @property
+    def user_id(self):
+        """Gets the user_id of this InlineResponse2003Data.
+
+        the user that started the service
+
+        :return: The user_id of this InlineResponse2003Data.
+        :rtype: str
+        """
+        return self._user_id
+
+    @user_id.setter
+    def user_id(self, user_id):
+        """Sets the user_id of this InlineResponse2003Data.
+
+        the user that started the service
+
+        :param user_id: The user_id of this InlineResponse2003Data.
+        :type user_id: str
+        """
+        if user_id is None:
+            raise ValueError("Invalid value for `user_id`, must not be `None`")
+
+        self._user_id = user_id