Merge ~sylvain-pineau/checkbox-ng:classic-session-autorestart into checkbox-ng:master

[Sylvain Pineau]

This patchset allows checkbox classic snaps to run test plans like com.canonical.certification::warm-boot-stress-test w/o using remote (and even if a slave daemon is running in the background).

This method is not the best as upon resume (on a Ubuntu server image) the tester won't get any information about the progress of the stress tests and won't get any UI to interact with the running session.

The best solution is to switch to checkbox remote and use an external launcher to drive the tests from another machine.

But still, checkbox classic snaps were missing the ability to run such test plans. To workaround the lack of session feedback I've tweaked the systemd unit to redirect all I/O to /dev/console, example: https://i.imgur.com/T3jf7Ew.jpg

The main fix below is a change in the allowed call sequence from session assistant as the logic to determine the best restart strategy now requires knowledge about the kind of session to resume (local vs remote basically).

[Sylvain Pineau (sylvain-pineau)]

Tested using checkbox-snappy-classic with this launcher (remotely and not but still with the remote service enabled):

[launcher]
app_id = com.canonical.certification:checkbox-test
launcher_version = 1
stock_reports = text, submission_files

[test plan]
unit = com.canonical.certification::warm-boot-stress-test
forced = yes

[test selection]
forced = no

[ui]
output = hide-resource-and-attachment

[environment]
STRESS_BOOT_ITERATIONS=2
STRESS_BOOT_WAIT_DELAY=10
STRESS_BOOT_WAKEUP_DELAY=15

[Sylvain Pineau (sylvain-pineau)]

Local test command line:

ubuntu@ubuntu:~$ STRESS_BOOT_ITERATIONS=2 STRESS_BOOT_WAIT_DELAY=10 STRESS_BOOT_WAKEUP_DELAY=15 checkbox-snappy-classic.checkbox-cli launcher ./mylauncher

[Sylvain Pineau (sylvain-pineau)]

self-approved

review: Approve

[Devices Certification Bot (ce-certification-qa)]

The merge was fine but running tests failed.

[xenial] [12:31:13] starting container
Device project added to xenial-testing
[bionic] [12:31:17] starting container
Device project added to bionic-testing
[xenial] [12:31:24] provisioning container
[bionic] [12:31:32] provisioning container
[bionic] [12:32:51] Unable to provision requirements in container!
[bionic] output: https://paste.ubuntu.com/p/NDp28hq9S9/
[bionic] [12:32:54] Fixing file permissions in source directory
[bionic] Destroying failed container to reclaim resources
[xenial] [12:32:59] Unable to provision requirements in container!
[xenial] output: https://paste.ubuntu.com/p/Rr7fNX8Jcz/
[xenial] [12:33:01] Fixing file permissions in source directory
[xenial] Destroying failed container to reclaim resources

review: Needs Fixing

