From 1751f46bbe2d08dac96bd791fecf177681793cc6 Mon Sep 17 00:00:00 2001 From: Piotr Bartman-Szwarc Date: Thu, 23 May 2024 13:22:42 +0200 Subject: [PATCH 01/21] updater: non-interactive updater --- qui/updater/intro_page.py | 8 +++--- qui/updater/progress_page.py | 7 ++++-- qui/updater/updater.py | 47 +++++++++++++++++++++++++++--------- 3 files changed, 44 insertions(+), 18 deletions(-) diff --git a/qui/updater/intro_page.py b/qui/updater/intro_page.py index a1adf1c9..740a42ac 100644 --- a/qui/updater/intro_page.py +++ b/qui/updater/intro_page.py @@ -179,8 +179,8 @@ def select_rows_ignoring_conditions(self, cliargs, dom0): args = [a for a in dir(cliargs) if not a.startswith("_")] for arg in args: - if arg in ("dom0", "no-restart", "restart", "max_concurrency", - "log"): + if arg in ("dom0", "no_restart", "restart", "max_concurrency", + "log", "non_interactive"): continue value = getattr(cliargs, arg) if value: @@ -191,8 +191,8 @@ def select_rows_ignoring_conditions(self, cliargs, dom0): continue value = ",".join(vms_without_dom0) cmd.append(f"--{arg.replace('_', '-')}") - if isinstance(value, str): - cmd.append(value) + if isinstance(value, (str, int)): + cmd.append(str(value)) to_update = set() if cmd[2:]: diff --git a/qui/updater/progress_page.py b/qui/updater/progress_page.py index 4ea9f2ab..7697f658 100644 --- a/qui/updater/progress_page.py +++ b/qui/updater/progress_page.py @@ -44,7 +44,8 @@ def __init__( log, header_label, next_button, - cancel_button + cancel_button, + callback ): self.builder = builder self.log = log @@ -54,6 +55,7 @@ def __init__( self.vms_to_update = None self.exit_triggered = False self.update_thread = None + self.after_update_callback = callback self.update_details = QubeUpdateDetails(self.builder) @@ -127,9 +129,10 @@ def perform_update(self, settings): if templs: self.update_templates(templs, settings) - GLib.idle_add(self.next_button.set_sensitive, True) GLib.idle_add(self.header_label.set_text, l("Update finished")) GLib.idle_add(self.cancel_button.set_visible, False) + GLib.idle_add(self.next_button.set_sensitive, True) + self.after_update_callback() def update_admin_vm(self, admins): """Runs command to update dom0.""" diff --git a/qui/updater/updater.py b/qui/updater/updater.py index df6ab01c..d0fec9bc 100644 --- a/qui/updater/updater.py +++ b/qui/updater/updater.py @@ -3,6 +3,7 @@ # pylint: disable=wrong-import-position,import-error import argparse import logging +import sys import time import importlib.resources @@ -12,7 +13,7 @@ show_dialog_with_icon, RESPONSES_OK from qui.updater.progress_page import ProgressPage from qui.updater.updater_settings import Settings, OverridenSettings -from qui.updater.summary_page import SummaryPage +from qui.updater.summary_page import SummaryPage, RestartStatus from qui.updater.intro_page import IntroPage gi.require_version('Gtk', '3.0') # isort:skip @@ -44,6 +45,7 @@ def __init__(self, qapp, cliargs): self.do_nothing = False self.connect("activate", self.do_activate) self.cliargs = cliargs + self.retcode = 0 log_handler = logging.FileHandler( QubesUpdater.LOGPATH, encoding='utf-8') @@ -104,7 +106,9 @@ def perform_setup(self, *_args, **_kwargs): self.log, self.header_label, self.next_button, - self.cancel_button + self.cancel_button, + callback=lambda: self.next_clicked(None) + if self.cliargs.non_interactive else lambda: None ) self.summary_page = SummaryPage( self.builder, @@ -125,7 +129,7 @@ def perform_setup(self, *_args, **_kwargs): overriden_restart = None if self.cliargs.restart: overriden_restart = True - elif self.cliargs.no_restart: + elif self.cliargs.no_restart: overriden_restart = False overrides = OverridenSettings( @@ -201,15 +205,27 @@ def next_clicked(self, _emitter, skip_intro=False): vm_updated=self.progress_page.vms_to_update, settings=self.settings ) - self.summary_page.show(*self.progress_page.get_update_summary()) + updated, no_updates, failed = ( + self.progress_page.get_update_summary()) + if failed: + self.retcode = 1 + if failed or not self.cliargs.non_interactive: + self.summary_page.show(updated, no_updates, failed) + else: + self.restart_phase() elif self.summary_page.is_visible: - self.main_window.hide() - self.log.debug("Hide main window") - # ensuring that main_window will be hidden - while Gtk.events_pending(): - Gtk.main_iteration() - self.summary_page.restart_selected_vms() - self.exit_updater() + self.restart_phase() + + def restart_phase(self): + self.main_window.hide() + self.log.debug("Hide main window") + # ensuring that main_window will be hidden + while Gtk.events_pending(): + Gtk.main_iteration() + self.summary_page.restart_selected_vms() + if self.summary_page.status == RestartStatus.ERROR: + self.retcode = 2 + self.exit_updater() def cancel_clicked(self, _emitter): self.log.debug("Cancel clicked") @@ -296,6 +312,11 @@ def parse_args(args): parser.add_argument('--dom0', action='store_true', help='Target dom0') + parser.add_argument('--non-interactive', action='store_true', + help='Run the updater GUI in non-interactive mode. ' + 'Interaction will be required in the event ' + 'of an update error.') + args = parser.parse_args(args) return args @@ -304,7 +325,8 @@ def parse_args(args): def skip_intro_if_args(args): return args is not None and (args.templates or args.standalones or args.skip or args.update_if_stale or args.all - or args.targets or args.dom0) + or args.targets or args.dom0 + or args.non_interactive) def main(args=None): @@ -312,6 +334,7 @@ def main(args=None): qapp = Qubes() app = QubesUpdater(qapp, cliargs) app.run() + sys.exit(app.retcode) if __name__ == '__main__': From 819fdd9d8899519079a62c0e579b1fc6106cdfab Mon Sep 17 00:00:00 2001 From: Piotr Bartman-Szwarc Date: Mon, 27 May 2024 10:10:35 +0200 Subject: [PATCH 02/21] updater: unify cli options with cli updater --- qui/updater/intro_page.py | 3 ++- qui/updater/updater.py | 33 +++++++++++++++++++++------------ qui/updater/updater_settings.py | 11 ++++++----- 3 files changed, 29 insertions(+), 18 deletions(-) diff --git a/qui/updater/intro_page.py b/qui/updater/intro_page.py index 740a42ac..112d6cf7 100644 --- a/qui/updater/intro_page.py +++ b/qui/updater/intro_page.py @@ -179,7 +179,8 @@ def select_rows_ignoring_conditions(self, cliargs, dom0): args = [a for a in dir(cliargs) if not a.startswith("_")] for arg in args: - if arg in ("dom0", "no_restart", "restart", "max_concurrency", + if arg in ("dom0", "no_restart", "restart", "apply_to_sys", + "apply_to_all", "no_apply", "max_concurrency", "log", "non_interactive"): continue value = getattr(cliargs, arg) diff --git a/qui/updater/updater.py b/qui/updater/updater.py index d0fec9bc..709771bb 100644 --- a/qui/updater/updater.py +++ b/qui/updater/updater.py @@ -21,7 +21,7 @@ from qubesadmin import Qubes # using locale.gettext is necessary for Gtk.Builder translation support to work -# in most cases gettext is better, but it cannot handle Gtk.Builder/glade files +# in most cases, gettext is better, but it cannot handle Gtk.Builder/glade files import locale from locale import gettext as l @@ -126,14 +126,20 @@ def perform_setup(self, *_args, **_kwargs): settings_image = Gtk.Image.new_from_pixbuf(settings_pixbuf) self.button_settings.set_image(settings_image) - overriden_restart = None - if self.cliargs.restart: - overriden_restart = True + overridden_apply_to_sys = None + overridden_apply_to_other = None + if self.cliargs.apply_to_all: + overridden_apply_to_sys = True + overridden_apply_to_other = True + elif self.cliargs.restart: + overridden_apply_to_sys = True elif self.cliargs.no_restart: - overriden_restart = False + overridden_apply_to_sys = False + overridden_apply_to_other = False overrides = OverridenSettings( - restart=overriden_restart, + apply_to_sys=overridden_apply_to_sys, + apply_to_other=overridden_apply_to_other, max_concurrency=self.cliargs.max_concurrency, update_if_stale=self.cliargs.update_if_stale, ) @@ -186,7 +192,6 @@ def cell_data_func(_column, cell, model, it, data): width = self.intro_page.vm_list.get_preferred_width().natural_width self.main_window.resize(width + 50, int(width * 1.2)) self.main_window.set_position(Gtk.WindowPosition.CENTER_ALWAYS) - # return 0 def open_settings_window(self, _emitter): self.settings.show() @@ -283,12 +288,16 @@ def parse_args(args): '(default: number of cpus)', type=int) restart_gr = parser.add_mutually_exclusive_group() - restart_gr.add_argument('--restart', action='store_true', - help='Restart AppVMs whose template ' - 'has been updated.') - restart_gr.add_argument('--no-restart', action='store_true', - help='Do not restart AppVMs whose template ' + restart_gr.add_argument('--restart', '--apply-to-sys', '-r', + action='store_true', + help='Restart Service VMs whose template ' 'has been updated.') + restart_gr.add_argument('--apply-to-all', '-R', + action='store_true', + help='Restart Service VMs and shutdown AppVMs ' + 'whose template has been updated.') + restart_gr.add_argument('--no-apply', action='store_true', + help='Do not restart/shutdown any AppVMs.') group = parser.add_mutually_exclusive_group() group.add_argument('--targets', action='store', diff --git a/qui/updater/updater_settings.py b/qui/updater/updater_settings.py index 4b5d45f4..056effb8 100644 --- a/qui/updater/updater_settings.py +++ b/qui/updater/updater_settings.py @@ -39,7 +39,8 @@ @dataclasses.dataclass(frozen=True) class OverridenSettings: - restart: Optional[bool] = None + apply_to_sys: Optional[bool] = None + apply_to_other: Optional[bool] = None max_concurrency: Optional[int] = None update_if_stale: Optional[int] = None @@ -148,8 +149,8 @@ def update_if_stale(self) -> int: @property def restart_service_vms(self) -> bool: """Return the current (set by this window or manually) option value.""" - if self.overrides.restart is not None: - return self.overrides.restart + if self.overrides.apply_to_sys is not None: + return self.overrides.apply_to_sys result = get_boolean_feature( self.vm, "qubes-vm-update-restart-servicevms", @@ -168,8 +169,8 @@ def restart_service_vms(self) -> bool: @property def restart_other_vms(self) -> bool: """Return the current (set by this window or manually) option value.""" - if self.overrides.restart is not None: - return self.overrides.restart + if self.overrides.apply_to_other is not None: + return self.overrides.apply_to_other return get_boolean_feature( self.vm, "qubes-vm-update-restart-other", Settings.DEFAULT_RESTART_OTHER_VMS) From d7c282ee1b1411ce9bfe74eac4bec9c1c3d50252 Mon Sep 17 00:00:00 2001 From: Piotr Bartman-Szwarc Date: Mon, 27 May 2024 11:05:55 +0200 Subject: [PATCH 03/21] updater: return 100 if no vm was updated --- qui/updater/updater.py | 7 +++++-- qui/updater/updater_settings.py | 9 +++++---- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/qui/updater/updater.py b/qui/updater/updater.py index 709771bb..359ea61b 100644 --- a/qui/updater/updater.py +++ b/qui/updater/updater.py @@ -12,7 +12,7 @@ from qubes_config.widgets.gtk_utils import load_icon_at_gtk_size, load_theme, \ show_dialog_with_icon, RESPONSES_OK from qui.updater.progress_page import ProgressPage -from qui.updater.updater_settings import Settings, OverridenSettings +from qui.updater.updater_settings import Settings, OverriddenSettings from qui.updater.summary_page import SummaryPage, RestartStatus from qui.updater.intro_page import IntroPage @@ -137,7 +137,7 @@ def perform_setup(self, *_args, **_kwargs): overridden_apply_to_sys = False overridden_apply_to_other = False - overrides = OverridenSettings( + overrides = OverriddenSettings( apply_to_sys=overridden_apply_to_sys, apply_to_other=overridden_apply_to_other, max_concurrency=self.cliargs.max_concurrency, @@ -214,6 +214,9 @@ def next_clicked(self, _emitter, skip_intro=False): self.progress_page.get_update_summary()) if failed: self.retcode = 1 + if updated == 0 and failed == 0: + # no updates + self.retcode = 100 if failed or not self.cliargs.non_interactive: self.summary_page.show(updated, no_updates, failed) else: diff --git a/qui/updater/updater_settings.py b/qui/updater/updater_settings.py index 056effb8..f55f5090 100644 --- a/qui/updater/updater_settings.py +++ b/qui/updater/updater_settings.py @@ -37,8 +37,9 @@ GObject.SignalFlags.RUN_LAST, GObject.TYPE_PYOBJECT, (GObject.TYPE_PYOBJECT,)) + @dataclasses.dataclass(frozen=True) -class OverridenSettings: +class OverriddenSettings: apply_to_sys: Optional[bool] = None apply_to_other: Optional[bool] = None max_concurrency: Optional[int] = None @@ -59,7 +60,7 @@ def __init__( qapp, log, refresh_callback: Callable, - overrides: OverridenSettings = OverridenSettings(), + overrides: OverriddenSettings = OverriddenSettings(), ): self.qapp = qapp self.log = log @@ -71,7 +72,7 @@ def __init__( self.builder.set_translation_domain("desktop-linux-manager") glade_ref = (importlib.resources.files('qui') / - 'updater_settings.glade') + 'updater_settings.glade') with importlib.resources.as_file(glade_ref) as path: self.builder.add_from_file(str(path)) @@ -222,7 +223,7 @@ def _limit_concurrency_toggled(self, _emitter=None): ) def show(self): - """Show hidden window.""" + """Show a hidden window.""" self.load_settings() self.settings_window.show_all() self._show_restart_exceptions() From 32508dd456dc00296e20660e5cc81dd5e7619d87 Mon Sep 17 00:00:00 2001 From: Piotr Bartman-Szwarc Date: Mon, 27 May 2024 12:28:54 +0200 Subject: [PATCH 04/21] updater: update tests --- qui/updater/tests/test_progress_page.py | 30 ++++++++++--- qui/updater/tests/test_updater.py | 58 ++++++++++++++++++++++++- qui/updater/updater.py | 2 +- qui/updater/updater_settings.py | 5 ++- 4 files changed, 85 insertions(+), 10 deletions(-) diff --git a/qui/updater/tests/test_progress_page.py b/qui/updater/tests/test_progress_page.py index 63e0a917..e5bca68b 100644 --- a/qui/updater/tests/test_progress_page.py +++ b/qui/updater/tests/test_progress_page.py @@ -43,8 +43,10 @@ def test_init_update( mock_threading.return_value = mock_thread mock_log = Mock() + mock_callback = Mock() sut = ProgressPage( - real_builder, mock_log, mock_label, mock_next_button, mock_cancel_button + real_builder, mock_log, mock_label, mock_next_button, + mock_cancel_button, mock_callback ) sut.progress_list = mock_tree_view @@ -61,6 +63,7 @@ def test_init_update( assert mock_label.halign == Gtk.Align.CENTER assert sut.progress_list.model == all_vms_list.list_store_raw + mock_callback.assert_not_called() @patch('gi.repository.GLib.idle_add') @@ -69,8 +72,10 @@ def test_perform_update( mock_next_button, mock_cancel_button, mock_label, updatable_vms_list ): mock_log = Mock() + mock_callback = Mock() sut = ProgressPage( - real_builder, mock_log, mock_label, mock_next_button, mock_cancel_button + real_builder, mock_log, mock_label, mock_next_button, + mock_cancel_button, mock_callback ) sut.vms_to_update = updatable_vms_list @@ -91,6 +96,7 @@ def __call__(self, vm_rows, *args, **kwargs): call(mock_label.set_text, "Update finished"), call(mock_cancel_button.set_visible, False)] idle_add.assert_has_calls(calls, any_order=True) + mock_callback.assert_called_once() @patch('gi.repository.GLib.idle_add') @@ -108,8 +114,10 @@ def test_update_admin_vm( mock_list_store ): mock_log = Mock() + mock_callback = Mock() sut = ProgressPage( - real_builder, mock_log, mock_label, mock_next_button, mock_cancel_button + real_builder, mock_log, mock_label, mock_next_button, + mock_cancel_button, mock_callback ) admins = ListWrapper(UpdateRowWrapper, mock_list_store) @@ -130,6 +138,7 @@ def test_update_admin_vm( idle_add.assert_has_calls(calls) if not interrupted: mock_subprocess.assert_called() + mock_callback.assert_not_called() @patch('gi.repository.GLib.idle_add') @@ -145,8 +154,10 @@ def test_update_templates( mock_next_button, mock_cancel_button, mock_label, mock_text_view ): mock_log = Mock() + mock_callback = Mock() sut = ProgressPage( - real_builder, mock_log, mock_label, mock_next_button, mock_cancel_button + real_builder, mock_log, mock_label, mock_next_button, + mock_cancel_button, mock_callback ) sut.do_update_templates = Mock() @@ -172,6 +183,7 @@ def test_update_templates( idle_add.assert_has_calls(calls, any_order=True) if not interrupted: sut.do_update_templates.assert_called() + mock_callback.assert_not_called() @patch('subprocess.Popen') @@ -200,8 +212,10 @@ def poll(self): mock_subprocess.return_value = MockPorc() mock_log = Mock() + mock_callback = Mock() sut = ProgressPage( - real_builder, mock_log, mock_label, mock_next_button, mock_cancel_button + real_builder, mock_log, mock_label, mock_next_button, + mock_cancel_button, mock_callback ) sut.read_stderrs = lambda *_args, **_kwargs: None sut.read_stdouts = lambda *_args, **_kwargs: None @@ -223,6 +237,7 @@ def poll(self): 'fedora-35,fedora-36,test-standalone'], stderr=subprocess.PIPE, stdout=subprocess.PIPE)] mock_subprocess.assert_has_calls(calls) + mock_callback.assert_not_called() def test_get_update_summary( @@ -230,8 +245,10 @@ def test_get_update_summary( mock_next_button, mock_cancel_button, mock_label, updatable_vms_list ): mock_log = Mock() + mock_callback = Mock() sut = ProgressPage( - real_builder, mock_log, mock_label, mock_next_button, mock_cancel_button + real_builder, mock_log, mock_label, mock_next_button, + mock_cancel_button, mock_callback ) updatable_vms_list[0].set_status(UpdateStatus.NoUpdatesFound) @@ -246,6 +263,7 @@ def test_get_update_summary( assert vm_updated_num == 1 assert vm_no_updates_num == 1 assert vm_failed_num == 2 + mock_callback.assert_not_called() def test_set_active_row(real_builder, updatable_vms_list): diff --git a/qui/updater/tests/test_updater.py b/qui/updater/tests/test_updater.py index f1d1019b..31352e97 100644 --- a/qui/updater/tests/test_updater.py +++ b/qui/updater/tests/test_updater.py @@ -18,7 +18,9 @@ # along with this program; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, # USA. -from unittest.mock import patch, call +from unittest.mock import patch, call, Mock + +import pytest from qui.updater.updater import QubesUpdater, parse_args @@ -31,3 +33,57 @@ def test_setup(populate_vm_list, _mock_logging, __mock_logging, test_qapp): sut.perform_setup() calls = [call(sut.qapp, sut.settings)] populate_vm_list.assert_has_calls(calls) + + +@patch('logging.FileHandler') +@patch('logging.getLogger') +@patch('qui.updater.intro_page.IntroPage.populate_vm_list') +@pytest.mark.parametrize( + "update_results, ret_code", + ( + pytest.param((0, 0, 0), 100, id="nothing to do"), + pytest.param((0, 0, 1), 1, id="failed"), + pytest.param((0, 1, 0), 100, id="no updates"), + pytest.param((0, 1, 1), 1, id="no updates + failed"), + pytest.param((1, 0, 0), 0, id="success"), + pytest.param((1, 0, 1), 1, id="success + failed"), + pytest.param((1, 1, 0), 0, id="success + no updated"), + pytest.param((1, 1, 1), 1, id="all"), + ) +) +def test_retcode(_populate_vm_list, _mock_logging, __mock_logging, + update_results, ret_code, test_qapp): + sut = QubesUpdater(test_qapp, parse_args(())) + sut.perform_setup() + + sut.intro_page.get_vms_to_update = Mock() + vms_to_update = Mock() + sut.intro_page.get_vms_to_update.return_value = vms_to_update + + def set_vms(_vms_to_update, _settings): + sut.progress_page.vms_to_update = _vms_to_update + sut.progress_page.init_update = Mock(side_effect=set_vms) + + sut.next_clicked(None) + + assert not sut.intro_page.active + assert sut.progress_page.is_visible + sut.progress_page.init_update.assert_called_once_with( + vms_to_update, sut.settings) + + # set sut.summary_page.is_populated = False + sut.summary_page.list_store = None + def populate(**_kwargs): + sut.summary_page.list_store = [] + sut.summary_page.populate_restart_list = Mock(side_effect=populate) + sut.progress_page.get_update_summary = Mock() + sut.progress_page.get_update_summary.return_value = update_results + sut.summary_page.show = Mock() + sut.summary_page.show.return_value = None + + sut.next_clicked(None) + + sut.summary_page.populate_restart_list.assert_called_once_with( + restart=True, vm_updated=vms_to_update, settings=sut.settings) + assert sut.retcode == ret_code + sut.summary_page.show.assert_called_once_with(*update_results) diff --git a/qui/updater/updater.py b/qui/updater/updater.py index 359ea61b..192721ef 100644 --- a/qui/updater/updater.py +++ b/qui/updater/updater.py @@ -133,7 +133,7 @@ def perform_setup(self, *_args, **_kwargs): overridden_apply_to_other = True elif self.cliargs.restart: overridden_apply_to_sys = True - elif self.cliargs.no_restart: + elif self.cliargs.no_apply: overridden_apply_to_sys = False overridden_apply_to_other = False diff --git a/qui/updater/updater_settings.py b/qui/updater/updater_settings.py index f55f5090..f80a65f7 100644 --- a/qui/updater/updater_settings.py +++ b/qui/updater/updater_settings.py @@ -195,11 +195,12 @@ def load_settings(self): self._init_restart_servicevms = self.restart_service_vms self._init_restart_other_vms = self.restart_other_vms self.restart_servicevms_checkbox.set_sensitive( - not self.overrides.restart) + not self.overrides.apply_to_sys) self.restart_servicevms_checkbox.set_active( self._init_restart_servicevms) self.restart_other_checkbox.set_active(self._init_restart_other_vms) - self.restart_other_checkbox.set_sensitive(not self.overrides.restart) + self.restart_other_checkbox.set_sensitive( + not self.overrides.apply_to_other) self._init_max_concurrency = self.max_concurrency self._init_limit_concurrency = self._init_max_concurrency is not None From 22e6599dd9a47885e1dccb89da511eaee226fe7f Mon Sep 17 00:00:00 2001 From: Piotr Bartman-Szwarc Date: Tue, 28 May 2024 16:25:53 +0200 Subject: [PATCH 05/21] updater: show success dialog in non-interactive mode --- .../en/LC_MESSAGES/desktop-linux-manager.po | 9 ++++++ qui/updater/summary_page.py | 2 +- qui/updater/updater.py | 30 +++++++++++++++++-- 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/locale/en/LC_MESSAGES/desktop-linux-manager.po b/locale/en/LC_MESSAGES/desktop-linux-manager.po index b53de3bd..b555fdd1 100644 --- a/locale/en/LC_MESSAGES/desktop-linux-manager.po +++ b/locale/en/LC_MESSAGES/desktop-linux-manager.po @@ -548,3 +548,12 @@ msgstr "" msgid "failed to update." msgstr "" + +msgid "Qubes OS is up to date." +msgstr "" + +msgid "All selected qubes have been updated." +msgstr "" + +msgid "There are no updates available for the selected Qubes." +msgstr "" diff --git a/qui/updater/summary_page.py b/qui/updater/summary_page.py index 7fd7ef19..cd60b7b0 100644 --- a/qui/updater/summary_page.py +++ b/qui/updater/summary_page.py @@ -251,7 +251,7 @@ def restart_selected_vms(self): time.sleep(0.01) if self.restart_thread.is_alive(): - # show wainting dialog + # show waiting dialog spinner = Gtk.Spinner() spinner.start() dialog = show_dialog(None, l("Applying updates to qubes"), l( diff --git a/qui/updater/updater.py b/qui/updater/updater.py index 192721ef..f3b6398b 100644 --- a/qui/updater/updater.py +++ b/qui/updater/updater.py @@ -220,11 +220,12 @@ def next_clicked(self, _emitter, skip_intro=False): if failed or not self.cliargs.non_interactive: self.summary_page.show(updated, no_updates, failed) else: - self.restart_phase() + self._restart_phase() + self._show_final_dialog() elif self.summary_page.is_visible: - self.restart_phase() + self._restart_phase() - def restart_phase(self): + def _restart_phase(self): self.main_window.hide() self.log.debug("Hide main window") # ensuring that main_window will be hidden @@ -235,6 +236,29 @@ def restart_phase(self): self.retcode = 2 self.exit_updater() + def _show_final_dialog(self): + """ + In a non-interactive mode, we should show the user a success + confirmation. + """ + if not self.cliargs.non_interactive: + return + + if (not self.cliargs.skip and not self.cliargs.targets + and self.retcode in (0, 100)): + msg = "Qubes OS is up to date." + elif self.retcode == 0: + msg = "All selected qubes have been updated." + elif self.retcode == 100: + msg = "There are no updates available for the selected Qubes." + show_dialog_with_icon( + None, + l("Success"), + l(msg), + buttons=RESPONSES_OK, + icon_name="qubes-check-yes" + ) + def cancel_clicked(self, _emitter): self.log.debug("Cancel clicked") if self.summary_page.is_visible: From 47bea9595bc0f8a3fa255c8fde9e9b0bac11b7cb Mon Sep 17 00:00:00 2001 From: Piotr Bartman-Szwarc Date: Tue, 28 May 2024 16:40:18 +0200 Subject: [PATCH 06/21] updater: show success dialog if nothing to do --- qui/updater/updater.py | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/qui/updater/updater.py b/qui/updater/updater.py index f3b6398b..dda45641 100644 --- a/qui/updater/updater.py +++ b/qui/updater/updater.py @@ -65,12 +65,7 @@ def do_activate(self, *_args, **_kwargs): else: self.log.debug("Secondary activation") if self.do_nothing: - show_dialog_with_icon( - None, l("Quit"), - l("Nothing to do."), - buttons=RESPONSES_OK, - icon_name="check_yes" - ) + self._show_final_dialog() self.window_close() else: self.main_window.present() @@ -221,7 +216,8 @@ def next_clicked(self, _emitter, skip_intro=False): self.summary_page.show(updated, no_updates, failed) else: self._restart_phase() - self._show_final_dialog() + if self.cliargs.non_interactive: + self._show_final_dialog() elif self.summary_page.is_visible: self._restart_phase() @@ -238,12 +234,11 @@ def _restart_phase(self): def _show_final_dialog(self): """ - In a non-interactive mode, we should show the user a success - confirmation. - """ - if not self.cliargs.non_interactive: - return + We should show the user a success confirmation. + In the case of non-interactive mode or if there is nothing to do, + we should show some feedback to the user. + """ if (not self.cliargs.skip and not self.cliargs.targets and self.retcode in (0, 100)): msg = "Qubes OS is up to date." From 4c824ec3a3e852354dfd5291a339b15c5e7ca2eb Mon Sep 17 00:00:00 2001 From: Piotr Bartman-Szwarc Date: Tue, 28 May 2024 18:37:45 +0200 Subject: [PATCH 07/21] updater: fix selection if the intro page is skipped --- qui/updater/intro_page.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/qui/updater/intro_page.py b/qui/updater/intro_page.py index 112d6cf7..8b264490 100644 --- a/qui/updater/intro_page.py +++ b/qui/updater/intro_page.py @@ -179,7 +179,7 @@ def select_rows_ignoring_conditions(self, cliargs, dom0): args = [a for a in dir(cliargs) if not a.startswith("_")] for arg in args: - if arg in ("dom0", "no_restart", "restart", "apply_to_sys", + if arg in ("dom0", "restart", "apply_to_sys", "apply_to_all", "no_apply", "max_concurrency", "log", "non_interactive"): continue @@ -195,10 +195,7 @@ def select_rows_ignoring_conditions(self, cliargs, dom0): if isinstance(value, (str, int)): cmd.append(str(value)) - to_update = set() - if cmd[2:]: - to_update = self._get_stale_qubes(cmd) - + to_update = self._get_stale_qubes(cmd) to_update = self._handle_cli_dom0(dom0, to_update, cliargs) for row in self.list_store: From 32bf2909828b721699cdf8f3c544dd0e5abf9b33 Mon Sep 17 00:00:00 2001 From: Piotr Bartman-Szwarc Date: Mon, 10 Jun 2024 14:46:37 +0200 Subject: [PATCH 08/21] updater: unify cli args with backend updater --- qui/updater/intro_page.py | 21 ++-- qui/updater/tests/test_intro_page.py | 46 ++++++--- qui/updater/updater.py | 137 +++++++++++++++++++-------- 3 files changed, 143 insertions(+), 61 deletions(-) diff --git a/qui/updater/intro_page.py b/qui/updater/intro_page.py index 8b264490..16343ba9 100644 --- a/qui/updater/intro_page.py +++ b/qui/updater/intro_page.py @@ -113,7 +113,7 @@ def refresh_update_list(self, update_if_stale): if not self.active: return - cmd = ['qubes-vm-update', '--dry-run', + cmd = ['qubes-vm-update', '--quiet', '--dry-run', '--update-if-stale', str(update_if_stale)] to_update = self._get_stale_qubes(cmd) @@ -175,13 +175,14 @@ def select_rows(self): in self.head_checkbox.allowed def select_rows_ignoring_conditions(self, cliargs, dom0): - cmd = ['qubes-vm-update', '--dry-run'] + cmd = ['qubes-vm-update', '--dry-run', '--quiet'] args = [a for a in dir(cliargs) if not a.startswith("_")] for arg in args: - if arg in ("dom0", "restart", "apply_to_sys", - "apply_to_all", "no_apply", "max_concurrency", - "log", "non_interactive"): + if arg in ("non_interactive", "non_default_select", + "dom0", + "restart", "apply_to_sys", "apply_to_all", "no_apply", + "max_concurrency", "log"): continue value = getattr(cliargs, arg) if value: @@ -192,10 +193,16 @@ def select_rows_ignoring_conditions(self, cliargs, dom0): continue value = ",".join(vms_without_dom0) cmd.append(f"--{arg.replace('_', '-')}") - if isinstance(value, (str, int)): + if not isinstance(value, bool): cmd.append(str(value)) - to_update = self._get_stale_qubes(cmd) + to_update = set() + non_default_select = [ + '--' + arg for arg in cliargs.non_default_select if arg != 'dom0'] + non_default = [a for a in cmd if a in non_default_select] + if non_default or cliargs.non_interactive: + to_update = self._get_stale_qubes(cmd) + to_update = self._handle_cli_dom0(dom0, to_update, cliargs) for row in self.list_store: diff --git a/qui/updater/tests/test_intro_page.py b/qui/updater/tests/test_intro_page.py index 207c3b44..be44c02c 100644 --- a/qui/updater/tests/test_intro_page.py +++ b/qui/updater/tests/test_intro_page.py @@ -176,51 +176,71 @@ def test_on_checkbox_toggled( # templates_and_standalones: mocked selection of templates and standalones # derived_qubes: mocked selection of derived qubes # expected_selection: gui should select what - "args, tmpls_and_stndas, derived_qubes, expected_selection", + "args, tmpls_and_stndas, derived_qubes, expected_selection, expected_args", ( # `qubes-update-gui --all` # Target all updatable VMs (AdminVM, TemplateVMs and StandaloneVMs) pytest.param( ('--all',), ",".join(_tmpls_and_stndas).encode(), - ",".join(_derived_qubes).encode(), _non_derived_qubes), + ",".join(_derived_qubes).encode(), _non_derived_qubes, ('--all',), + id="all"), # `qubes-update-gui --update-if-stale 10` # Target all TemplateVMs and StandaloneVMs with known updates or for # which last update check was more than <10> days ago. pytest.param( - ('--update-if-stale', '10'), b'fedora-36', b'', {'fedora-36'}), + ('--non-interactive', '--update-if-stale', '10'), + b'fedora-36', b'', {'fedora-36'}, ('--update-if-stale', '10'), + id="if-stale"), # `qubes-update-gui --targets dom0,fedora-36` # Comma separated list of VMs to target pytest.param( ('--targets', 'dom0,fedora-36'), b'fedora-36', - b'', {'dom0', 'fedora-36'}), + b'', {'dom0', 'fedora-36'}, ('--targets', 'fedora-36'), + id="targets"), # `qubes-update-gui --standalones` # Target all StandaloneVMs pytest.param( ('--standalones',), b'', - ",".join(_standalones).encode(), _standalones), + ",".join(_standalones).encode(), _standalones, ('--standalones',), + id="standalones"), # `qubes-update-gui --dom0` # Target dom0 - pytest.param(('--dom0',), b'', b'', {'dom0'}), + pytest.param(('--dom0',), b'', b'', {'dom0'}, None, id="dom0"), # `qubes-update-gui --dom0 --skip dom0` # Comma separated list of VMs to be skipped, # works with all other options. - pytest.param(('--dom0', '--skip', 'dom0'), b'', b'', set()), + pytest.param(('--dom0', '--skip', 'dom0'), b'', b'', set(), None, + id="skip all"), # `qubes-update-gui --skip dom0` - pytest.param(('--skip', 'dom0'), b'', b'', set()), + pytest.param(('--skip', 'dom0'), b'', b'', set(), None, + id="skip dom0"), + # `qubes-update-gui --skip garbage-name` + pytest.param(('--skip', 'garbage-name'), + ",".join(_tmpls_and_stndas).encode(), + ",".join(_derived_qubes).encode(), _tmpls_and_stndas, + ('--skip', 'garbage-name'), + id="skip non dom0"), # `qubes-update-gui --targets dom0 --skip dom0` # the same as `qubes-update-gui --dom0 --skip dom0` pytest.param( - ('--targets', 'dom0', '--skip', 'dom0'), b'', b'', set()), + ('--targets', 'dom0', '--skip', 'dom0'), b'', b'', set(), + None, id="skip all targets"), # `qubes-update-gui --templates dom0 --skip fedora-36,garbage-name` + # expected args are in alphabetical order pytest.param(('--templates', '--skip', 'fedora-36,garbage-name'), ",".join(_templates.difference({"fedora-36"})).encode(), b'', - _templates.difference({"fedora-36"})), + _templates.difference({"fedora-36"}), + ('--skip', 'fedora-36,garbage-name', '--templates'), + id="templates with skip"), + pytest.param(('--force-update',), b'', b'', set(), None, + id="force-update"), ), ) def test_select_rows_ignoring_conditions( mock_subprocess, args, tmpls_and_stndas, derived_qubes, expected_selection, + expected_args, real_builder, test_qapp, mock_next_button, mock_settings, mock_list_store ): @@ -250,8 +270,10 @@ def test_select_rows_ignoring_conditions( assert to_update == expected_selection - at_most_dom0_selected = not tmpls_and_stndas + derived_qubes - if at_most_dom0_selected: + if expected_args is None: mock_subprocess.assert_not_called() + else: + mock_subprocess.assert_called_with( + ['qubes-vm-update', '--dry-run', '--quiet', *expected_args]) diff --git a/qui/updater/updater.py b/qui/updater/updater.py index dda45641..cb9d5403 100644 --- a/qui/updater/updater.py +++ b/qui/updater/updater.py @@ -29,6 +29,10 @@ locale.textdomain('desktop-linux-manager') +class ArgumentError(Exception): + """Nonsense arguments""" + + class QubesUpdater(Gtk.Application): # pylint: disable=too-many-instance-attributes @@ -126,7 +130,7 @@ def perform_setup(self, *_args, **_kwargs): if self.cliargs.apply_to_all: overridden_apply_to_sys = True overridden_apply_to_other = True - elif self.cliargs.restart: + elif self.cliargs.apply_to_sys: overridden_apply_to_sys = True elif self.cliargs.no_apply: overridden_apply_to_sys = False @@ -182,6 +186,15 @@ def cell_data_func(_column, cell, model, it, data): return self.next_clicked(None, skip_intro=True) else: + # default update_if_stale -> do nothing + if self.cliargs.update_if_available: + self.intro_page.head_checkbox.state = ( + self.intro_page.head_checkbox.SAFE) + self.intro_page.select_rows() + elif self.cliargs.force_update: + self.intro_page.head_checkbox.state = ( + self.intro_page.head_checkbox.ALL) + self.intro_page.select_rows() self.log.info("Show intro page.") self.main_window.show_all() width = self.intro_page.vm_list.get_preferred_width().natural_width @@ -239,8 +252,16 @@ def _show_final_dialog(self): In the case of non-interactive mode or if there is nothing to do, we should show some feedback to the user. """ - if (not self.cliargs.skip and not self.cliargs.targets - and self.retcode in (0, 100)): + non_default_select = any( + [getattr(self.cliargs, arg) + for arg in self.cliargs.non_default_select if arg != 'all']) + if ( + ( # at least all vms with available updates was updated + (self.cliargs.all and not self.cliargs.skip) + or not non_default_select + ) + and self.retcode in (0, 100) + ): msg = "Qubes OS is up to date." elif self.retcode == 0: msg = "All selected qubes have been updated." @@ -301,63 +322,95 @@ def exit_updater(self, _emitter=None): def parse_args(args): parser = argparse.ArgumentParser() + default_update_if_stale = 7 parser.add_argument('--log', action='store', default='WARNING', help='Provide logging level. Values: DEBUG, INFO, ' 'WARNING (default), ERROR, CRITICAL') + parser.add_argument('--max-concurrency', action='store', help='Maximum number of VMs configured simultaneously ' '(default: number of cpus)', type=int) - restart_gr = parser.add_mutually_exclusive_group() - restart_gr.add_argument('--restart', '--apply-to-sys', '-r', - action='store_true', - help='Restart Service VMs whose template ' - 'has been updated.') - restart_gr.add_argument('--apply-to-all', '-R', - action='store_true', - help='Restart Service VMs and shutdown AppVMs ' - 'whose template has been updated.') - restart_gr.add_argument('--no-apply', action='store_true', - help='Do not restart/shutdown any AppVMs.') - - group = parser.add_mutually_exclusive_group() - group.add_argument('--targets', action='store', - help='Comma separated list of VMs to target') - group.add_argument('--all', action='store_true', - help='Target all updatable VMs (AdminVM, ' - 'TemplateVMs and StandaloneVMs)') - group.add_argument('--update-if-stale', action='store', - help='Target all TemplateVMs and StandaloneVMs with ' - 'known updates or for which last update check was ' - 'more than N days ago.', - type=int) - - parser.add_argument('--skip', action='store', - help='Comma separated list of VMs to be skipped, ' - 'works with all other options.', default="") - parser.add_argument('--templates', action='store_true', - help='Target all TemplatesVMs') - parser.add_argument('--standalones', action='store_true', - help='Target all StandaloneVMs') - parser.add_argument('--dom0', action='store_true', - help='Target dom0') - - parser.add_argument('--non-interactive', action='store_true', + + restart = parser.add_mutually_exclusive_group() + restart.add_argument( + '--apply-to-sys', '--restart', '-r', + action='store_true', + help='Restart not updated ServiceVMs whose template has been updated.') + restart.add_argument( + '--apply-to-all', '-R', action='store_true', + help='Restart not updated ServiceVMs and shutdown not updated AppVMs ' + 'whose template has been updated.') + restart.add_argument( + '--no-apply', action='store_true', + help='DEFAULT. Do not restart/shutdown any AppVMs.') + + update_state = parser.add_mutually_exclusive_group() + update_state.add_argument( + '--force-update', action='store_true', + help='Attempt to update all targeted VMs ' + 'even if no updates are available') + update_state.add_argument( + '--update-if-stale', action='store', + help='DEFAULT. ' + 'Attempt to update targeted VMs with known updates available ' + 'or for which last update check was more than N days ago. ' + '(default: %(default)d)', + type=int, default=default_update_if_stale) + update_state.add_argument( + '--update-if-available', action='store_true', + help='Update targeted VMs with known updates available.') + + parser.add_argument( + '--skip', action='store', + help='Comma separated list of VMs to be skipped, ' + 'works with all other options. ' + 'If present, skip manual selection of qubes to update.', + default="") + parser.add_argument( + '--targets', action='store', + help='Comma separated list of updatable VMs to target. ' + 'If present, skip manual selection of qubes to update.') + parser.add_argument( + '--templates', '-T', action='store_true', + help='Target all updatable TemplateVMs. ' + 'If present, skip manual selection of qubes to update.') + parser.add_argument( + '--standalones', '-S', action='store_true', + help='Target all updatable StandaloneVMs. ' + 'If present, skip manual selection of qubes to update.') + parser.add_argument( + '--all', action='store_true', + help='DEFAULT. Target AdminVM, TemplateVMs and StandaloneVMs.' + 'Use explicitly with "--targets" to include both. ' + 'If explicitly present, skip manual selection of qubes to update.') + parser.add_argument( + '--dom0', action='store_true', help='Target dom0. ' + 'If present, skip manual selection of qubes to update.') + + parser.add_argument('--non-interactive', '-n', action='store_true', help='Run the updater GUI in non-interactive mode. ' 'Interaction will be required in the event ' 'of an update error.') args = parser.parse_args(args) + args.non_default_select = { + 'skip', 'targets', 'templates', 'standalones', 'all', 'dom0'} + + if args.update_if_stale < 0: + raise ArgumentError("Wrong value for --update-if-stale") + if args.update_if_stale == default_update_if_stale: + args.update_if_stale = None + return args def skip_intro_if_args(args): - return args is not None and (args.templates or args.standalones or args.skip - or args.update_if_stale or args.all - or args.targets or args.dom0 - or args.non_interactive) + auto_select = [getattr(args, arg) for arg in args.non_default_select + ] + [args.non_interactive] + return any(auto_select) def main(args=None): From a67700037ae4b6c2c8336a60e048bb0e14d189f8 Mon Sep 17 00:00:00 2001 From: Piotr Bartman-Szwarc Date: Tue, 11 Jun 2024 12:05:05 +0200 Subject: [PATCH 09/21] updater: get default value of '--update-if-stale' from dom0 settings --- qui/updater/tests/test_intro_page.py | 2 +- qui/updater/tests/test_updater.py | 4 ++-- qui/updater/updater.py | 12 +++++++++--- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/qui/updater/tests/test_intro_page.py b/qui/updater/tests/test_intro_page.py index be44c02c..9f33f168 100644 --- a/qui/updater/tests/test_intro_page.py +++ b/qui/updater/tests/test_intro_page.py @@ -264,7 +264,7 @@ def test_select_rows_ignoring_conditions( result += b'Following qubes will be updated: ' + derived_qubes mock_subprocess.return_value = result - cliargs = parse_args(args) + cliargs = parse_args(args, test_qapp) sut.select_rows_ignoring_conditions(cliargs, test_qapp.domains['dom0']) to_update = {row.name for row in sut.list_store if row.selected} diff --git a/qui/updater/tests/test_updater.py b/qui/updater/tests/test_updater.py index 31352e97..510501ed 100644 --- a/qui/updater/tests/test_updater.py +++ b/qui/updater/tests/test_updater.py @@ -29,7 +29,7 @@ @patch('logging.getLogger') @patch('qui.updater.intro_page.IntroPage.populate_vm_list') def test_setup(populate_vm_list, _mock_logging, __mock_logging, test_qapp): - sut = QubesUpdater(test_qapp, parse_args(())) + sut = QubesUpdater(test_qapp, parse_args((), test_qapp)) sut.perform_setup() calls = [call(sut.qapp, sut.settings)] populate_vm_list.assert_has_calls(calls) @@ -53,7 +53,7 @@ def test_setup(populate_vm_list, _mock_logging, __mock_logging, test_qapp): ) def test_retcode(_populate_vm_list, _mock_logging, __mock_logging, update_results, ret_code, test_qapp): - sut = QubesUpdater(test_qapp, parse_args(())) + sut = QubesUpdater(test_qapp, parse_args((), test_qapp)) sut.perform_setup() sut.intro_page.get_vms_to_update = Mock() diff --git a/qui/updater/updater.py b/qui/updater/updater.py index cb9d5403..c3c8be8d 100644 --- a/qui/updater/updater.py +++ b/qui/updater/updater.py @@ -19,6 +19,7 @@ gi.require_version('Gtk', '3.0') # isort:skip from gi.repository import Gtk, Gdk, Gio # isort:skip from qubesadmin import Qubes +import qubesadmin.exc # using locale.gettext is necessary for Gtk.Builder translation support to work # in most cases, gettext is better, but it cannot handle Gtk.Builder/glade files @@ -320,9 +321,14 @@ def exit_updater(self, _emitter=None): self.release() -def parse_args(args): +def parse_args(args, app): parser = argparse.ArgumentParser() - default_update_if_stale = 7 + try: + default_update_if_stale = int(app.domains["dom0"].features.get( + "qubes-vm-update-update-if-stale", Settings.DEFAULT_UPDATE_IF_STALE) + ) + except qubesadmin.exc.QubesDaemonAccessError: + default_update_if_stale = Settings.DEFAULT_UPDATE_IF_STALE parser.add_argument('--log', action='store', default='WARNING', help='Provide logging level. Values: DEBUG, INFO, ' @@ -414,8 +420,8 @@ def skip_intro_if_args(args): def main(args=None): - cliargs = parse_args(args) qapp = Qubes() + cliargs = parse_args(args, qapp) app = QubesUpdater(qapp, cliargs) app.run() sys.exit(app.retcode) From 644299ae46b236ccf41bf354b6d8d1a68ac9726a Mon Sep 17 00:00:00 2001 From: Piotr Bartman-Szwarc Date: Tue, 11 Jun 2024 12:29:59 +0200 Subject: [PATCH 10/21] updater: return 130 if user cancels updating --- qui/updater/progress_page.py | 13 ++++++++----- qui/updater/tests/test_progress_page.py | 9 +++++---- qui/updater/tests/test_updater.py | 22 +++++++++++++--------- qui/updater/updater.py | 12 +++++++----- 4 files changed, 33 insertions(+), 23 deletions(-) diff --git a/qui/updater/progress_page.py b/qui/updater/progress_page.py index 7697f658..12562ccb 100644 --- a/qui/updater/progress_page.py +++ b/qui/updater/progress_page.py @@ -461,16 +461,19 @@ def get_update_summary(self): 2. number of vms that tried to update but no update was found, 3. vms that update was canceled before starting. """ - vm_updated_num = len( + updated = len( [row for row in self.vms_to_update if row.status == UpdateStatus.Success]) - vm_no_updates_num = len( + no_updates = len( [row for row in self.vms_to_update if row.status == UpdateStatus.NoUpdatesFound]) - vm_failed_num = len( + failed = len( [row for row in self.vms_to_update - if row.status in (UpdateStatus.Error, UpdateStatus.Cancelled)]) - return vm_updated_num, vm_no_updates_num, vm_failed_num + if row.status == UpdateStatus.Error]) + cancelled = len( + [row for row in self.vms_to_update + if row.status == UpdateStatus.Cancelled]) + return updated, no_updates, failed, cancelled class Ticker: diff --git a/qui/updater/tests/test_progress_page.py b/qui/updater/tests/test_progress_page.py index e5bca68b..d313aa1d 100644 --- a/qui/updater/tests/test_progress_page.py +++ b/qui/updater/tests/test_progress_page.py @@ -258,11 +258,12 @@ def test_get_update_summary( sut.vms_to_update = updatable_vms_list - vm_updated_num, vm_no_updates_num, vm_failed_num = sut.get_update_summary() + updated, no_updates, failed, cancelled = sut.get_update_summary() - assert vm_updated_num == 1 - assert vm_no_updates_num == 1 - assert vm_failed_num == 2 + assert updated == 1 + assert no_updates == 1 + assert failed == 1 + assert cancelled == 1 mock_callback.assert_not_called() diff --git a/qui/updater/tests/test_updater.py b/qui/updater/tests/test_updater.py index 510501ed..54d0d37a 100644 --- a/qui/updater/tests/test_updater.py +++ b/qui/updater/tests/test_updater.py @@ -41,14 +41,16 @@ def test_setup(populate_vm_list, _mock_logging, __mock_logging, test_qapp): @pytest.mark.parametrize( "update_results, ret_code", ( - pytest.param((0, 0, 0), 100, id="nothing to do"), - pytest.param((0, 0, 1), 1, id="failed"), - pytest.param((0, 1, 0), 100, id="no updates"), - pytest.param((0, 1, 1), 1, id="no updates + failed"), - pytest.param((1, 0, 0), 0, id="success"), - pytest.param((1, 0, 1), 1, id="success + failed"), - pytest.param((1, 1, 0), 0, id="success + no updated"), - pytest.param((1, 1, 1), 1, id="all"), + pytest.param((0, 0, 0, 0), 100, id="nothing to do"), + pytest.param((0, 0, 1, 0), 1, id="failed"), + pytest.param((0, 0, 0, 1), 130, id="cancelled"), + pytest.param((0, 0, 1, 1), 130, id="failed + cancelled"), + pytest.param((0, 1, 0, 0), 100, id="no updates"), + pytest.param((0, 1, 1, 0), 1, id="no updates + failed"), + pytest.param((1, 0, 0, 0), 0, id="success"), + pytest.param((1, 0, 1, 0), 1, id="success + failed"), + pytest.param((1, 1, 0, 0), 0, id="success + no updated"), + pytest.param((1, 1, 1, 1), 130, id="all"), ) ) def test_retcode(_populate_vm_list, _mock_logging, __mock_logging, @@ -86,4 +88,6 @@ def populate(**_kwargs): sut.summary_page.populate_restart_list.assert_called_once_with( restart=True, vm_updated=vms_to_update, settings=sut.settings) assert sut.retcode == ret_code - sut.summary_page.show.assert_called_once_with(*update_results) + expected_summary = (update_results[0], update_results[1], + update_results[2] + update_results[3]) + sut.summary_page.show.assert_called_once_with(*expected_summary) diff --git a/qui/updater/updater.py b/qui/updater/updater.py index c3c8be8d..534eff2c 100644 --- a/qui/updater/updater.py +++ b/qui/updater/updater.py @@ -219,15 +219,17 @@ def next_clicked(self, _emitter, skip_intro=False): vm_updated=self.progress_page.vms_to_update, settings=self.settings ) - updated, no_updates, failed = ( + updated, no_updates, failed, cancelled = ( self.progress_page.get_update_summary()) - if failed: - self.retcode = 1 - if updated == 0 and failed == 0: + if updated == 0: # no updates self.retcode = 100 + if failed: + self.retcode = 1 + if cancelled: + self.retcode = 130 if failed or not self.cliargs.non_interactive: - self.summary_page.show(updated, no_updates, failed) + self.summary_page.show(updated, no_updates, failed + cancelled) else: self._restart_phase() if self.cliargs.non_interactive: From 462ce5366eb5eb399ec9e022db0be29a1fe0a0e5 Mon Sep 17 00:00:00 2001 From: Piotr Bartman-Szwarc Date: Tue, 11 Jun 2024 12:30:37 +0200 Subject: [PATCH 11/21] updater: make test reproducible --- qui/updater/intro_page.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qui/updater/intro_page.py b/qui/updater/intro_page.py index 16343ba9..3ddf4326 100644 --- a/qui/updater/intro_page.py +++ b/qui/updater/intro_page.py @@ -191,7 +191,7 @@ def select_rows_ignoring_conditions(self, cliargs, dom0): vms_without_dom0 = vms.difference({"dom0"}) if not vms_without_dom0: continue - value = ",".join(vms_without_dom0) + value = ",".join(sorted(vms_without_dom0)) cmd.append(f"--{arg.replace('_', '-')}") if not isinstance(value, bool): cmd.append(str(value)) From f92e565a86012d4056d6f741f326dedd00818b41 Mon Sep 17 00:00:00 2001 From: Piotr Bartman-Szwarc Date: Tue, 11 Jun 2024 12:39:06 +0200 Subject: [PATCH 12/21] updater: set progress as 100 if update fails or was cancelled --- qui/updater/progress_page.py | 1 + 1 file changed, 1 insertion(+) diff --git a/qui/updater/progress_page.py b/qui/updater/progress_page.py index 12562ccb..4c4b63c2 100644 --- a/qui/updater/progress_page.py +++ b/qui/updater/progress_page.py @@ -381,6 +381,7 @@ def handle_err_line(self, untrusted_line, rows): try: if status == "done": update_status = UpdateStatus.from_name(info) + GLib.idle_add(rows[name].set_update_progress, 100) GLib.idle_add(rows[name].set_status, update_status) except KeyError: return From 9f59b616db3a348475b6e299fd4f28ffbfa4eafc Mon Sep 17 00:00:00 2001 From: Piotr Bartman-Szwarc Date: Tue, 11 Jun 2024 15:03:07 +0200 Subject: [PATCH 13/21] updater: apply the same selection criteria to dom0 --- qui/updater/intro_page.py | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/qui/updater/intro_page.py b/qui/updater/intro_page.py index 3ddf4326..135d41f0 100644 --- a/qui/updater/intro_page.py +++ b/qui/updater/intro_page.py @@ -229,12 +229,16 @@ def _get_stale_qubes(self, cmd): @staticmethod def _handle_cli_dom0(dom0, to_update, cliargs): - if not cliargs.targets and not cliargs.all: - if bool(dom0.features.get( - 'updates-available', False)): - to_update.add('dom0') - if cliargs.dom0 or cliargs.all: - to_update.add("dom0") + default_select = not any( + [getattr(cliargs, arg) + for arg in cliargs.non_default_select if arg != 'all']) + if (default_select or cliargs.all or cliargs.dom0) and ( + cliargs.force_update + or bool(dom0.features.get('updates-available', False)) + or is_stale(dom0, cliargs.update_if_stale) + ): + to_update.add('dom0') + if cliargs.targets and "dom0" in cliargs.targets.split(","): to_update.add("dom0") if cliargs.skip and "dom0" in cliargs.skip.split(","): @@ -242,6 +246,20 @@ def _handle_cli_dom0(dom0, to_update, cliargs): return to_update +def is_stale(vm, expiration_period): + if expiration_period is None: + return False + today = datetime.today() + last_update_str = vm.features.get( + 'last-updates-check', + datetime.fromtimestamp(0).strftime('%Y-%m-%d %H:%M:%S') + ) + last_update = datetime.fromisoformat(last_update_str) + if (today - last_update).days > expiration_period: + return True + return False + + class UpdateRowWrapper(RowWrapper): COLUMN_NUM = 9 _SELECTION = 1 From 91bd5c4ea9a4e7cbf304ea26217e736391d6fc6a Mon Sep 17 00:00:00 2001 From: Piotr Bartman-Szwarc Date: Tue, 11 Jun 2024 17:52:29 +0200 Subject: [PATCH 14/21] updater: make pylint happy --- qui/updater/intro_page.py | 4 ++-- qui/updater/updater.py | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/qui/updater/intro_page.py b/qui/updater/intro_page.py index 135d41f0..7035025d 100644 --- a/qui/updater/intro_page.py +++ b/qui/updater/intro_page.py @@ -230,8 +230,8 @@ def _get_stale_qubes(self, cmd): @staticmethod def _handle_cli_dom0(dom0, to_update, cliargs): default_select = not any( - [getattr(cliargs, arg) - for arg in cliargs.non_default_select if arg != 'all']) + (getattr(cliargs, arg) + for arg in cliargs.non_default_select if arg != 'all')) if (default_select or cliargs.all or cliargs.dom0) and ( cliargs.force_update or bool(dom0.features.get('updates-available', False)) diff --git a/qui/updater/updater.py b/qui/updater/updater.py index 534eff2c..97e2ceab 100644 --- a/qui/updater/updater.py +++ b/qui/updater/updater.py @@ -256,8 +256,9 @@ def _show_final_dialog(self): we should show some feedback to the user. """ non_default_select = any( - [getattr(self.cliargs, arg) - for arg in self.cliargs.non_default_select if arg != 'all']) + (getattr(self.cliargs, arg) + for arg in self.cliargs.non_default_select if arg != 'all')) + msg = "Nothing to do." if ( ( # at least all vms with available updates was updated (self.cliargs.all and not self.cliargs.skip) From ceec6a0617273ef8bb53c7e34bc403181effeb6d Mon Sep 17 00:00:00 2001 From: Piotr Bartman-Szwarc Date: Tue, 11 Jun 2024 18:25:20 +0200 Subject: [PATCH 15/21] updater: updaate dom0 tests --- qui/updater/intro_page.py | 14 +++++++++----- qui/updater/tests/test_intro_page.py | 12 ++++++++---- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/qui/updater/intro_page.py b/qui/updater/intro_page.py index 7035025d..173f083f 100644 --- a/qui/updater/intro_page.py +++ b/qui/updater/intro_page.py @@ -232,11 +232,15 @@ def _handle_cli_dom0(dom0, to_update, cliargs): default_select = not any( (getattr(cliargs, arg) for arg in cliargs.non_default_select if arg != 'all')) - if (default_select or cliargs.all or cliargs.dom0) and ( - cliargs.force_update - or bool(dom0.features.get('updates-available', False)) - or is_stale(dom0, cliargs.update_if_stale) - ): + if (( + default_select and cliargs.non_interactive + or cliargs.all + or cliargs.dom0 + ) and ( + cliargs.force_update + or bool(dom0.features.get('updates-available', False)) + or is_stale(dom0, cliargs.update_if_stale) + )): to_update.add('dom0') if cliargs.targets and "dom0" in cliargs.targets.split(","): diff --git a/qui/updater/tests/test_intro_page.py b/qui/updater/tests/test_intro_page.py index 9f33f168..3f1f6755 100644 --- a/qui/updater/tests/test_intro_page.py +++ b/qui/updater/tests/test_intro_page.py @@ -181,15 +181,18 @@ def test_on_checkbox_toggled( # `qubes-update-gui --all` # Target all updatable VMs (AdminVM, TemplateVMs and StandaloneVMs) pytest.param( - ('--all',), ",".join(_tmpls_and_stndas).encode(), - ",".join(_derived_qubes).encode(), _non_derived_qubes, ('--all',), + ('--all', '--force-update'), + ",".join(_tmpls_and_stndas).encode(), + ",".join(_derived_qubes).encode(), _non_derived_qubes, + ('--all', '--force-update'), id="all"), # `qubes-update-gui --update-if-stale 10` # Target all TemplateVMs and StandaloneVMs with known updates or for # which last update check was more than <10> days ago. pytest.param( ('--non-interactive', '--update-if-stale', '10'), - b'fedora-36', b'', {'fedora-36'}, ('--update-if-stale', '10'), + b'fedora-36', b'', {'fedora-36', 'dom0'}, + ('--update-if-stale', '10'), id="if-stale"), # `qubes-update-gui --targets dom0,fedora-36` # Comma separated list of VMs to target @@ -205,7 +208,8 @@ def test_on_checkbox_toggled( id="standalones"), # `qubes-update-gui --dom0` # Target dom0 - pytest.param(('--dom0',), b'', b'', {'dom0'}, None, id="dom0"), + pytest.param(('--dom0', '--force-update'), b'', b'', {'dom0'}, + None, id="dom0"), # `qubes-update-gui --dom0 --skip dom0` # Comma separated list of VMs to be skipped, # works with all other options. From 713df4937120f9ece5723dbffa4527261812a294 Mon Sep 17 00:00:00 2001 From: Piotr Bartman-Szwarc Date: Fri, 14 Jun 2024 19:54:55 +0200 Subject: [PATCH 16/21] updater: show only one confirmation to the user --- qui/updater/summary_page.py | 32 +++++++++------ qui/updater/tests/test_summary_page.py | 6 +-- qui/updater/tests/test_updater.py | 57 ++++++++++++++++++++++++++ qui/updater/updater.py | 24 ++++++----- 4 files changed, 94 insertions(+), 25 deletions(-) diff --git a/qui/updater/summary_page.py b/qui/updater/summary_page.py index cd60b7b0..504e6b3e 100644 --- a/qui/updater/summary_page.py +++ b/qui/updater/summary_page.py @@ -237,7 +237,7 @@ def select_rows(self): AppVMType.EXCLUDED in self.head_checkbox.allowed ) - def restart_selected_vms(self): + def restart_selected_vms(self, show_only_error: bool): self.log.debug("Start restarting") self.restart_thread = threading.Thread( target=self.perform_restart @@ -272,7 +272,7 @@ def restart_selected_vms(self): spinner.stop() dialog.destroy() self.log.debug("Hide restart dialog") - self._show_status_dialog() + self._show_status_dialog(show_only_error) def perform_restart(self): @@ -298,10 +298,8 @@ def perform_restart(self): self.restart_vms(to_restart) self.shutdown_domains(to_shutdown) - if not self.err: + if self.status is RestartStatus.NONE: self.status = RestartStatus.OK - else: - self.status = RestartStatus.ERROR def shutdown_domains(self, to_shutdown): """ @@ -317,6 +315,7 @@ def shutdown_domains(self, to_shutdown): self.err += vm.name + " cannot shutdown: " + str(err) + '\n' self.log.error("Cannot shutdown %s because %s", vm.name, str(err)) + self.status = RestartStatus.ERROR_TMPL_DOWN asyncio.run(wait_for_domain_shutdown(wait_for)) @@ -336,9 +335,10 @@ def restart_vms(self, to_restart): except qubesadmin.exc.QubesVMError as err: self.err += vm.name + " cannot start: " + str(err) + '\n' self.log.error("Cannot start %s because %s", vm.name, str(err)) + self.status = RestartStatus.ERROR_APP_DOWN - def _show_status_dialog(self): - if self.status == RestartStatus.OK: + def _show_status_dialog(self, show_only_error: bool): + if self.status == RestartStatus.OK and not show_only_error: show_dialog_with_icon( None, l("Success"), @@ -346,12 +346,13 @@ def _show_status_dialog(self): buttons=RESPONSES_OK, icon_name="qubes-check-yes" ) - elif self.status == RestartStatus.ERROR: + elif self.status.is_error(): show_error(None, "Failure", l("During restarting following errors occurs: ") + self.err ) self.log.error("Restart error: %s", self.err) + self.status = RestartStatus.ERROR_APP_START class RestartRowWrapper(RowWrapper): @@ -421,10 +422,17 @@ class AppVMType: class RestartStatus(Enum): - ERROR = 0 - OK = 1 - NOTHING_TO_DO = 2 - NONE = 3 + OK = 0 + NOTHING_TO_DO = 100 + NONE = -1 + ERROR_TMPL_DOWN = 11 + ERROR_APP_DOWN = 12 + ERROR_APP_START = 13 + + def is_error(self: 'RestartStatus') -> bool: + return self in [ + RestartStatus.ERROR_TMPL_DOWN, RestartStatus.ERROR_APP_DOWN, + RestartStatus.ERROR_APP_START] class RestartHeaderCheckbox(HeaderCheckbox): diff --git a/qui/updater/tests/test_summary_page.py b/qui/updater/tests/test_summary_page.py index 25de45d6..58573f83 100644 --- a/qui/updater/tests/test_summary_page.py +++ b/qui/updater/tests/test_summary_page.py @@ -212,7 +212,7 @@ def test_populate_restart_list( "alive_requests_max, status", (pytest.param(3, RestartStatus.OK), pytest.param(0, RestartStatus.OK), - pytest.param(1, RestartStatus.ERROR), + pytest.param(1, RestartStatus.ERROR_TMPL_DOWN), pytest.param(1, RestartStatus.NOTHING_TO_DO), ), ) @@ -259,7 +259,7 @@ def set_deletable(self, deletable): sut.status = status # ACT - sut.restart_selected_vms() + sut.restart_selected_vms(show_only_error=False) # ASSERT @@ -288,7 +288,7 @@ def set_deletable(self, deletable): calls = [call(None, "Success", "All qubes were restarted/shutdown successfully.", RESPONSES_OK, icon)] - if status == RestartStatus.ERROR: + if status.is_error(): calls = [call(None, "Failure", "During restarting following errors occurs: " + sut.err, RESPONSES_OK, icon)] diff --git a/qui/updater/tests/test_updater.py b/qui/updater/tests/test_updater.py index 54d0d37a..aea2a323 100644 --- a/qui/updater/tests/test_updater.py +++ b/qui/updater/tests/test_updater.py @@ -23,6 +23,8 @@ import pytest from qui.updater.updater import QubesUpdater, parse_args +from qui.updater.summary_page import SummaryPage, RestartStatus +from qubes_config.widgets import gtk_utils @patch('logging.FileHandler') @@ -91,3 +93,58 @@ def populate(**_kwargs): expected_summary = (update_results[0], update_results[1], update_results[2] + update_results[3]) sut.summary_page.show.assert_called_once_with(*expected_summary) + + +@patch('threading.Thread') +@patch('qui.updater.updater.show_dialog_with_icon') +@patch('qui.updater.summary_page.show_dialog_with_icon') +@patch('logging.FileHandler') +@patch('logging.getLogger') +@patch('qui.updater.intro_page.IntroPage.populate_vm_list') +def test_dialog(_populate_vm_list, _mock_logging, __mock_logging, + dialog, dialog2, thread, test_qapp, monkeypatch): + monkeypatch.setattr(SummaryPage, "perform_restart", lambda *_: None) + sut = QubesUpdater(test_qapp, parse_args((), test_qapp)) + sut.perform_setup() + + sut.cliargs.non_interactive = True + + sut.intro_page.get_vms_to_update = Mock() + vms_to_update = Mock() + sut.intro_page.get_vms_to_update.return_value = vms_to_update + + def set_vms(_vms_to_update, _settings): + sut.progress_page.vms_to_update = _vms_to_update + sut.progress_page.init_update = Mock(side_effect=set_vms) + + sut.next_clicked(None) + + assert not sut.intro_page.active + assert sut.progress_page.is_visible + sut.progress_page.init_update.assert_called_once_with( + vms_to_update, sut.settings) + + # set sut.summary_page.is_populated = False + sut.summary_page.list_store = None + def populate(**_kwargs): + sut.summary_page.list_store = [] + sut.summary_page.populate_restart_list = Mock(side_effect=populate) + sut.progress_page.get_update_summary = Mock() + sut.progress_page.get_update_summary.return_value = (1, 0, 0, 0) + sut.summary_page.show = Mock() + sut.summary_page.show.return_value = None + + def ok(**_kwargs): + sut.summary_page.status = RestartStatus.OK + t = Mock() + t.is_alive = Mock(return_value=False) + return t + thread.side_effect = ok + + sut.summary_page.status = RestartStatus.OK + sut.next_clicked(None) + + dialog2.assert_has_calls(calls=[call( + None, "Success", "Qubes OS is up to date.", + buttons=gtk_utils.RESPONSES_OK, icon_name="qubes-check-yes")]) + dialog.assert_not_called() diff --git a/qui/updater/updater.py b/qui/updater/updater.py index 97e2ceab..a7e158cc 100644 --- a/qui/updater/updater.py +++ b/qui/updater/updater.py @@ -70,7 +70,7 @@ def do_activate(self, *_args, **_kwargs): else: self.log.debug("Secondary activation") if self.do_nothing: - self._show_final_dialog() + self._show_success_dialog() self.window_close() else: self.main_window.present() @@ -228,27 +228,31 @@ def next_clicked(self, _emitter, skip_intro=False): self.retcode = 1 if cancelled: self.retcode = 130 - if failed or not self.cliargs.non_interactive: + if failed or cancelled or not self.cliargs.non_interactive: self.summary_page.show(updated, no_updates, failed + cancelled) else: - self._restart_phase() - if self.cliargs.non_interactive: - self._show_final_dialog() + # at this point retcode is in (0, 100) + self._restart_phase( + show_only_error=self.cliargs.non_interactive) + # at thi point retcode is in (0, 100) + # or an error message have been already shown + if self.cliargs.non_interactive and self.retcode in (0, 100): + self._show_success_dialog() elif self.summary_page.is_visible: self._restart_phase() - def _restart_phase(self): + def _restart_phase(self, show_only_error: bool = True): self.main_window.hide() self.log.debug("Hide main window") # ensuring that main_window will be hidden while Gtk.events_pending(): Gtk.main_iteration() - self.summary_page.restart_selected_vms() - if self.summary_page.status == RestartStatus.ERROR: - self.retcode = 2 + self.summary_page.restart_selected_vms(show_only_error) + if self.summary_page.status.is_error(): + self.retcode = self.summary_page.status.value self.exit_updater() - def _show_final_dialog(self): + def _show_success_dialog(self): """ We should show the user a success confirmation. From 73174ddea08ac7e886ccd4566f7367431aa2b47f Mon Sep 17 00:00:00 2001 From: Piotr Bartman-Szwarc Date: Mon, 17 Jun 2024 15:21:28 +0200 Subject: [PATCH 17/21] updater: fix force updating --- qui/updater/progress_page.py | 1 + qui/updater/tests/test_progress_page.py | 1 + 2 files changed, 2 insertions(+) diff --git a/qui/updater/progress_page.py b/qui/updater/progress_page.py index 4c4b63c2..914d2986 100644 --- a/qui/updater/progress_page.py +++ b/qui/updater/progress_page.py @@ -330,6 +330,7 @@ def do_update_templates( ['qubes-vm-update', '--show-output', '--just-print-progress', + '--force-update', *args, '--targets', targets], stderr=subprocess.PIPE, stdout=subprocess.PIPE) diff --git a/qui/updater/tests/test_progress_page.py b/qui/updater/tests/test_progress_page.py index d313aa1d..6b151101 100644 --- a/qui/updater/tests/test_progress_page.py +++ b/qui/updater/tests/test_progress_page.py @@ -233,6 +233,7 @@ def poll(self): ['qubes-vm-update', '--show-output', '--just-print-progress', + '--force-update', '--targets', 'fedora-35,fedora-36,test-standalone'], stderr=subprocess.PIPE, stdout=subprocess.PIPE)] From 17111064c37bf7d9e45a8d3384456d7c1fce3fed Mon Sep 17 00:00:00 2001 From: Piotr Bartman-Szwarc Date: Mon, 17 Jun 2024 18:22:34 +0200 Subject: [PATCH 18/21] updater: make pylint happy --- qui/updater/updater.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qui/updater/updater.py b/qui/updater/updater.py index a7e158cc..c2f59f73 100644 --- a/qui/updater/updater.py +++ b/qui/updater/updater.py @@ -13,7 +13,7 @@ show_dialog_with_icon, RESPONSES_OK from qui.updater.progress_page import ProgressPage from qui.updater.updater_settings import Settings, OverriddenSettings -from qui.updater.summary_page import SummaryPage, RestartStatus +from qui.updater.summary_page import SummaryPage from qui.updater.intro_page import IntroPage gi.require_version('Gtk', '3.0') # isort:skip From 5abe6256365ec59759fe9abcd770cb5fc596b1a9 Mon Sep 17 00:00:00 2001 From: Piotr Bartman-Szwarc Date: Fri, 21 Jun 2024 13:42:11 +0200 Subject: [PATCH 19/21] updater: test is_stale(dom0) --- qui/updater/tests/conftest.py | 4 ++++ qui/updater/tests/test_intro_page.py | 24 +++++++++++++++++------- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/qui/updater/tests/conftest.py b/qui/updater/tests/conftest.py index c4e941ed..2025881f 100644 --- a/qui/updater/tests/conftest.py +++ b/qui/updater/tests/conftest.py @@ -40,6 +40,10 @@ @pytest.fixture def test_qapp(): + return test_qapp_impl() + + +def test_qapp_impl(): """Test QubesApp""" qapp = QubesTest() qapp._local_name = 'dom0' # pylint: disable=protected-access diff --git a/qui/updater/tests/test_intro_page.py b/qui/updater/tests/test_intro_page.py index 3f1f6755..93559c8c 100644 --- a/qui/updater/tests/test_intro_page.py +++ b/qui/updater/tests/test_intro_page.py @@ -24,7 +24,7 @@ from unittest.mock import patch from unittest.mock import Mock -from qubes_config.tests.conftest import test_qapp_impl +from qui.updater.tests.conftest import test_qapp_impl from qui.updater.intro_page import IntroPage, UpdateRowWrapper, UpdatesAvailable from qui.updater.updater import parse_args from qui.updater.utils import ListWrapper, HeaderCheckbox @@ -159,11 +159,10 @@ def test_on_checkbox_toggled( assert not sut.checkbox_column_button.get_active() -_domains = {vm.name for vm in test_qapp_impl().domains} -_templates = {vm.name for vm in test_qapp_impl().domains - if vm.klass == "TemplateVM"} -_standalones = {vm.name for vm in test_qapp_impl().domains - if vm.klass == "StandaloneVM"} +doms = test_qapp_impl().domains +_domains = {vm.name for vm in doms} +_templates = {vm.name for vm in doms if vm.klass == "TemplateVM"} +_standalones = {vm.name for vm in doms if vm.klass == "StandaloneVM"} _tmpls_and_stndas = _templates.union(_standalones) _non_derived_qubes = {"dom0"}.union(_tmpls_and_stndas) _derived_qubes = _domains.difference(_non_derived_qubes) @@ -193,7 +192,12 @@ def test_on_checkbox_toggled( ('--non-interactive', '--update-if-stale', '10'), b'fedora-36', b'', {'fedora-36', 'dom0'}, ('--update-if-stale', '10'), - id="if-stale"), + id="if-stale with dom0"), + pytest.param( + ('--non-interactive', '--update-if-stale', '10'), + b'fedora-36', b'', {'fedora-36'}, + ('--update-if-stale', '10'), + id="if-stale without dom0"), # `qubes-update-gui --targets dom0,fedora-36` # Comma separated list of VMs to target pytest.param( @@ -268,6 +272,12 @@ def test_select_rows_ignoring_conditions( result += b'Following qubes will be updated: ' + derived_qubes mock_subprocess.return_value = result + if (expected_args == ('--update-if-stale', '10') + and expected_selection == {'fedora-36'}): + test_qapp.expected_calls[ + ('dom0', 'admin.vm.feature.Get', 'last-updates-check', None)] = \ + b'0\x00' + b'3020-01-01 00:00:00' + cliargs = parse_args(args, test_qapp) sut.select_rows_ignoring_conditions(cliargs, test_qapp.domains['dom0']) to_update = {row.name for row in sut.list_store if row.selected} From ff9900e8e23941ac5f7b95a005b855aafc65a472 Mon Sep 17 00:00:00 2001 From: Piotr Bartman-Szwarc Date: Fri, 21 Jun 2024 15:06:29 +0200 Subject: [PATCH 20/21] updater: test apply and update flags --- qui/updater/tests/test_updater.py | 72 +++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/qui/updater/tests/test_updater.py b/qui/updater/tests/test_updater.py index aea2a323..7aa930aa 100644 --- a/qui/updater/tests/test_updater.py +++ b/qui/updater/tests/test_updater.py @@ -37,6 +37,78 @@ def test_setup(populate_vm_list, _mock_logging, __mock_logging, test_qapp): populate_vm_list.assert_has_calls(calls) +@patch('logging.FileHandler') +@patch('logging.getLogger') +@patch('subprocess.check_output') +@patch('qui.updater.intro_page.IntroPage.select_rows_ignoring_conditions') +@patch('qui.updater.intro_page.IntroPage.get_vms_to_update') +def test_setup_non_interactive_nothing_to_do( + get_vms, select, subproc, _mock_logging, __mock_logging, test_qapp): + sut = QubesUpdater(test_qapp, parse_args(('-n',), test_qapp)) + subproc.return_value = b'' + get_vms.return_value = () + sut.perform_setup() + select.assert_called_once() + get_vms.assert_called_once() + + +@patch('logging.FileHandler') +@patch('logging.getLogger') +@patch('qui.updater.intro_page.IntroPage.populate_vm_list') +@patch('qui.updater.intro_page.IntroPage.select_rows') +def test_setup_update_if_available( + select, populate_vm_list, _mock_logging, __mock_logging, test_qapp): + sut = QubesUpdater( + test_qapp, parse_args(('--update-if-available',), test_qapp)) + sut.perform_setup() + calls = [call(sut.qapp, sut.settings)] + populate_vm_list.assert_has_calls(calls) + select.assert_called_once() + assert (sut.intro_page.head_checkbox.state == + sut.intro_page.head_checkbox.SAFE) + + +@patch('logging.FileHandler') +@patch('logging.getLogger') +@patch('qui.updater.intro_page.IntroPage.populate_vm_list') +@patch('qui.updater.intro_page.IntroPage.select_rows') +def test_setup_force_update( + select, populate_vm_list, _mock_logging, __mock_logging, test_qapp): + sut = QubesUpdater( + test_qapp, parse_args(('--force-update',), test_qapp)) + sut.perform_setup() + calls = [call(sut.qapp, sut.settings)] + populate_vm_list.assert_has_calls(calls) + select.assert_called_once() + assert (sut.intro_page.head_checkbox.state == + sut.intro_page.head_checkbox.ALL) + + +@patch('logging.FileHandler') +@patch('logging.getLogger') +@patch('qui.updater.intro_page.IntroPage.populate_vm_list') +@patch('qui.updater.intro_page.IntroPage.select_rows') +@patch('qui.updater.updater_settings.get_boolean_feature') +@pytest.mark.parametrize( + "args, sys, non_sys", + ( + pytest.param(('--apply-to-all',), True, True, id="all"), + pytest.param(('--apply-to-sys',), True, None, id="sys"), + pytest.param(('--no-apply',), False, False, id="none"), + ) +) +def test_setup_apply( + get_feature, __select, populate_vm_list, _mock_logging, __mock_logging, test_qapp, args, sys, non_sys): + sut = QubesUpdater( + test_qapp, parse_args(args, test_qapp)) + sut.perform_setup() + calls = [call(sut.qapp, sut.settings)] + populate_vm_list.assert_has_calls(calls) + assert sut.settings.restart_service_vms == sys + assert (non_sys is not None and sut.settings.restart_other_vms == non_sys + or sut.settings.overrides.apply_to_other is None) + + @patch('logging.FileHandler') @patch('logging.getLogger') @patch('qui.updater.intro_page.IntroPage.populate_vm_list') From 15dc1f19eac1acf6bc25e84292a8b30fe4a30f15 Mon Sep 17 00:00:00 2001 From: Piotr Bartman-Szwarc Date: Wed, 26 Jun 2024 17:54:19 +0200 Subject: [PATCH 21/21] updater: add --signal-no-updates flag --- qui/updater/updater.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/qui/updater/updater.py b/qui/updater/updater.py index c2f59f73..383dfb36 100644 --- a/qui/updater/updater.py +++ b/qui/updater/updater.py @@ -345,6 +345,10 @@ def parse_args(args, app): help='Maximum number of VMs configured simultaneously ' '(default: number of cpus)', type=int) + parser.add_argument( + '--signal-no-updates', action='store_true', + help='Return exit code 100 instread of 0 ' + 'if there is no updates available.') restart = parser.add_mutually_exclusive_group() restart.add_argument( @@ -431,6 +435,8 @@ def main(args=None): cliargs = parse_args(args, qapp) app = QubesUpdater(qapp, cliargs) app.run() + if app.retcode == 100 and not app.cliargs.signal_no_updates: + app.retcode = 0 sys.exit(app.retcode)