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

[Sylvain Pineau]

Improved version of https://code.launchpad.net/~sylvain-pineau/checkbox-ng/+git/checkbox-ng/+merge/362546

There's no Resumed state, the resume event info happening on the slave is sent via the payload data.
the run_jobs method gets a new argument to process it to essentially display the resumed job id and its outcome.

On slave side, the new version of this MR saves the launcher settings in order to set them again when resuming (i.g auto_retry parameters). It gets also support for jobs altering the result via $CHECKBOX_DATA/__result.

One bonus since I bumped the API, stderr streams are colored in RED.

Tested using the same env/methodology as the previous MR (and to see how the new one evolved).

Warning: I still consider this version as a first set of commits to fully support reboots/autorestart/noreturn jobs. rebooting once works fine but for jobs like 30 suspends + 3 reboots handled by the job itself it won't as the service will start on boot and try to resume the session. For such complex scenario disabling the systemd unit and activating it again via the __respawn way is probably the best solution but deserves its own MR.

[Maciej Kisielewski (kissiel)]

Amazing work, huge +1.
I’d only change that reraising. Good to land after testing that we in fact don’t need that try/except block

review: Approve

[Sylvain Pineau (sylvain-pineau)]

I've removed the re raised exception. tested, ctrl-c lead exactly to the same screen. Good catch!

review: Needs Resubmitting

[Jonathan Cave (jocave)]

One tiny flake warning and a question if I may...

review: Needs Information

[Sylvain Pineau (sylvain-pineau)]

flake error fixed.

Regarding your question, of course /tmp will go away if you reboot your development system. But the debug mode was put in place to actually avoid having to reboot the system. Interrupting the slave with a CTRL-C will do the same (i.e going through all the strategy methods, prime&diffuse). So yes, the debug mode is not meant to validate the restart strategy with "real" reboots. It's just a convenience way to simulate them and speed up the debugging/development of features related to UC restarts on your development machine (i.e your laptop running classic)

[Jonathan Cave (jocave)]

Understood, thanks.

review: Approve

diff --git a/checkbox_ng/launcher/remote.py b/checkbox_ng/launcher/remote.py
index 03c600c..b884a4f 100644
--- a/checkbox_ng/launcher/remote.py
+++ b/checkbox_ng/launcher/remote.py
@@ -44,11 +44,13 @@ from plainbox.impl.color import Colorizer
 from plainbox.impl.launcher import DefaultLauncherDefinition
 from plainbox.impl.secure.sudo_broker import SudoProvider
 from plainbox.impl.session.remote_assistant import RemoteSessionAssistant
+from plainbox.impl.session.restart import RemoteSnappyRestartStrategy
 from plainbox.vendor import rpyc
 from plainbox.vendor.rpyc.utils.server import ThreadedServer
 from checkbox_ng.urwid_ui import test_plan_browser
 from checkbox_ng.urwid_ui import CategoryBrowser
 from checkbox_ng.urwid_ui import interrupt_dialog
+from checkbox_ng.urwid_ui import resume_dialog
 from checkbox_ng.launcher.run import NormalUI
 from checkbox_ng.launcher.stages import MainLoopStage
 from checkbox_ng.launcher.stages import ReportsStage
@@ -82,7 +84,10 @@ class SimpleUI(NormalUI, MainLoopStage):
         print(SimpleUI.C.header(header, fill='-'))
 
     def green_text(text, end='\n'):
-        print(SimpleUI.C.GREEN(text), end)
+        print(SimpleUI.C.GREEN(text), end=end)
+
+    def red_text(text, end='\n'):
+        print(SimpleUI.C.RED(text), end=end)
 
     def horiz_line():
         print(SimpleUI.C.WHITE('-' * 80))
@@ -124,7 +129,22 @@ class RemoteSlave(Command):
 
         SessionAssistantSlave.session_assistant = RemoteSessionAssistant(
             lambda s: [sys.argv[0] + ' remote-service --resume'])
-        if ctx.args.resume:
+        snap_data = os.getenv('SNAP_DATA')
+        remote_restart_stragegy_debug = os.getenv('REMOTE_RESTART_DEBUG')
+        if snap_data or remote_restart_stragegy_debug:
+            if remote_restart_stragegy_debug:
+                strategy = RemoteSnappyRestartStrategy(debug=True)
+            else:
+                strategy = RemoteSnappyRestartStrategy()
+            if os.path.exists(strategy.session_resume_filename):
+                with open(strategy.session_resume_filename, 'rt') as f:
+                    session_id = f.readline()
+                try:
+                    SessionAssistantSlave.session_assistant.resume_by_id(
+                        session_id)
+                except StopIteration:
+                    print("Couldn't resume the session")
+        elif ctx.args.resume:
             try:
                 SessionAssistantSlave.session_assistant.resume_last()
             except StopIteration:
