Project

General

Profile

Actions
Copy link

action #33388

closed

openQA Project (public) - 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

Added by nicksinger almost 7 years ago. Updated over 5 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
mgriessmeier
Category:
Enhancement to existing tests
Target version:
SUSE QA (private) - Milestone 25
Start date:
2019-02-17
Due date:
% Done:

0%

Estimated time:
Difficulty:
easy

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) {

Related issues 5 (0 open — 5 closed)

Related to openQA Project (public) - action #38486: [functional][u] add capability flags to os-autoinst backends (or tests)Rejectedmgriessmeier2018-07-17

Actions
Related to openQA Tests (public) - action #38423: [sle][functional][u][hard] Refactor first_boot to unify duplicated behavior for remote backendRejectedzluo2018-07-16

Actions
Blocked by openQA Project (public) - coordination #21838: [functional][u][saga] PowerVM backendClosednicksinger2017-08-082018-07-31

Actions
Blocks qe-yam - action #41546: [functional][y] Change architecture checks with functionality of the #33388 to detect VNC installationResolvedsyrianidou_sofia2018-09-25

Actions
Precedes openQA Tests (public) - action #45008: [functional][u] Further improvements on splitting backend codeResolvedzluo2019-04-16

Actions
Actions
Copy link
 #1

Updated by nicksinger almost 7 years ago

  • Copied from action #33340: [tools][functional][u][medium][pvm] Enable graphical installation for the powerVM backend added
Actions
Copy link
 #2

Updated by nicksinger almost 7 years ago

Actions
Copy link
 #3

Updated by nicksinger almost 7 years ago

  • Copied from deleted (action #33340: [tools][functional][u][medium][pvm] Enable graphical installation for the powerVM backend)
Actions
Copy link
 #4

Updated by nicksinger almost 7 years ago

  • Copied to action #33697: [tools][hard][pvm] Enable the powerVM backend to conduct multimachine tests added
Actions
Copy link
 #5

Updated by nicksinger almost 7 years ago

  • Copied to deleted (action #33697: [tools][hard][pvm] Enable the powerVM backend to conduct multimachine tests)
Actions
Copy link
 #6

Updated by okurz almost 7 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
Actions
Copy link
 #7

Updated by okurz over 6 years ago

  • Due date set to 2018-08-14
  • Target version changed from Milestone 17 to Milestone 18
Actions
Copy link
 #8

Updated by okurz over 6 years ago

  • Target version changed from Milestone 18 to Milestone 18
Actions
Copy link
 #9

Updated by okurz over 6 years ago

  • Related to action #38486: [functional][u] add capability flags to os-autoinst backends (or tests) added
Actions
Copy link
 #10

Updated by okurz over 6 years ago

  • Related to action #38423: [sle][functional][u][hard] Refactor first_boot to unify duplicated behavior for remote backend added
Actions
Copy link
 #11

Updated by okurz over 6 years ago

  • Copied to coordination #38819: [qe-core][tools][functional][epic] Refactor use of backends added
Actions
Copy link
 #12

Updated by okurz over 6 years ago

  • Copied to deleted (coordination #38819: [qe-core][tools][functional][epic] Refactor use of backends)
Actions
Copy link
 #13

Updated by okurz over 6 years ago

  • Description updated (diff)
  • Status changed from New to Workable
  • Parent task set to #38819
Actions
Copy link
 #14

Updated by okurz over 6 years ago

  • Description updated (diff)
  • Estimated time set to 3.00 h
  • Difficulty set to easy
Actions
Copy link
 #15

Updated by SLindoMansilla over 6 years ago

  • Status changed from Workable to In Progress
  • Assignee set to SLindoMansilla
Actions
Copy link
 #16

Updated by SLindoMansilla over 6 years ago

  • Status changed from In Progress to Feedback
Actions
Copy link
 #17

Updated by okurz over 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

Actions
Copy link
 #18

Updated by SLindoMansilla over 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

Actions
Copy link
 #19

Updated by mgriessmeier over 6 years ago

  • Due date changed from 2018-08-28 to 2018-09-11

PR is still WIP
move due to vacation

Actions
Copy link
 #21

Updated by SLindoMansilla over 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.

Actions
Copy link
 #22

Updated by mgriessmeier over 6 years ago

  • Due date changed from 2018-09-11 to 2018-09-25
Actions
Copy link
 #23

Updated by SLindoMansilla over 6 years ago

  • Status changed from Workable to In Progress
Actions
Copy link
 #24

Updated by SLindoMansilla over 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.

Actions
Copy link
 #25

Updated by riafarov over 6 years ago

  • Start date set to 5000-01-01

due to changes in a related task

Actions
Copy link
 #26

Updated by riafarov over 6 years ago

  • Blocks action #41546: [functional][y] Change architecture checks with functionality of the #33388 to detect VNC installation added
Actions
Copy link
 #27

Updated by okurz over 6 years ago

Wow, not sure if we want to wait that long ;)

Actions
Copy link
 #28

Updated by okurz over 6 years ago

  • Start date set to 5000-01-01

due to changes in a related task

Actions
Copy link
 #29

Updated by okurz over 6 years ago

  • Start date set to 5000-01-01

due to changes in a related task

Actions
Copy link
 #30

Updated by okurz over 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)
Actions
Copy link
 #31

Updated by okurz about 6 years ago

  • Status changed from In Progress to Workable
  • Assignee deleted (SLindoMansilla)

as discussed in planning, not really working on that right now.

Actions
Copy link
 #32

Updated by szarate about 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

Actions
Copy link
 #33

Updated by okurz about 6 years ago

  • Target version changed from Milestone 20 to Milestone 21
Actions
Copy link
 #35

Updated by okurz about 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 :)

Actions
Copy link
 #36

Updated by szarate about 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.

Actions
Copy link
 #37

Updated by szarate about 6 years ago

  • Project changed from openQA Project (public) to openQA Tests (public)
  • Category changed from 132 to Enhancement to existing tests

Also, this belongs to test code, not backend.

Actions
Copy link
 #38

Updated by okurz about 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.

Actions
Copy link
 #39

Updated by szarate about 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.

Actions
Copy link
 #40

Updated by szarate about 6 years ago

  • Status changed from In Progress to Feedback

Setting to feedback, waiting for either merge or more feedback on the PR.

Actions
Copy link
 #41

Updated by szarate about 6 years ago

  • Start date set to 5000-01-01

due to changes in a related task

Actions
Copy link
 #42

Updated by szarate about 6 years ago

  • Precedes action #45008: [functional][u] Further improvements on splitting backend code added
Actions
Copy link
 #43

Updated by okurz almost 6 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.

Actions
Copy link
 #44

Updated by okurz almost 6 years ago

  • Priority changed from Normal to High
  • Target version changed from Milestone 21 to Milestone 22
Actions
Copy link
 #45

Updated by mgriessmeier almost 6 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"

Actions
Copy link
 #46

Updated by szarate almost 6 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

Actions
Copy link
 #47

Updated by okurz almost 6 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

Actions
Copy link
 #48

Updated by okurz almost 6 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.

Actions
Copy link
 #49

Updated by okurz almost 6 years ago

  • Status changed from New to Workable
Actions
Copy link
 #50

Updated by szarate almost 6 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.

Actions
Copy link
 #51

Updated by okurz almost 6 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.

Actions
Copy link
 #52

Updated by mgriessmeier over 5 years ago

  • Target version changed from Milestone 23 to Milestone 24

moving to M24

Actions
Copy link
 #53

Updated by mgriessmeier over 5 years ago

  • Target version changed from Milestone 24 to Milestone 25
Actions
Copy link
 #54

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.

Actions
Copy link
 #55

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

Actions
Copy link
 #56

Updated by zluo over 5 years ago

tests/installation/bootloader.pm
lib/main_common.pm

modified now.

Actions
Copy link
 #57

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.

Actions
Copy link
 #58

Updated by SLindoMansilla over 5 years ago

  • Blocked by action #51725: [s390x][kvm] test fails in bootloader_zkvm added
Actions
Copy link
 #59

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

Actions
Copy link
 #60

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

Actions
Copy link
 #61

Updated by zluo over 5 years ago

mark: need to file a ticket for https://openqa.suse.de/tests/2912470#settings

Actions
Copy link
 #62

Updated by zluo over 5 years ago

check issue with reboot after installation:
https://openqa.suse.de/tests/2915198

done, looks good.

Actions
Copy link
 #64

Updated by zluo over 5 years ago

  • Status changed from In Progress to Feedback
Actions
Copy link
 #66

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.

Actions
Copy link
 #67

Updated by zluo over 5 years ago

https://openqa.suse.de/tests/2933890 system_role works fine.

Actions
Copy link
 #68

Updated by zluo over 5 years ago

  • Status changed from Feedback to In Progress
Actions
Copy link
 #69

Updated by zluo over 5 years ago

  • Status changed from In Progress to Feedback

cannot set as resolved atm.

Actions
Copy link
 #70

Updated by zluo over 5 years ago

  • Blocked by deleted (action #51725: [s390x][kvm] test fails in bootloader_zkvm)
Actions
Copy link
 #71

Updated by zluo over 5 years ago

  • Status changed from Feedback to Resolved
Actions
Copy link
 #72

Updated by osukup over 5 years ago

  • Status changed from Resolved to Feedback
Actions
Copy link
 #73

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.

Actions
Copy link
 #74

Updated by szarate over 5 years ago

  • Assignee changed from zluo to mgriessmeier
Actions
Copy link
 #75

Updated by mgriessmeier over 5 years ago

  • Status changed from Feedback to Resolved
Actions
Copy link

Also available in: Atom PDF