github: require reef job to pass in mergify config #910

phlogistonjohn · 2023-08-02T15:32:55Z

It does what it says on the tin.

This is a change to the mergify configuration and thus will require a manual merge upon approval.

.github/mergify.yml

anoopcs9

Request to add missing check for quincy job in mergify configuration.

phlogistonjohn · 2023-08-07T17:17:45Z

Request to add missing check for quincy job in mergify configuration.

Thank you for the reminder. PR Updated.

anoopcs9

lgtm.

phlogistonjohn · 2023-08-08T14:05:23Z

So reef build is failing there's yet another nfs ganesha packaging related issue. I'm not sure how long this one will last. We can try to wait it out or we can manually merge before the nfs ganesha issue is fixed. We have to manually merge this PR at some time because it changes the mergify config.

Opinions? - @anoopcs9 @ansiwen

anoopcs9 · 2023-08-08T17:11:10Z

So reef build is failing there's yet another nfs ganesha packaging related issue. I'm not sure how long this one will last. We can try to wait it out or we can manually merge before the nfs ganesha issue is fixed. We have to manually merge this PR at some time because it changes the mergify config.

Opinions? - @anoopcs9 @ansiwen

I noticed the failure and initiated a discussion with Kaleb as there was an update pushed yesterday for nfs-ganesha from CentOS Storage SIG. Is that OK if I give it a day or two for fixing the installation issues?

phlogistonjohn · 2023-08-08T17:42:19Z

I noticed the failure and initiated a discussion with Kaleb as there was an update pushed yesterday for nfs-ganesha from CentOS Storage SIG. Is that OK if I give it a day or two for fixing the installation issues?

Fine by me.

anoopcs9 · 2023-08-09T07:16:13Z

So reef build is failing there's yet another nfs ganesha packaging related issue.

Current installation issue with nfs-ganesha is due to the fact that we execute a yum update -y prior to installing the required Ceph packages.

Reef is configured to build images with nfs-ganesha v5 packages. They do have a set of clean up instructions within the Dockerfile where /etc/selinux is removed. With updated packages for nfs-ganesha-5.x/nfs-ganesha-selinux-5.x, an update fails to find /etc/selinux/config resulting in an installation error as seen in the GitHub CI logs.

#6 35.42 /var/tmp/rpm-tmp.tz17GR: line 2: /etc/selinux/config: No such file or directory
#6 35.42 error: %prein(nfs-ganesha-selinux-5.5-2.el8s.noarch) scriptlet failed, exit status 1
#6 35.42 
#6 35.43 Error in PREIN scriptlet in rpm package nfs-ganesha-selinux
#6 35.43   Running scriptlet: nfs-ganesha-5.5-2.el8s.x86_64                         2/12 
#6 35.43 error: nfs-ganesha-selinux-5.5-2.el8s.noarch: install failed
. . .
#6 36.30 Error in POSTTRANS scriptlet in rpm package nfs-ganesha 
#6 36.30 error: nfs-ganesha-selinux-5.4-1.el8s.noarch: erase skipped
#6 36.30 /var/tmp/rpm-tmp.QHy4Za: line 2: /etc/selinux/config: No such file or directory
#6 36.30 warning: %posttrans(nfs-ganesha-5.5-2.el8s.x86_64) scriptlet failed, exit status 1

I'm not sure how long this one will last.

Issue will be fixed once the Reef image gets rebuilt with latest nfs-ganesha-5.x packages. But if I understand correctly this rebuild will only happen with next update to v18.2.

In this context I would like to rethink on the need for yum update -y in our Dockerfile.

phlogistonjohn · 2023-08-09T11:35:00Z

AFAIR we update the packages because the RPM repository doesn't necessarily have the same packages that are in the container image any more. Since the point is to get the devel packages we ended up failing the install because the matching dev/non-dev packages were not present.
Hypothetical example:

  foo-5.6.7 is installed in the container
  foo-5.6.12 and foo-devel-5.6.12 is in the repo
  foo-devel-5.6.7 is not in the repo

So the yum/dnf update prevents failing the devel package install in this case.

I'm not inclined to remove this step, which has been working for years at this point because of a bug we know will be fixed. I'd be willing to try and put in an nfs-ganesha specific workaround if we can. However, I'm willing to continue the discussion too. :-)

anoopcs9 · 2023-08-09T13:59:42Z

AFAIR we update the packages because the RPM repository doesn't necessarily have the same packages that are in the container image any more. Since the point is to get the devel packages we ended up failing the install because the matching dev/non-dev packages were not present. Hypothetical example:
  foo-5.6.7 is installed in the container
  foo-5.6.12 and foo-devel-5.6.12 is in the repo
  foo-devel-5.6.7 is not in the repo

Are you talking about Ceph related packages or in general? In any case I think DNF is capable of detecting updates in such situations. Out of my curiosity I created a similar situation on my Fedora 38 machine:

$ rpm -q ncurses ncurses-libs ncurses-base
ncurses-6.4-2.20230114.fc38
ncurses-libs-6.4-2.20230114.fc38
ncurses-base-6.4-2.20230114.fc38

$ sudo dnf list ncurses --showduplicates --forcearch x86_64
Installed Packages
ncurses.x86_64                                                                        6.4-2.20230114.fc38                                                                         @@commandline
Available Packages
ncurses.x86_64                                                                        6.4-3.20230114.fc38                                                                         fedora       

$ sudo dnf list ncurses-devel --showduplicates --forcearch x86_64
Available Packages
ncurses-devel.x86_64                                                                         6.4-3.20230114.fc38                                                                         fedora