@@ -236,7 +256,8 @@ class RemoteMaster(Command, ReportsStage, MainLoopStage):
                     'idle': self.new_session,
                     'running': self.wait_and_continue,
                     'finalizing': self.finish_session,
-                    'testsselected': self.run_jobs,
+                    'testsselected': partial(
+                        self.run_jobs, resumed_session_info=payload),
                     'bootstrapping': self.restart,
                     'bootstrapped': partial(
                         self.select_jobs, all_jobs=payload),
@@ -363,6 +384,7 @@ class RemoteMaster(Command, ReportsStage, MainLoopStage):
             return True
 
     def finish_session(self):
+        print(self.C.header("Results"))
         if self.launcher.local_submission:
             # Disable SIGINT while we save local results
             tmp_sig = signal.signal(signal.SIGINT, signal.SIG_IGN)
@@ -379,7 +401,24 @@ class RemoteMaster(Command, ReportsStage, MainLoopStage):
         self.wait_for_job()
         self.run_jobs()
 
-    def run_jobs(self):
+    def _handle_last_job_after_resume(self, resumed_session_info):
+        if self.launcher.ui_type == 'silent':
+            time.sleep(20)
+        else:
+            resume_dialog(10)
+        jobs_repr = self.sa.get_jobs_repr([resumed_session_info['last_job']])
+        job = jobs_repr[-1]
+        SimpleUI.header(job['name'])
+        print(_("ID: {0}").format(job['id']))
+        print(_("Category: {0}").format(job['category_name']))
+        SimpleUI.horiz_line()
+        print(
+            _("Outcome") + ": " +
+            SimpleUI.C.result(self.sa.get_job_result(job['id'])))
+
+    def run_jobs(self, resumed_session_info=None):
+        if resumed_session_info:
+            self._handle_last_job_after_resume(resumed_session_info)
         _logger.info("master: Running jobs.")
         jobs = self.sa.get_session_progress()
         _logger.debug("master: Jobs to be run:\n%s",
@@ -406,7 +445,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 stream, line in payload:
+                    if stream == 'stderr':
+                        SimpleUI.red_text(line, end='')
+                    else:
+                        SimpleUI.green_text(line, end='')
             if state == 'running':
                 time.sleep(0.5)
                 while True:
diff --git a/checkbox_ng/urwid_ui.py b/checkbox_ng/urwid_ui.py
index 8820b6b..992a471 100644
--- a/checkbox_ng/urwid_ui.py
+++ b/checkbox_ng/urwid_ui.py
@@ -22,6 +22,8 @@
 ============================================================
 """
 
+import time
+
 from gettext import gettext as _
 import urwid
 
@@ -621,6 +623,59 @@ def interrupt_dialog(host):
         return None
 
 
+class CountdownWidget(urwid.BigText):
+
+    def __init__(self, duration):
+        self._started = time.time()
+        self._duration = duration
+        self.set_text('{0:.1f}'.format(duration))
+        self.font = urwid.HalfBlock6x5Font()
+        super().__init__(self.get_text()[0], self.font)
+
+    def update(self):
+        remaining = self._duration + self._started - time.time()
+        if remaining <= 0:
+            remaining = 0
+        text = '{0:.1f}'.format(remaining)
+        self.set_text(text)
+        print('\33]2;Auto resume remote session in %s\007' % text, end='')
+        if remaining:
+            return True
+        else:
+            raise urwid.ExitMainLoop
+
+
+def resume_dialog(duration):
+    palette = [
+        ('body', 'light gray', 'black', 'standout'),
+        ('header', 'black', 'light gray', 'bold'),
+        ('buttnf', 'black', 'light gray'),
+        ('buttn', 'light gray', 'black', 'bold'),
+        ('foot', 'light gray', 'black'),
+        ('start', 'dark green,bold', 'black'),
+    ]
+    footer_text = [
+        ('Press '), ('<CTRL + C>'),
+        (" to open the cancellation menu")]
+    timer = CountdownWidget(duration)
+    timer_pad = urwid.Padding(timer, align='center', width='clip')
+    timer_fill = urwid.Filler(timer_pad)
+    title = _("Checkbox slave is about to resume the session!")
+    header = urwid.AttrWrap(urwid.Padding(urwid.Text(title), left=1), 'header')
+    footer = urwid.AttrWrap(
+        urwid.Padding(urwid.Text(footer_text), left=1), 'foot')
+    frame = urwid.Frame(urwid.AttrWrap(urwid.LineBox(timer_fill), 'body'),
+                        header=header, footer=footer)
+
+    def update_timer(loop, timer):
+        if timer.update():
+            loop.set_alarm_in(0.1, update_timer, timer)
+
+    loop = urwid.MainLoop(frame, palette)
+    update_timer(loop, timer)
+    loop.run()
+
+
 def add_widget(id, widget):
     """Add the widget for a given id."""
     _widget_cache[id] = widget
diff --git a/plainbox/impl/session/assistant.py b/plainbox/impl/session/assistant.py
index 1f67655..9e0c652 100644
--- a/plainbox/impl/session/assistant.py
+++ b/plainbox/impl/session/assistant.py
@@ -675,7 +675,6 @@ class SessionAssistant:
                     io_log_filename=self._runner.get_record_path_for_job(job),
                 ).get_result()
                 self._context.state.update_job_result(job, result)
-                self._metadata.running_job_name = None
                 self._manager.checkpoint()
         if self._restart_strategy is not None:
             self._restart_strategy.diffuse_application_restart(self._app_id)
@@ -686,6 +685,8 @@ class SessionAssistant:
         else:
             UsageExpectation.of(self).allowed_calls = {
                 self.select_test_plan: "to save test plan selection",
+                self.use_alternate_configuration: (
+                    "use an alternate configuration system"),
             }
         return self._metadata
 
diff --git a/plainbox/impl/session/remote_assistant.py b/plainbox/impl/session/remote_assistant.py
index abd6233..96786d1 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):
@@ -126,7 +127,7 @@ class BackgroundExecutor(Thread):
 class RemoteSessionAssistant():
     """Remote execution enabling wrapper for the SessionAssistant"""
 
-    REMOTE_API_VERSION = 4
+    REMOTE_API_VERSION = 5
 
     def __init__(self, cmd_callback):
         _logger.debug("__init__()")
@@ -154,6 +155,7 @@ class RemoteSessionAssistant():
         self._jobs_count = 0
         self._job_index = 0
         self._currently_running_job = None  # XXX: yuck!
+        self._last_job = None
         self._current_comments = ""
         self._last_response = None
         self._normal_user = ''
@@ -234,6 +236,8 @@ class RemoteSessionAssistant():
 
         self._sa.update_app_blob(json.dumps(
             {'description': session_desc, }).encode("UTF-8"))
+        self._sa.update_app_blob(json.dumps(
+            {'launcher': configuration['launcher'], }).encode("UTF-8"))
 
         self._session_id = self._sa.get_session_id()
         tps = self._sa.get_test_plans()
@@ -393,6 +397,8 @@ class RemoteSessionAssistant():
             payload = (
                 self._job_index, self._jobs_count, self._currently_running_job
             )
+        if self._state == TestsSelected and not self._currently_running_job:
+            payload = {'last_job': self._last_job}
         elif self._state == Started:
             payload = self._available_testplans
         elif self._state == Interacting:
@@ -491,6 +497,9 @@ class RemoteSessionAssistant():
         self._state = TestsSelected
         return candidates
 
+    def get_job_result(self, job_id):
+        return self._sa.get_job_state(job_id).result
+
     def get_jobs_repr(self, job_ids, offset=0):
         """
         Translate jobs into a {'field': 'val'} representations.
@@ -535,24 +544,55 @@ class RemoteSessionAssistant():
         return test_info_list
 
     def resume_last(self):
+        last = next(self._sa.get_resumable_sessions())
+        self.resume_by_id(last.id)
+
+    def resume_by_id(self, session_id):
         self._launcher = DefaultLauncherDefinition()
         self._sa.select_providers(*self._launcher.providers)
-        last = next(self._sa.get_resumable_sessions())
-        meta = self._sa.resume_session(last.id)
+        resume_candidates = list(self._sa.get_resumable_sessions())
+        _logger.warning("Resuming session: %r", session_id)
+        meta = self._sa.resume_session(session_id)
         app_blob = json.loads(meta.app_blob.decode("UTF-8"))
+        launcher = app_blob['launcher']
+        self._launcher.read_string(launcher)
+        self._sa.use_alternate_configuration(self._launcher)
         test_plan_id = app_blob['testplan_id']
         self._sa.select_test_plan(test_plan_id)
         self._sa.bootstrap()
-        result = MemoryJobResult({
+        self._last_job = meta.running_job_name
+
+        result_dict = {
             'outcome': IJobResult.OUTCOME_PASS,
-            'comments': _("Passed after resuming execution")
-        })
-        last_job = meta.running_job_name
-        if last_job:
+            'comments': _("Automatically passed after resuming execution"),
+        }
+        result_path = os.path.join(
+            self._sa.get_session_dir(), 'CHECKBOX_DATA', '__result')
+        if os.path.exists(result_path):
+            try:
+                with open(result_path, 'rt') as f:
+                    result_dict = json.load(f)
+                    # the only really important field in the result is
+                    # 'outcome' so let's make sure it doesn't contain
+                    # anything stupid
+                    if result_dict.get('outcome') not in [
+                            'pass', 'fail', 'skip']:
+                        result_dict['outcome'] = IJobResult.OUTCOME_PASS
+            except json.JSONDecodeError as e:
+                pass
+        result = MemoryJobResult(result_dict)
+        if self._last_job:
             try:
-                self._sa.use_job_result(last_job, result)
+                self._sa.use_job_result(self._last_job, result)
             except KeyError:
-                raise SystemExit(last_job)
+                raise SystemExit(self._last_job)
+
+        if self._launcher.auto_retry:
+            for job_id in [job.id for job in self.get_auto_retry_candidates()]:
+                job_state = self._sa.get_job_state(job_id)
+                job_state.attempts = self._launcher.max_attempts - len(
+                    job_state.result_history)
+
         self._state = TestsSelected
 
     def finalize_session(self):
diff --git a/plainbox/impl/session/restart.py b/plainbox/impl/session/restart.py
index eaa1f4d..0b497e5 100644
--- a/plainbox/impl/session/restart.py
+++ b/plainbox/impl/session/restart.py
@@ -197,6 +197,37 @@ class SnappyRestartStrategy(IRestartStrategy):
         subprocess.call(['sudo', 'rm', filename])
 
 
+class RemoteSnappyRestartStrategy(IRestartStrategy):
+
+    """
+    Remote Restart strategy for checkbox snaps.
+    """
+
+    def __init__(self, debug=False):
+        self.debug = debug
+        self.session_resume_filename = self.get_session_resume_filename()
+
+    def get_session_resume_filename(self) -> str:
+        if self.debug:
+            return '/tmp/session_resume'
+        snap_data = os.getenv('SNAP_DATA')
+        return os.path.join(snap_data, 'session_resume')
+
+    def prime_application_restart(self, app_id: str,
+                                  session_id: str, cmd: str) -> None:
+        with open(self.session_resume_filename, 'wt') as f:
+            f.write(session_id)
+
+    def diffuse_application_restart(self, app_id: str) -> None:
+        try:
+            os.remove(self.session_resume_filename)
+        except OSError as exc:
+            if exc.errno == errno.ENOENT:
+                pass
+            else:
+                raise
+
+
 def detect_restart_strategy() -> IRestartStrategy:
     """
     Detect the restart strategy for the current environment.
@@ -206,17 +237,41 @@ def detect_restart_strategy() -> IRestartStrategy:
     :raises LookupError:
         When no such object can be found.
     """
+    # XXX: RemoteSnappyRestartStrategy debug
+    remote_restart_stragegy_debug = os.getenv('REMOTE_RESTART_DEBUG')
+    if remote_restart_stragegy_debug:
+        return RemoteSnappyRestartStrategy(debug=True)
     # If we are running as a confined Snappy app this variable will have been
     # set by the launcher script
     if on_ubuntucore():
-        return SnappyRestartStrategy()
-        
+        try:
+            slave_status = subprocess.check_output(
+                ['snapctl', 'get', 'slave'], universal_newlines=True).rstrip()
+            if slave_status == 'disabled':
+                return SnappyRestartStrategy()
+            else:
+                return RemoteSnappyRestartStrategy()
+        except subprocess.CalledProcessError:
+            return SnappyRestartStrategy()
+
+    # Classic + remote service enabled
+    snap_data = os.getenv('SNAP_DATA')
+    if snap_data:
+        try:
+            slave_status = subprocess.check_output(
+                ['snapctl', 'get', 'slave'], universal_newlines=True).rstrip()
+            if slave_status == 'enabled':
+                return RemoteSnappyRestartStrategy()
+        except subprocess.CalledProcessError:
+            pass
+
     if os.path.isdir('/etc/xdg/autostart'):
         # NOTE: Assume this is a terminal application
         return XDGRestartStrategy(app_terminal=True)
 
     raise LookupError("Unable to find appropriate strategy.""")
 
+
 def get_strategy_by_name(name: str) -> type:
     """
     Get restart strategy class identified by a string.