Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Containers: use serial terminal optinally for the tests #10424

Merged
merged 1 commit into from
Jun 3, 2020
Merged

Containers: use serial terminal optinally for the tests #10424

merged 1 commit into from
Jun 3, 2020

Conversation

jlausuch
Copy link
Contributor

@jlausuch jlausuch commented Jun 1, 2020

Using this option we gain 2 things:

  1. Execution time reduced more than half.
  2. Everything is logged to serial_termina.txt

It is possible to disable it by setting VIRTIO_CONSOLE=0.

@jlausuch
Copy link
Contributor Author

jlausuch commented Jun 1, 2020

@pdostal @punkioudi please be aware of this. It's a big change in how tests are executed, so I don't want to convince everyone to use it, that's why it's optional. It won't affect QAM jobs, but you might be interested as well.

@asmorodskyi
Copy link
Member

I thought that we have VIRTIO_TERMINAL to make such decisions , not sure why we need one more variable . Anyway if we will decide to keep it after all I would move this variable inside select_serial_terminal function but generally I think VIRTIO_TERMINAL is enough .
@pevik you might be interested in this discussion

@pevik
Copy link
Contributor

pevik commented Jun 1, 2020

@jlausuch yes, this functionality has been already implemented with VIRTIO_CONSOLE=0
https://github.com/os-autoinst/os-autoinst-distri-opensuse/blob/master/lib/opensusebasetest.pm#L1078

@jlausuch
Copy link
Contributor Author

jlausuch commented Jun 1, 2020

I thought that we have VIRTIO_TERMINAL to make such decisions , not sure why we need one more variable . Anyway if we will decide to keep it after all I would move this variable inside select_serial_terminal function but generally I think VIRTIO_TERMINAL is enough .
@pevik you might be interested in this discussion

I guess you mean VIRTIO_CONSOLE, not VIRTIO_TERMINAL. I thought the same and started using VIRTIO_CONSOLE variable, but then I realized that all container jobs have VIRTIO_CONSOLE=1 See this or this. Does it mean that the variable should be =0 in those jobs?
If so, I will replace all calls select_console("root-console"); to $self->select_serial_terminal and force VIRTIO_CONSOLE=0 in the jobs that are not from us. WDYT?

@pevik
Copy link
Contributor

pevik commented Jun 2, 2020

