updated launcher to update each templateVM only once per run

launcher/sdw_updater_gui/Updater.py

@@ -35,7 +35,7 @@
 # The are the TemplateVMs that require full patch level at boot in order to start the client,
 # as well as their associated TemplateVMs.
 # In the future, we could use qvm-prefs to extract this information.
-current_templates = {
+current_vms = {
     "fedora": "fedora-31",
     "sd-viewer": "sd-large-buster-template",
     "sd-app": "sd-small-buster-template",
@@ -46,6 +46,8 @@
     "sd-gpg": "sd-small-buster-template",
 }
 
+current_templates = set([val for key, val in current_vms.items() if key != "dom0"])
+
 
 def get_dom0_path(folder):
     return os.path.join(os.path.expanduser("~"), folder)
@@ -76,7 +78,7 @@ def migration_is_required():
     return result
 
 
-def apply_updates(vms=current_templates.keys()):
+def apply_updates(vms=current_templates):
     """
     Apply updates to all TemplateVMs
     """
@@ -85,7 +87,7 @@ def apply_updates(vms=current_templates.keys()):
     progress_start = 15
     sdlog.info("Applying all updates")
 
-    for progress_current, vm in enumerate(vms):
+    for progress_current, vm in enumerate(vms, 1):
         upgrade_results = UpdateStatus.UPDATES_FAILED
 
         if vm == "dom0":
@@ -97,7 +99,7 @@ def apply_updates(vms=current_templates.keys()):
         else:
             upgrade_results = _apply_updates_vm(vm)
 
-        progress_percentage = int(progress_start + ((progress_current + 1) / len(vms)) * 100 - 25)
+        progress_percentage = int(progress_start + ((progress_current) / len(vms)) * 100 - 25)
         if progress_percentage < progress_start:
             progress_percentage = progress_start
         yield vm, progress_percentage, upgrade_results
@@ -145,28 +147,18 @@ def _apply_updates_vm(vm):
     Apply updates to a given TemplateVM. Any update to the base fedora template
     will require a reboot after the upgrade.
     """
-    sdlog.info("Updating {}:{}".format(vm, current_templates[vm]))
+    sdlog.info("Updating {}".format(vm))
     try:
         subprocess.check_call(
-            [
-                "sudo",
-                "qubesctl",
-                "--skip-dom0",
-                "--targets",
-                current_templates[vm],
-                "state.sls",
-                "update.qubes-vm",
-            ]
+            ["sudo", "qubesctl", "--skip-dom0", "--targets", vm, "state.sls", "update.qubes-vm"]
         )
     except subprocess.CalledProcessError as e:
         sdlog.error(
-            "An error has occurred updating {}. Please contact your administrator.".format(
-                current_templates[vm]
-            )
+            "An error has occurred updating {}. Please contact your administrator.".format(vm)
         )
         sdlog.error(str(e))
         return UpdateStatus.UPDATES_FAILED
-    sdlog.info("{} update successful".format(current_templates[vm]))
+    sdlog.info("{} update successful".format(vm))
     return UpdateStatus.UPDATES_OK
 
 
@@ -374,11 +366,8 @@ def shutdown_and_start_vms():
         "sd-log",
     ]
 
-    # All TemplateVMs minus dom0
-    sdw_templates = [val for key, val in current_templates.items() if key != "dom0"]
-
     sdlog.info("Shutting down SDW TemplateVMs for updates")
-    for vm in sdw_templates:
+    for vm in sorted(current_templates):
         _safely_shutdown_vm(vm)
 
     sdlog.info("Shutting down SDW AppVMs for updates")

launcher/tests/test_updater.py

@@ -13,6 +13,7 @@
 updater = SourceFileLoader("Updater", path_to_script).load_module()
 from Updater import UpdateStatus  # noqa: E402
 from Updater import current_templates  # noqa: E402
+from Updater import current_vms  # noqa: E402
 
 temp_dir = TemporaryDirectory().name
 
@@ -56,7 +57,11 @@
 
 
 def test_updater_vms_present():
-    assert len(updater.current_templates) == 8
+    assert len(updater.current_vms) == 8
+
+
+def test_updater_templatevms_present():
+    assert len(updater.current_templates) == 4
 
 
 @mock.patch("Updater._write_updates_status_flag_to_disk")
@@ -303,7 +308,7 @@ def test_apply_updates_dom0_failure(mocked_info, mocked_error, mocked_call):
     mocked_error.assert_has_calls(error_log)
 
 
-@pytest.mark.parametrize("vm", current_templates.keys())
+@pytest.mark.parametrize("vm", current_templates)
 @mock.patch("subprocess.check_call", side_effect="0")
 @mock.patch("Updater.sdlog.error")
 @mock.patch("Updater.sdlog.info")
@@ -313,30 +318,18 @@ def test_apply_updates_vms(mocked_info, mocked_error, mocked_call, vm):
         assert result == UpdateStatus.UPDATES_OK
 
         mocked_call.assert_called_once_with(
-            [
-                "sudo",
-                "qubesctl",
-                "--skip-dom0",
-                "--targets",
-                current_templates[vm],
-                "state.sls",
-                "update.qubes-vm",
-            ]
+            ["sudo", "qubesctl", "--skip-dom0", "--targets", vm, "state.sls", "update.qubes-vm"]
         )
         assert not mocked_error.called
 
 
-@pytest.mark.parametrize("vm", current_templates.keys())
+@pytest.mark.parametrize("vm", current_templates)
 @mock.patch("subprocess.check_call", side_effect=subprocess.CalledProcessError(1, "check_call"))
 @mock.patch("Updater.sdlog.error")
 @mock.patch("Updater.sdlog.info")
 def test_apply_updates_vms_fails(mocked_info, mocked_error, mocked_call, vm):
     error_calls = [
-        call(
-            "An error has occurred updating {}. Please contact your administrator.".format(
-                current_templates[vm]
-            )
-        ),
+        call("An error has occurred updating {}. Please contact your administrator.".format(vm)),
         call("Command 'check_call' returned non-zero exit status 1."),
     ]
     result = updater._apply_updates_vm(vm)
@@ -423,7 +416,7 @@ def test_overall_update_status_reboot_not_done_previously(
     assert not mocked_error.called
 
 
-@pytest.mark.parametrize("vm", current_templates.keys())
+@pytest.mark.parametrize("vm", current_vms.keys())
 @mock.patch("subprocess.check_output")
 @mock.patch("Updater.sdlog.error")
 @mock.patch("Updater.sdlog.info")
@@ -435,7 +428,7 @@ def test_safely_shutdown(mocked_info, mocked_error, mocked_output, vm):
     assert not mocked_error.called
 
 
-@pytest.mark.parametrize("vm", current_templates.keys())
+@pytest.mark.parametrize("vm", current_vms.keys())
 @mock.patch(
     "subprocess.check_output", side_effect=["0", "0", "0"],
 )
@@ -452,7 +445,7 @@ def test_safely_start(mocked_info, mocked_error, mocked_output, vm):
     assert not mocked_error.called
 
 
-@pytest.mark.parametrize("vm", current_templates.keys())
+@pytest.mark.parametrize("vm", current_vms.keys())
 @mock.patch(
     "subprocess.check_output", side_effect=subprocess.CalledProcessError(1, "check_output"),
 )
@@ -468,7 +461,7 @@ def test_safely_start_fails(mocked_info, mocked_error, mocked_output, vm):
     mocked_error.assert_has_calls(call_list)
 
 
-@pytest.mark.parametrize("vm", current_templates.keys())
+@pytest.mark.parametrize("vm", current_vms.keys())
 @mock.patch(
     "subprocess.check_output", side_effect=subprocess.CalledProcessError(1, "check_output"),
 )
@@ -510,11 +503,7 @@ def test_shutdown_and_start_vms(
         call("fedora-31"),
         call("sd-large-buster-template"),
         call("sd-small-buster-template"),
-        call("sd-small-buster-template"),
-        call("sd-large-buster-template"),
-        call("sd-small-buster-template"),
         call("whonix-gw-15"),
-        call("sd-small-buster-template"),
     ]
     app_vm_calls = [
         call("sd-app"),
@@ -562,11 +551,7 @@ def test_shutdown_and_start_vms_sysvm_fail(
         call("fedora-31"),
         call("sd-large-buster-template"),
         call("sd-small-buster-template"),
-        call("sd-small-buster-template"),
-        call("sd-large-buster-template"),
-        call("sd-small-buster-template"),
         call("whonix-gw-15"),
-        call("sd-small-buster-template"),
     ]
     error_calls = [
         call("Error while killing system VM: sys-firewall"),