action #33388
closedopenQA Project - coordination #14626: [epic] backend and console capabilities interface to increase extensibility and code reuse
coordination #38819: [qe-core][tools][functional][epic] Refactor use of backends
[functional][u][easy][pvm] Implement proper split from other backends
0%
Description
Motivation¶
The current implementation of the spvm backend makes use of functions ( e.g. use_ssh_serial_console
located in lib/ipmi_backend_utils.pm
) from other "backends".
This is good enough for a first proof of concept but for the sake of clarity and further maintainability this should reside in a separate file which is shared/included from the backends using it.
Acceptance criteria¶
- AC1: Have clearly seperated backends with shared common functions
- AC2: Do not include any functions from other backends (e.g. from ipmi into spvm)
Suggestions¶
- Extract method "use_ssh_serial_console" into a common lib, e.g. "ssh_remote_console_backend_something_foobar"
- Call
git grep 'BACKEND.*pvm'
and only work on these occurences, that excludes e.g. boot_from_pxe.pm - Find occurences like in https://github.com/os-autoinst/os-autoinst-distri-opensuse/blob/master/lib/susedistribution.pm#L554 where we look for "s390x || ipmi || spvm" and replace this with an explicit function returning a boolean value for what it really is, e.g. "no_local_tty" or "remote_backend" or "relies_on_ssh_terminal"
- Create followup sibling ticket for all necessary work for other backends
Further details¶
Focus on spvm, leave the rest for the parent epic. "Having a single verification run is enough" says okurz
Example for a change:
- if (check_var('BACKEND', 'foo') || check_var('BACKEND', 'bar')) {
+ sub backend_is_remote { return check_var('BACKEND', 'foo') || check_var('BACKEND', 'bar') }
+ if (backend_is_remote) {
Updated by nicksinger over 6 years ago
- Copied from action #33340: [tools][functional][u][medium][pvm] Enable graphical installation for the powerVM backend added
Updated by nicksinger over 6 years ago
- Blocked by coordination #21838: [functional][u][saga] PowerVM backend added
Updated by nicksinger over 6 years ago
- Copied from deleted (action #33340: [tools][functional][u][medium][pvm] Enable graphical installation for the powerVM backend)
Updated by nicksinger over 6 years ago
- Copied to action #33697: [tools][hard][pvm] Enable the powerVM backend to conduct multimachine tests added
Updated by nicksinger over 6 years ago
- Copied to deleted (action #33697: [tools][hard][pvm] Enable the powerVM backend to conduct multimachine tests)
Updated by okurz over 6 years ago
- Subject changed from [tools][functional][easy][pvm] Implement proper split from other backends to [tools][functional][u][easy][pvm] Implement proper split from other backends
- Target version set to Milestone 17
Updated by okurz over 6 years ago
- Due date set to 2018-08-14
- Target version changed from Milestone 17 to Milestone 18
Updated by okurz over 6 years ago
- Target version changed from Milestone 18 to Milestone 18
Updated by okurz about 6 years ago
- Related to action #38486: [functional][u] add capability flags to os-autoinst backends (or tests) added
Updated by okurz about 6 years ago
- Related to action #38423: [sle][functional][u][hard] Refactor first_boot to unify duplicated behavior for remote backend added
Updated by okurz about 6 years ago
- Copied to coordination #38819: [qe-core][tools][functional][epic] Refactor use of backends added
Updated by okurz about 6 years ago
- Copied to deleted (coordination #38819: [qe-core][tools][functional][epic] Refactor use of backends)
Updated by okurz about 6 years ago
- Description updated (diff)
- Status changed from New to Workable
- Parent task set to #38819
Updated by okurz about 6 years ago
- Description updated (diff)
- Estimated time set to 3.00 h
- Difficulty set to easy
Updated by SLindoMansilla about 6 years ago
- Status changed from Workable to In Progress
- Assignee set to SLindoMansilla
Updated by SLindoMansilla about 6 years ago
- Status changed from In Progress to Feedback
Updated by okurz about 6 years ago
- Due date changed from 2018-08-14 to 2018-08-28
bulk move to next sprint as could not be discussed in SR
Updated by SLindoMansilla about 6 years ago
- Status changed from Feedback to In Progress
PR is WIP until I clarify with Alice the details of the affected test suite: https://openqa.suse.de/tests/1495366
Updated by mgriessmeier about 6 years ago
- Due date changed from 2018-08-28 to 2018-09-11
PR is still WIP
move due to vacation
Updated by mgriessmeier about 6 years ago
please also take this in account when you do the refactoring ;)
https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/5714/files#diff-141f4b5a48eaecb0c631a0de23e41a51R270
Updated by SLindoMansilla about 6 years ago
- Status changed from In Progress to Workable
I have problems executing IPMI jobs on my webui.
And general problems with my openQA instance.
Updated by mgriessmeier about 6 years ago
- Due date changed from 2018-09-11 to 2018-09-25
Updated by SLindoMansilla about 6 years ago
- Status changed from Workable to In Progress
Updated by SLindoMansilla about 6 years ago
- Due date changed from 2018-09-25 to 2018-10-09
Moving to sprint 27. Not able to finish on sprint 26.
Updated by riafarov about 6 years ago
- Blocks action #41546: [functional][y] Change architecture checks with functionality of the #33388 to detect VNC installation added
Updated by okurz about 6 years ago
Wow, not sure if we want to wait that long ;)
Updated by okurz about 6 years ago
- Due date set to 2018-10-23
- Target version changed from Milestone 18 to Milestone 20
- Start date deleted (
5000-01-01)
Updated by okurz almost 6 years ago
- Status changed from In Progress to Workable
- Assignee deleted (
SLindoMansilla)
as discussed in planning, not really working on that right now.
Updated by szarate almost 6 years ago
- Status changed from Workable to In Progress
- Assignee set to szarate
I'll take a look to it, with #38423 in mind
Updated by okurz almost 6 years ago
- Target version changed from Milestone 20 to Milestone 21
Updated by szarate almost 6 years ago
Updated by okurz almost 6 years ago
@szarate, this ticket was estimated as "easy" but is above our mean cycle time. Is the status "In Progress" correct or would you consider this task "Blocked" waiting for another ticket first or "Feedback" waiting on feedback in the PR? What I saw in the PR makes sense to me but needs a bit more love to pull the changes through in the test modules I guess :)
Updated by szarate almost 6 years ago
@okurz, please take a look at the last changes and my two questions (I actually updated the PR yesterday). There are some verification runs that could be ran, can you help with that?
What I saw in the PR makes sense to me but needs a bit more love to pull the changes through in the test modules I guess :)
Can you elaborate? I don't understand too much the sentence.
Updated by szarate almost 6 years ago
- Project changed from openQA Project to openQA Tests
- Category changed from 132 to Enhancement to existing tests
Also, this belongs to test code, not backend.
Updated by okurz almost 6 years ago
szarate wrote:
@okurz, please take a look at the last changes and my two questions (I actually updated the PR yesterday). There are some verification runs that could be ran, can you help with that?
I don't yet see why it would be necessary to run verification runs as I do not see any bigger, risky changes yet.
What I saw in the PR makes sense to me but needs a bit more love to pull the changes through in the test modules I guess :)
Can you elaborate? I don't understand too much the sentence.
Easy, follow all suggestions and ensure the ACs are covered
szarate wrote:
Also, this belongs to test code, not backend.
Agreed – for now – as we still have #38486 for a more backend-centric proposal which we can base on refactoring the test code first.
Updated by szarate almost 6 years ago
I don't yet see why it would be necessary to run verification runs as I do not see any bigger, risky changes yet.
@okurz I planned initially on doing more changes, but I guess there's no reason to keep losig time on trying to get verification runs.
Easy, follow all suggestions and ensure the ACs are covered
I think they're covered by now. for suggestion #2, there's already is_remote_backend, which I think is sufficient as a first step (it was there already, I just moved it to a different place).
Agreed – for now – as we still have #38486 for a more backend-centric proposal which we can base on refactoring the test code first.
Updated by szarate almost 6 years ago
- Status changed from In Progress to Feedback
Setting to feedback, waiting for either merge or more feedback on the PR.
Updated by szarate almost 6 years ago
- Precedes action #45008: [functional][u] Further improvements on splitting backend code added
Updated by okurz over 5 years ago
https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/6316 is merged, thanks for that. What I see as missing according to the suggestions in the description is the still many "if backend spvm":
$ git grep 'BACKEND.*pvm'
lib/Utils/Backends.pm: check_var('BACKEND', 'spvm');
lib/bootloader_setup.pm: type_string_very_slow "console=tty " unless (check_var('BACKEND', 'ipmi') || check_var('BACKEND', 'spvm'));
lib/bootloader_setup.pm: if (check_var("BACKEND", "spvm")) {
lib/main_common.pm: elsif (check_var('BACKEND', 'spvm')) {
lib/main_common.pm: if (!check_var('BACKEND', 'ipmi') && !is_hyperv_in_gui && !get_var("LIVECD") && !check_var('BACKEND', 'spvm')) {
lib/main_common.pm: and !check_var('BACKEND', 'spvm')
lib/power_action_utils.pm: console('root-ssh')->kill_ssh if check_var('BACKEND', 'ipmi') || check_var('BACKEND', 'spvm');
lib/susedistribution.pm: if (check_var('BACKEND', 'ikvm') || check_var('BACKEND', 'ipmi') || check_var('BACKEND', 'spvm')) {
lib/susedistribution.pm: if (check_var('BACKEND', 'ipmi') || check_var('BACKEND', 's390x') || get_var('S390_ZKVM') || check_var('BACKEND', 'spvm')) {
lib/susedistribution.pm: $hostname = get_required_var('SUT_IP') if check_var('BACKEND', 'ipmi') || check_var('BACKEND', 'spvm');
lib/susedistribution.pm: elsif (check_var('BACKEND', 'ipmi') || check_var('BACKEND', 'spvm')) {
lib/susedistribution.pm: elsif ($console =~ m/root-console$/ && check_var('BACKEND', 'spvm')) {
lib/susedistribution.pm: if (check_var('BACKEND', 's390x') || get_var('S390_ZKVM') || check_var('BACKEND', 'ipmi') || check_var('BACKEND', 'spvm')) {
lib/utils.pm: return if (check_var('BACKEND', 'spvm')) || (check_var('BACKEND', 'ipmi'));
tests/installation/installation_overview.pm: $need_ssh = 1 if check_var('BACKEND', 'spvm'); # we better be able to login
tests/installation/logs_from_installation_system.pm: if (check_var('BACKEND', 'ipmi') || check_var('BACKEND', 'spvm')) {
tests/installation/reboot_after_installation.pm: if (check_var('BACKEND', 'spvm')) {
the first occurence within lib/Utils/Backends.pm
is fine but we should try to use more is_remote_backend
or similar in the test modules.
Updated by okurz over 5 years ago
- Priority changed from Normal to High
- Target version changed from Milestone 21 to Milestone 22
Updated by mgriessmeier over 5 years ago
how to proceed here to not let the ticket rot in Feedback?
If there is anything left, I suggest to either create a new ticket or put it back to workable with suggestions of "next steps"
Updated by szarate over 5 years ago
- Status changed from Feedback to Resolved
I call it resolved, as okurz addresed his own comments in the child ticket: https://progress.opensuse.org/issues/45008#note-5
Updated by okurz over 5 years ago
- Status changed from Resolved to Feedback
- Assignee changed from szarate to okurz
Well, so far we have not fulfilled what was written in the suggestions or ACs but let's wait for the PR https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/6600 as referenced in #45008#note-5
Updated by okurz over 5 years ago
- Status changed from Feedback to New
- Assignee deleted (
okurz) - Priority changed from High to Normal
PR merged, no message received about broken tests. Next steps as mentioned in the suggestion can be followed on with.
Updated by szarate over 5 years ago
- Subject changed from [tools][functional][u][easy][pvm] Implement proper split from other backends to [functional][u][easy][pvm] Implement proper split from other backends
- Status changed from Workable to Blocked
- Assignee set to okurz
@okurz, can you please set expectations for this ticket? also, there's the child ticket, can you also give it a look? so that we can decide to pick it up or not? I'd close it tbh.
Setting to blocked until PO clarifies.
Updated by okurz over 5 years ago
- Status changed from Blocked to Workable
- Assignee deleted (
okurz) - Target version changed from Milestone 22 to Milestone 23
- Start date changed from 5000-01-01 to 2019-02-17
As stated in #33388#note-48 I expect to follow "next steps as mentioned in the suggestion". E.g. we can now use the method "Utils::Backends::has_ttys" in more places, e.g. when checking if we should execute the console font test.
Updated by mgriessmeier over 5 years ago
- Target version changed from Milestone 23 to Milestone 24
moving to M24
Updated by mgriessmeier over 5 years ago
- Target version changed from Milestone 24 to Milestone 25
Updated by zluo over 5 years ago
- Status changed from Workable to In Progress
- Assignee set to zluo
take over and check this. maybe directly working on ticket https://progress.opensuse.org/issues/51236 make sense now.
Updated by zluo over 5 years ago
Compilation looks good for Backends.pm, bootloader_setup.pm
http://f40.suse.de/tests/3746
http://f40.suse.de/tests/3751
add more changes...
Updated by zluo over 5 years ago
tests/installation/bootloader.pm
lib/main_common.pm
modified now.
Updated by zluo over 5 years ago
WIP PR:
https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/7503
Adding documentation into Utils::Backends is required.
Updated by SLindoMansilla over 5 years ago
- Blocked by action #51725: [s390x][kvm] test fails in bootloader_zkvm added
Updated by zluo over 5 years ago
check:
https://openqa.suse.de/tests/2915205 (s390x) okay with issue for hostname_inst which should not be there
https://openqa.suse.de/tests/2915204 (ppc64le) okay with issue for hostname_inst which should not be there
https://openqa.suse.de/tests/2912003 (aarch64) okay
https://openqa.suse.de/tests/2915206 (spvm) okay with issue for hostname_inst which should not be there
Updated by zluo over 5 years ago
check:
https://openqa.suse.de/tests/2915201 (xen) okay with issue for hostname_inst which should not be there
https://openqa.suse.de/tests/2912213 (svirt-xen-pv) okay
https://openqa.suse.de/tests/2915202 (s390x-kvm) okay
https://openqa.suse.de/tests/2915203 (svirt-hyperv) okay
Updated by zluo over 5 years ago
mark: need to file a ticket for https://openqa.suse.de/tests/2912470#settings
Updated by zluo over 5 years ago
check issue with reboot after installation:
https://openqa.suse.de/tests/2915198
done, looks good.
Updated by zluo over 5 years ago
Updated by zluo over 5 years ago
to check on osd:
https://openqa.suse.de/tests/2934076
https://openqa.suse.de/tests/2934082
Updated by zluo over 5 years ago
issues with hostname_inst from ppc64le-spvm and system_role failure from s390x-kvm-sle12 are not related.
Updated by zluo over 5 years ago
https://openqa.suse.de/tests/2933890 system_role works fine.
Updated by zluo over 5 years ago
- Status changed from In Progress to Feedback
cannot set as resolved atm.
Updated by zluo over 5 years ago
- Blocked by deleted (action #51725: [s390x][kvm] test fails in bootloader_zkvm)
Updated by osukup over 5 years ago
- Status changed from Resolved to Feedback
Updated by szarate over 5 years ago
I vote to close this ticket and work better on defining #38819 rather than comming up with more and more tasks that are simply created out of nothing.
Assigning to PO to later discuss this.
Updated by mgriessmeier over 5 years ago
- Status changed from Feedback to Resolved