action #104520
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 over 1 year 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
#2
Updated by tinita over 1 year ago
- Description updated (diff)
#3
Updated by cdywan over 1 year ago
I changed my existing PR to an untested proof of concept https://github.com/os-autoinst/os-autoinst/pull/1895
#4
Updated by tinita over 1 year ago
Is this in progress?
#5
Updated by cdywan over 1 year 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.
#6
Updated by cdywan over 1 year ago
- Subject changed from Move svirt extract_asset code from os-autoinst-distri-opensuse to os-autoinst/backend/svirt.pm to Move svirt extract_asset code from os-autoinst-distri-opensuse to os-autoinst/backend/svirt.pm size:M
- Description updated (diff)
#7
Updated by cdywan over 1 year 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 over 1 year 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 over 1 year 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" added
#10
Updated by cdywan over 1 year ago
- Due date changed from 2022-01-13 to 2022-01-21
I'll admit that context switching with topics has slowed me down here, and then I ran into #76813 trying verify the distri-opensuse changes. So I'm bumping the due date now.
#11
Updated by cdywan over 1 year 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 over 1 year ago
- Related to action #99663: Use more perl signatures - os-autoinst size:M added
#13
Updated by cdywan over 1 year 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 over 1 year 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 over 1 year 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
#16
Updated by tinita over 1 year ago
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
#17
Updated by cdywan over 1 year 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.
#18
Updated by cdywan over 1 year ago
- Status changed from In Progress to Feedback
Branch merged, waiting to confirm that this is working fine in production
#19
Updated by okurz over 1 year 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 over 1 year 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
#21
Updated by cdywan over 1 year ago
- Due date deleted (
2022-01-28) - Status changed from Feedback to Blocked
The obvious next step would be to understand where the unit tests deviate from production since the vmname was not initialized as expected. But we should conduct #105690 first.
#22
Updated by okurz over 1 year ago
- Parent task set to #105699
#23
Updated by cdywan over 1 year ago
- Status changed from Blocked to Workable
- Assignee deleted (
cdywan)
#24
Updated by okurz over 1 year ago
- Priority changed from Normal to Low
#25
Updated by okurz over 1 year ago
- Target version changed from Ready to future