tmt icon indicating copy to clipboard operation
tmt copied to clipboard

Support reboot from shell during `prepare` and `finish`

Open FrNecas opened this issue 4 years ago • 11 comments

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 avatar Sep 23 '21 21:09 FrNecas

@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 :-)

lkotek avatar Jul 28 '22 15:07 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.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

FrNecas avatar Jul 28 '22 16:07 FrNecas

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

@FrNecas Thanks for sharing the ideas! I will let you know if there is anything 'unexpected' I need to help with :)

lkotek avatar Jul 29 '22 13:07 lkotek

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.

lkotek avatar Oct 10 '22 11:10 lkotek

Note taken from https://github.com/teemtee/tmt/issues/2869#issuecomment-2061383019:

relates to: VROOM-18747

lukaszachy avatar Apr 18 '24 07:04 lukaszachy

More and more users run into the problem, #3649 is another fresh example. Let's give it a high priority. Added to the backlog.

psss avatar Apr 08 '25 14:04 psss

  • liberating scripts like tmt-reboot of their execute oppressor in https://github.com/teemtee/tmt/pull/3832, and preventing their use outside of execute, 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 execute and its behavior (pid file, locking, etc.). prepare and finish do not provide this structure.

happz avatar Jun 23 '25 08:06 happz

Dropping from the milestone - work is in progress, it just takes several steps that do not fit into a single milestone.

happz avatar Jul 03 '25 07:07 happz

  • extracting tmt-abort properties to a standalone class we can then reuse
  • I plan to add this new "abort context" to shell and ansible plugins, and teach them to follow tmt-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.

happz avatar Jul 03 '25 20:07 happz

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.

happz avatar Oct 27 '25 21:10 happz

Current state of things:

  • https://github.com/teemtee/tmt/pull/4351 to enable handling of tmt-reboot in shell plugin
    • 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 shell plugin to store phase results
      • https://github.com/teemtee/tmt/pull/4263 to "standardize" creation of report files

happz avatar Nov 20 '25 13:11 happz

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.

happz avatar Dec 10 '25 19:12 happz

👏 👏 Nice happy to see this went through!

BeeGrech avatar Dec 10 '25 20:12 BeeGrech