1. We seem to be reinventing the 'log in as root' work a lot - could that not be moved into a function in one of the parent classes or something?

2. What is the reason for the introduction of console_password_required2 and the change to root_logged_in?

3. Using the tag console_f22_installed for the 'did the upgraded release show up' needle seems wrong - it'll be out of date as soon as 22 ships. Why not have it tagged console_upgrade_successful or something, then we can keep adding needles with that tag as appropriate? In that same needle, why bother logging in and catting the release file, when its contents are in the login screen already, above the prompt?

4. Along the same lines, this implementation effectively 'hard codes' the 'from' release in templates. I'd like us to do that better, though I'm not quite sure what the exact best design would be. Perhaps it would be good to derive the 'from' release from the $BUILD value, perhaps defaulting to $BUILD.release minus 1, with a var for making it $BUILD.release minus 2? Then we just have to make sure the test host has an appropriately-named disk image for the given release.

5. The $fedup_url in upgrade_run_minimal.pm is not really sufficiently sophisticated, unfortunately: this test will probably not run as of right now, because the $BUILD for Final TC1 will be 22_Final_TC1, but the directory on the mirrors is 22_TC1. (Another case of us - well, me - and releng not using exactly the same versioning philosophy, by which I mean I'm right and they're idiots. :>) fedfind copes with this, but the fedfind CLI at present doesn't give a convenient way to display just the base mirror path for a given release; I could add that capability, that might be the 'best' way to do this...in fact I might make it possible to output this specific directory (/foo/bar/'generic'_product/arch/os) for any given fedfind 'release', that seems useful in possibly more than one way. That should also make it much more robust for tests that aren't of TCs/RCs (like nightlies). To get the value from fedup I guess we can redirect the command's output to a file and then incorporate the contents of the file into the fedup command (type_string "fedup --network ".$to_version." --instrepo $(cat /tmp/fedfind.output) or whatever). I might try and play with this later today.

Thanks!

https://www.happyassassin.net/cgit/fedfind/commit/?id=47eaa4d9470387737dc0035f8ebf37a589216f88 takes care of providing the 'generic' URL from fedfind.

In D358#6488, @adamwill wrote:

1. We seem to be reinventing the 'log in as root' work a lot - could that not be moved into a function in one of the parent classes or something?

I have not searched the code, but if it really is the same, I think it would be worth moving into a standalone function

2. What is the reason for the introduction of console_password_required2 and the change to root_logged_in?

Check the needles ;) Font/color changes IIRC (...required2), and fix of the area, which was catching some pixels of the line above (root_logged_in). Pro-tip - you can use the @garretrazile's tool from openqa_fedora_tools repo for browsing needles.

3. Using the tag console_f22_installed for the 'did the upgraded release show up' needle seems wrong - it'll be out of date as soon as 22 ships. Why not have it tagged console_upgrade_successful or something, then we can keep adding needles with that tag as appropriate? In that same needle, why bother logging in and catting the release file, when its contents are in the login screen already, above the prompt?

Well, imagine a situation, where you have F22/F23/F24 screens with the same tag (console_upgrade_successful), and the upgrade from 22 to 23 fails, in such a way, that you boot the system to 22. The needle would match, and provide false positive...
Good catch with the contents of release though!

4. Along the same lines, this implementation effectively 'hard codes' the 'from' release in templates...

I'm not really sure what you mean. Could you be more specific?

5. The $fedup_url in upgrade_run_minimal.pm is not really sufficiently sophisticated, unfortunately: this test will probably not run as of right now, because the $BUILD for Final TC1 will be 22_Final_TC1, but the directory on the mirrors is 22_TC1. (Another case of us - well, me - and releng not using exactly the same versioning philosophy, by which I mean I'm right and they're idiots. :>) fedfind copes with this, but the fedfind CLI at present doesn't give a convenient way to display just the base mirror path for a given release; I could add that capability, that might be the 'best' way to do this...in fact I might make it possible to output this specific directory (/foo/bar/'generic'_product/arch/os) for any given fedfind 'release', that seems useful in possibly more than one way. That should also make it much more robust for tests that aren't of TCs/RCs (like nightlies). To get the value from fedup I guess we can redirect the command's output to a file and then incorporate the contents of the file into the fedup command (type_string "fedup --network ".$to_version." --instrepo $(cat /tmp/fedfind.output) or whatever). I might try and play with this later today.

That sounds great!

In D358#6600, @jskladan wrote:

In D358#6488, @adamwill wrote:

3. Using the tag console_f22_installed for the 'did the upgraded release show up' needle seems wrong - it'll be out of date as soon as 22 ships. Why not have it tagged console_upgrade_successful or something, then we can keep adding needles with that tag as appropriate? In that same needle, why bother logging in and catting the release file, when its contents are in the login screen already, above the prompt?

