Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WIP Intial Qubes 4.2 Updater Integration #967

Closed
wants to merge 20 commits into from
Closed

WIP Intial Qubes 4.2 Updater Integration #967

wants to merge 20 commits into from

Conversation

deeplow
Copy link
Contributor

@deeplow deeplow commented Mar 27, 2024

Status

Work in progress. Based on 4_2-compat-deb12-base, only @deeplow's commits are relevant to this PR.

TODO:

  • Evaluate if error states are still appropriate
  • Add / fix tests
  • Consider if it makes sense to update even after a migration was applied (this was not the case before)

Description of Changes

Fixes #899 . Keeps most of the existing updater logic.

Changes proposed in this pull request:

  • updates templates via the CLI Qubes Updater instead of via salt
  • removes functions deprecated by the new Qubes updater

Testing

  1. make dev
  2. launch updater
  3. Observe templates starting as they are updated
  4. Observe app qubes getting restarted in background
  5. See "successfully completed"
  6. Inspect SDW-launcher logs to ensure correct updating of templates.

Deployment

Any special considerations for deployment? Will only apply to new installs

Checklist

If you have made changes to the provisioning logic

  • All tests (make test) pass in dom0

Note: tests don't pass due to the WIP nature of the base 4.2 branch. However, it seems that launcher-specific tests weren't passing already and it was unclear how to run the tests.

If you have added or removed files

  • I have updated MANIFEST.in and rpm-build/SPECS/securedrop-workstation-dom0-config.spec

If documentation is required

  • I have opened a PR in the docs repo for these changes, or will do so later
  • I would appreciate help with the documentation

zenmonkeykstop and others added 15 commits March 22, 2024 10:53
@zenmonkeykstop
Specify 4.2 for workstation CI
70c8939
@eaon @zenmonkeykstop
Early Qubes 4.2 preparations
970875a 
(cherry picked from commit 98b5bb3)
@eaon @zenmonkeykstop
Convert the remaining policies into the r5 format
79e3ac8 
(cherry picked from commit 99b72a5)
@zenmonkeykstop
update expected fedora base template name
3f1e683 
(cherry picked from commit e873481)
@zenmonkeykstop
minor fix to fedora setup
1103fc2 
(cherry picked from commit 4d6ca1a)
@cfm @zenmonkeykstop
[WIP] the qvm-check conditional isn't doing the trick; as a workaroun…
2c50f7e 
…d, don't abort on qvm-shutdown failure

(cherry picked from commit 9f9d1d3)
@zenmonkeykstop
Updated GHA workflows to use Fedora-37 as appropriate
94368a1
@zenmonkeykstop
Update default system template to fedora-39
9c6b97c
@zenmonkeykstop
Update PyYaml to 6.0.1 in dev reqs.
c1f6075
@zenmonkeykstop
Updated launcher test to invoke python3 as python alias not present i…
5acc850 
…n CI
@zenmonkeykstop
added safe.directory value to git config
c0101b5
@zenmonkeykstop
Setting permissions on configfiles for workstation CI
99aeb15
@zenmonkeykstop
Setting permissions on configfiles for workstation CI
667714f
@zenmonkeykstop
EXPERIMENT: install debian-12-minimal and use it to clone the templat…
35a8905 
…es - this will likely fail to install fully as stuff in current template hasn't been added
Split dom0 updates from template ones
a7ec40e 
Done in preparation for 4.2 Qubes Updater integration, where the
template upgrade part will be replaced entirely.
@deeplow
Copy link
Contributor Author

deeplow commented Mar 27, 2024 •
edited
Loading

Open Implementation Questions

What to do with the progress bar?

Currently this PR functionally removed a lot of meaningful progress reporting. I see several alternatives:

  1. Keep it as is. Even with reduced reporting (10%, 15%, 75%, 100%)
  2. Remove progress bar entirely.
  3. Remove progress bar and launch GUI updater for visual progress indication
  4. Parse CLI updater for progress reporting. With the --show-output we should be able to know when template has finished.

Should we use the GUI Updater?

Launching the Qubes GUI updater could be helpful in two aspects

Advantages

  • progress indication
  • failure reporting - the administrator could quickly identified the failed qubes instead of having to read through logs

