diff --git a/checkbox-ng/checkbox_ng/launcher/agent.py b/checkbox-ng/checkbox_ng/launcher/agent.py index 4ef3e055c..f515c4e9d 100644 --- a/checkbox-ng/checkbox_ng/launcher/agent.py +++ b/checkbox-ng/checkbox_ng/launcher/agent.py @@ -1,8 +1,7 @@ # This file is part of Checkbox. # -# Copyright 2017-2019 Canonical Ltd. +# Copyright 2017-2023 Canonical Ltd. # Written by: -# Maciej Kisielewski # # Checkbox is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License version 3, @@ -20,11 +19,15 @@ functionality. """ import gettext +import json import logging import os import socket import sys +from checkbox_ng import app_context +from plainbox.impl.config import Configuration from plainbox.impl.secure.sudo_broker import is_passwordless_sudo +from plainbox.impl.session.assistant import ResumeCandidate from plainbox.impl.session.remote_assistant import RemoteSessionAssistant from plainbox.impl.session.restart import RemoteDebRestartStrategy from plainbox.impl.session.restart import RemoteSnappyRestartStrategy @@ -36,7 +39,6 @@ class SessionAssistantAgent(rpyc.Service): - session_assistant = None controlling_controller_conn = None controller_blaster = None @@ -58,10 +60,14 @@ def exposed_register_controller_blaster(self, callable): def on_connect(self, conn): try: if SessionAssistantAgent.controller_blaster: - msg = 'Forcefully disconnected by new controller from {}:{}'.format( - conn._config['endpoints'][1][0], conn._config['endpoints'][1][1]) + msg = "Forcefully disconnected by new controller from {}:{}".format( + conn._config["endpoints"][1][0], + conn._config["endpoints"][1][1], + ) SessionAssistantAgent.controller_blaster(msg) - old_controller = SessionAssistantAgent.controlling_controller_conn + old_controller = ( + SessionAssistantAgent.controlling_controller_conn + ) if old_controller is not None: old_controller.close() SessionAssistantAgent.controller_blaster = None @@ -82,7 +88,7 @@ def on_disconnect(self, conn): self.controlling_controller_conn = None -class RemoteAgent(): +class RemoteAgent: """ Run checkbox instance as a agent @@ -91,14 +97,23 @@ class RemoteAgent(): part should be run on system-under-test. """ - name = 'agent' + name = "agent" def invoked(self, ctx): if os.geteuid(): raise SystemExit(_("Checkbox agent must be run by root!")) if not is_passwordless_sudo(): raise SystemExit( - _("System is not configured to run sudo without a password!")) + _("System is not configured to run sudo without a password!") + ) + if ctx.args.resume: + msg = ( + "--resume is deprecated and will be removed soon. " + "Automated sessions are now always resumed. " + "Manual sessions can be resumed from the welcome screen." + ) + _logger.warning(msg) + agent_port = ctx.args.port # Check if able to connect to the agent port as indicator of there @@ -106,45 +121,35 @@ def invoked(self, ctx): def agent_port_open(): sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) sock.settimeout(0.5) - result = sock.connect_ex(('127.0.0.1', agent_port)) + result = sock.connect_ex(("127.0.0.1", agent_port)) sock.close() return result + if agent_port_open() == 0: - raise SystemExit(_("Found port {} is open. Is Checkbox agent" - " already running?").format(agent_port)) + raise SystemExit( + _( + "Found port {} is open. Is Checkbox agent" + " already running?" + ).format(agent_port) + ) SessionAssistantAgent.session_assistant = RemoteSessionAssistant( - lambda s: [sys.argv[0] + 'agent']) - snap_data = os.getenv('SNAP_DATA') - snap_rev = os.getenv('SNAP_REVISION') - remote_restart_strategy_debug = os.getenv('REMOTE_RESTART_DEBUG') - if (snap_data and snap_rev) or ctx.args.resume: - if remote_restart_strategy_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() - SessionAssistantAgent.session_assistant.resume_by_id( - session_id) - elif ctx.args.resume: - # XXX: explicitly passing None to not have to bump Remote API - # TODO: remove on the next Remote API bump - SessionAssistantAgent.session_assistant.resume_by_id(None) - else: - _logger.info("RemoteDebRestartStrategy") - if remote_restart_strategy_debug: - strategy = RemoteDebRestartStrategy(debug=True) - else: - strategy = RemoteDebRestartStrategy() - if os.path.exists(strategy.session_resume_filename): - with open(strategy.session_resume_filename, 'rt') as f: - session_id = f.readline() - _logger.info( - "RemoteDebRestartStrategy resume_by_id %r", session_id) + lambda s: [sys.argv[0] + "agent"] + ) + + # the agent is meant to be run only as a service, + # and we always resume if the session was automated, + # so we don't need to encode check whether we should resume + + sessions = list(ctx.sa.get_resumable_sessions()) + if sessions: + # the sessions are ordered by time, so the first one is the most + # recent one + if is_the_session_noninteractive(sessions[0]): SessionAssistantAgent.session_assistant.resume_by_id( - session_id) + sessions[0].id + ) + self._server = ThreadedServer( SessionAssistantAgent, port=agent_port, @@ -152,15 +157,35 @@ def agent_port_open(): "allow_all_attrs": True, "allow_setattr": True, "sync_request_timeout": 1, - "propagate_SystemExit_locally": True + "propagate_SystemExit_locally": True, }, ) SessionAssistantAgent.session_assistant.terminate_cb = ( - self._server.close) + self._server.close + ) self._server.start() def register_arguments(self, parser): - parser.add_argument('--resume', action='store_true', help=_( - "resume last session")) - parser.add_argument('--port', type=int, default=18871, help=_( - "port to listen on")) + parser.add_argument( + "--resume", action="store_true", help=_("resume last session") + ) + parser.add_argument( + "--port", type=int, default=18871, help=_("port to listen on") + ) + + +def is_the_session_noninteractive( + resumable_session: "ResumeCandidate", +) -> bool: + """ + Check if given session is non-interactive. + + To determine that we need to take the original launcher that had been used + when the session was started, recreate it as a proper Launcher object, and + check if it's in fact non-interactive. + """ + # app blob is a bytes string with a utf-8 encoded json + # let's decode it and parse it as json + app_blob = json.loads(resumable_session.metadata.app_blob.decode("utf-8")) + launcher = Configuration.from_text(app_blob["launcher"], "resumed session") + return launcher.sections["ui"].get("type") == "silent" diff --git a/checkbox-ng/checkbox_ng/launcher/checkbox_cli.py b/checkbox-ng/checkbox_ng/launcher/checkbox_cli.py index 5b6dc0da3..2c58f8943 100644 --- a/checkbox-ng/checkbox_ng/launcher/checkbox_cli.py +++ b/checkbox-ng/checkbox_ng/launcher/checkbox_cli.py @@ -80,7 +80,7 @@ def main(): "slave": "run-agent", "service": "run-agent", "master": "control", - "remote": "control" + "remote": "control", } known_cmds = list(commands.keys()) @@ -141,12 +141,7 @@ def main(): subcmd = commands[args.subcommand]() subcmd.register_arguments(subcmd_parser) sub_args = subcmd_parser.parse_args(sys.argv[subcmd_index + 1 :]) - sa = SessionAssistant( - "com.canonical:checkbox-cli", - "0.99", - "0.99", - ["restartable"], - ) + sa = SessionAssistant() ctx = Context(sub_args, sa) try: socket.getaddrinfo("localhost", 443) # 443 for HTTPS diff --git a/checkbox-ng/checkbox_ng/launcher/test_agent.py b/checkbox-ng/checkbox_ng/launcher/test_agent.py index 85472661b..5d679922c 100644 --- a/checkbox-ng/checkbox_ng/launcher/test_agent.py +++ b/checkbox-ng/checkbox_ng/launcher/test_agent.py @@ -16,9 +16,13 @@ # You should have received a copy of the GNU General Public License # along with Checkbox. If not, see . +import json from unittest import TestCase, mock +from checkbox_ng.launcher.agent import is_the_session_noninteractive from checkbox_ng.launcher.agent import RemoteAgent +from plainbox.impl.session.assistant import ResumeCandidate +from plainbox.impl.session.state import SessionMetaData class AgentTests(TestCase): @@ -61,3 +65,24 @@ def test_invoked_ok( server = threaded_server_mock.return_value # the server was started self.assertTrue(server.start.called) + + +class IsTheSessionNonInteractiveTests(TestCase): + def test_a_non_interactive_one(self): + a_non_interactive_launcher = """ + [ui] + type = silent + """ + app_blob = json.dumps({"launcher": a_non_interactive_launcher}) + metadata = SessionMetaData(app_blob=app_blob.encode("utf-8")) + + candidate = ResumeCandidate("an_id", metadata) + self.assertTrue(is_the_session_noninteractive(candidate)) + + def test_an_interactive(self): + a_non_interactive_launcher = "" # the defautl one is interactive + app_blob = json.dumps({"launcher": a_non_interactive_launcher}) + metadata = SessionMetaData(app_blob=app_blob.encode("utf-8")) + + candidate = ResumeCandidate("an_id", metadata) + self.assertFalse(is_the_session_noninteractive(candidate)) diff --git a/checkbox-ng/plainbox/impl/session/assistant.py b/checkbox-ng/plainbox/impl/session/assistant.py index 33caae857..698009341 100644 --- a/checkbox-ng/plainbox/impl/session/assistant.py +++ b/checkbox-ng/plainbox/impl/session/assistant.py @@ -34,6 +34,9 @@ import time from tempfile import SpooledTemporaryFile + +from checkbox_ng.app_context import application_name + from plainbox.abc import IJobResult from plainbox.abc import IJobRunnerUI from plainbox.abc import ISessionStateTransport @@ -123,7 +126,11 @@ class SessionAssistant: # TODO: create a flowchart of possible states def __init__( - self, app_id, app_version=None, api_version="0.99", api_flags=() + self, + app_id=application_name(), + app_version=None, + api_version="0.99", + api_flags=(), ): """ Initialize a new session assistant. diff --git a/checkbox-ng/plainbox/impl/session/remote_assistant.py b/checkbox-ng/plainbox/impl/session/remote_assistant.py index 34dd7f51a..2de252f2a 100644 --- a/checkbox-ng/plainbox/impl/session/remote_assistant.py +++ b/checkbox-ng/plainbox/impl/session/remote_assistant.py @@ -162,7 +162,7 @@ def __init__(self, cmd_callback): def _reset_sa(self): _logger.info("Resetting RSA") self._state = Idle - self._sa = SessionAssistant("service", api_flags={SA_RESTARTABLE}) + self._sa = SessionAssistant() self._be = None self._session_id = "" self._jobs_count = 0 @@ -292,7 +292,8 @@ def start_session(self, configuration): configuration["launcher"], "Remote launcher" ) self._launcher.update_from_another( - launcher_from_controller, "Remote launcher") + launcher_from_controller, "Remote launcher" + ) session_title = ( self._launcher.get_value("launcher", "session_title") or session_title @@ -326,8 +327,6 @@ def start_session(self, configuration): } ).encode("UTF-8") self._sa.update_app_blob(new_blob) - self._sa.configure_application_restart(self._cmd_callback) - self._session_id = self._sa.get_session_id() tps = self._sa.get_test_plans() filtered_tps = set() @@ -702,7 +701,8 @@ def resume_by_id(self, session_id=None): app_blob["launcher"], "Remote launcher" ) self._launcher.update_from_another( - launcher_from_controller, "Remote launcher") + launcher_from_controller, "Remote launcher" + ) self._sa.use_alternate_configuration(self._launcher) self._normal_user = app_blob.get( @@ -740,6 +740,14 @@ def resume_by_id(self, session_id=None): result_dict["outcome"] = IJobResult.OUTCOME_PASS except json.JSONDecodeError: pass + else: + the_job = self._sa.get_job(self._last_job) + if the_job.plugin == "shell": + if "noreturn" in the_job.get_flag_set(): + result_dict["outcome"] = IJobResult.OUTCOME_PASS + else: + result_dict["outcome"] = IJobResult.OUTCOME_CRASH + result = MemoryJobResult(result_dict) if self._last_job: try: diff --git a/checkbox-ng/plainbox/impl/session/test_assistant.py b/checkbox-ng/plainbox/impl/session/test_assistant.py index 5ae48ccf1..c86c158f3 100644 --- a/checkbox-ng/plainbox/impl/session/test_assistant.py +++ b/checkbox-ng/plainbox/impl/session/test_assistant.py @@ -26,35 +26,36 @@ from plainbox.vendor import morris -@mock.patch('plainbox.impl.session.assistant.get_providers') +@mock.patch("plainbox.impl.session.assistant.get_providers") class SessionAssistantTests(morris.SignalTestCase): """Tests for the SessionAssitant class.""" - APP_ID = 'app-id' - APP_VERSION = '1.0' - API_VERSION = '0.99' + APP_ID = "app-id" + APP_VERSION = "1.0" + API_VERSION = "0.99" API_FLAGS = [] def setUp(self): """Common set-up code.""" self.sa = SessionAssistant( - self.APP_ID, self.APP_VERSION, self.API_VERSION, self.API_FLAGS) + self.APP_ID, self.APP_VERSION, self.API_VERSION, self.API_FLAGS + ) # Monitor the provider_selected signal since some tests check it self.watchSignal(self.sa.provider_selected) # Create a few mocked providers that tests can use. # The all-important plainbox provider - self.p1 = mock.Mock(spec_set=Provider1, name='p1') - self.p1.namespace = 'com.canonical.plainbox' - self.p1.name = 'com.canonical.plainbox:special' + self.p1 = mock.Mock(spec_set=Provider1, name="p1") + self.p1.namespace = "com.canonical.plainbox" + self.p1.name = "com.canonical.plainbox:special" # An example 3rd party provider - self.p2 = mock.Mock(spec_set=Provider1, name='p2') - self.p2.namespace = 'pl.zygoon' - self.p2.name = 'pl.zygoon:example' + self.p2 = mock.Mock(spec_set=Provider1, name="p2") + self.p2.namespace = "pl.zygoon" + self.p2.name = "pl.zygoon:example" # A Canonical certification provider - self.p3 = mock.Mock(spec_set=Provider1, name='p3') - self.p3.namespace = 'com.canonical.certification' - self.p3.name = 'com.canonical.certification:stuff' + self.p3 = mock.Mock(spec_set=Provider1, name="p3") + self.p3.namespace = "com.canonical.certification" + self.p3.name = "com.canonical.certification:stuff" def _get_mock_providers(self): """Get some mocked provides for testing.""" @@ -64,16 +65,22 @@ def test_expected_call_sequence(self, mock_get_providers): """Track the sequence of allowed method calls.""" mock_get_providers.return_value = self._get_mock_providers() # SessionAssistant.start_new_session() must now be allowed - self.assertIn(self.sa.start_new_session, - UsageExpectation.of(self.sa).allowed_calls) + self.assertIn( + self.sa.start_new_session, + UsageExpectation.of(self.sa).allowed_calls, + ) # Call SessionAssistant.start_new_session() self.sa.start_new_session("just for testing") # SessionAssistant.start_new_session() must no longer allowed - self.assertNotIn(self.sa.start_new_session, - UsageExpectation.of(self.sa).allowed_calls) + self.assertNotIn( + self.sa.start_new_session, + UsageExpectation.of(self.sa).allowed_calls, + ) # SessionAssistant.select_test_plan() must now be allowed - self.assertIn(self.sa.select_test_plan, - UsageExpectation.of(self.sa).allowed_calls) + self.assertIn( + self.sa.select_test_plan, + UsageExpectation.of(self.sa).allowed_calls, + ) # Use the manager to tidy up after the tests when normally you wouldnt # be allowed to self.sa._manager.destroy()