Well, imagine a situation, where you have F22/F23/F24 screens with the same tag (console_upgrade_successful), and the upgrade from 22 to 23 fails, in such a way, that you boot the system to 22. The needle would match, and provide false positive...

Oh yes, good point, I clearly didn't think that one through :)

4. Along the same lines, this implementation effectively 'hard codes' the 'from' release in templates...

I'm not really sure what you mean. Could you be more specific?

Basically I was wanting to make it easier to re-use the test in future, and looking at the templates file, where in this change we simply stick key => "HDD_1", value => "disk_f21.img" right in there. We'd have to go in and change that every release. What I was wondering is whether we can figure a way to have the image name derived from the $BUILD value instead. Your point about knowing whether the upgrade completed is another issue with doing this, though - I guess it's fine to just land it as-is and if I want to propose tweaks to make it smarter I can.

5. The $fedup_url in upgrade_run_minimal.pm is not really sufficiently sophisticated, unfortunately: this test will probably not run as of right now, because the $BUILD for Final TC1 will be 22_Final_TC1, but the directory on the mirrors is 22_TC1. (Another case of us - well, me - and releng not using exactly the same versioning philosophy, by which I mean I'm right and they're idiots. :>) fedfind copes with this, but the fedfind CLI at present doesn't give a convenient way to display just the base mirror path for a given release; I could add that capability, that might be the 'best' way to do this...in fact I might make it possible to output this specific directory (/foo/bar/'generic'_product/arch/os) for any given fedfind 'release', that seems useful in possibly more than one way. That should also make it much more robust for tests that aren't of TCs/RCs (like nightlies). To get the value from fedup I guess we can redirect the command's output to a file and then incorporate the contents of the file into the fedup command (type_string "fedup --network ".$to_version." --instrepo $(cat /tmp/fedfind.output) or whatever). I might try and play with this later today.

That sounds great!

So I did this and shipped a new fedfind...but now I come to test it I realize I need to redirect some progress messages to stderr instead of stdout, so another new release coming :) But basically you'll be able to do fedfind images -r 22 -m Final -c TC1 -g > /tmp/fedfind.out and the output is https://dl.fedoraproject.org/pub/alt/stage/22_TC1/Server (representing the top-level of the 'generic' tree for the release requested), so you'll want something like --instrepo $(cat /tmp/fedfind.output)/".get_var("ARCH")"/os.

OK, with current git master fedfind, you should be able to use the -g thing. I haven't cut a release yet. Split the BUILD var, pass the values you get in as -r, -m, -c, add -g, redirect the output to a file, use the contents of the file as a base to construct the URL.

In D358#6676, @adamwill wrote:

OK, with current git master fedfind, you should be able to use the -g thing. I haven't cut a release yet. Split the BUILD var, pass the values you get in as -r, -m, -c, add -g, redirect the output to a file, use the contents of the file as a base to construct the URL.

In D358#6656, @adamwill wrote:

Basically I was wanting to make it easier to re-use the test in future, and looking at the templates file, where in this change we simply stick key => "HDD_1", value => "disk_f21.img" right in there. We'd have to go in and change that every release. What I was wondering is whether we can figure a way to have the image name derived from the $BUILD value instead. Your point about knowing whether the upgrade completed is another issue with doing this, though - I guess it's fine to just land it as-is and if I want to propose tweaks to make it smarter I can.

I'm not sure that it's possible to do this in an easy, and straightforward way (but I have not investigated the matter too much) - the HDD_1 parameter is used when constructing the qemu command, to attach the right image before the virt machine is started. It could be added to the client.pl call, when sheduling the jobs, but then we'd need to separate those into a group, like you did with (e.g.) the Live tests, and I'm not really sure that is a good way of doing it.

I absolutely understand the inner need to get this done in some kind of universal way, but (at the moment) the time spent on doing that is much greater, than changing the template, and the respective value in the OpenQA's web interface, once every couple of months. Especially since we need to create the image of previously installed system by hand anyway (right now, because of some bugs in virt-builder & selinux interaction).

Now I think about it a bit, it might be easier to have openqa_trigger pass in the fedup URL as a variable, then you don't have to worry about getting fedfind into the test VM. Either is possible, though.

I don't really have time to go deep with it, so I'll merge it as it is (with small hacks here and there) and open new tasks:

1. We seem to be reinventing the 'log in as root' work a lot - could that not be moved into a function in one of the parent classes or something?

T479

5. The $fedup_url in upgrade_run_minimal.pm is not really sufficiently sophisticated,...

T478