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

zKVM: Add capability for Upgrades and booting images #2196

Merged
merged 3 commits into from
Dec 9, 2016

Conversation

Soulofdestiny
Copy link
Contributor

No description provided.

@Soulofdestiny
Copy link
Contributor Author

requires: os-autoinst/os-autoinst#661

Verification for Upgrade: http://10.160.5.109/tests/4336

may need some refactoring, feedback welcome - therefor the notready label -> @mnowaksuse @coolo


# temporary use of hardcoded '+4' to workaround messed up network setup on z/KVM
my $vtap = $svirt->instance + 4;
my $repo = "ftp://openqa.suse.de/" . get_var('REPO_0');
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it is a good idea to hardcode this here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but use what instead? I'm not aware of any variable providing this

Copy link
Member

Choose a reason for hiding this comment

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

Maybe using a jobsetting: e.g get_var("SUSEMIRROR") which is used in few places, might be wise to keep using that one... but i'm guessing in most cases it is not present but only http|https is supported (According to bmwqemu)

Copy link
Member

Choose a reason for hiding this comment

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

ok, it's only moved here so fine for this PR but
@Soulofdestiny please replace the hardcoded values in all test files accordingly. I am awaiting your PR then :-)

Copy link
Contributor Author

@Soulofdestiny Soulofdestiny Dec 9, 2016

Choose a reason for hiding this comment

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

well, found one which should be always set...

@@ -21,6 +21,12 @@ sub run() {
record_soft_failure 'workaround missing release notes';
return;
}

# workaround for bsc#1014178
if (check_screen('zfcp-popup')) {
Copy link
Member

Choose a reason for hiding this comment

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

this will waste 30 seconds in every run unless the screen is shown. Either make this a check_screen …, 0; or use a multi-tag assert_screen in a later step

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agreed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed


# clean up on s390pb
type_string("rm -f /var/lib/libvirt/images/$name && echo 'OK'\n");
assert_screen('svirt-image-cleaned-up', 30);
Copy link
Member

Choose a reason for hiding this comment

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

leave out default timeout

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agreed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -58,6 +58,10 @@ sub extract_assets {

upload_asset("/var/lib/libvirt/images/$name", 1, 1);
assert_screen('svirt-asset-upload-hdd-image-uploaded', 1000);

# clean up on s390pb
type_string("rm -f /var/lib/libvirt/images/$name && echo 'OK'\n");
Copy link
Member

Choose a reason for hiding this comment

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

why not just

assert_script_run "rm -f /var/lib/libvirt/images/$name";

Copy link
Contributor Author

Choose a reason for hiding this comment

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

generally agreed, just want to stick to the previous ones to keep it consistent

Copy link
Contributor

Choose a reason for hiding this comment

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

this code does not run on the SUT but on the hypervisor, so it has no access to the serial console of the SUT.

We have limited space on s390pb, so we can't store every single image there
Let's remove it after it got uploaded to the worker
@Soulofdestiny
Copy link
Contributor Author

}
else {
loadtest "installation/bootloader_svirt";
}
Copy link
Member

Choose a reason for hiding this comment

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

that's not how it should be, please open a progress issue to improve the two bootloader implementations.

DRY. You are calling the same code multiple times, please extract method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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


# temporary use of hardcoded '+4' to workaround messed up network setup on z/KVM
my $vtap = $svirt->instance + 4;
my $repo = "ftp://openqa.suse.de/" . get_var('REPO_0');
Copy link
Member

Choose a reason for hiding this comment

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

ok, it's only moved here so fine for this PR but
@Soulofdestiny please replace the hardcoded values in all test files accordingly. I am awaiting your PR then :-)

@okurz okurz merged commit eb7d612 into os-autoinst:master Dec 9, 2016
@Soulofdestiny Soulofdestiny deleted the feature/zkvm_boot_existing branch February 14, 2017 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants