-
Notifications
You must be signed in to change notification settings - Fork 269
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
Add support for new SLE15 system roles 'textmode' and 'minimal' #3718
Add support for new SLE15 system roles 'textmode' and 'minimal' #3718
Conversation
Specify 'all-packages' in test variable 'ADDONS' to add default sles/sled modules using the 'all-packages' medium specified in an ISO. Additional addons specified comma-separated are still supported. Verification run: local, see screenshot SLE needles MR: https://gitlab.suse.de/openqa/os-autoinst-needles-sles/merge_requests/526 Related progress issue: https://progress.opensuse.org/issues/25456
Based on the existing test variable 'SYSTEM_ROLE' we can explicitly select the new SLE15 system roles 'textmode' or 'minimal' as well as the existant ones. As this also influences the initial bootup target the 'default_desktop' method is updated accordingly. Verification local, see screenshots. Related SLE needles MR: https://gitlab.suse.de/openqa/os-autoinst-needles-sles/merge_requests/527 Related progress issues: * minimal: https://progress.opensuse.org/issues/26038 * textmode: https://progress.opensuse.org/issues/26042
Includes #3717 . If I would have needed to separate then there would be a conflicting line in main.pm. If you see a reason to split it, please tell me. |
# are not preselected, to crosscheck or just recreate automatic selections | ||
# manually | ||
our %SLE15_DEFAULT_MODULES = ( | ||
sles => 'base,script,desktop,serverapp', |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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:
- I am just moving the definition here, did not change it
- 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
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
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
# manually | ||
our %SLE15_DEFAULT_MODULES = ( | ||
sles => 'base,script,desktop,serverapp', | ||
sled => 'base,script,desktop' |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
@@ -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'); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
|
||
use strict; | ||
use base "y2logsstep"; | ||
use testapi; | ||
use utils 'sle_version_at_least'; | ||
|
||
|
||
my %role_hotkey = ( | ||
default => 's', # sles with gnome |
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
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 ;)
@@ -29,7 +29,8 @@ our @EXPORT = qw( | |||
registration_bootloader_params | |||
yast_scc_registration | |||
skip_registration | |||
SLE15_MODULES | |||
%SLE15_MODULES | |||
%SLE15_DEFAULT_MODULES |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Based on the existing test variable 'SYSTEM_ROLE' we can explicitly select the
new SLE15 system roles 'textmode' or 'minimal' as well as the existant ones.
As this also influences the initial bootup target the 'default_desktop' method
is updated accordingly.
Verification local, see screenshots:
textmode:
minimal:
Related SLE needles MR:
https://gitlab.suse.de/openqa/os-autoinst-needles-sles/merge_requests/527
Related progress issues: