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.

Sign up for GitHub

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

Jump to bottom

Add support for new SLE15 system roles 'textmode' and 'minimal' #3718

Merged
merged 2 commits into from
Oct 17, 2017
Merged

Add support for new SLE15 system roles 'textmode' and 'minimal' #3718

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e05348c
Add support for SLE15 'all packages' medium as addon
okurz Oct 15, 2017
d8c8e48
Add support for new SLE15 system roles 'textmode' and 'minimal'
okurz Oct 15, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions lib/registration.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ our @EXPORT = qw(
registration_bootloader_params
yast_scc_registration
skip_registration
SLE15_MODULES
%SLE15_MODULES
%SLE15_DEFAULT_MODULES
Copy link
Member Author

Choose a reason for hiding this comment

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

@yxususe , @rwx788 I don't know why your previous changes worked but do you think this makes problems for suseconnect now?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I guess it's correct way, as you've done it. But it works with both ways.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I can confirm both way worked.

);

# We already have needles with names which are different we would use here
Expand All @@ -43,6 +44,14 @@ our %SLE15_MODULES = (
serverapp => 'Server-Applications'
);

# The expected modules of a default installation per product. Use them if they
# are not preselected, to crosscheck or just recreate automatic selections
# manually
our %SLE15_DEFAULT_MODULES = (
sles => 'base,script,desktop,serverapp',
Copy link
Contributor

Choose a reason for hiding this comment

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

the script in here is actually a bug

Copy link
Member Author

@okurz okurz Oct 16, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

really? which one?

but anyway:

  1. I am just moving the definition here, did not change it
  2. It's a bug anyway (which we track already) that the modules are not preselected, at least when registering against SCC and having a product selected

Copy link
Contributor

Choose a reason for hiding this comment

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

We will discuss this once scc shows the right default modules - no need to discuss this now :)

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, that's what I'm thinking. Anyway I reported a bug about this. Also here we use the same list for the all-packages medium where we don't assume there are any modules pre-selected

sled => 'base,script,desktop'
Copy link
Contributor

Choose a reason for hiding this comment

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

this looks incomplete. It should contain at least productivity as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

true but different story. For now I am just changing the scope of the variable.

);

