ospkgs: fix checks on `osimage`.`pkgdir` #7380

samveen · 2023-05-01T16:01:25Z

The PR is to fix issue #7370

The code for OSPKGDIR handling in the ospkgs postscript has a bug in the rhel section where the the tracking index is used before incrementing, causing the url path for the 2nd repo to be overwritten by the path of the 3rd one, and the 3rd one is left with a blank repo url.

~~a classic case of use before increment bug~~

The pkgdir handling code has slowly mutated into multiple overlapping sections such that bugs have been introduced into the processing. This PR changes the pkgdir handling code to be clearer and simpler in organization to:

Allow easier review
Avoid the introduction of subtle bugs.

samveen · 2023-05-01T16:20:34Z

@gurevichmark @besawn for your kind attention.

besawn · 2023-05-01T20:04:44Z

@samveen Thank you for the contribution. @gurevichmark and I would like to try to figure out why our internal tests haven't been catching this issue before we merge this PR. We will follow up soon, thanks.

samveen · 2023-05-02T08:06:34Z

@samveen Thank you for the contribution. @gurevichmark and I would like to try to figure out why our internal tests haven't been catching this issue before we merge this PR. We will follow up soon, thanks.

@besawn Would you point me to the OSImage definitions being used for the internal tests, particularly the pkgdir attribute, which should be able to help me give you the point of failure in the tests.

As for the github workflow checks, the environment used in the tests is Ubuntu , which will never trigger the bug:

Run perl github_action_xcat_test.pl
Current OS information:
$VAR1 = [
          'NAME="Ubuntu"',
          'VERSION="20.04.6 LTS (Focal Fossa)"',
          'ID=ubuntu',
          'ID_LIKE=debian',
          'PRETTY_NAME="Ubuntu 20.04.6 LTS"',
          'VERSION_ID="20.04"',
          'HOME_URL="https://www.ubuntu.com/"',
          'SUPPORT_URL="https://help.ubuntu.com/"',
          'BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"',
          'PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"',
          'VERSION_CODENAME=focal',
          'UBUNTU_CODENAME=focal'
        ];

gurevichmark · 2023-05-02T17:50:06Z

@samveen Thanks for your contribution. I was able to recreate this problem in our environment. However, I think the solution under #7371 is better.

With an osimage which has
pkgdir=/install/rhels8.6.0/ppc64le,/install/post/otherpkgs/rhels8.4.0/ppc64le/xcat/xcat-core,
if I try the solution you propose in this PR, I get 4 .repo files generated:

[root@f1n03k06 yum.repos.d]# grep baseurl /etc/yum.repos.d/*
/etc/yum.repos.d/xCAT-rhels8.6.0-path0.repo:baseurl=http://f1n03k05:80/install/rhels8.6.0/ppc64le
/etc/yum.repos.d/xCAT-rhels8.6.0-path1.repo:baseurl=http://f1n03k05:80/install/rhels8.6.0/ppc64le/BaseOS
/etc/yum.repos.d/xCAT-rhels8.6.0-path2.repo:baseurl=http://f1n03k05:80/install/rhels8.6.0/ppc64le/AppStream
/etc/yum.repos.d/xCAT-rhels8.6.0-path3.repo:baseurl=http://f1n03k05:80/install/post/otherpkgs/rhels8.4.0/ppc64le/xcat/xcat-core
[root@f1n03k06 yum.repos.d]#

and the first one is a problem:

[root@f1n03k06 yum.repos.d]# yum makecache
Updating Subscription Management repositories.
xCAT-rhels8.6.0-path0                           3.2 kB/s | 196  B     00:00
Errors during downloading metadata for repository 'xCAT-rhels8.6.0-path0':
  - Status code: 404 for http://f1n03k05:80/install/rhels8.6.0/ppc64le/repodata/repomd.xml (IP: 10.10.113.5)
Error: Failed to download metadata for repo 'xCAT-rhels8.6.0-path0': Cannot download repomd.xml: Cannot download repodata/repomd.xml: All mirrors were tried
xCAT-rhels8.6.0-path1                           219 kB/s | 2.8 kB     00:00
xCAT-rhels8.6.0-path2                           250 kB/s | 3.2 kB     00:00
xCAT-rhels8.6.0-path3                           110 kB/s | 3.0 kB     00:00
Ignoring repositories: xCAT-rhels8.6.0-path0
Metadata cache created.
[root@f1n03k06 yum.repos.d]#

If I use the solution proposed in #7371 I get 3 .repo files generated:

[root@f1n03k06 yum.repos.d]# grep baseurl /etc/yum.repos.d/*
/etc/yum.repos.d/xCAT-rhels8.6.0-path0.repo:baseurl=http://f1n03k05:80/install/rhels8.6.0/ppc64le/BaseOS
/etc/yum.repos.d/xCAT-rhels8.6.0-path1.repo:baseurl=http://f1n03k05:80/install/rhels8.6.0/ppc64le/AppStream
/etc/yum.repos.d/xCAT-rhels8.6.0-path2.repo:baseurl=http://f1n03k05:80/install/post/otherpkgs/rhels8.4.0/ppc64le/xcat/xcat-core
[root@f1n03k06 yum.repos.d]#

