fix downloading assets to different filename

[garretraziel]

I've just found out that it's not possible to set both ASSET and ASSET_URL to download asset from given URL, but then use different filename. This fixes it.

[aaannz]

Right, tidy failed and can you come up with some test for this feature?

[garretraziel]

Done.

[aaannz]

Is the asset file (t/data/openqa/factory/other/vmlinuz.img.20160516) needed for the test?

[aaannz]

Otherwise LGTM

[garretraziel]

Hmm, it was because asset_register function from Scheduler.pm script was showing warning about missing file vmlinuz.img.20160516, but I thought that it's caused by some weird mocking that is happening in testing environment. But I've just tested it and this same warning is shown during real run - if I post both "KERNEL_URL" and "KERNEL", it shows error about missing file in "asset_register", but then it downloads file from "KERNEL_URL" and correctly saves it to filename from "KERNEL". I can remove that asset file from test and add asset name 'vmlinuz.img.20160516' invalid or does not exist as expected warning instead, but question is if there isn't some bigger problem with registering assets.

[aaannz]

I think OpenQA::Utils::asset_type_from_setting should be adapted not to register assets if both asset name and asset _URL is set. And I guess this should work for numbered assets too (i.e. ISO_1_URL and ISO_1 set at the same time)

[coveralls]

Coverage increased (+0.03%) to 64.844% when pulling 403ce25 on garretraziel:master into abbc935 on os-autoinst:master.

[coveralls]

Coverage increased (+0.03%) to 64.844% when pulling 403ce25 on garretraziel:master into abbc935 on os-autoinst:master.

[garretraziel]

Either way, it shows warning about missing file but it works, so I've just added this warning as expected and removed unnecessary asset.

[aaannz]

I have to disagree with you. This shouldn't be expected warning. But instead of fixing this early attempt to register assets I would remove it and just rely on job_grab`s asset registration.
@coolo do you know what was the use case to register assets early? The comment mentions only that when no jobs are generated no assets would be registered, which seems fine to me.

[coolo]

It's fine in this scenario, but horrible in our first rsync, then schedule scenario. Because it leaves all the assets that never actually ran tests (but got obsoleted by next build) without any reference in the database - which means they are dead files filling disk space without the GRU having a chance to know which job group's limit they should apply to.

[AdamWill]

hey, folks (@aaannz @coolo ), @garretraziel is still waiting for this; are you waiting for him to do something?

[sysrich]

I believe @coolo 's comment might have been a request to ensure that all assets are referenced in the database so that they can be cleaned up afterwards..at least, that's why I haven't merged this yet..

@aaannz are you working on this?

[aaannz]

I am not working on this. @coolo answered why early registration can't be removed, but in that case I stand by this comment to not register asset when both VAR and VAR_URL is provided.
When someone in the future will be debugging why her assets aren't showing up, warning about invalid asset (we already know is false positive) will be misleading.

[coolo]

I assigned the issue to @aaannz to signal that you have to convince him before this is merged.

[AdamWill]

@garretraziel can you try to do whatever is necessary to make @aaannz happy here? This is causing us problems in production now, because vmlinuz and initrd.img are just stored in factory/other on the servers and constantly reused, they are never redownloaded; our F25 ARM tests are using F24 kernel/initramfs at present.

I can do some kind of ugly hack around this (I guess a script that runs hourly and wipes the files if no tests are scheduled/running) but we really need this to fix it properly.

[garretraziel]

I'll try and fix it. I have tried rebasing, but it looks that it doesn't work anymore, so I need to dig deeper into the code.

[garretraziel]

Since I've deleted forked repository and I can't update this pull request anymore, I have created new pull request with the same code in #920.