teemtee / tmt

Test Management Tool
MIT License
82 stars 122 forks source link

Remove tmt scripts from 'system under test' after test run complete #1655

Open BeeGrech opened 1 year ago

BeeGrech commented 1 year ago

Scripts leftover for features are left on the test machine.

Example:
# ls /usr/local/bin
brewkoji_install.sh          ip6ll                        polarion_tool_doc.pdf        setup_guest.sh               tipcTS
bug2patch.py                 ipv6.sh                      polarion_tool_doc.tex        setup_ovs_dpdk_tunnel.sh     tmt-abort
bug_report.py                mcast                        rhts-abort                   setup_ovs_dpdk_vhostuser.sh  tmt-file-submit
checksctp                    mcast_client                 rhts-reboot                  setup_ovs_kernel.sh          tmt-reboot
conn_client                  mcast_server                 rhts-report-result           setup_ovs_offload.sh         tmt-report-result
conn_server                  move_to_polarion.py          rhts-submit-log              setup_ovs_offload_tunnel.sh  tools_polarion/
container_img.sh             myftp                        rhts_submit_log              setup_sriov.sh               top_client
copy_googlesheet.py          nagle_rcv                    rstrnt-abort                 setup_trex.sh                top_server
erratainfo.py                nagle_snd                    rstrnt-reboot                sriov_report/                vm_img.sh
fdp.py                       netns_1_net.sh               rstrnt-report-log            start_binary_search.sh       vmsh
gen_report.py                netns_2_net.sh               rstrnt-report-result         stream_client                withsctp
get_cpu.py                   netns_3_net.sh               runtest                      stream_server                zstream_reminder.sh

...
 '/usr/local/bin/rhts-abort
 '/usr/local/bin/rhts-reboot
 /usr/local/bin/rhts-report-result
 '/usr/local/bin/rhts-submit-log
 '/usr/local/bin/rhts_submit_log
/usr/local/bin/tmt*
'/usr/local/bin/tmt-abort
 '/usr/local/bin/tmt-file-submit
 '/usr/local/bin/tmt-reboot
/usr/local/bin/tmt-report-result

In my use case, leaving the rhts scripts creates issues with using restraint against this test machine until the scripts are removed.

The tool should clean up leftover scripts at the completion of testing.

idorax commented 1 year ago

From the source code, we can see there are some scripts having path /usr/local/bin,

$ find . -type f | xargs grep "/usr/local/bin"
./tmt/steps/provision/__init__.py:                "&& ln -sf /root/pkg/bin/rsync /usr/local/bin/rsync")
./tmt/steps/provision/__init__.py:                "&& ln -sf /root/pkg/bin/rsync /usr/local/bin/rsync")
./tmt/steps/execute/__init__.py:    path="/usr/local/bin/tmt-reboot",
./tmt/steps/execute/__init__.py:        "/usr/local/bin/rstrnt-reboot",
./tmt/steps/execute/__init__.py:        "/usr/local/bin/rhts-reboot"],
./tmt/steps/execute/__init__.py:    path="/usr/local/bin/tmt-report-result",
./tmt/steps/execute/__init__.py:        "/usr/local/bin/rstrnt-report-result",
./tmt/steps/execute/__init__.py:        "/usr/local/bin/rhts-report-result"],
./tmt/steps/execute/__init__.py:    path="/usr/local/bin/tmt-file-submit",
./tmt/steps/execute/__init__.py:        "/usr/local/bin/rstrnt-report-log",
./tmt/steps/execute/__init__.py:        "/usr/local/bin/rhts-submit-log",
./tmt/steps/execute/__init__.py:        "/usr/local/bin/rhts_submit_log"],
./tmt/steps/execute/__init__.py:    path="/usr/local/bin/tmt-abort",
./tmt/steps/execute/__init__.py:        "/usr/local/bin/rstrnt-abort",
./tmt/steps/execute/__init__.py:        "/usr/local/bin/rhts-abort"],

... leaving the rhts scripts creates issues with using restraint against this test machine until the scripts are removed.

@BeeGrech, do you mean we should remove these scripts

/usr/local/bin/rsync
/usr/local/bin/rhts-abort
/usr/local/bin/rhts-reboot
/usr/local/bin/rhts-report-result
/usr/local/bin/rhts-submit-log
/usr/local/bin/rhts_submit_log
/usr/local/bin/rstrnt-abort
/usr/local/bin/rstrnt-reboot
/usr/local/bin/rstrnt-report-log
/usr/local/bin/rstrnt-report-result
/usr/local/bin/tmt-abort
/usr/local/bin/tmt-file-submit
/usr/local/bin/tmt-reboot
/usr/local/bin/tmt-report-result

once tmt run ... is done?

idorax commented 1 year ago

The scripts are created by ExecutePlugin.prepare_scripts(),

