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

Write sdw-last-updated after upgrades in REBOOT_REQUIRED case #465

Merged
merged 1 commit into from
Feb 28, 2020

Conversation

eloquence
Copy link
Member

@eloquence eloquence commented Feb 21, 2020

Status

Ready for review.

Description

The UpdateThread never newly discovers that a reboot is required, it only picks up a reboot requirement from prior runs. Even if we added logic to discover the reboot requirement earlier, we won't know for sure whether updates have been successfully applied until the UpgradeThreads have completed.

Resolves #462

Test plan

  1. Note the contents of ~/.securedrop_launcher/sdw-last-updated or delete it from disk.
  2. Check if you have updates available in fedora-30 by running dnf check-update inside the template. If you do not, attempt to ensure a system state in which updates are available.
  3. Force a run of the preflight updater by running /opt/securedrop/launcher-sdw-launcher.py --skip-delta 0 in dom0. While the command runs, observe its output in ~/.securedrop-launcher/launcher.log.
  4. Wait for the update check to finish.
    • Observe that ~/.securedrop_launcher/sdw-last-updated has not been modified yet.
  5. Apply the available updates. Do not reboot yet.
    • Observe that ~/.securedrop_launcher/sdw-last-updated has been updated.
  6. Restart the updater and re-check for updates.
    • Observe that ~/.securedrop_launcher/sdw-last-updated has not been modified again.

Checklist

  • No packaging changes required
  • make test succeeds in dom0 (haven't re-run; no provisioning changes in this PR)

The UpdateThread never newly discovers that a reboot is required, it
only picks up a reboot requirement from prior runs. Even if we added
logic to discover the reboot requirement earlier, we won't know for
sure whether updates have been successfully applied until the
UpgradeThreads have completed.
@eloquence
Copy link
Member Author

eloquence commented Feb 21, 2020

Prelim testing with a REBOOT_REQUIRED case suggests that it now works as intended; will write up a more formal test plan.

@zenmonkeykstop
Copy link
Contributor

  1. Note the contents of ~/.securedrop_launcher/sdw-last-updated or delete it from disk.
  2. Check if you have updates available in fedora-30 by running dnf check-update inside the template. If you do not, attempt to ensure a system state in which updates are available.
  3. Force a run of the preflight updater by running /opt/securedrop/launcher-sdw-launcher.py --skip-delta 0 in dom0. While the command runs, observe its output in ~/.securedrop-launcher/launcher.log.
  4. Wait for the update check to finish.
    • Observe that ~/.securedrop_launcher/sdw-status has not been modified yet.
  5. Apply the available updates. Do not reboot yet.
    • Observe that ~/.securedrop_launcher/sdw-status has been updated.
  6. Restart the updater and re-check for updates.
    • Observe that ~/.securedrop_launcher/sdw-status has not been modified again. FAIL - updater checks for updates again and finds that updates are required. `sdw-update-status is updated

@eloquence
Copy link
Member Author

eloquence commented Feb 25, 2020

If you do not, attempt to ensure a system state in which updates are available.

What step did you take here, if any?

@zenmonkeykstop
Copy link
Contributor

dnf downgrade zlib in the fedora-30 template, then shutdown template, before running the updater.

@eloquence
Copy link
Member Author

Does dnf check-update continue to report updates are available in the template? Would you mind uploading the launcher.log to a Gist or similar, as well? Let me know if you already have any theories as to what's causing it to report that updates are still available; my expectation is that it would, after the "check for updates", show the "reboot required" screen.

@zenmonkeykstop
Copy link
Contributor

Ok, forcing an update check after a reboot right now to see if it clears it, but will do some more troubleshooting once that's complete.

@zenmonkeykstop
Copy link
Contributor

Update check after reboot sets the status to the correct value and subsequent launcher runs within the default interval go straight to client. I think it just doesn't handle the case when the updater is rerun before rebooting, which is unlikely to happen as users are specifically told to reboot and given a handy button.

@eloquence
Copy link
Member Author

The test plan incorrectly referred to the file we expect to be modified as sdw-status; the expected modification is to sdw-last-udpated (which is the file that's parsed by sdw-notify, and contains the date of the last successful update, regardless of the current update status). My apologies for the error. Seeing if I can reproduce the issue you encountered with the updater incorrectly reporting that updates are required.

@eloquence
Copy link
Member Author

updater checks for updates again and finds that updates are required.

If I interpret the description correctly, I'm unable to reproduce this, @zenmonkeykstop. If I downgrade zlib and then run the updater twice, it advances to the "Reboot required" screen after the check for updates. It does not report that updates are available:

Screenshot_2020-02-26_14-56-10

It does run the check unnecessarily, which is already tracked in freedomofpress/securedrop-updater#17. Is that what you meant? That's the behavior both in master and in this branch.

@zenmonkeykstop
Copy link
Contributor

Reran this on a fresh install (make clean && make all):

  1. Note the contents of ~/.securedrop_launcher/sdw-last-updated or delete it from disk. deleted
  2. Check if you have updates available in fedora-30 by running dnf check-update inside the template. If you do not, attempt to ensure a system state in which updates are available.updates available
  3. Force a run of the preflight updater by running /opt/securedrop/launcher-sdw-launcher.py --skip-delta 0 in dom0. While the command runs, observe its output in ~/.securedrop-launcher/launcher.log.
  4. Wait for the update check to finish.
    • Observe that ~/.securedrop_launcher/sdw-last-updated has not been modified yet.
  5. Apply the available updates. Do not reboot yet.
    • Observe that ~/.securedrop_launcher/sdw-last-updated has been updated.
  6. Restart the updater and re-check for updates.
    • Observe that ~/.securedrop_launcher/sdw-last-updated has not been modified again.

LGTM

Copy link
Contributor

@zenmonkeykstop zenmonkeykstop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test plan passing on clean install! WFM

@zenmonkeykstop zenmonkeykstop merged commit addae8c into master Feb 28, 2020
cfm pushed a commit that referenced this pull request Apr 1, 2024
Write `sdw-last-updated` after upgrades in REBOOT_REQUIRED case
@legoktm legoktm deleted the fix-sdw-last-updated branch May 28, 2024 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sdw-last-updated not created/updated after update that requires reboot
2 participants