Support reboot from shell during `prepare` and `finish`
In one of the discussions around reboot, it was mentioned that it may be nice to also support it not only in execute -h internal but also in prepare -h shell. @psss mentioned that the same effect should be achieved using ansible and its reboot feature but it could be useful to provide the same thing for shell preparation (more options are nice, aren't they?). I see it as a nice-to-have, should not be hard to implement (logic can be reused) but not a priority.
prepare:
- name: Prepare the system and reboot
how: shell
script: |
if [ -z "$REBOOT_COUNT" ] || [ "$REBOOT_COUNT" -eq 0 ]; then
dnf install -y something
touch /etc/some-file
tmt-reboot
fi
@FrNecas Hi, I would like to take this issue if possible. I was looking for something to start with. This functionality is something I have a use for and it is already marked like good first issue :-)
Sure, I just hope that I estimated it correctly to be an easy issue and that no problems arise :) I think that generalising the logic for reboot handling a bit and moving it from execute/internal.py elsewhere (maybe introducing a new class -- PluginSupportingReboot or something along those lines) should suffice. Also do not forget about the finish step as mentioned in #1351 . Feel free to ping me if you need any introduction to tmt's code base or any help with the issue @lkotek
Sure, I just hope that I estimated it correctly to be an easy issue and that no problems arise :) I think that generalising the logic for reboot handling a bit and moving it from
execute/internal.pyelsewhere (maybe introducing a new class --PluginSupportingRebootor something along those lines) should suffice. Also do not forget about the finish step as mentioned in #1351 . Feel free to ping me if you need any introduction to tmt's code base or any help with the issue @lkotek
@FrNecas Thanks for sharing the ideas! I will let you know if there is anything 'unexpected' I need to help with :)
Hi @FrNecas, I prepared PR containing the changes for support of reboot in case of prepare and finish steps. I extracted the functionality of the key methods responsible for reboot (the most important _handle_reboot), modified them a bit, so it was possible to use it in prepare steps and finally put them into new class CommonReboot. Regarding support in finish step I used the same approach we currently use for ansible step (class from the prepare step is completely reused).
Sorry it took longer than expected, there was multiple changes related to reboot recently forcing me to rethink & rewrite it. What do you think about it, please? Please let me know if there is anything to be improved/changed.
Note taken from https://github.com/teemtee/tmt/issues/2869#issuecomment-2061383019:
relates to: VROOM-18747
More and more users run into the problem, #3649 is another fresh example. Let's give it a high priority. Added to the backlog.
- liberating scripts like
tmt-rebootof theirexecuteoppressor in https://github.com/teemtee/tmt/pull/3832, and preventing their use outside ofexecute, for now, in https://github.com/teemtee/tmt/pull/3833 - I would consider https://github.com/teemtee/tmt/pull/3835 (or similar PR) as a blocker for solving this issue, as I need to expose new environment variables, and I dare not touch the current implementation of plan environment composition.
- the next step would be to do something about the content of these scripts that are too dependent on
executeand its behavior (pid file, locking, etc.).prepareandfinishdo not provide this structure.
Dropping from the milestone - work is in progress, it just takes several steps that do not fit into a single milestone.
- extracting
tmt-abortproperties to a standalone class we can then reuse - I plan to add this new "abort context" to
shellandansibleplugins, and teach them to followtmt-abort - once that works - when I get back from PTO - the same will happen to reboot code: extracted into a reusable class, reconnecting the dots, adding it to prepare/finish, teach them to use it, and fix scripts to work in that context.
It's getting close! I still have several PRs in flight, and some work in my git stash to spawn more PRs, but, put together, this is how tmt-reboot from prepare looks like, as a draft:
/
discover
how: shell
order: 50
summary: 1 test selected
/foo
provision
queued provision.provision task #1: default-0
provision.provision task #1: default-0
how: virtual
order: 50
ansible: GuestAnsible(group=None, vars={})
qcow: Fedora-Cloud-Base-Generic-42-1.1.x86_64.qcow2
effective hardware: {}
memory: 2048 MB
disk: 40 GB
name: tmt-018-iItgMtlS
key: /var/tmp/tmt/run-018/provision/default-0/id_ecdsa
progress: booting...
console: /var/tmp/tmt/run-018/provision/default-0/logs/console.txt
primary address: 127.0.0.1
topology address: 127.0.0.1
port: 10023
multihost name: default-0
arch: x86_64
distro: Fedora Linux 42 (Cloud Edition)
kernel: 6.14.0-63.fc42.x86_64
package manager: dnf5
selinux: yes
systemd: yes
rsync: yes
is superuser: yes
is_container: no
queued push task #1: push to default-0
push task #1: push to default-0
summary: 1 guest provisioned
prepare
queued push task #1: push to default-0
push task #1: push to default-0
queued prepare task #1: essential-requires on default-0
queued prepare task #2: default-0 on default-0
queued prepare task #3: default-1 on default-0
prepare task #1: essential-requires on default-0
how: install
summary: Install essential required packages
name: essential-requires
order: 30
where: default-0
package: 1 package requested
/usr/bin/flock
cmd: rpm -q --whatprovides /usr/bin/flock || dnf5 install -y /usr/bin/flock
out: util-linux-core-2.40.4-7.fc42.x86_64
prepare task #2: default-0 on default-0
how: shell
order: 50
overview: 1 script found
script: /bin/true
cmd: /var/tmp/tmt/run-018/tree/outer-tmt-prepare-wrapper.sh-default-0-default-0
prepare task #3: default-1 on default-0
how: shell
order: 50
overview: 1 script found
script:
if [ "$TMT_REBOOT_COUNT" = "0" ]; then
echo "Before reboot: TMT_REBOOT_COUNT=$TMT_REBOOT_COUNT"
echo "Trigger one then!"
tmt-reboot
else
echo "After reboot: TMT_REBOOT_COUNT=$TMT_REBOOT_COUNT"
fi
cmd: /var/tmp/tmt/run-018/tree/outer-tmt-prepare-wrapper.sh-default-1-default-0
out: Before reboot: TMT_REBOOT_COUNT=0
out: Trigger one then!
script:
if [ "$TMT_REBOOT_COUNT" = "0" ]; then
echo "Before reboot: TMT_REBOOT_COUNT=$TMT_REBOOT_COUNT"
echo "Trigger one then!"
tmt-reboot
else
echo "After reboot: TMT_REBOOT_COUNT=$TMT_REBOOT_COUNT"
fi
cmd: /var/tmp/tmt/run-018/tree/outer-tmt-prepare-wrapper.sh-default-1-default-0
err: Warning: Permanently added '[127.0.0.1]:10023' (ED25519) to the list of known hosts.
out: After reboot: TMT_REBOOT_COUNT=1
queued pull task #1: pull from default-0
pull task #1: pull from default-0
summary: 3 preparations applied
Logging needs to be improved, and there are some rough edges when it comes to exceptions, but all in all, it behaves as if in a test.
Current state of things:
- https://github.com/teemtee/tmt/pull/4351 to enable handling of
tmt-rebootinshellplugin- https://github.com/teemtee/tmt/pull/4306 to make composition of environments easier
- https://github.com/teemtee/tmt/pull/4320 to include guest in phase results
- https://github.com/teemtee/tmt/pull/4307 to teach
shellplugin to store phase results- https://github.com/teemtee/tmt/pull/4263 to "standardize" creation of report files
https://github.com/teemtee/tmt/pull/4351 is now up for a review. That should be the last piece needed for prepare and finish to support tmt-reboot.
👏 👏 Nice happy to see this went through!