Merge ~sylvain-pineau/checkbox-ng:remote_deamon_controller into checkbox-ng:master

[Sylvain Pineau]

The code that use the slave daemon to act as a new controller for root job commands

[Jonathan Cave (jocave)]

I'm not sure I fully understand the concept of what this is looking to do yet.

The intention is to "hand over" jobs that want to run as the root user to the remote-slave service? Why is this much better that just using localhost remote (assuming know bugs are fixed)? The obvious disadvantage being that if you start a session locally, you can't disconnect the front end and then re-connect somewhere else.

review: Needs Information

[Chris Wayne (cwayne)]

I'm looking at it as an intermediary step until full local-remote is ready
for widespread use

On Thu, Dec 13, 2018, 11:11 AM Jonathan Cave <<email address hidden>
wrote:

> Review: Needs Information
>
> I'm not sure I fully understand the concept of what this is looking to do
> yet.
>
> The intention is to "hand over" jobs that want to run as the root user to
> the remote-slave service? Why is this much better that just using localhost
> remote (assuming know bugs are fixed)? The obvious disadvantage being that
> if you start a session locally, you can't disconnect the front end and then
> re-connect somewhere else.
> --
>
> https://code.launchpad.net/~sylvain-pineau/checkbox-ng/+git/checkbox-ng/+merge/360477
> Your team Checkbox Administrators is subscribed to branch
> checkbox-ng:master.
>

[Sylvain Pineau (sylvain-pineau)]

Remote for everyone is the prio #1, back to in progress. I'll re assess the need after xmas.

Unmerged commits

ca2e28b... by Sylvain Pineau on 2018-12-10

subcommands:run: Add support for sideloaded providers

322a34f... by Sylvain Pineau on 2018-12-10

runner: Only run the job command via remote, not the full job

Doing so keeps all the logic to skip or not jobs on master

3ff0225... by Sylvain Pineau on 2018-12-10

session:assistant: log "Finalizing session" events as info not warning msg

Too noisy when used with remote slave

7472231... by Sylvain Pineau on 2018-12-10

remote: Properly render stderr stream lines in red

1e8da85... by Sylvain Pineau on 2018-12-10

remote_assistant: Fix logger name

255d44a... by Sylvain Pineau on 2018-12-10

ctrl: New execution controller that gains root by using the remote deamon

diff --git a/checkbox_ng/launcher/remote.py b/checkbox_ng/launcher/remote.py
index 70bdc60..34e8fec 100644
--- a/checkbox_ng/launcher/remote.py
+++ b/checkbox_ng/launcher/remote.py
@@ -84,6 +84,9 @@ class SimpleUI(NormalUI, MainLoopStage):
     def green_text(text, end='\n'):
         print(SimpleUI.C.GREEN(text), end)
 
+    def red_text(text, end='\n'):
+        print(SimpleUI.C.RED(text), end)
+
     def horiz_line():
         print(SimpleUI.C.WHITE('-' * 80))
 
@@ -431,7 +434,11 @@ class RemoteMaster(Command, ReportsStage, MainLoopStage):
         while True:
             state, payload = self.sa.monitor_job()
             if payload and not self._is_bootstrapping:
-                SimpleUI.green_text(payload, end='')
+                for line in payload:
+                    if line[0] == 'stderr':
+                        SimpleUI.red_text(payload, end='')
+                    else:
+                        SimpleUI.green_text(payload, end='')
             if state == 'running':
                 time.sleep(0.5)
                 while True:
diff --git a/checkbox_ng/launcher/subcommands.py b/checkbox_ng/launcher/subcommands.py
index 169d38a..4e4c2cf 100644
--- a/checkbox_ng/launcher/subcommands.py
+++ b/checkbox_ng/launcher/subcommands.py
@@ -771,8 +771,20 @@ class Run(Command, MainLoopStage):
                 "0.99",
                 ["restartable"],
             )
