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

Show reboot when no updates are required (and user did not reboot) #436

Merged
merged 1 commit into from
Feb 4, 2020

Conversation

emkll
Copy link
Contributor

@emkll emkll commented Jan 29, 2020

Status

Ready for review

Description of Changes

Fixes #435

Testing

More updates scenario

  • All other VMs are up-to-date
  • Ensure your fedora-30 template needs an update (sudo dnf downgrade zlib)
  • run the updater, apply updates, and close the updater (click cancel button)
  • downgrade a non-reboot-after-update-vm (eg, sd-app-buster-template but not fedora-30 nor dom0) by downgrading a package
  • run the updater, and observe the reboot is requested after the updates are applied to sd-app-buster-template

Up-to-date scenario

  • All other VMs are up-to-date
  • Ensure your fedora-30 template needs an update (sudo dnf downgrade zlib)
  • run the updater, apply updates, and close the updater (click cancel button)
  • run the updater again, observer reboot screen after checking for updates

@kushaldas kushaldas self-assigned this Jan 30, 2020
Copy link
Contributor

@kushaldas kushaldas left a comment

Choose a reason for hiding this comment

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

  • Run for the first time, I was required a few updates
  • after update & reboot
  • After reboot, it says more update needed, asked to reboot again, rebooted.
  • finally, no more reboot option
  • Downgraded zlib in the Fedora 30, now the updater asked to update
  • asked for a reboot (this should not happen).

@emkll
Copy link
Contributor Author

emkll commented Jan 30, 2020

@kushaldas that is expected behavior: by design, any update to fedora-30 will require a reboot. We could file a follow-up to reboot the sys-{usb,net,firewall} after updating the fedora template but the order might be complicated and would disrupt user actions should they be doing something else on the Workstation.

the test plan states:

downgrade a non-reboot-after-update-vm (sd-app-buster-template) by downgrading a package

I have clarified the test plan to explicitly exclude fedora-30

@kushaldas
Copy link
Contributor

I was testing based on


    - All other VMs are up-to-date
     -Ensure your fedora-30 template needs an update (sudo dnf downgrade zlib)
    - run the updater, apply updates, and close the updater
    - run the updater again, observer reboot screen after checking for updates

I never understood that close the updater means, close the application even when the updater said "reboot". This explains better now.

@emkll
Copy link
Contributor Author

emkll commented Jan 30, 2020

ahh yes, sorry that was unclear @kushaldas . "close the updater" can be done by clicking the "cancel" button. Updated the test plan, sorry for the confusion

Copy link
Contributor

@redshiftzero redshiftzero left a comment

Choose a reason for hiding this comment

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

thanks @emkll! lgtm and test plan all works as advertised

@kushaldas you can either dismiss your review if you're happy with this as is and merge else if you also want to do a full review feel free

@kushaldas kushaldas dismissed their stale review February 4, 2020 05:57

test plan updated.

Copy link
Contributor

@kushaldas kushaldas left a comment

Choose a reason for hiding this comment

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

Worked good. Approved.

@kushaldas kushaldas merged commit 05ddb1f into master Feb 4, 2020
@kushaldas kushaldas deleted the 435-reboot-no-updates branch February 4, 2020 05:59
cfm pushed a commit that referenced this pull request Apr 1, 2024
Show reboot when no updates are required (and user did not reboot)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Launching updater when reboot is still required results in error instead of reboot request
3 participants