action #104520
coordination #109668: [saga][epic] Stable and updated non-qemu backends for SLE validation
coordination #109656: [epic] Stable non-qemu backends
coordination #105699: [epic] 5 whys follow-up to s390x svirt jobs incomplete with unable to extract assets:.*/var/lib/libvirt/images/a.img" size:S
Move svirt extract_asset code from os-autoinst-distri-opensuse to os-autoinst/backend/svirt.pm size:M auto_review:"unable to extract assets: Can't call method.+name.+on an undefined value":retry
0%
Description
Observation¶
The test https://openqa.suse.de/tests/7917723 failed because the code was changed to always execute do_extract_assets
when there were PUBLISH_HDD_*
settings.
However, do_extract_assets
is only implemented in 3 backend classes in os-autoinst currently.
The test in question, svirt_upload_assets.pm
implements extract_assets itself, and it does this using the same special setting PUBLISH_HDD_*
as os-autoinst:
https://github.com/os-autoinst/os-autoinst-distri-opensuse/blob/master/tests/shutdown/svirt_upload_assets.pm
This only worked because PUBLISH_HDD_ was simply ignored for anything but qemu so far.
The code above should be moved to backend::svirt, and after that we can hopefully remove the check for qemu.
Acceptance criteria¶
- AC1: Assets for svirt are published via os-autoinst backend code
Suggestions¶
- Remove svirt_upload_assets from the schedule after deploying the new backend code
- Early return in svirt_upload_assets if the backend method is implement
if backend::svirt->can('do_extract_assets')
Related issues
History
#1
Updated by tinita 5 months ago
- Copied from action #104499: s390 tests fail with backend died: unable to extract assets: Too many arguments for subroutine 'backend::baseclass::do_extract_assets added
#3
Updated by cdywan 5 months ago
I changed my existing PR to an untested proof of concept https://github.com/os-autoinst/os-autoinst/pull/1895
#5
Updated by cdywan 4 months ago
- Status changed from New to Feedback
- Assignee set to cdywan
tinita wrote:
Is this in progress?
I guess it's in Feedback. Wasn't consciously my intention to take it right away, but then I started reaching out to relevant experts to make sure we know what we're doing here.
#7
Updated by cdywan 4 months ago
- Status changed from Feedback to In Progress
- Discussed on Slack
- Make
svirt_upload_assets
a no-op if the backend implements the functionality https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/13974 - Implement
do_extract_assets
for svirt https://github.com/os-autoinst/os-autoinst/pull/1895
#8
Updated by cdywan 4 months ago
cdywan wrote:
- Make
svirt_upload_assets
a no-op if the backend implements the functionality https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/13974
./script/openqa-clone-job --within-instance http://openqa.suse.de --skip-deps 7917723 _GROUP=0 CASEDIR="https://github.com/kalikiana/os-autoinst-distri-opensuse.git#svirt_upload_assets_skip" Created job #7968548: sle-12-SP4-Server-DVD-Updates-s390x-Build20211229-1-mru-install-minimal-with-addons@s390x-kvm-sle12 -> http://openqa.suse.de/t7968548
#9
Updated by cdywan 4 months ago
- Related to action #76813: [tools] Test using svirt backend fails with auto_review:"Error connecting to VNC server.*: IO::Socket::INET: connect: Connection refused":retry added
#11
Updated by cdywan 4 months ago
I put some more verbose thoughts into review comments. Currently I'd describe my progress as follows:
- I'd like to use TDD here, since there was some confusion on the expected behavior of the function, but I'm unsure from reading e.g. qemu code what exactly is expected (e.g. why would svirt need its own image path handling and uploading) and we don't seem to have a test specifically for
do_extract_assets
. - I tried to re-use the
Qemu
module to avoid duplication but I'm not sure it really makes sense to set it up just to convert images - so I'm thinking to split out Qemu:LDriveDevice::gen_qemu_img_convert, if that's all we need
#12
Updated by okurz 4 months ago
- Related to action #99663: Use more perl signatures - os-autoinst size:M added
#13
Updated by cdywan 4 months ago
- Due date changed from 2022-01-21 to 2022-01-28
Moved from "defined" to "can" now, made the unit test work and added docs for the variables which is now required. Bumping the due date since this https://github.com/os-autoinst/os-autoinst-distri-opensuse/pull/13974 also hasn't been merged yet.
#14
Updated by cdywan 4 months ago
Unfortunately it looks like something went wrong and causes failures in jobs now:
console svirt does not exist at /usr/lib/os-autoinst/backend/baseclass.pm line 686. backend::baseclass::console(backend::svirt=HASH(0x10042805f70), "svirt") called at /usr/lib/os-autoinst/backend/svirt.pm line 181 backend::svirt::do_extract_assets(backend::svirt=HASH(0x10042805f70), HASH(0x10042349640)) called at /usr/lib/os-autoinst/backend/driver.pm line 80 backend::driver::extract_assets(backend::driver=HASH(0x10042912848), HASH(0x10042349640)) called at /usr/lib/os-autoinst/OpenQA/Isotovideo/Utils.pm line 163 eval {...} called at /usr/lib/os-autoinst/OpenQA/Isotovideo/Utils.pm line 163 OpenQA::Isotovideo::Utils::handle_generated_assets(OpenQA::Isotovideo::CommandHandler=HASH(0x10042cb0e78), 1) called at /usr/bin/isotovideo line 409 [2022-01-26T08:26:33.975697+01:00] [warn] !!! bmwqemu::serialize_state: unable to extract assets: Can't call method "name" on an undefined value at /usr/lib/os-autoinst/backend/svirt.pm line 181. backend::svirt::do_extract_assets(backend::svirt=HASH(0x10042805f70), HASH(0x10042349640)) called at /usr/lib/os-autoinst/backend/driver.pm line 80 backend::driver::extract_assets(backend::driver=HASH(0x10042912848), HASH(0x10042349640)) called at /usr/lib/os-autoinst/OpenQA/Isotovideo/Utils.pm line 163 eval {...} called at /usr/lib/os-autoinst/OpenQA/Isotovideo/Utils.pm line 163 OpenQA::Isotovideo::Utils::handle_generated_assets(OpenQA::Isotovideo::CommandHandler=HASH(0x10042cb0e78), 1) called at /usr/bin/isotovideo line 409
Edit: Rollback triggered, will propose a fix shortly.
#15
Updated by cdywan 4 months ago
- Subject changed from Move svirt extract_asset code from os-autoinst-distri-opensuse to os-autoinst/backend/svirt.pm size:M to Move svirt extract_asset code from os-autoinst-distri-opensuse to os-autoinst/backend/svirt.pm size:M auto_review:"unable to extract assets: Can't call method.+name.+on an undefined value":retry
#17
Updated by cdywan 4 months ago
tinita wrote:
btw, do we still need the
auto_review:"unable to extract assets: Can't call method.+name.+on an undefined value":retry
in the title?
edit: oh sorry, you just added it
For the record, I used host=openqa.suse.de ./openqa-monitor-incompletes
to identify affected jobs anyway but put the regex in case I missed one.
The above branch now has test coverage for all uses of the "vmname" and addresses the problem by not relying on the console when assets are being prepared.
#19
Updated by okurz 4 months ago
- Related to action #105690: s390x svirt jobs incomplete with auto_review:"unable to extract assets:.*/var/lib/libvirt/images/a.img":retry added
#20
Updated by cdywan 4 months ago
- Blocked by coordination #105699: [epic] 5 whys follow-up to s390x svirt jobs incomplete with unable to extract assets:.*/var/lib/libvirt/images/a.img" size:S added