+            # side-load providers local-providers
+            side_load_path = os.path.expandvars(os.path.join(
+                '/home', '$USER', 'providers'))
+            additional_providers = ()
+            if os.path.exists(side_load_path):
+                print(self._C.RED(_(
+                    "WARNING: using side-loaded providers")))
+                os.environ['PROVIDERPATH'] = ''
+                embedded_providers = EmbeddedProvider1PlugInCollection(
+                    side_load_path)
+                additional_providers = embedded_providers.get_all_plugin_objects()
             self._configure_restart()
-            try_selecting_providers(self.sa, '*')
+            try_selecting_providers(
+                self.sa, '*', additional_providers=additional_providers)
             self.sa.start_new_session(self.ctx.args.title or 'checkbox-run')
             tps = self.sa.get_test_plans()
             self._configure_report()
diff --git a/plainbox/impl/ctrl.py b/plainbox/impl/ctrl.py
index ef08867..72a78b7 100644
--- a/plainbox/impl/ctrl.py
+++ b/plainbox/impl/ctrl.py
@@ -44,6 +44,7 @@ import itertools
 import json
 import logging
 import os
+import time
 import tempfile
 import subprocess
 import sys
@@ -55,6 +56,7 @@ from plainbox.abc import IJobResult
 from plainbox.abc import ISessionStateController
 from plainbox.i18n import gettext as _
 from plainbox.impl import get_plainbox_dir
+from plainbox.impl.color import Colorizer
 from plainbox.impl.depmgr import DependencyDuplicateError
 from plainbox.impl.depmgr import DependencyMissingError
 from plainbox.impl.resource import ExpressionCannotEvaluateError
@@ -74,11 +76,14 @@ from plainbox.impl.unit.unit import MissingParam
 from plainbox.impl.validation import Severity
 from plainbox.vendor import morris
 from plainbox.vendor import extcmd
