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

wicked: Upload files via serial console #7565

Merged
merged 3 commits into from
Jul 16, 2019

Conversation

cfconrad
Copy link
Contributor

In wicked testsuites, we often have the case, that we would like to transfer files between worker and SUT without having a working network connection network.
We are using a second virtio-terminal for it by adding VIRTIO_CONSOLE_NUM=2.

This PR needs following changes deployed:


sub run {
my ($self, $ctx) = @_;
$self->select_serial_terminal;
add_serial_console('hvc1');
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 could call it somewhere generic with a condition of BACKEND==qemu and VIRTIO_CONSOLE_NUM>1 and is SLES. But I didn't found a good place.

lib/serial_terminal.pm Outdated Show resolved Hide resolved
@jlausuch
Copy link
Contributor

Impressive job! Just a few small modifications and good to go.

@cfconrad cfconrad force-pushed the wicked_offline_upload branch 3 times, most recently from 83c500d to f1bdebd Compare June 3, 2019 12:55
@cfconrad cfconrad force-pushed the wicked_offline_upload branch 2 times, most recently from 611eea4 to 825ad64 Compare July 11, 2019 09:35
@cfconrad cfconrad changed the title [WIP] wicked: Upload files via serial console wicked: Upload files via serial console Jul 11, 2019
@cfconrad
Copy link
Contributor Author

VR with latest os-autoinst: http://cfconrad-vm.qa.suse.de/tests/4941#step/t08_setup_second_card/127

The testsuite needs to add the VIRTIO_CONSOLE_NUM=2 variable.

@cfconrad
Copy link
Contributor Author

This http://cfconrad-vm.qa.suse.de/tests/4945/file/serial_terminal.txt is how it looks if the job was run with VIRTIO_CONSOLE_NUM<2. It sill works but the base64 stuff is visible in serial.txt.

lib/wickedbase.pm Outdated Show resolved Hide resolved
@cfconrad
Copy link
Contributor Author

Sandbox VR with download and upload http://cfconrad-vm.qa.suse.de/tests/4955

@jlausuch
Copy link
Contributor

@rwx788 @okurz @coolgw @foursixnine and others, please review this new option to transfer files from/to OpenQA VM via serial. Useful when there is no network configured in the VM (i.e. networking tests).

lib/serial_terminal.pm Outdated Show resolved Hide resolved
lib/serial_terminal.pm Outdated Show resolved Hide resolved
@asmorodskyi
Copy link
Member

LGTM beside to minor comments which I did before

Copy link
Member

@rwx788 rwx788 left a comment

Choose a reason for hiding this comment

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

Looks very promising! Well done!

Copy link
Member

@foursixnine foursixnine left a comment

Choose a reason for hiding this comment

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

Looks really good!

I recently came across the qemu -drive file=fat:rw:some/directory ... which would expose a drive on the qemu guest. (But has some drawbacks regarding the filesystem + needing to mount the disk et al)... Not to question your idea, but did you check that one also?

lib/serial_terminal.pm Outdated Show resolved Hide resolved
lib/serial_terminal.pm Outdated Show resolved Hide resolved
lib/serial_terminal.pm Show resolved Hide resolved
lib/serial_terminal.pm Outdated Show resolved Hide resolved
@@ -20,6 +20,8 @@ use testapi qw(is_serial_terminal :DEFAULT);
use serial_terminal;
use Carp;
use Mojo::File 'path';
use serial_terminal 'upload_file';
Copy link
Member

Choose a reason for hiding this comment

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

You are already exporting it by default, and in line 20 it is already imported :D

Copy link
Contributor Author

@cfconrad cfconrad Jul 15, 2019

Choose a reason for hiding this comment

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

Oh, thx I overseen it :)
What do you mean with exporting it by default? I only see @EXPORT in serial_terminal.pm.

Copy link
Member

Choose a reason for hiding this comment

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

Any symbol inside @EXPORT will be imported when use Foo; is used :), @EXPORT_OK on the other hand behaves a little bit different, as it will allow the module to export those symbols, but on demand (use Mojo::File qw(path) for example...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@foursixnine thx for pointing me at this. I twisted it.
What would you recommend, using the :DEFAULT export as long as it exists?

tests/wicked/before_test.pm Show resolved Hide resolved
@cfconrad
Copy link
Contributor Author

I recently came across the qemu -drive file=fat:rw:some/directory ... which would expose a drive on the qemu guest. (But has some drawbacks regarding the filesystem + needing to mount the disk et al)... Not to question your idea, but did you check that one also?

We thought about that too. But we took this approach as this will work, as long as you have a "almost silent" serial console. The -drive approach is backend depended.

VIRTIO_CONSOLE_NUM is used on QEMU backend, to enable more
virtio-consoles. We need to add these consoles also in os-autoinst so
the test can communicate with them.
Using serial terminal to transfer files between worker and SUT
Use serial_file::upload_file() for uploading log files. This avoid the
need of working network after test.

Allow upload files via serial on first console:
If the test was run with VIRTIO_CONSOLE_NUM < 2, the upload should still
work.
The disadvantage is, that the file is visible on serial.txt in base64
format.
Copy link
Member

@foursixnine foursixnine left a comment

Choose a reason for hiding this comment

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

Looks fine for me

@foursixnine foursixnine merged commit 947b62a into os-autoinst:master Jul 16, 2019
@cfconrad cfconrad deleted the wicked_offline_upload branch July 16, 2019 08:25
@cfconrad cfconrad mentioned this pull request Jul 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants