Repository: jina-ai/serve
Pull request: fix: rely on control request to set ready docker pea
**Changes introduced**
Instead of relying on reading messages from the logs to set ready signal to the Pod, it relies in network connection with the Pea running inside docker.

To ensure that the PretrainedModelException is properly passed when building executors in the hub, we are still basing ourselves on reading log messages. Not sure if there is a solution for it

**Discussion**
Since there are 2 processes where the `ContainerPea` lives right now, if we start the `docker` container in a `detach=False` way, we may have better control on how its resources are used, and maybe we can avoid all this inner communication? I am not an expert but in non-detached mode we may be able to extract exceptions and errors?

Problem statement:
Find a better way of communication between `ContainerPea` and `BasePea`
**Describe the feature**
Right now `BasePea` only knows when `ContainerPea` is ready (or any other needed communication by reading a specific message in its logstream).

This logic is really fragile and we should find a better way of communicating with the `ContainerPea` to have a reliable way to communicate.

So for instance, `BasePea` uses `events` to communicate between processes, with `ContainerPea` we may need to have some sort of socket based communication method or some `docker` built-in logic

Hints from issue discussion:
control port is reserved for this purpose
Then it should be used to communicate that Pea is ready, instead of reading from the logs a specific message. In the code seems that no clear way was found at the time it was implemented
I guess this is because ctrl port use ipc protocol by default, which is not container-pass-through. And I didn't have time to work on this details. Feel free to improve ctrl port, as it is needed in `jinad` and remote control

Commit message: fix: rely on control request to set ready docker pea

Patch to review:
diff --git a/jina/peapods/container.py b/jina/peapods/container.py
index 7b41c82da8d4e..cf7a2b6f7036a 100644
--- a/jina/peapods/container.py
+++ b/jina/peapods/container.py
@@ -5,6 +5,8 @@
 from pathlib import Path
 
 from .pea import BasePea
+from .zmq import send_ctrl_message
+from ..proto import jina_pb2
 from .. import __ready_msg__, __unable_to_load_pretrained_model_msg__
 from ..helper import is_valid_local_config_source, kwargs2list, get_non_defaults_args
 from ..logging import JinaLogger
@@ -65,14 +67,13 @@ def post_init(self):
         _args = kwargs2list(non_defaults)
         self._container = self._client.containers.run(self.args.uses, _args,
                                                       detach=True, auto_remove=True,
-                                                      ports={'%d/tcp' % v: v for v in
+                                                      ports={f'{v}': v for v in
                                                              _expose_port},
                                                       name=self.name,
                                                       volumes=_volumes,
                                                       network_mode=net_mode,
                                                       entrypoint=self.args.entrypoint,
-                                                      # network='mynetwork',
-                                                      # publish_all_ports=True
+                                                      # publish_all_ports=True # This looks like something I would activate
                                                       )
         # wait until the container is ready
         self.logger.info('waiting ready signal from the container')
@@ -82,18 +83,20 @@ def loop_body(self):
         import docker
 
         logger = JinaLogger('🐳', **vars(self.args))
+        ready = False
         with logger:
             try:
                 for line in self._container.logs(stream=True):
                     msg = line.strip().decode()
                     # this is shabby, but it seems the only easy way to detect is_ready signal meanwhile
                     # print all error message when fails
-                    if __ready_msg__ in msg:
+                    logger.info(line.strip().decode())
+                    if not ready and self.get_ready:
                         self.is_ready.set()
                         self.logger.success(__ready_msg__)
+                        ready = True
                     if __unable_to_load_pretrained_model_msg__ in msg:
                         self.is_pretrained_model_exception.set()
-                    logger.info(line.strip().decode())
             except docker.errors.NotFound:
                 self.logger.error('the container can not be started, check your arguments, entrypoint')
 
@@ -109,6 +112,18 @@ def loop_teardown(self):
         if getattr(self, '_client', None):
             self._client.close()
 
+    @property
+    def status(self):
+        """Send the control signal ``STATUS`` to itself and return the status """
+        if getattr(self, 'ctrl_addr'):
+            return send_ctrl_message(self.ctrl_addr, jina_pb2.Request.ControlRequest.STATUS,
+                                     timeout=self.args.timeout_ctrl)
+
+    @property
+    def get_ready(self) -> bool:
+        status = self.status
+        return status and status.envelope.status.code == jina_pb2.Status.READY
+
     def close(self) -> None:
         self.send_terminate_signal()
         if not self.daemon:
diff --git a/tests/unit/peapods/test_container.py b/tests/unit/peapods/test_container.py
index 1d7555f4cc29f..d1312a69fde37 100644
--- a/tests/unit/peapods/test_container.py
+++ b/tests/unit/peapods/test_container.py
@@ -182,3 +182,14 @@ def test_tail_host_docker2local(docker_image_built):
     with f:
         assert getattr(f._pod_nodes['d12'].tail_args, 'host_out') == localhost
         f.dry_run()
+
+
+def test_container_status():
+    args = set_pea_parser().parse_args(['--uses', img_name,
+                                        '--uses-internal',
+                                        os.path.join(cur_dir, '../mwu-encoder/mwu_encoder_ext.yml')])
+    pea = ContainerPea(args)
+    assert not pea.get_ready
+    with pea:
+        time.sleep(1.)
+        assert pea.get_ready


Code review comments:
diff_hunk: @@ -109,6 +112,18 @@ def loop_teardown(self):
         if getattr(self, '_client', None):
             self._client.close()
 
+    @property
+    def status(self):
+        """Send the control signal ``STATUS`` to itself and return the status """
+        if getattr(self, 'ctrl_addr'):
+            return send_ctrl_message(self.ctrl_addr, jina_pb2.Request.ControlRequest.STATUS,
+                                     timeout=self.args.timeout_ctrl)
+
+    @property
+    def get_ready(self) -> bool:
path: jina/peapods/container.py
text: @user1:
```suggestion
    def is_ready(self) -> bool:
```

diff_hunk: @@ -65,37 +67,50 @@ def post_init(self):
         _args = kwargs2list(non_defaults)
         self._container = self._client.containers.run(self.args.uses, _args,
                                                       detach=True, auto_remove=True,
-                                                      ports={'%d/tcp' % v: v for v in
+                                                      ports={f'{v}': v for v in
                                                              _expose_port},
                                                       name=self.name,
                                                       volumes=_volumes,
                                                       network_mode=net_mode,
                                                       entrypoint=self.args.entrypoint,
-                                                      # network='mynetwork',
-                                                      # publish_all_ports=True
+                                                      # publish_all_ports=True # This looks like something I would activate
                                                       )
         # wait until the container is ready
         self.logger.info('waiting ready signal from the container')
 
     def loop_body(self):
         """Direct the log from the container to local console """
-        import docker
+        import asyncio
+        def check_ready():
+            import asyncio
+            while not self.is_ready:
+                asyncio.sleep(0.1)
+            self.is_ready_event.set()
+            self.logger.success(__ready_msg__)
+            return True
+
+        async def _loop_body():
+            import asyncio
path: jina/peapods/container.py
text: @user1:
Can we move the import to the top of the file, since this is a standard Python import? 

@author:
Done!