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.

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

Fix Updater only restarting sys-* qubes #1128

Merged
merged 1 commit into from
Jul 8, 2024

Conversation

deeplow
Copy link
Contributor

@deeplow deeplow commented Jul 8, 2024

After an update to qubes-vm-update (which the SDW updater uses under the hood) --restart was changed to only restart sys-* qubes. The new qubes-vm-update flag --apply-to-all has the behavior we intend.

Status

Ready for review

Description of Changes

Fixes #1127 by replacing qubes-vm-update's flags from --restart to --apply-to-all

Testing

Run the updater and ensure sd-app, sd-log, sd-gpg, etc. (all app qubes) are restarted after a successful update process.

Deployment

no special considerations.

Checklist

If you have made changes to the provisioning logic

  • All tests (make test) pass in dom0
    • ⚠️ not on a prod

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

After an update to qubes-vm-update [1] (which the SDW updater uses
under the hood) `--restart` was changed to only restart `sys-*`
qubes. The new qubes-vm-update flag --apply-to-all has the behavior
we intend.

Fixes freedomofpress#1127

[1]: QubesOS/qubes-core-admin-linux@93078d0b
@legoktm legoktm self-assigned this Jul 8, 2024
@legoktm
Copy link
Member

legoktm commented Jul 8, 2024

I manually applied this change to a 1.0.0rc2 system and then:

  • Downloaded an old securedrop-client deb, installed it into the small template so there would be 1 package needing an upgrade.
  • Deleted dom0's ~/.securedrop_updater
  • Rebooted Qubes
  • Updater notification showed up, proceeded. sd-gpg, sd-proxy and sd-log VMs were running at this point
  • sd-small-bookworm-template upgraded and shutdown
  • sd-gpg, sd-log, sd-proxy shutdown
  • sd-app starts, which triggers start of sd-log
  • Updater concludes, lets me "Continue" to start client (as there were no dom0 updates)

Is that the expected behavior, that it shuts down all the VMs instead of restarting them? And then the updater manually starts sd-app?

@legoktm
Copy link
Member

legoktm commented Jul 8, 2024

Is that the expected behavior, that it shuts down all the VMs instead of restarting them?

Based on https://github.com/QubesOS/qubes-core-admin-linux/blob/6299958c831a764474b65554db0758dffbd41d9b/vmupdate/vmupdate.py#L384-L387, I believe so. Only servicevms are restarted, the others are just shutdown.

@legoktm
Copy link
Member

legoktm commented Jul 8, 2024

(SDW CI failure is #1006 - OK to ignore.)

@deeplow
Copy link
Contributor Author

deeplow commented Jul 8, 2024

Is that the expected behavior, that it shuts down all the VMs instead of restarting them? And then the updater manually starts sd-app?

Based on https://github.com/QubesOS/qubes-core-admin-linux/blob/6299958c831a764474b65554db0758dffbd41d9b/vmupdate/vmupdate.py#L384-L387, I believe so. Only servicevms are restarted, the others are just shutdown.

Yes, that is consistent with --help:

--apply-to-all, -R Restart Service VMs and shutdown AppVMs whose template
has been updated.

I think it's fine as it its, but please do let me know if you disagree.

@legoktm legoktm added this pull request to the merge queue Jul 8, 2024
Merged via the queue into freedomofpress:main with commit 9ee0ba3 Jul 8, 2024
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Investigate updater not restarting some qubes
2 participants