dev: vagrant: downgrade kernel, https, better messaging#3751
Conversation
|
The original Ubuntu 24.04 GA kernel doesn't have a bug with mouse positioning that the HWE kernel is showing. Using https for apt avoids a problem with what may be stale proxies. This PR also adds some clearer messaging regarding whether the vagrant guest machine finished provisioning successfully or not, via notification popups in the machine. |
| subject="Please wait" | ||
| body="Please wait for provisioning to finish. $(date -Is)" | ||
| systemd-run --quiet --machine vagrant@.host --user notify-send --expire-time=0 "${subject}" "${body}" |
There was a problem hiding this comment.
🚩 systemd-run notification failure would abort provisioning under set -e
The systemd-run --quiet --machine vagrant@.host --user notify-send call at line 45 runs before any provisioning work. If the host doesn't have the right systemd/machinectl configuration (e.g., non-systemd host, wrong permissions), this command will fail and set -e will abort the entire script before any useful provisioning happens. The notifications are a UX nicety but probably shouldn't be fatal. The same concern applies to the systemd-run call inside on_exit at line 38 — if the error-path notification itself fails under set -e, it could mask the original error message (though since it's the last command in the trap, the impact is limited). This is likely fine for the known Vagrant box environment, but fragile if the box configuration changes.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
I will let it rely on systemd-run working. Also, BTW the systemd-run and the notification happens in the guest not on the containing host. (Even though it says .host).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3751 +/- ##
=======================================
Coverage 81.31% 81.31%
=======================================
Files 620 620
Lines 39080 39080
Branches 6376 6376
=======================================
Hits 31778 31778
Misses 6330 6330
Partials 972 972 ☔ View full report in Codecov by Sentry. |
1d5e847 to
a8d3c3a
Compare
marksvc
left a comment
There was a problem hiding this comment.
@marksvc made 3 comments and resolved 3 discussions.
Reviewable status: 0 of 1 files reviewed, all discussions resolved.
| subject="Please wait" | ||
| body="Please wait for provisioning to finish. $(date -Is)" | ||
| systemd-run --quiet --machine vagrant@.host --user notify-send --expire-time=0 "${subject}" "${body}" |
There was a problem hiding this comment.
I will let it rely on systemd-run working. Also, BTW the systemd-run and the notification happens in the guest not on the containing host. (Even though it says .host).
|
|
||
| subject="Please wait" | ||
| body="Please wait for provisioning to finish. $(date -Is)" | ||
| systemd-run --quiet --machine vagrant@.host --user notify-send --expire-time=0 "${subject}" "${body}" |
There was a problem hiding this comment.
🔴 systemd-run notification failure aborts entire provisioning due to set -e
The systemd-run call on line 45 sends a purely informational "Please wait" notification to the host. Because set -e is active (line 25), if this command fails for any reason (e.g., the host doesn't have notify-send, or machinectl registration isn't configured for the VM), the entire provisioning script immediately exits before performing any useful work (npm install, dotnet install, kernel downgrade). The EXIT trap then fires, reporting "Provisioning failed (code N)" — all because a cosmetic notification couldn't be delivered.
The same systemd-run call inside on_exit (deploy/vagrant/sfdev/Vagrantfile:38) is less of a concern since it's the last command in the trap handler and the script is already exiting. But the one on line 45 gates the entire provisioning process.
| systemd-run --quiet --machine vagrant@.host --user notify-send --expire-time=0 "${subject}" "${body}" | |
| systemd-run --quiet --machine vagrant@.host --user notify-send --expire-time=0 "${subject}" "${body}" || true |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
I will let it rely on systemd-run working. Also, BTW the systemd-run and the notification happens in the guest not on the containing host. (Even though it says .host).
| echo "== Downgrading kernel to GA ==" | ||
| retry sudo apt-get --assume-yes install --install-recommends linux-generic | ||
| # Must reboot after removing modules. | ||
| sudo DEBIAN_FRONTEND=noninteractive apt-get --assume-yes purge linux-generic-hwe-\* linux-hwe-\* linux-modules\*-6.17.\* |
There was a problem hiding this comment.
🚩 apt-get purge with HWE glob patterns may fail if packages are absent
At line 86, apt-get purge uses glob patterns like linux-generic-hwe-\* and linux-modules\*-6.17.\*. If the base box has already been updated (or was never on HWE), these patterns may match zero packages, causing apt-get to return a non-zero exit code (E: Unable to locate package). Under set -e, this would abort provisioning. Currently the base box is expected to have HWE packages, so this works today, but it's fragile against future base box changes. Wrapping with || true or checking whether HWE packages exist first would make it more resilient.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
I don't retry the purge because it's deleting packages that are installed. Testing in situations of the packages being present and not being present show the command to work as desired.
marksvc
left a comment
There was a problem hiding this comment.
@marksvc made 2 comments and resolved 2 discussions.
Reviewable status: 0 of 1 files reviewed, all discussions resolved.
|
|
||
| subject="Please wait" | ||
| body="Please wait for provisioning to finish. $(date -Is)" | ||
| systemd-run --quiet --machine vagrant@.host --user notify-send --expire-time=0 "${subject}" "${body}" |
There was a problem hiding this comment.
I will let it rely on systemd-run working. Also, BTW the systemd-run and the notification happens in the guest not on the containing host. (Even though it says .host).
| echo "== Downgrading kernel to GA ==" | ||
| retry sudo apt-get --assume-yes install --install-recommends linux-generic | ||
| # Must reboot after removing modules. | ||
| sudo DEBIAN_FRONTEND=noninteractive apt-get --assume-yes purge linux-generic-hwe-\* linux-hwe-\* linux-modules\*-6.17.\* |
There was a problem hiding this comment.
I don't retry the purge because it's deleting packages that are installed. Testing in situations of the packages being present and not being present show the command to work as desired.
a8d3c3a to
d5bb4b0
Compare
| body="Destroy and recreate this vagrant machine. See WARNING-NOT-PROVISIONED.txt on the desktop for details. $(date -Is)" | ||
| fi | ||
| echo >&2 "${subject}: ${body}" | ||
| systemd-run --quiet --machine vagrant@.host --user notify-send --expire-time=0 "${subject}" "${body}" |
There was a problem hiding this comment.
🚩 EXIT trap's systemd-run could alter final exit status
The on_exit function at line 38 ends with a systemd-run call. If provisioning succeeds (exit 0) but this notification command fails, the last command in the EXIT trap has a non-zero exit status. In some bash versions, this can override the script's final exit status, causing Vagrant to interpret a successful provisioning as a failure. While the main-line systemd-run at line 45 was reported as a bug, this trap-side instance has its own subtle risk. Adding || true here as well would make the behavior robust across bash versions.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
(I'm going to rely on systemd-run working.)
marksvc
left a comment
There was a problem hiding this comment.
@marksvc made 1 comment and resolved 1 discussion.
Reviewable status: 0 of 2 files reviewed, all discussions resolved.
| body="Destroy and recreate this vagrant machine. See WARNING-NOT-PROVISIONED.txt on the desktop for details. $(date -Is)" | ||
| fi | ||
| echo >&2 "${subject}: ${body}" | ||
| systemd-run --quiet --machine vagrant@.host --user notify-send --expire-time=0 "${subject}" "${body}" |
There was a problem hiding this comment.
(I'm going to rely on systemd-run working.)
pmachapman
left a comment
There was a problem hiding this comment.
I liked the notifications prompting me to restart, and that the provisioning was successful.
@pmachapman reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on marksvc).
This change is