and

[root@f1n03k06 yum.repos.d]# yum makecache
Updating Subscription Management repositories.
xCAT-rhels8.6.0-path0                           156 kB/s | 2.8 kB     00:00
xCAT-rhels8.6.0-path1                           202 kB/s | 3.2 kB     00:00
xCAT-rhels8.6.0-path2                           238 kB/s | 3.0 kB     00:00
Metadata cache created.
[root@f1n03k06 yum.repos.d]#

samveen · 2023-05-03T09:29:04Z

@gurevichmark

Is the first element of the osimage.pkgdir NOT a YUM repos? The path is meant to be a repo (have the repodata/repomd.xml and associated filelists). Please check on the management server and confirm.
If this is not the case (i.e. first element is not a yum repo), then the code logic of ospkgs has retained code logic from RHEL6/7 repository layouts and will need a cleanup. See line 284 and it's preceding check
I've rewritten the logic to be clearer. Please review.

gurevichmark · 2023-05-11T16:24:49Z

@samveen Is there a case where the solution under #7371 does not work ?

I have tried that fix with multiple pkgdir entries on RH8 where BaseOS and AppStream need to be appended, and with multiple pkgdir entries on RH7.6 where it is just a path specified in pkgdir. Both produce correct set of .repo files.

conxuro · 2023-05-15T10:51:06Z

I have tested the new ospkgs and it works for me. Thanks for the fix!

Take into account the two commits (6397efd a95e293) since the first one (6397efd) standalone didn't work. I tried it before the solution proposed in #7371 and does the same issue but with the first repo file with void URL instead the third one. However, with the commit a95e293 now the postscript does the indexing properly without wrong URLs.

In addition, not directly related to this bug/fix, the ospkgs postscript creates one repo file per repository but the files created at install time have merged the two first repos into one file (that causes a discrepancy with the number of repo files before and after).

cat /etc/yum.repos.d/local-repository-0.repo
[local-rocky8.6-x86_64--install-rocky8.6-x86_64-BaseOS]
name=xCAT configured yum repository for /install/rocky8.6/x86_64/BaseOS
baseurl=http://10.3.0.1:80//install/rocky8.6/x86_64/BaseOS
enabled=1
gpgcheck=0

[local-rocky8.6-x86_64--install-rocky8.6-x86_64-AppStream]
name=xCAT configured yum repository for /install/rocky8.6/x86_64/AppStream
baseurl=http://10.3.0.1:80//install/rocky8.6/x86_64/AppStream
enabled=1
gpgcheck=0

I think this behavior and the change of the repo files names (local*.repo vs xCAT*.repo) can cause confusions and can be more prone to errors.

samveen · 2023-05-18T16:10:08Z

@conxuro

thank you for testing and the feedback.
6397efd is based on incorrect assumptions and is thus by itself a toxic commit.
a95e293 reverts the previous commit and re-does the code cleanly and hopefully correctly.
The local repository names created during the kickstart process is done by adding appropriate cat commands to the pre-install script section of the kickstart files for the nodes (as generated by xCAT-server/lib/perl/xCAT/Template.pm), and are expected to be transient for the duration of the install. (git grep INSTALL_SOURCES_IN_PRE for more info)

ospkgs: fix index use before increment bug

6397efd

samveen mentioned this pull request May 1, 2023

Fix #7370 ospkgs issue #7371

Open

besawn added this to the 2.16.6 milestone May 1, 2023

rewrite path logic to be cleaner, more understandable, and shorter

a95e293

samveen changed the title ~~ospkgs: fix index use before increment bug~~ ospkgs: fix checks on osimage.pkgdir May 5, 2023

Obihoernchen modified the milestones: 2.16.6, 2.17 Jan 29, 2024

Obihoernchen added the component:os_provision label Feb 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ospkgs: fix checks on `osimage`.`pkgdir` #7380

ospkgs: fix checks on `osimage`.`pkgdir` #7380

samveen commented May 1, 2023 •

edited

samveen commented May 1, 2023

besawn commented May 1, 2023

samveen commented May 2, 2023 •

edited

gurevichmark commented May 2, 2023

samveen commented May 3, 2023 •

edited

gurevichmark commented May 11, 2023

conxuro commented May 15, 2023 •

edited

samveen commented May 18, 2023

ospkgs: fix checks on osimage.pkgdir #7380

Are you sure you want to change the base?

ospkgs: fix checks on osimage.pkgdir #7380

Conversation

samveen commented May 1, 2023 • edited

samveen commented May 1, 2023

besawn commented May 1, 2023

samveen commented May 2, 2023 • edited

gurevichmark commented May 2, 2023

samveen commented May 3, 2023 • edited

gurevichmark commented May 11, 2023

conxuro commented May 15, 2023 • edited

samveen commented May 18, 2023

ospkgs: fix checks on `osimage`.`pkgdir` #7380

ospkgs: fix checks on `osimage`.`pkgdir` #7380

samveen commented May 1, 2023 •

edited

samveen commented May 2, 2023 •

edited

samveen commented May 3, 2023 •

edited

conxuro commented May 15, 2023 •

edited