diff --git a/plainbox/impl/session/assistant.py b/plainbox/impl/session/assistant.py
index 90edfaf..6e8921e 100644
--- a/plainbox/impl/session/assistant.py
+++ b/plainbox/impl/session/assistant.py
@@ -253,7 +253,7 @@ class SessionAssistant:
         """
         UsageExpectation.of(self).enforce()
         if self._restart_strategy is None:
-            self._restart_strategy = detect_restart_strategy()
+            self._restart_strategy = detect_restart_strategy(self)
         self._restart_cmd_callback = cmd_callback
         # Prevent second call to this method and to the
         # use_alternate_restart_strategy() method.
@@ -524,6 +524,13 @@ class SessionAssistant:
             self.hand_pick_jobs: "select jobs to run (w/o a test plan)",
             self.finalize_session: "to finalize session",
         }
+        if SA_RESTARTABLE in self._flags:
+            allowed_calls = UsageExpectation.of(self).allowed_calls
+            allowed_calls[self.configure_application_restart] = (
+                "configure automatic restart capability")
+            allowed_calls[self.use_alternate_restart_strategy] = (
+                "configure automatic restart capability")
+
 
     @raises(KeyError, UnexpectedMethodCall)
     def resume_session(self, session_id: str,
diff --git a/plainbox/impl/session/remote_assistant.py b/plainbox/impl/session/remote_assistant.py
index c93e7df..1307bb1 100644
--- a/plainbox/impl/session/remote_assistant.py
+++ b/plainbox/impl/session/remote_assistant.py
@@ -161,7 +161,6 @@ class RemoteSessionAssistant():
         _logger.info("Resetting RSA")
         self._state = Idle
         self._sa = SessionAssistant('service', api_flags={SA_RESTARTABLE})
-        self._sa.configure_application_restart(self._cmd_callback)
         self._be = None
         self._session_id = ""
         self._jobs_count = 0
@@ -244,12 +243,15 @@ class RemoteSessionAssistant():
         _logger.debug("start_session: %r", configuration)
         session_title = 'checkbox-slave'
         session_desc = 'checkbox-slave session'
+        session_type = 'checkbox-slave'
 
         self._launcher = load_configs()
         if configuration['launcher']:
             self._launcher.read_string(configuration['launcher'], False)
-            session_title = self._launcher.session_title
-            session_desc = self._launcher.session_desc
+            if self._launcher.session_title:
+                session_title = self._launcher.session_title
+            if self._launcher.session_desc:
+                session_desc = self._launcher.session_desc
 
         self._sa.use_alternate_configuration(self._launcher)
 
@@ -268,7 +270,10 @@ class RemoteSessionAssistant():
         self._sa.update_app_blob(json.dumps(
             {'description': session_desc, }).encode("UTF-8"))
         self._sa.update_app_blob(json.dumps(
+            {'type': session_type, }).encode("UTF-8"))
+        self._sa.update_app_blob(json.dumps(
             {'launcher': configuration['launcher'], }).encode("UTF-8"))
+        self._sa.configure_application_restart(self._cmd_callback)
 
         self._session_id = self._sa.get_session_id()
         tps = self._sa.get_test_plans()
diff --git a/plainbox/impl/session/restart.py b/plainbox/impl/session/restart.py
index 6a33483..bd6a735 100644
--- a/plainbox/impl/session/restart.py
+++ b/plainbox/impl/session/restart.py
@@ -22,6 +22,7 @@
 
 import abc
 import errno
+import json
 import os
 import shlex
 import subprocess
@@ -149,6 +150,9 @@ class SnappyRestartStrategy(IRestartStrategy):
         section = 'Service'
         config.add_section(section)
         config.set(section, 'Type', 'oneshot')
+        config.set(section, 'StandardOutput', 'tty')
+        config.set(section, 'StandardError', 'tty')
+        config.set(section, 'TTYPath', '/dev/console')
         if os.getenv('USER'):
             config.set(section, 'User', os.getenv('USER'))
 
@@ -229,10 +233,11 @@ class RemoteSnappyRestartStrategy(IRestartStrategy):
                 raise
 
 
-def detect_restart_strategy() -> IRestartStrategy:
+def detect_restart_strategy(session=None) -> IRestartStrategy:
     """
     Detect the restart strategy for the current environment.
-
+    :param session:
+        The current session object.
     :returns:
         A restart strategy object.
     :raises LookupError:
@@ -255,16 +260,30 @@ def detect_restart_strategy() -> IRestartStrategy:
         except subprocess.CalledProcessError:
             return SnappyRestartStrategy()
 
-    # Classic + remote service enabled
+    # Classic snaps
     snap_data = os.getenv('SNAP_DATA')
+    try:
+        if session:
+            app_blob = json.loads(session._context.state.metadata.app_blob.decode('UTF-8'))
+            session_type = app_blob.get("type")
+        else:
+            session_type = None
+    except AttributeError:
+        session_type = None
     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
+        # Classic snaps w/ remote service enabled and in use
+        if session_type == "checkbox-slave":
+            try:
+                slave_status = subprocess.check_output(
+                    ['snapctl', 'get', 'slave'],
+                    universal_newlines=True).rstrip()
+                if slave_status == 'enabled':
+                    return RemoteSnappyRestartStrategy()
+            except subprocess.CalledProcessError:
+                pass
+        # Classic snaps w/o remote service
+        else:
+            return SnappyRestartStrategy()
 
     if os.path.isdir('/etc/xdg/autostart'):
         # NOTE: Assume this is a terminal application