Looks we should remove the scripts created by ExecutePlugin.prepare_scripts() from the guest. @lukaszachy and @psss, what do you think?

thrix commented 1 year ago

Hmm, I am uncertain if this is worth it. Expecting the machine is clean after testing can be hard to achieve. I would rather document it as a known thing personally. If we accept this, these problems might appear:

  1. The testing will not be reproducable after tmt execution is done, we will need an option to skip this cleanup, this will lead to more complexity. I will need to again run tmt prepare, before I login to the machine and try to manually run my tests which expect these files to be present.

  2. Shouldn't we also cleanup created artifacts after testing, or some other things?? Questions like this might appear.

BeeGrech commented 1 year ago

@BeeGrech, do you mean we should remove these scripts

/usr/local/bin/rsync
/usr/local/bin/rhts-abort
/usr/local/bin/rhts-reboot
/usr/local/bin/rhts-report-result
/usr/local/bin/rhts-submit-log
/usr/local/bin/rhts_submit_log
/usr/local/bin/rstrnt-abort
/usr/local/bin/rstrnt-reboot
/usr/local/bin/rstrnt-report-log
/usr/local/bin/rstrnt-report-result
/usr/local/bin/tmt-abort
/usr/local/bin/tmt-file-submit
/usr/local/bin/tmt-reboot
/usr/local/bin/tmt-report-result

once tmt run ... is done?

@idorax Yes, that would be ideal, at least the scripts from /usr/local/bin/*

Understand Miro's comments here, however in comparison to other artifacts, these scripts modify the default behavior of installed binaries. when i run restraint, these new local scripts are executed and restraint functionality is broken because of missing new variables inserted into the TMT version of the scripts

idorax commented 1 year ago

@thrix and @BeeGrech, how about removing them via tmt clean since it may introduce other potential problems?

BeeGrech commented 1 year ago

@idorax something like tmt clean test-device ?

idorax commented 1 year ago

@idorax something like tmt clean test-device ?

Almost yes, something like tmt clean scripts as currently tmt clean supports tmt clean runs, tmt clean guests and tmt clean images. If we add tmt clean scripts, tmt clean --help looks like:

$ tmt clean --help
Usage: tmt clean [OPTIONS] COMMAND1 [ARGS]... [COMMAND2 [ARGS]...]...

  Clean workdirs, guests, images or scripts installed by tmt.

...<snip>...

Commands:
  runs    Clean workdirs of past runs.
  guests  Stop running guests of runs.
  images  Remove images of supported provision methods.
  scripts Remove scripts installed by execution plugins.
BeeGrech commented 1 year ago

Yeah I think it sounds good, possibly make it so it could be added to fmf files?

idorax commented 1 year ago

possibly make it so it could be added to fmf files?

@BeeGrech, would you please explain more? I don't know what (related to tmt clean) should be added to fmf files.

(To have more expert suggestions, let me CC) @psss, @lukaszachy and @thrix, what dou you think?

BeeGrech commented 1 year ago

Im sorry, I meant in perhaps supported through the Finish step

idorax commented 1 year ago

in perhaps supported through the Finish step

@BeeGrech, looks like it's back to the issue pointed by @thrix,

The testing will not be reproducable after tmt execution is done, we will need an option to skip this cleanup, this will lead to more complexity. I will need to again run tmt prepare, before I login to the machine and try to manually run my tests which expect these files to be present.

So, it seems simpler if we manually run tmt clean ... to remove scripts installed by tmt.

BeeGrech commented 1 year ago

Ah ok I miss understood, my original intention was meant for TMT cleaning automatically. tmt clean ... should suffice :)

comps commented 1 year ago

Wouldn't a better solution to this be simply installing the files outside of common PATH locations, plus exporting that outside location as part of PATH for the actual executed tests, so they can still access those commands?

Something like /opt/tmt would be pretty standard.

happz commented 1 year ago

Wouldn't a better solution to this be simply installing the files outside of common PATH locations, plus exporting that outside location as part of PATH for the actual executed tests, so they can still access those commands?

Something like /opt/tmt would be pretty standard.

Oh, I like this.

comps commented 1 year ago

Oh, I like this.

Actually, /opt might be mounted noexec, a more standard location for out-of-PATH executables is /usr/libexec/tmt.

lukaszachy commented 1 year ago

Wouldn't a better solution to this be simply installing the files outside of common PATH locations, plus exporting that outside location as part of PATH for the actual executed tests, so they can still access those commands?

Not really. Test can use sudo / su (which clear custom PATH).

comps commented 1 year ago

Not really. Test can use sudo / su (which clear custom PATH).

But that also clears other environment variables, incl. everything TMT_* (unless they are configured to be inherited in sudoers). I would say it's unreasonable to expect test-framework-related functionality to work after an environment reset.