+from plainbox.vendor import rpyc
+
 
 __all__ = [
     'CheckBoxSessionStateController',
     'RootViaPTL1ExecutionController',
     'RootViaPkexecExecutionController',
+    'RootViaRemoteDeamonController',
     'RootViaSudoExecutionController',
     'UserJobExecutionController',
     'checkbox_session_state_ctrl',
@@ -810,7 +815,7 @@ class UserJobExecutionController(CheckBoxExecutionController):
                 return -1
             if job.user is not None:
                 if os.getuid() == 0:
-                    return 4
+                    return 5
                 else:
                     return -1
             return 1
@@ -1305,6 +1310,62 @@ class RootViaPkexecExecutionController(
             return 0
 
 
+class RootViaRemoteDeamonController(
+        CheckBoxDifferentialExecutionController):
+    """
+    Execution controller that gains root by using the remote deamon.
+    """
+
+    _REMOTE_CONN = None
+
+    C = Colorizer()
+
+    def execute_remote_job(self, job, job_state, config):
+        sa = self._REMOTE_CONN.root.get_sa()
+        sa._reset_sa()
+        configuration = dict()
+        configuration['launcher'] = None
+        sa.start_session(configuration)
+        sa._sa.hand_pick_jobs([job.id])
+        sa.save_todo_list([job.id])
+        sa.run_job_command(job.id)
+        while True:
+            state, payload = sa.monitor_job()
+            if payload:
+                for line in payload:
+                    if line[0] == 'stderr':
+                        print(self.C.RED(line[1]), end='')
+                    else:
+                        print(self.C.GREEN(line[1]), end='')
+            if state == 'running':
+                time.sleep(0.1)
+            else:
+                result = sa.finish_job_command()
+                sa.finalize_session()
+                return result
+
+    def get_execution_command(self, job, job_state, config, session_dir,
+                              nest_dir):
+        return None
+
+    def get_checkbox_score(self, job):
+        """
+        Compute how applicable this controller is for the specified job.
+
+        :returns:
+            4 for jobs with a user override, zero otherwise
+        """
+        if job.user is not None:
+            try:
+                conn = rpyc.connect('localhost', 18871)
+                self._REMOTE_CONN = conn
+            except OSError:
+                return -1
+            return 4
+        else:
+            return 0
+
+
 class RootViaSudoExecutionController(
         CheckBoxDifferentialExecutionController):
     """
diff --git a/plainbox/impl/runner.py b/plainbox/impl/runner.py
index 7807a8f..03fff56 100644
--- a/plainbox/impl/runner.py
+++ b/plainbox/impl/runner.py
@@ -309,28 +309,21 @@ class JobRunner(IJobRunner):
         self._session_dir = session_dir
         if execution_ctrl_list is None:
             logger.debug("execution_ctrl_list not passed to JobRunner")
-            if sys.platform == 'linux' or sys.platform == 'linux2':
-                from plainbox.impl.ctrl import RootViaPkexecExecutionController
-                from plainbox.impl.ctrl import RootViaPTL1ExecutionController
-                from plainbox.impl.ctrl import RootViaSudoExecutionController
-                from plainbox.impl.ctrl import UserJobExecutionController
-                from plainbox.impl.ctrl import QmlJobExecutionController
-                execution_ctrl_list = [
-                    RootViaPTL1ExecutionController(provider_list),
-                    RootViaPkexecExecutionController(provider_list),
-                    # XXX: maybe this one should be only used on command line
-                    RootViaSudoExecutionController(provider_list),
-                    UserJobExecutionController(provider_list),
-                    QmlJobExecutionController(provider_list),
-                ]
-            elif sys.platform == 'win32':
-                from plainbox.impl.ctrl import UserJobExecutionController
-                execution_ctrl_list = [
-                    UserJobExecutionController(provider_list)
-                ]
-            else:
-                logger.warning("Unsupported platform: %s", sys.platform)
-                execution_ctrl_list = []
+            from plainbox.impl.ctrl import RootViaPkexecExecutionController
+            from plainbox.impl.ctrl import RootViaPTL1ExecutionController
+            from plainbox.impl.ctrl import RootViaRemoteDeamonController
+            from plainbox.impl.ctrl import RootViaSudoExecutionController
+            from plainbox.impl.ctrl import UserJobExecutionController
+            from plainbox.impl.ctrl import QmlJobExecutionController
+            execution_ctrl_list = [
+                RootViaPTL1ExecutionController(provider_list),
+                RootViaPkexecExecutionController(provider_list),
+                RootViaRemoteDeamonController(provider_list),
+                # XXX: maybe this one should be only used on command line
+                RootViaSudoExecutionController(provider_list),
+                UserJobExecutionController(provider_list),
+                QmlJobExecutionController(provider_list),
+            ]
         self._jobs_io_log_dir = jobs_io_log_dir
         # NOTE: deprecated
         self._command_io_delegate = command_io_delegate
@@ -787,24 +780,38 @@ class JobRunner(IJobRunner):
             return JobResultBuilder(
                 outcome=IJobResult.OUTCOME_NOT_SUPPORTED,
                 comments=_('No suitable execution controller is available)'))
-        # Run the embedded command
-        start_time = time.time()
-        return_code, record_path = self._run_command(
-            job, job_state, config, ctrl)
-        execution_duration = time.time() - start_time
-        # Convert the return of the command to the outcome of the job
-        if return_code == 0:
-            outcome = IJobResult.OUTCOME_PASS
-        elif return_code < 0:
-            outcome = IJobResult.OUTCOME_CRASH
-        else:
-            outcome = IJobResult.OUTCOME_FAIL
-        # Create a result object and return it
-        return JobResultBuilder(
-            outcome=outcome,
-            return_code=return_code,
-            io_log_filename=record_path,
-            execution_duration=execution_duration)
+        try:
+            result = ctrl.execute_remote_job(job, job_state, config)
+            params = {}
+            io_log_filename = getattr(
+                result, 'io_log_filename', None)
+            if io_log_filename:
+                params['io_log_filename'] = io_log_filename
+            if result.return_code is not None:
+                params['return_code'] = result.return_code
+            if result.execution_duration:
+                params['execution_duration'] = result.execution_duration
+            params['outcome'] = result.outcome
+            return JobResultBuilder(**params)
+        except AttributeError:
+            # Run the embedded command
+            start_time = time.time()
+            return_code, record_path = self._run_command(
+                job, job_state, config, ctrl)
+            execution_duration = time.time() - start_time
+            # Convert the return of the command to the outcome of the job
+            if return_code == 0:
+                outcome = IJobResult.OUTCOME_PASS
+            elif return_code < 0:
+                outcome = IJobResult.OUTCOME_CRASH
+            else:
+                outcome = IJobResult.OUTCOME_FAIL
+            # Create a result object and return it
+            return JobResultBuilder(
+                outcome=outcome,
+                return_code=return_code,
+                io_log_filename=record_path,
+                execution_duration=execution_duration)
 
     def _prepare_io_handling(self, job, config):
         ui_io_delegate = self._command_io_delegate
diff --git a/plainbox/impl/session/assistant.py b/plainbox/impl/session/assistant.py
index d85c467..2cebb4f 100644
--- a/plainbox/impl/session/assistant.py
+++ b/plainbox/impl/session/assistant.py
@@ -1278,6 +1278,72 @@ class SessionAssistant:
             if jsm[job.id].result.outcome is None
         ]
 
+    #@raises(ValueError, TypeError, UnexpectedMethodCall)
+    def run_job_command(
+        self, job_id: str, ui: 'Union[str, IJobRunnerUI]',
+        native: bool
+    ) -> 'JobResultBuilder':
+        """
+        Run a job with the specific identifier.
+        """
+        if isinstance(ui, IJobRunnerUI):
+            pass
+        elif isinstance(ui, str):
+            if ui == 'silent':
+                ui = _SilentUI()
+            elif ui == 'piano':
+                ui = _PianoUI()
+            else:
+                raise ValueError("unknown user interface: {!r}".format(ui))
+        else:
+            raise TypeError("incorrect UI type")
+        start_time = time.time()
+        job_state = self._context.state.job_state_map[job_id]
+        job = job_state.job
+        ui.considering_job(job, job_state)
+        ui.about_to_start_running(job, job_state)
+        self._context.state.metadata.running_job_name = job.id
+        self._manager.checkpoint()
+        autorestart = (self._restart_strategy is not None and
+                       'autorestart' in job.get_flag_set())
+        if autorestart:
+            restart_cmd = ' '.join(
+                shlex.quote(cmd_part)
+                for cmd_part in self._restart_cmd_callback(
+                    self._manager.storage.id))
+            self._restart_strategy.prime_application_restart(
+                self._app_id, self._manager.storage.id, restart_cmd)
+        ui.started_running(job, job_state)
+        if 'noreturn' in job.get_flag_set():
+            # 'share' the information how to respawn the application
+            # once all the test actions are performed.
+            # tests can read this from $PLAINBOX_PROVIDER_SHARE envvar
+            checkbox_data_dir = os.path.join(
+                self.get_session_dir(), 'CHECKBOX_DATA')
+            if not os.path.exists(checkbox_data_dir):
+                os.mkdir(checkbox_data_dir)
+            respawn_cmd_file = os.path.join(
+                checkbox_data_dir, '__respawn_checkbox')
+            if self._restart_cmd_callback:
+                with open(respawn_cmd_file, 'wt') as f:
+                    f.writelines(self._restart_cmd_callback(
+                        self.get_session_id()))
+        if not native:
+            builder = self._runner.run_job(
+                job, job_state, self._config, ui
+            ).get_builder()
+        else:
+            builder = JobResultBuilder(
+                outcome=IJobResult.OUTCOME_UNDECIDED,
+            )
+        builder.execution_duration = time.time() - start_time
+        if autorestart:
+            self._restart_strategy.diffuse_application_restart(
+                self._app_id)
+        self._manager.checkpoint()
+        ui.finished_running(job, job_state, builder.get_result())
+        return builder
+
     @raises(ValueError, TypeError, UnexpectedMethodCall)
     def run_job(
         self, job_id: str, ui: 'Union[str, IJobRunnerUI]',
@@ -1501,7 +1567,7 @@ class SessionAssistant:
             # leave the same usage expectations
             return
         if SessionMetaData.FLAG_SUBMITTED not in self._metadata.flags:
-            _logger.warning("Finalizing session that hasn't been submitted "
+            _logger.info("Finalizing session that hasn't been submitted "
                             "anywhere: %s", self._manager.storage.id)
         self._metadata.flags.remove(SessionMetaData.FLAG_INCOMPLETE)
         self._manager.checkpoint()
diff --git a/plainbox/impl/session/remote_assistant.py b/plainbox/impl/session/remote_assistant.py
index e1a9930..fa28ae9 100644
--- a/plainbox/impl/session/remote_assistant.py
+++ b/plainbox/impl/session/remote_assistant.py
@@ -42,7 +42,7 @@ from checkbox_ng.launcher.run import SilentUI
 
 _ = gettext.gettext
 
-_logger = logging.getLogger("plainbox.session.assistant2")
+_logger = logging.getLogger("plainbox.session.remote_assistant")
 
 Interaction = namedtuple('Interaction', ['kind', 'message', 'extra'])
 
@@ -74,7 +74,8 @@ class BufferedUI(SilentUI):
         self.clear_buffers()
 
     def got_program_output(self, stream_name, line):
-        self._queue.put(line.decode(sys.stdout.encoding, 'replace'))
+        self._queue.put(
+            (stream_name, line.decode(sys.stdout.encoding, 'replace')))
         self._whole_queue.put(line)
 
     def whole_output(self):
@@ -85,9 +86,9 @@ class BufferedUI(SilentUI):
 
     def get_output(self):
         """Returns all the output queued up since previous call."""
-        output = ''
+        output = []
         while not self._queue.empty():
-            output += self._queue.get()
+            output.append(self._queue.get())
         return output
 
     def clear_buffers(self):
@@ -270,6 +271,18 @@ class RemoteSessionAssistant():
         self._state = TestsSelected
 
     @allowed_when(TestsSelected)
+    def run_job_command(self, job_id):
+        _logger.debug("run_job: %r", job_id)
+        self._job_index = self._jobs_count - len(
+            self._sa.get_dynamic_todo_list()) + 1
+        self._currently_running_job = job_id
+        job = self._sa.get_job(job_id)
+        if job.command:
+            self._state = Running
+            self._be = BackgroundExecutor(
+                self, job_id, self._sa.run_job_command)
+
+    @allowed_when(TestsSelected)
     def run_job(self, job_id):
         """
         Depending on the type of the job, run_job can yield different number
@@ -434,6 +447,17 @@ class RemoteSessionAssistant():
                 self._state = Idle
             else:
                 self._state = TestsSelected
+
+    def finish_job_command(self):
+        # assert the thread completed
+        self.session_change_lock.acquire(blocking=False)
+        self._session_change_lock.release()
+        result = self._be.wait().get_result()
+        if self._state != Bootstrapping:
+            if not self._sa.get_dynamic_todo_list():
+                self._state = Idle
+            else:
+                self._state = TestsSelected
         return result
 
     def get_jobs_repr(self, job_ids, offset=0):
diff --git a/plainbox/impl/test_ctrl.py b/plainbox/impl/test_ctrl.py
index a211b0e..63cd01e 100644
--- a/plainbox/impl/test_ctrl.py
+++ b/plainbox/impl/test_ctrl.py
@@ -613,12 +613,12 @@ class UserJobExecutionControllerTests(CheckBoxExecutionControllerTestsMixIn,
     @mock.patch('os.getuid')
     def test_get_checkbox_score_as_root(self, mock_getuid, mock_plat):
         """
-        verify that score for jobs with an user override is 4 if I am root
+        verify that score for jobs with an user override is 5 if I am root
         """
         mock_plat.return_value = 'linux'
         mock_getuid.return_value = 0  # Pretend to be root
         self.job.user = 'root'
-        self.assertEqual(self.ctrl.get_checkbox_score(self.job), 4)
+        self.assertEqual(self.ctrl.get_checkbox_score(self.job), 5)
 
     @mock.patch.dict('os.environ', clear=True)
     def test_get_execution_environment_resets_locales(self):