Project

General

Profile

action #117028

[kernel][core][alp] Possible improvements for transactional installations

Added by pcervinka 5 months ago. Updated 4 days ago.

Status:
Workable
Priority:
Normal
Assignee:
-
Category:
Refactor/Code Improvements
Target version:
-
Start date:
2022-09-22
Due date:
% Done:

0%

Estimated time:
Difficulty:
Tags:

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?


Related issues

Related to openQA Tests - action #115196: [qe-core] Prepare for ALP - Schedule Databases testsuite for ALPBlocked2022-08-31

History

#1 Updated by pcervinka 5 months ago

  • Project changed from openQA Tests to qam-qasle-collaboration
  • Category deleted (Refactor/Code Improvements)

#2 Updated by MDoucha 5 months 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 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.

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 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?

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().

#3 Updated by szarate 4 months ago

  • Related to action #115196: [qe-core] Prepare for ALP - Schedule Databases testsuite for ALP added

#4 Updated by szarate 4 months 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 the install_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 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.

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 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?

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().

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...

#5 Updated by MDoucha 4 months 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.

#6 Updated by pcervinka 4 months 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?

#7 Updated by pcervinka 4 months 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 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?

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().

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.

#8 Updated by okurz 4 months 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.

#9 Updated by jstehlik 4 months ago

  • Status changed from New to Workable

Followup to be planned beginning of November.

#10 Updated by MDoucha 4 months ago

pcervinka wrote:

1. Universal function for package installation

Related PR: https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/15683

#11 Updated by jstehlik 3 months ago

  • Project changed from qam-qasle-collaboration to openQA Tests
  • Assignee set to pcervinka

Petr and Santi have scheduled a call on this. To be moved into another project.

#12 Updated by szarate 2 months 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

#13 Updated by pcervinka 2 months 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

#14 Updated by pcervinka 2 months ago

  • Assignee deleted (pcervinka)

#15 Updated by szarate 4 days ago

  • Category set to Refactor/Code Improvements

Also available in: Atom PDF