Virtio console is enabled by default (VIRTIO_CONSOLE=1), because we realized there is really no reason to have it disabled (see os-autoinst/os-autoinst#1045, merged as os-autoinst/os-autoinst@7226ced). You need to explicitly set VIRTIO_CONSOLE=0 to disable it.

I should document this in https://github.com/os-autoinst/os-autoinst-distri-opensuse/blob/master/lib/opensusebasetest.pm#L1078, because in general docs it's somehow reflected, but VIRTIO_CONSOLE=1 is specific to openSUSE/SLES tests https://github.com/os-autoinst/openQA/blob/master/docs/WritingTests.asciidoc#using-a-serial-terminal.

VIRTIO_CONSOLE=1 is set on machine setup on machines 64bit, aarch64-virtio, ppc64le-virtio. But because it's default on, on machines aarch64 and ppc64le it's also enabled (it's not in defined in https://openqa.suse.de/tests/4295384#settings, but it's on https://openqa.suse.de/tests/4295384/file/vars.json).

VIRTIO_CONSOLE is for qemu backed. svirt backend has SERIAL_CONSOLE, which is not enabled by default. Thus from many machines using svirt is enabled only on s390x-kvm-sle15 (and maybe s390x-kvm-sle12), but not on vmware and hyperv (which still doesn't have this support, see https://progress.opensuse.org/issues/55985).

@pevik
Copy link
Contributor

pevik commented Jun 2, 2020

I also improved doc, can you please have a look whether it's really an improvement?

@jlausuch
Copy link
Contributor Author

jlausuch commented Jun 2, 2020

Virtio console is enabled by default (VIRTIO_CONSOLE=1), because we realized there is really no reason to have it disabled (see os-autoinst/os-autoinst#1045, merged as os-autoinst/os-autoinst@7226ced). You need to explicitly set VIRTIO_CONSOLE=0 to disable it.

I should document this in https://github.com/os-autoinst/os-autoinst-distri-opensuse/blob/master/lib/opensusebasetest.pm#L1078, because in general docs it's somehow reflected, but VIRTIO_CONSOLE=1 is specific to openSUSE/SLES tests https://github.com/os-autoinst/openQA/blob/master/docs/WritingTests.asciidoc#using-a-serial-terminal.

VIRTIO_CONSOLE=1 is set on machine setup on machines 64bit, aarch64-virtio, ppc64le-virtio. But because it's default on, on machines aarch64 and ppc64le it's also enabled (it's not in defined in https://openqa.suse.de/tests/4295384#settings, but it's on https://openqa.suse.de/tests/4295384/file/vars.json).

VIRTIO_CONSOLE is for qemu backed. svirt backend has SERIAL_CONSOLE, which is not enabled by default. Thus from many machines using svirt is enabled only on s390x-kvm-sle15 (and maybe s390x-kvm-sle12), but not on vmware and hyperv (which still doesn't have this support, see https://progress.opensuse.org/issues/55985).

Thanks for clarification. I will remove the new variable and update the PR.

@punkioudi
Copy link
Contributor

punkioudi commented Jun 2, 2020

@ggkioulis this might interests you too

Copy link
Contributor

@cfconrad cfconrad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Using this option we gain 2 things:
1) Execution time reduced more than half.
2) Everything is logged to serial_termina.txt

It is possible to disable it by setting VIRTIO_CONSOLE=0.
@@ -30,7 +30,8 @@ use strict;
use warnings;

sub run {
select_console("root-console");
my ($self) = @_;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we excluding it for SLE . does it make sense to touch it ? do we plan also migrate to VIRTIO_CONSOLE=1 in o3 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might include it in the future and all these tests are a common subset of container tests, which I think should be similar the way they are handled.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason why not? BTW where are these tests in o3?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A test called extra_tests_textmode_containers in Tumbleweed builds e.g. https://openqa.opensuse.org/tests/1282756
and Leap https://openqa.opensuse.org/tests/1282504#

@@ -30,7 +30,8 @@ use strict;
use warnings;

sub run {
select_console("root-console");
my ($self) = @_;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason why not? BTW where are these tests in o3?

@jlausuch
Copy link
Contributor Author

jlausuch commented Jun 2, 2020

@Vogtinator @pdostal : Would it be fine for you to merge this PR and if you don't want this feature then set VIRTIO_CONSOLE=0 in QAM and O3 side?

@Vogtinator
Copy link
Member

@Vogtinator @pdostal : Would it be fine for you to merge this PR and if you don't want this feature then set VIRTIO_CONSOLE=0 in QAM and O3 side?

Please add openSUSE TW and Leap 15.1 verification runs. If those pass, LGTM.

@pevik
Copy link
Contributor

pevik commented Jun 2, 2020

@Vogtinator I tested it for openSUSE:

BTW on my host 11 min vs. 33 min => virtio console is 3x faster :).
IMHO virtio console is OK for o3. It has problems on some old SLES versions (SLE12 SP1 maybe, @mimi1vx probably remember which one).

@jlausuch
Copy link
Contributor Author

jlausuch commented Jun 2, 2020

@Vogtinator I tested it for openSUSE:

BTW on my host 11 min vs. 33 min => virtio console is 3x faster :).
IMHO virtio console is OK for o3. It has problems on some old SLES versions (SLE12 SP1 maybe, @mimi1vx probably remember which one).

Thanks! I was about to do it :)
btw, correct link for Tumbleweed: http://quasar.suse.cz/tests/5280

@pevik
Copy link
Contributor

pevik commented Jun 3, 2020

I did it before, because I was myself curious how much the speedup is. Although prefer virtio/serial console anyway (to get text output), but I have to admit for tests with minimal output and long running commands the speedup can be not that significant.

@Vogtinator
Copy link
Member

@Vogtinator I tested it for openSUSE:

BTW on my host 11 min vs. 33 min => virtio console is 3x faster :).

Great!

@asmorodskyi asmorodskyi merged commit a3aa6a4 into os-autoinst:master Jun 3, 2020
@pevik
Copy link
Contributor

pevik commented Jun 3, 2020

@mdoucha @mimi1vx @dzedro FYI not sure if this affect some SLE12 QAM tests. Hope you already have VIRTIO_CONSOLE=0 for isos which need to have virtio disabled.

@jlausuch
Copy link
Contributor Author

jlausuch commented Jun 3, 2020

@mdoucha @mimi1vx @dzedro FYI not sure if this affect some SLE12 QAM tests. Hope you already have VIRTIO_CONSOLE=0 for isos which need to have virtio disabled.

The biggest negative impact is that there will be more frames on the Details view.

@pevik
Copy link
Contributor

pevik commented Jun 3, 2020

@jlausuch FYI I meant impact on SLE12-SP1 ppc64le "util-linux on sle12/sle12sp1 rejects root login on any console which isnt listed in /etc/securetty , from SP2 is this file ignored" (https://progress.opensuse.org/issues/18980#note-14), it has been fixed long time ago for LTP:#2820, #6352 (https://progress.opensuse.org/issues/18980, https://progress.opensuse.org/issues/18980)

but not sure about current status for the other tests, this old @dzedro complain was closed, so I guess correct setup has been done
https://progress.opensuse.org/issues/45842

@jlausuch jlausuch deleted the containers_use_serial branch June 3, 2020 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants