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

Fix SLE11-SP4 Incidents #17196

Merged
merged 2 commits into from Jun 1, 2023
Merged

Conversation

@dzedro dzedro added the qe-core Tag used to filter PR's created by QE-Core's members or are assigned to them label May 31, 2023
@dzedro dzedro requested a review from mdoucha May 31, 2023 14:00
@github-actions
Copy link

Great PR! Please pay attention to the following items before merging:

Files matching lib/**.pm:

  • Consider adding or extending unit tests in t/

This is an automatically generated QA checklist based on modified files.

lib/qam.pm Outdated
my ($url, $name) = @_;
my $gpg = get_var('BUILD') =~ m/^MR:/ ? "-G" : "";
my $system_repos = script_output('zypper lr -u');
unless ($system_repos =~ /$url/) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this regex check could easily break due to unescaped special characters in $url. It'd be safer to call $repolist = zypper_repos('-u') and then grep { $_->{uri} eq $url } @$repolist

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, I don't know, but I don't think there should be any special characters in this urls, but now we will never find out. 😉

lib/qam.pm Outdated
add_repo_if_not_present("http://dist.suse.de/ibs/SUSE/Updates/SLE-Product-SLES/15-SP3-ERICSSON/$arch/update/", '15-SP3-ERICSSON-Updates');
}
elsif (is_sle('=15-SP4')) {
add_repo_if_not_present("http://dist.suse.de/ibs/SUSE/Updates/SLE-Product-SLES/15-SP4-ERICSSON/$arch/update/", '15-SP4-ERICSSON-Updates');
Copy link
Contributor

Choose a reason for hiding this comment

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

When there are so many repos, it'd be cleaner to define an array and loop over it:

my @repo_list = (
    {cond => '=12-SP2', name => '12-SP2-LTSS-ERICSSON-Updates', uri => '...'},
    {...}
);

for my $repo in (@repo_list) {
    add_repo_if_not_present($repo->{uri}, $repo->{name}) if is_sle($repo->{cond});
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had feeling to make it better, this is great. Thank you

}

# https://progress.opensuse.org/issues/90522
sub add_extra_customer_repositories {
Copy link
Member

Choose a reason for hiding this comment

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

I thought this already existed 🧠 ... deja vu!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, only add_test_repositories for updates, but this is "extension" for the extra repos. :)

@foursixnine
Copy link
Member

waiting on @mdoucha's comments

@dzedro dzedro force-pushed the sle11_extreme branch 2 times, most recently from 0186e85 to fb37c20 Compare May 31, 2023 21:43
It was on multiple places and repeating check if the repo is not added already.
Also add ERICSSON SLE15 SP3 & SP4 repos.
@mdoucha
Copy link
Contributor

mdoucha commented Jun 1, 2023

Code looks good but please add one more verification run.

@dzedro
Copy link
Contributor Author

dzedro commented Jun 1, 2023

Code looks good but please add one more verification run.

https://dzedro.suse.cz/tests/1379#step/patch_and_reboot/19
https://dzedro.suse.cz/tests/1375 kernel_update 29128
https://dzedro.suse.cz/tests/1376 kernel_update 29130

Copy link
Contributor

@mdoucha mdoucha left a comment

Choose a reason for hiding this comment

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

LGTM

@foursixnine foursixnine merged commit 17db97c into os-autoinst:master Jun 1, 2023
7 checks passed
@dzedro dzedro deleted the sle11_extreme branch June 1, 2023 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
qe-core Tag used to filter PR's created by QE-Core's members or are assigned to them
Projects
None yet
3 participants