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 default modules for SLES15 media migration #4753
Conversation
@@ -15,7 +15,7 @@ use strict; | |||
use base "y2logsstep"; | |||
use testapi; | |||
use utils 'addon_license'; | |||
use version_utils 'is_sle'; | |||
use version_utils qw/is_sle sle_version_at_least/; |
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.
should be use qw(A B) format, the same as line 54
@@ -43,6 +43,19 @@ sub handle_all_packages_medium { | |||
# The legacy module is required if upgrade from previous version (bsc#1066338) | |||
push @addons, 'legacy' if get_var('UPGRADE') && !grep(/^legacy$/, @addons); | |||
|
|||
# For SLES12SPx and SLES11SPx to SLES15 migration, follow modules are need at least |
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.
Should we restrict the change to SLES? Otherwise the media upgrade should be failed for SLED 12-SP3 -> 15
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.
add sled check in line 53
# SLE-Module-Server-Applications15 | ||
# SLE-Module-Web-Scripting15 | ||
if (get_var('UPGRADE') && !sle_version_at_least('15', version_variable => 'HDDVERSION')) { | ||
my @default_addon = qw/contm desktop legacy serverapp script/; |
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 variable name "default_addon" is some bit confusing, here you try to define a list of modules which are required to upgrade SLE 11/12 to 15. Please think about it.
# SLE-Module-Legacy15 | ||
# SLE-Module-Server-Applications15 | ||
# SLE-Module-Web-Scripting15 | ||
if (get_var('UPGRADE') && !sle_version_at_least('15', version_variable => 'HDDVERSION')) { |
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.
Seems sle_version_at_least is already deprecated, is there another way to do this check?
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.
is_sle doesn't support setup version_variable, such as version_variable => 'HDDVERSION'
.
I suppose update is_sle
is a better solution for it.
# SLE-Module-Legacy15 | ||
# SLE-Module-Server-Applications15 | ||
# SLE-Module-Web-Scripting15 | ||
if (get_var('UPGRADE') && !sle_version_at_least('15', version_variable => 'HDDVERSION') && !check_var('SLE_PRODUCT', 'sled')) { |
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.
@ldevulder @alvarocarvajald @asmorodskyi comments on this? Specifically the latest portion about !sled, maybe the same applies for other products?
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.
hard to say , it's related to upgrade only. and I have no information about SLE 12 SP3 with HPC module to SLE 15 HPC upgrade . what modules should / may be selected there. Not saying that it is normal , just stating the fact. And based on this statement - I am ok with this PR
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.
For sles4sap
we need base,desktop,serverapp,sapapp,ha
, but currently we don't have migration tests in place in o.s.d., so its inclusion into this part of the code can wait for the time being ... but if you don't mind adding it now, it will helps us in the long run.
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.
Same as @alvarocarvajald for HA, no upgrade tests in o.s.d. at this time. Should be added in the future.
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.
ok then we can keep it as is, no over-engineering without any tests
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.
No upgrade test for sles4sap product yet.
But HA extension is already covered in migration testing, for example:
media_upgrade_sles11sp4+ha+geo@64bit : https://openqa.suse.de/tests/1604474
media_upgrade_sles12sp3+ha+geo_allpatterns@64bit : https://openqa.suse.de/tests/1604110
proxyscc_upgrade_sles12sp3+ha+geo@64bit : https://openqa.suse.de/tests/1604102
You can find more upgrade tests for HA in SLE 15 job groups:
https://openqa.suse.de/group_overview/111
https://openqa.suse.de/group_overview/145
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.
@QingmingSu, thx! I will try to re-use these tests. The only think I have to do is to publish a HDD from these tests to after re-use it with all the HA stack tests.
# SLE-Module-Desktop-Applications15 | ||
# SLE-Module-Legacy15 | ||
# SLE-Module-Server-Applications15 | ||
# SLE-Module-Web-Scripting15 |
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 suggest to not spell out the modules in a comment. If the situation will change the list below will change but I fear someone will forget to update the comments. I suggest to remove this list from the comment
# SLE-Module-Server-Applications15 | ||
# SLE-Module-Web-Scripting15 | ||
if (get_var('UPGRADE') && !sle_version_at_least('15', version_variable => 'HDDVERSION') && !check_var('SLE_PRODUCT', 'sled')) { | ||
my @demand_addon = qw(contm desktop legacy serverapp script); |
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 wonder if this is a good place to define (yet another) list of modules. Should we maybe put all lists together in products/sle/main.pm ?
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 agree we should define a list for all modules. But in this case, firstly those domand module is only be needed for media migration, it is not a common list for all prodcut upgrade. And secondly this list should be a tempory one, we don't have a fixed list for sles15 media migraion right now. So I suppose we can add it here for now,.
Add follow modules for SLES12SP&SLSE11 to SLES15 migration SLE-Product-SLES15 SLE-Module-Basesystem15 SLE-Module-Containers15 SLE-Module-Desktop-Applications15 SLE-Module-Legacy15 SLE-Module-Server-Applications15 SLE-Module-Web-Scripting15 -see poo#34021
ok, let's go ahead with it. The extension of |
Add follow modules for SLES12SPx&SLSE11 to SLES15 migration
SLE-Product-SLES15
SLE-Module-Basesystem15
SLE-Module-Containers15
SLE-Module-Desktop-Applications15
SLE-Module-Legacy15
SLE-Module-Server-Applications15
SLE-Module-Web-Scripting15
See poo#34021
Related ticket: https://progress.opensuse.org/issues/34021
Verification run: http://10.67.17.147/tests/2256