sub fill_in_registration_data {
my ($addon, $uc_addon);
if (!get_var("HDD_SCC_REGISTERED")) {
Expand Down Expand Up @@ -128,11 +137,7 @@ sub fill_in_registration_data {
if (match_has_tag 'bsc#1056413') {
record_soft_failure('bsc#1056413');
# Add expected modules to select them manually, as not preselected
my %addons = (
sles => 'base,script,desktop,serverapp',
sled => 'base,script,desktop'
);
my $addons = $addons{get_required_var('SLE_PRODUCT')} . (get_var('SCC_ADDONS') ? ',' . get_var('SCC_ADDONS') : '');
my $addons = $SLE15_DEFAULT_MODULES{get_required_var('SLE_PRODUCT')} . (get_var('SCC_ADDONS') ? ',' . get_var('SCC_ADDONS') : '');
set_var('SCC_ADDONS', $addons);
}
}
Expand Down
2 changes: 2 additions & 0 deletions products/sle/main.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ sub default_desktop {
return 'gnome' if get_var('BASE_VERSION', '') =~ /^12/;
# In sle15 we add repos manually to make a workaround of missing SCC, gnome will be installed as default system.
return 'gnome' if get_var('ADDONURL', '') =~ /(desktop|server)/;
return 'textmode' if (get_var('SYSTEM_ROLE') && !check_var('SYSTEM_ROLE', 'default'));
return 'gnome' if check_var_array('ADDONS', 'all-packages');
Copy link
Contributor

Choose a reason for hiding this comment

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

this is strange reasoning. If you use the packages DVD or not has no relevance to the default system role - only what modules you pick from it.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, this is what is defined by 'ADDONS=all-packages'. ADDONS is a test variable so it is our decision within tests what we do with it. We could of course layer this on the second layer and select modules from the all packages medium based on the content of another variable but right now I don't see the need for it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer if you could set SYSTEM_ROLE=default from within handle_all_packages_medium and depend on this variable only. The whole logic in here grows just too much - the ADDONURL match is already too much if you ask me.

But I'm just saying

Copy link
Member Author

Choose a reason for hiding this comment

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

problem is the test schedule evaluation so evaluating SYSTEM_ROLE within handle_all_packages_medium is too late to influence the schedule. I hear what you are saying but honestly I like the default_desktop method better than the previous approach of hiding the expected desktop somewhere spread.

return (get_var('SCC_REGISTER') && !check_var('SCC_REGISTER', 'installation')) ? 'textmode' : 'gnome';
}

Expand Down
51 changes: 34 additions & 17 deletions tests/installation/addon_products_sle.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,38 @@ use base "y2logsstep";
use testapi;
use utils qw(addon_license sle_version_at_least);
use qam 'advance_installer_window';
use registration '%SLE15_DEFAULT_MODULES';

sub handle_all_packages_medium {
assert_screen 'addon-products-all_packages';
send_key 'alt-s';
# We could reuse SCC_MODULES or another list. For now just hardcode what
# corresponds to the selected SLE15 product because the "all packages"
# addon medium and feature of installer is only available for SLE >= 15
# anyway
foreach (split(/,/, $SLE15_DEFAULT_MODULES{get_required_var('SLE_PRODUCT')})) {
send_key 'home';
send_key_until_needlematch "addon-products-all_packages-$_-highlighted", 'down';
send_key 'spc';
}
send_key $cmd{next};
}

sub handle_addon {
my ($addon) = @_;
return handle_all_packages_medium if $addon eq 'all-packages';
addon_license($addon);
# might involve some network lookup of products, licenses, etc.
assert_screen 'addon-products', 90;
send_key "tab"; # select addon-products-$addon
wait_still_screen 10;
if (check_var('VIDEOMODE', 'text')) { # textmode need more tabs, depends on add-on count
send_key_until_needlematch "addon-list-selected", 'tab';
}
send_key "pgup";
wait_still_screen 2;
send_key_until_needlematch "addon-products-$addon", 'down';
}

sub run {
my ($self) = @_;
Expand All @@ -27,12 +59,7 @@ sub run {
$self->process_unsigned_files([qw(inst-addon addon-products)]);
assert_screen [qw(inst-addon addon-products)];
if (get_var("ADDONS")) {
if (match_has_tag('inst-addon')) {
send_key 'alt-k'; # install with addons
}
else {
send_key 'alt-a';
}
send_key match_has_tag('inst-addon') ? 'alt-k' : 'alt-a';
# the ISO_X variables must match the ADDONS list
my $sr_number = 0;
for my $addon (split(/,/, get_var('ADDONS'))) {
Expand All @@ -44,17 +71,7 @@ sub run {
send_key_until_needlematch 'addon-dvd-list', 'tab', 5; # jump into addon list
send_key_until_needlematch "addon-dvd-sr$sr_number", 'down', 10; # select addon in list
send_key 'alt-o'; # continue
addon_license($addon);
# might involve some network lookup of products, licenses, etc.
assert_screen 'addon-products', 90;
send_key "tab"; # select addon-products-$addon
wait_still_screen 10;
if (check_var('VIDEOMODE', 'text')) { # textmode need more tabs, depends on add-on count
send_key_until_needlematch "addon-list-selected", 'tab';
}
send_key "pgup";
wait_still_screen 2;
send_key_until_needlematch "addon-products-$addon", 'down';
handle_addon($addon);
if ((split(/,/, get_var('ADDONS')))[-1] ne $addon) { # if $addon is not first from all ADDONS
send_key 'alt-a'; # add another add-on
}
Expand Down
2 changes: 1 addition & 1 deletion tests/installation/releasenotes.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ sub run {
}

# no release-notes for WE and all modules
my @no_relnotes = qw(we lgm asmm certm contm pcm tcm wsm hpcm ids idu phub);
my @no_relnotes = qw(we lgm asmm certm contm pcm tcm wsm hpcm ids idu phub all-packages);

# No release-notes for basic modules on SLE 15
if (sle_version_at_least('15') && check_var('DISTRI', 'sle')) {
Expand Down
34 changes: 23 additions & 11 deletions tests/installation/system_role.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,33 +1,45 @@
# SUSE's openQA tests
#
# Copyright © 2016 SUSE LLC
# Copyright © 2016-2017 SUSE LLC
#
# Copying and distribution of this file, with or without modification,
# are permitted in any medium without royalty provided the copyright
# notice and this notice are preserved. This file is offered as-is,
# without any warranty.

# Summary: Check system role selection screen. Added in SLE 12 SP2
# Summary: Check system role selection screen or select system role. Added in SLE 12 SP2
# Maintainer: Jozef Pupava <jpupava@suse.com>
# Tags: poo#16650, poo#25850

use strict;
use base "y2logsstep";
use testapi;
use utils 'sle_version_at_least';


my %role_hotkey = (
default => 's', # sles with gnome
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, relying on hot keys on this screen sounds dangerous. One more system role and yast will reshuffle everything. For now we're lucky though as all system roles start with unique and unused letters :)

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, so what? I don't see a problem with that. When the roles change we want to update this. Now we need to update it. But otherwise we would have needles so everything is in vain … as is life ;)

textmode => 't',
minimal => 'm',
kvm => 'k',
xen => 'x',
);

sub change_system_role {
my ($system_role) = @_;
send_key 'alt-' . $role_hotkey{$system_role};
assert_screen "system-role-$system_role-selected";
# every system role other than default will end up in textmode
set_var('DESKTOP', 'textmode');
}

sub assert_system_role {
# Still initializing the system at this point, can take some time
assert_screen 'system-role-default-system', 180;
# Pick System Role; poo#16650
if (check_var('SYSTEM_ROLE', 'kvm')) {
send_key 'alt-k';
assert_screen 'system-role-kvm-virthost';
}
elsif (check_var('SYSTEM_ROLE', 'xen')) {
send_key 'alt-x';
assert_screen 'system-role-xen-virthost';
my $system_role = get_var('SYSTEM_ROLE', 'default');
if (get_var('SYSTEM_ROLE') && !check_var('SYSTEM_ROLE', 'default')) {
change_system_role($system_role);
}

send_key $cmd{next};
}

Expand Down