$ sudo dnf install ncurses-devel
Dependencies resolved.
===============================================================================================================================================================================================
 Package                                           Architecture                            Version                                               Repository                               Size
===============================================================================================================================================================================================
Installing:
 ncurses-devel                                     x86_64                                  6.4-3.20230114.fc38                                   fedora                                  549 k
Upgrading:
 ncurses                                           x86_64                                  6.4-3.20230114.fc38                                   fedora                                  412 k
 ncurses-base                                      noarch                                  6.4-3.20230114.fc38                                   fedora                                   87 k
 ncurses-libs                                      x86_64                                  6.4-3.20230114.fc38                                   fedora                                  333 k
Installing dependencies:
 ncurses-c++-libs                                  x86_64                                  6.4-3.20230114.fc38                                   fedora                                   37 k

Transaction Summary
===============================================================================================================================================================================================
Install  2 Packages
Upgrade  3 Packages

Total download size: 1.4 M
Is this ok [y/N]:

ansiwen · 2023-08-09T16:49:51Z

Are you talking about Ceph related packages or in general? In any case I think DNF is capable of detecting updates in such situations.

It was this fix: #510

It was definitely an issue. Some additional ceph packages couldn't be installed, because some other installed ceph packages were not matching. It did not always happen, but in certain times around releases IIRC. So we would need an alternative solution, we can't just remove the update.

Pull request has been modified.

anoopcs9 · 2023-08-16T07:22:25Z

Are you talking about Ceph related packages or in general? In any case I think DNF is capable of detecting updates in such situations.

It was this fix: #510

It was definitely an issue. Some additional ceph packages couldn't be installed, because some other installed ceph packages were not matching. It did not always happen, but in certain times around releases IIRC. So we would need an alternative solution, we can't just remove the update.

Hm.. I'm not convinced completely but I am good to go with #913 for now.

anoopcs9 · 2023-08-16T13:01:05Z

@Mergifyio rebase

Signed-off-by: John Mulligan <jmulligan@redhat.com>

After adding the reef job, it was noticed that there was no quincy job. Apparently we (probably me) forgot to add it at the time we added quincy to our CI matrix, so we're adding it now. Signed-off-by: John Mulligan <jmulligan@redhat.com>

mergify · 2023-08-16T13:01:24Z

rebase

✅ Branch has been successfully rebased

phlogistonjohn added the no-API This PR does not include any changes to the public API of a go-ceph package label Aug 2, 2023

phlogistonjohn requested review from anoopcs9 and ansiwen August 2, 2023 15:33

anoopcs9 reviewed Aug 3, 2023

View reviewed changes

.github/mergify.yml Show resolved Hide resolved

anoopcs9 requested changes Aug 7, 2023

View reviewed changes

phlogistonjohn requested a review from anoopcs9 August 7, 2023 17:17

anoopcs9 previously approved these changes Aug 8, 2023

View reviewed changes

phlogistonjohn force-pushed the jjm-reef-mergify branch from 1eaf69f to 5b7e1bd Compare August 14, 2023 19:45

phlogistonjohn mentioned this pull request Aug 14, 2023

containers: workaround for ganesha package error on reef #913

Merged

phlogistonjohn added 2 commits August 16, 2023 13:01

github: require reef job to pass in mergify config

ba7180c

Signed-off-by: John Mulligan <jmulligan@redhat.com>

anoopcs9 force-pushed the jjm-reef-mergify branch from 5b7e1bd to 098c248 Compare August 16, 2023 13:01

phlogistonjohn requested a review from anoopcs9 August 16, 2023 14:16

anoopcs9 approved these changes Aug 16, 2023

View reviewed changes

anoopcs9 merged commit c6272ee into ceph:master Aug 16, 2023
14 checks passed

phlogistonjohn deleted the jjm-reef-mergify branch August 16, 2023 17:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

github: require reef job to pass in mergify config #910

github: require reef job to pass in mergify config #910

phlogistonjohn commented Aug 2, 2023

anoopcs9 left a comment

phlogistonjohn commented Aug 7, 2023

anoopcs9 left a comment

phlogistonjohn commented Aug 8, 2023

anoopcs9 commented Aug 8, 2023

phlogistonjohn commented Aug 8, 2023

anoopcs9 commented Aug 9, 2023

phlogistonjohn commented Aug 9, 2023

anoopcs9 commented Aug 9, 2023

ansiwen commented Aug 9, 2023 •

edited

anoopcs9 commented Aug 16, 2023

anoopcs9 commented Aug 16, 2023

mergify bot commented Aug 16, 2023

github: require reef job to pass in mergify config #910

github: require reef job to pass in mergify config #910

Conversation

phlogistonjohn commented Aug 2, 2023

anoopcs9 left a comment

Choose a reason for hiding this comment

phlogistonjohn commented Aug 7, 2023

anoopcs9 left a comment

Choose a reason for hiding this comment

phlogistonjohn commented Aug 8, 2023

anoopcs9 commented Aug 8, 2023

phlogistonjohn commented Aug 8, 2023

anoopcs9 commented Aug 9, 2023

phlogistonjohn commented Aug 9, 2023

anoopcs9 commented Aug 9, 2023

ansiwen commented Aug 9, 2023 • edited

anoopcs9 commented Aug 16, 2023

anoopcs9 commented Aug 16, 2023

mergify bot commented Aug 16, 2023

✅ Branch has been successfully rebased

ansiwen commented Aug 9, 2023 •

edited