Disadvantages

  • However, we need to have first the launcher and then the updater GUI because dom0 has to be updated first and possibly migrations have to be applied. This creates a need to have a flow from the launcher to the updater.
  • The GUI updater has a few bugs that need to be polished before then:

@zenmonkeykstop zenmonkeykstop changed the base branch from main to 4_2-compat-keep-launcher March 29, 2024 23:16
@zenmonkeykstop zenmonkeykstop changed the base branch from 4_2-compat-keep-launcher to 4_2-compat-deb12-base March 29, 2024 23:17
@zenmonkeykstop zenmonkeykstop force-pushed the 4_2-compat-deb12-base branch 3 times, most recently from e3e5fe0 to 9f9ca1e Compare March 31, 2024 16:28
deeplow added 4 commits April 1, 2024 16:26
Replace template update logic with Qubes CLI updater
c7cbe62 
Replaces the Qubes CLI updater with the original template updating
process. This adds parallelization and deprecates the need to
restart app qubes which inherit from the updated templates.

NOTE: Template upgrades are now enforced after migrations.
Deprecate VM-related functions (replaced by 4.2 updater)
67351bb
Bump default template debian version name
d2c7e9d
Do not fail on empty updater outputs
32158c5 
Especially when testing, it is possible for the command to fail
without any ouput.
@deeplow
Copy link
Contributor Author

deeplow commented Apr 1, 2024

Fixed the tests and rebased due to some loose ends that made testing cleaner. I find that similarly to the CI, I cannot run the launcher tests with make check.

Internally it runs:

xvfb-run python3 -m pytest --cov-report term-missing --cov=sdw_notify --cov=sdw_updater_gui/ --cov=sdw_util -v tests/

This is also failing locally. I think it's related to some issue with pytest-cov. I was able to run them temporarily by recomming all coverag-related args:

xvfb-run python3 -m pytest  -v tests/

@zenmonkeykstop zenmonkeykstop force-pushed the 4_2-compat-deb12-base branch 2 times, most recently from 1573036 to b3f4634 Compare April 1, 2024 21:27
Replace launcher tests with 4.2 updater integration
4680ebf 
- deprecate tests that update VMs and associated app qube restarts
- add rest that checks Qubes 4.2 updater success / failure
- removes old update_generator logic

NOTE: some of the removed tests will have to be replaced with
integration tests since an external component (qubes updater) is
now used.
@zenmonkeykstop zenmonkeykstop force-pushed the 4_2-compat-deb12-base branch from a38de98 to 503d73d Compare April 2, 2024 15:16
@legoktm legoktm force-pushed the 4_2-compat-deb12-base branch 7 times, most recently from 426d6c1 to 1e36c74 Compare April 5, 2024 03:19
@zenmonkeykstop zenmonkeykstop force-pushed the 4_2-compat-deb12-base branch from 1e36c74 to 941f051 Compare April 9, 2024 14:20
@zenmonkeykstop zenmonkeykstop deleted the branch freedomofpress:4_2-compat-deb12-base April 9, 2024 21:42
@deeplow
Copy link
Contributor Author

deeplow commented Apr 10, 2024

Spent some time troubleshooting why it wasn't working after the main rebase. It turns out it's just the Debian templates that were actually failing, potentially due to lack of qubes-update-enabling packages (but I am not fully familiar on the in-template updater dependencies). So in the end the updater was just fine and doing its job.

@zenmonkeykstop
Copy link
Contributor

@deeplow, I can't reopen this from here - I think you need to switch the base to main first.

@deeplow
Copy link
Contributor Author

deeplow commented Apr 10, 2024

I can't seem to be able to switch base. Are you able to it? If not I'll resort to a new PR.

@legoktm
Copy link
Member

legoktm commented Apr 10, 2024

I tried restoring the branch in case it would help - it did not. I think you'll just have to create a new PR for it. :|

@deeplow deeplow mentioned this pull request Apr 10, 2024
Closed
5 tasks
@deeplow
Copy link
Contributor Author

deeplow commented Apr 10, 2024

Follow up PR here #979.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Investigate using Qubes updater in 4.2
5 participants
@deeplow @zenmonkeykstop @legoktm @eaon @cfm