action #117028
closed[qe-core] Possible improvements for transactional installations
Added by pcervinka about 2 years ago. Updated 6 months ago.
100%
Description
I have found few places for improvements during test development for alp.
We should discuss following points and if they are valid, try to fix/improve them.
1. Universal function for package installation¶
I noticed that existing code base, becoming messy with if-else for zypper and transactional-update installations.
One of the examples:
if (is_transactional) {
trup_call("pkg install qemu-linux-user");
check_reboot_changes;
} else {
zypper_call("in qemu-linux-user");
}
This can grow up in numbers with more tests adapted to alp/slem.
Maybe could we use similar approach like for liberty?
https://gitlab.suse.de/suse-liberty-linux/openqa-tests-sll/-/tree/master/lib/Utils/PackageManager
Example in test code:
$testapi::distri->get_package_manager()->install_package("ltp")
As you can see, test code just contained universal installation command, which was distro independent.
2. Existing trup_call
function needs root-console
¶
It means that code with $self->select_serial_terminal
doesn't work, console change needs to be done.
You can find this pattern in the code:
select_console 'root-console';
trup_call("--continue pkg $cmd", timeout => 2000);
Is there a way to fix that? trup_call
should be console independent.
3. Support for transactional command in script_run¶
We can run commands on the transaction system in the snapshot via transactional-update -c -d --quiet run
.
For example, we install something, want to just check something , but we can't use normal script_run
, because it will run on booted snapshot.
Would be worth to have something like script_run("COMMAND", transactional => [is_transactional| any other condition | 1])
which would wrap command in case we want transactional approach?
Updated by pcervinka about 2 years ago
- Project changed from openQA Tests (public) to 175
- Category deleted (
Refactor/Code Improvements)
Updated by MDoucha about 2 years ago
pcervinka wrote:
1. Universal function for package installation¶
Example in test code:
$testapi::distri->get_package_manager()->install_package("ltp")
As you can see, test code just contained universal installation command, which was distro independent.
This could be simplified to just install_package("ltp");
and all the $testapi::distri->get_package_manager()
boilerplate should be hidden inside the install_package()
helper function.
2. Existing
trup_call
function needsroot-console
¶It means that code with
$self->select_serial_terminal
doesn't work, console change needs to be done.You can find this pattern in the code:
select_console 'root-console'; trup_call("--continue pkg $cmd", timeout => 2000);
Is there a way to fix that?
trup_call
should be console independent.
Yes, this can be fixed, although it'd be easier to fix (or even write correctly from the start) if the console API design were sane.
3. Support for transactional command in script_run¶
We can run commands on the transaction system in the snapshot via
transactional-update -c -d --quiet run
.
For example, we install something, want to just check something , but we can't use normalscript_run
, because it will run on booted snapshot.Would be worth to have something like
script_run("COMMAND", transactional => [is_transactional| any other condition | 1])
which would wrap command in case we want transactional approach?
This should be handled by trup_call()
. When that function is called on a non-transactional system, it should fall back to plain script_run()
.
Updated by szarate about 2 years ago
- Related to action #115196: [qe-core] Prepare for ALP - Schedule Databases testsuite for ALP added
Updated by szarate about 2 years ago
MDoucha wrote:
pcervinka wrote:
1. Universal function for package installation¶
Example in test code:
$testapi::distri->get_package_manager()->install_package("ltp")
As you can see, test code just contained universal installation command, which was distro independent.
This could be simplified to just
install_package("ltp");
and all the$testapi::distri->get_package_manager()
boilerplate should be hidden inside theinstall_package()
helper function.
What I had thought about this, is to use module flags, and the pre_run
hooks, and do the installation of packages there, so that test modules could declare in a hash which packages it expects to have installed, and through an install wrapper use directly the abstract factory: $testapi::distri->packages->install(qw(ltp vim))
, and if needed use a facade for further needs...
2. Existing
trup_call
function needsroot-console
¶It means that code with
$self->select_serial_terminal
doesn't work, console change needs to be done.You can find this pattern in the code:
select_console 'root-console'; trup_call("--continue pkg $cmd", timeout => 2000);
Is there a way to fix that?
trup_call
should be console independent.Yes, this can be fixed, although it'd be easier to fix (or even write correctly from the start) if the console API design were sane.
Which console api in this case?
3. Support for transactional command in script_run¶
We can run commands on the transaction system in the snapshot via
transactional-update -c -d --quiet run
.
For example, we install something, want to just check something , but we can't use normalscript_run
, because it will run on booted snapshot.Would be worth to have something like
script_run("COMMAND", transactional => [is_transactional| any other condition | 1])
which would wrap command in case we want transactional approach?This should be handled by
trup_call()
. When that function is called on a non-transactional system, it should fall back to plainscript_run()
.
hmm... @Petr, here you mean any particular call to script_run, or any? for the earlier I'd go with what Martin suggests, for the latter, we might want to think a bit further... of course we could make script_run redefined in the test distribution, to be aware of a system being transactional or not, and whether we want it quiet or not for a given test module...
Updated by MDoucha about 2 years ago
szarate wrote:
MDoucha wrote:
Yes, this can be fixed, although it'd be easier to fix (or even write correctly from the start) if the console API design were sane.
Which console api in this case?
All of it, from the public testapi
functions all the way down to the backends.
Updated by pcervinka about 2 years ago
MDoucha wrote:
szarate wrote:
MDoucha wrote:
Yes, this can be fixed, although it'd be easier to fix (or even write correctly from the start) if the console API design were sane.
Which console api in this case?
All of it, from the public
testapi
functions all the way down to the backends.
@okurz are you aware of any plans or ideas which could help?
Updated by pcervinka about 2 years ago
szarate wrote:
MDoucha wrote:
pcervinka wrote:
3. Support for transactional command in script_run¶
We can run commands on the transaction system in the snapshot via
transactional-update -c -d --quiet run
.
For example, we install something, want to just check something , but we can't use normalscript_run
, because it will run on booted snapshot.Would be worth to have something like
script_run("COMMAND", transactional => [is_transactional| any other condition | 1])
which would wrap command in case we want transactional approach?This should be handled by
trup_call()
. When that function is called on a non-transactional system, it should fall back to plainscript_run()
.hmm... @Petr, here you mean any particular call to script_run, or any? for the earlier I'd go with what Martin suggests, for the latter, we might want to think a bit further... of course we could make script_run redefined in the test distribution, to be aware of a system being transactional or not, and whether we want it quiet or not for a given test module...
This particular case, i sorted for short term, but there could be better generic solution. This one just uses script_output
, idea is the same:
https://github.com/czerw/os-autoinst-distri-opensuse/blob/b53828660d486122b2bbc298ab3a791c50e95cbf/lib/bootloader_setup.pm#L144
https://github.com/czerw/os-autoinst-distri-opensuse/blob/b53828660d486122b2bbc298ab3a791c50e95cbf/lib/bootloader_setup.pm#L163
It could be any call, but maybe, i just want to generalize specific case, which is not need for wider use.
Updated by okurz about 2 years ago
pcervinka wrote:
@okurz are you aware of any plans or ideas which could help?
I do not know about any further plans or ideas regarding this going beyond this ticket itself.
Updated by jstehlik about 2 years ago
- Status changed from New to Workable
Followup to be planned beginning of November.
Updated by MDoucha about 2 years ago
pcervinka wrote:
1. Universal function for package installation¶
Related PR: https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/15683
Updated by jstehlik about 2 years ago
- Project changed from 175 to openQA Tests (public)
- Assignee set to pcervinka
Petr and Santi have scheduled a call on this. To be moved into another project.
Updated by szarate about 2 years ago
Some of the Thoughts I shared with Petr, 2 weeks ago:
thinking for instance in how the transnational update module will work, one can have a pre run module that looks for a packages method which should return a list of packages that the system under test should install, when it is found, it is passed directly to the install_packages() function, which defined in opensusedistribution, should allow for managing different types of installers, via InstallerFactory pattern (or maybe some other), the important part, is to be able to ask $distri->action($packages)
on test teardown, system is either rolled back in cases where btrfs is being used and it’s possible, or restored to a previous snapshot (ideally) when it’s a transactional system
it’s also posible that for isotovideo, a new step might be needed to achieve that, so that basetest::setup
and basetest::teardown
methods exist, and are capable somehow, of producing junit compatible output, this with the idea of having openqa test runs, to integrate with other systems, and to be able. to have standard interface that allows the reviewer to have better info ob the test that have been executed
questions arise when looking into the state of needles or what to do with them, but it should not be a huge problem in the end
the flag $basetest::packages
has to be a hash, or a dictionary that allows the caller to know what the test objects are going to be
Petr's suggestion is to: Consider installing everything from the beginning before the tests are executed
so in the end, we’re defining:
- setup
- teardown
- objects
This also would help us to avoid post_fail_hooks that have mixed roles, such as stopping services and collecting logs
With this, as a further step one should be able to easily write either a Perl::Tidy
rule or a Perl::Critic
check, that allows for verification of such things, like: no package install if testobjects are defined
We can continue this work on a new ticket, in the qe-core backlog, with the help of the rest of the teams
Updated by pcervinka about 2 years ago
Above idea looks fine, but we should make some follow up for details, as other ideas are already poping up https://github.com/pablo-herranz/os-autoinst-distri-opensuse/blob/9ea1f81305648b0fad9dfdab70f80dd46417737e/lib/Wrappers/packages.pm
Updated by szarate almost 2 years ago
- Category set to Refactor/Code Improvements
Updated by szarate almost 2 years ago
- Tags changed from alp, slem to alp, slem, platform-team
- Tracker changed from action to coordination
- Subject changed from [kernel][core][alp] Possible improvements for transactional installations to [qe-core] Possible improvements for transactional installations
- Status changed from Workable to New
See suggestion from: https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/16506#discussion_r1121460485
- Ideally, the creation of users can also be delegated to the setup step of the testsuite.
I had thought down a much better idea yesterday but forgot to write it down, so I better leave it here for posterity
PS: taking the ticket for QE-Core and will coordinate from there once the time comes
Updated by jlausuch almost 2 years ago
I like the idea of this ticket, then we can avoid those if-elses everywhere.
However, we need to tweak check_reboot_changes
as it will fail if the package is already installed as it expects a change in the new snapshot created by transactional-update
. I guess it's easy to change that behaviour.
There is another thing to avoid this, which is using process_reboot
after pkg installation, but it will reboot always regardless if the package is installed or not.
Updated by pcervinka about 1 year ago
Was updating maintenance part for kernel update recently and there is one outcome. transactional-update patch
don't indicate return code for a need of another patch
round if there is package manager update. It just prints it in output, but this is not comfortable as with zypper (which has multiple error codes). I created https://bugzilla.suse.com/show_bug.cgi?id=1216504 for further discussion. @jlausuch im just wondering, why it was not discovered earlier? For example why function fully_patch_system
was not adapted already for transactional systems, as it is function used across all maintenance tests. Adaptation is now done in https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/18012.
Updated by dzedro about 1 year ago
- Status changed from New to In Progress
- Assignee set to dzedro
Updated by jlausuch about 1 year ago
pcervinka wrote in #note-19:
Was updating maintenance part for kernel update recently and there is one outcome.
transactional-update patch
don't indicate return code for a need of another patch
round if there is package manager update. It just prints it in output, but this is not comfortable as with zypper (which has multiple error codes). I created https://bugzilla.suse.com/show_bug.cgi?id=1216504 for further discussion. @jlausuch im just wondering, why it was not discovered earlier? For example why functionfully_patch_system
was not adapted already for transactional systems, as it is function used across all maintenance tests. Adaptation is now done in https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/18012.
Not sure... The way we have always installed updates (aggregates and incidents) has been like this:
https://openqa.suse.de/tests/12746599#step/install_updates/341
e.g.
zypper -n --no-gpg-checks ar -f -n 'TEST_0' http://download.suse.de/ibs/SUSE:/Maintenance:/29290/SUSE_Updates_SLE-Micro_5.5_x86_64/ 'TEST_0'
zypper -n ref
...
zypper -n --no-gpg-checks ar -f -n 'TEST_25' http://download.suse.de/ibs/SUSE:/Maintenance:/31370/SUSE_Updates_SLE-Micro_5.5_x86_64/ 'TEST_25'
zypper -n ref
transactional-update -n up
Updated by szarate about 1 year ago
- Tags changed from alp, slem, platform-team to alp, slem, platform-team, qe-core-november-sprint
Updated by dzedro about 1 year ago · Edited
Is there test I could use to test the wrapper, I tried live patching https://dzedro.suse.cz/tests/146#step/qa_test_klp/17
Ignoring the missing packages, I guess live patching with transactional update is not even possible, because you have to do reboot to apply changes.
I'm looking for some, but transactional tests are separated and shared tests which I checked don't do install.
edit: found tests/kernel/update_kernel.pm from PR in comments
Updated by szarate about 1 year ago
- Sprint set to QE-Core: November Sprint 23 (Nov 15 - Dec 13)
Updated by dzedro about 1 year ago
Updated by pcervinka about 1 year ago
jlausuch wrote in #note-21:
transactional-update -n up
I believe, we should use patch
, instead of up
to validate particular update.
Updated by pcervinka about 1 year ago · Edited
Also patch in transactional-update doesn't properly work in case, when is a need for additional restart after package manager update. zypper handles this case, but not via transactional-update. See https://bugzilla.suse.com/show_bug.cgi?id=1216504
Updated by szarate about 1 year ago
- Tracker changed from coordination to action
Updated by szarate about 1 year ago
- Sprint changed from QE-Core: November Sprint 23 (Nov 15 - Dec 13) to QE-Core: December Sprint 23 (Dec 13 - Jan 10)
Updated by dzedro 11 months ago
- Related to action #153865: [qe-core] install_package with trup_reboot will fail if package is installed added
Updated by mgrifalconi 10 months ago
- Sprint changed from QE-Core: February Sprint 24 (Jan 31 - Feb 28) to QE-Core: March Sprint 24 (Mar 06 - Mar 28)
Updated by slo-gin 9 months ago
This ticket was set to High priority but was not updated within the SLO period. Please consider picking up this ticket or just set the ticket to the next lower priority.
Updated by szarate 5 months ago
- Related to action #164024: [qe-core] test fails in perl_bootloader - process_reboot and wait_boot seem to be doing the same thing added