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

use official centos repos for nfs-ganesha packages #2169

Closed
wants to merge 1 commit into from

Conversation

BlaineEXE
Copy link
Collaborator

@BlaineEXE BlaineEXE commented Nov 9, 2023

Do not use buildlogs.centos.org as a repo. Those locations are not guaranteed to be permanent storehouses for RPM builds, and they are generally assumed to also have non-release versions as well.

Instead, use official Centos mirrors for nfs-ganesha packages starting with ganesha v4. This commit also codifies the NFS repo for stream9, which changes its URL structure significantly from stream8. Though stream9 is not used yet, it will be coming soon, and it is important to record the change in NFS repo that happens in stream9 alongside these changes to make the upcoming transition easier.

To handle the additional complexity that now exists with ganesha repos differing between stream8 and stream9, and also differing between Ceph versions, this commit changes the old if,elsif,elsif,else approach to setting the ganesha repo in the Dockerfile to one where __GANESHA_REPO__ is defined and can be overridden in Ceph version directories.

All builds using stream8 or stream9 begin with a base __GANESHA_REPO__ that allows the ceph version directory to specify __GANESHA_VER__. This base __GANESHA_REPO__ should be suitable for main builds for the forseeable future. It is also suitable for Ceph reef (which uses __GANESHA_VER__ = 5) and Ceph quincy (which uses __GANESHA_VER__ = 4). Ceph main builds obviously use the latest __GANESHA_VER__ = 5 value.

The above applies to the Ceph main, Reef, and Quincy builds which were previously using buildlogs repos.

Ceph releases for Pacific and earlier include NFS-Ganesha packages that are built alongside the Ceph RPMs which have specific API compatibility options set. Therefore, __GANESHA_REPO__ is overridden for pacific and lower builds and is set to the same content that was previously selected via the if,elsif,elsif,else behavior.

Description of your changes:

Which issue is resolved by this Pull Request:
Resolves #

Checklist:

  • Documentation has been updated, if necessary.
  • Pending release notes updated with breaking and/or notable changes, if necessary.

@BlaineEXE BlaineEXE force-pushed the new-ganesha-repos branch 3 times, most recently from 91cd7d4 to 92c5056 Compare November 9, 2023 20:03
@BlaineEXE
Copy link
Collaborator Author

BlaineEXE commented Nov 9, 2023

Here is a breakdown of how the Dockerfiles look in staging after these changes:

main,centos,8

bash -c ' \
  if [ -n "nfs-ganesha nfs-ganesha-ceph nfs-ganesha-rgw nfs-ganesha-rados-grace nfs-ganesha-rados-urls sssd-client dbus-daemon rpcbind" ]; then \
    echo "[ganesha]" > /etc/yum.repos.d/ganesha.repo ; \
echo "name=ganesha" >> /etc/yum.repos.d/ganesha.repo ; \
echo "baseurl=http://mirror.centos.org/centos/8-stream/storage/\$basearch/nfsganesha-5/" >> /etc/yum.repos.d/ganesha.repo ; \
echo "gpgcheck=0" >> /etc/yum.repos.d/ganesha.repo ; \
echo "enabled=1" >> /etc/yum.repos.d/ganesha.repo ; \
  fi ; \

main,centos,9

Note that the baseurl value changes significantly for centos 9

bash -c ' \
  if [ -n "nfs-ganesha nfs-ganesha-ceph nfs-ganesha-rgw nfs-ganesha-rados-grace nfs-ganesha-rados-urls sssd-client dbus-daemon rpcbind" ]; then \
    echo "[ganesha]" > /etc/yum.repos.d/ganesha.repo ; \
echo "name=ganesha" >> /etc/yum.repos.d/ganesha.repo ; \
echo "baseurl=https://mirror.stream.centos.org/SIGs/9-stream/storage/\$basearch/nfsganesha-5/" >> /etc/yum.repos.d/ganesha.repo ; \
echo "gpgcheck=0" >> /etc/yum.repos.d/ganesha.repo ; \
echo "enabled=1" >> /etc/yum.repos.d/ganesha.repo ; \
  fi ; \

reef,centos,8

Note that this also updates the NFS packages to 5.7, which contains a critically-needed fix for cmount_path.

bash -c ' \
  if [ -n "nfs-ganesha-5.7-1.el8s nfs-ganesha-ceph-5.7-1.el8s nfs-ganesha-rgw-5.7-1.el8s nfs-ganesha-rados-grace-5.7-1.el8s nfs-ganesha-rados-urls-5.7-1.el8s sssd-client dbus-daemon rpcbind" ]; then \
    echo "[ganesha]" > /etc/yum.repos.d/ganesha.repo ; \
echo "name=ganesha" >> /etc/yum.repos.d/ganesha.repo ; \
echo "baseurl=http://mirror.centos.org/centos/8-stream/storage/\$basearch/nfsganesha-5/" >> /etc/yum.repos.d/ganesha.repo ; \
echo "gpgcheck=0" >> /etc/yum.repos.d/ganesha.repo ; \
echo "enabled=1" >> /etc/yum.repos.d/ganesha.repo ; \
  fi ; \

quincy,centos,8

bash -c ' \
  if [ -n "nfs-ganesha nfs-ganesha-ceph nfs-ganesha-rgw nfs-ganesha-rados-grace nfs-ganesha-rados-urls sssd-client dbus-daemon rpcbind" ]; then \
    echo "[ganesha]" > /etc/yum.repos.d/ganesha.repo ; \
echo "name=ganesha" >> /etc/yum.repos.d/ganesha.repo ; \
echo "baseurl=http://mirror.centos.org/centos/8-stream/storage/\$basearch/nfsganesha-4/" >> /etc/yum.repos.d/ganesha.repo ; \
echo "gpgcheck=0" >> /etc/yum.repos.d/ganesha.repo ; \
echo "enabled=1" >> /etc/yum.repos.d/ganesha.repo ; \
  fi ; \

pacific,centos,8

bash -c ' \
  if [ -n "nfs-ganesha nfs-ganesha-ceph nfs-ganesha-rgw nfs-ganesha-rados-grace nfs-ganesha-rados-urls sssd-client dbus-daemon rpcbind" ]; then \
    echo "[ganesha]" > /etc/yum.repos.d/ganesha.repo ; \
echo "name=ganesha" >> /etc/yum.repos.d/ganesha.repo ; \
echo "baseurl=https://download.ceph.com/nfs-ganesha/rpm-V3.5-stable/$CEPH_VERSION/el\$releasever/\$basearch/" >> /etc/yum.repos.d/ganesha.repo ; \
echo "gpgcheck=1" >> /etc/yum.repos.d/ganesha.repo ; \
echo "enabled=1" >> /etc/yum.repos.d/ganesha.repo  ; \
echo "[ganesha-noarch]" >> /etc/yum.repos.d/ganesha.repo ; \
echo "name=ganesha-noarch" >> /etc/yum.repos.d/ganesha.repo ; \
echo "baseurl=https://download.ceph.com/nfs-ganesha/rpm-V3.5-stable/$CEPH_VERSION/el\$releasever/noarch/" >> /etc/yum.repos.d/ganesha.repo ; \
echo "gpgcheck=1" >> /etc/yum.repos.d/ganesha.repo ; \
echo "enabled=1" >> /etc/yum.repos.d/ganesha.repo  ; \
  fi ; \

octopus,centos,8

bash -c ' \
  if [ -n "nfs-ganesha nfs-ganesha-ceph nfs-ganesha-rgw nfs-ganesha-rados-grace nfs-ganesha-rados-urls sssd-client dbus-daemon rpcbind" ]; then \
    echo "[ganesha]" > /etc/yum.repos.d/ganesha.repo ; \
echo "name=ganesha" >> /etc/yum.repos.d/ganesha.repo ; \
echo "baseurl=https://download.ceph.com/nfs-ganesha/rpm-V3.3-stable/$CEPH_VERSION/el\$releasever/\$basearch/" >> /etc/yum.repos.d/ganesha.repo ; \
echo "gpgcheck=1" >> /etc/yum.repos.d/ganesha.repo ; \
echo "enabled=1" >> /etc/yum.repos.d/ganesha.repo  ; \
echo "[ganesha-noarch]" >> /etc/yum.repos.d/ganesha.repo ; \
echo "name=ganesha-noarch" >> /etc/yum.repos.d/ganesha.repo ; \
echo "baseurl=https://download.ceph.com/nfs-ganesha/rpm-V3.3-stable/$CEPH_VERSION/el\$releasever/noarch/" >> /etc/yum.repos.d/ganesha.repo ; \
echo "gpgcheck=1" >> /etc/yum.repos.d/ganesha.repo ; \
echo "enabled=1" >> /etc/yum.repos.d/ganesha.repo  ; \
  fi ; \

nautilus,centos,7

bash -c ' \
  if [ -n "nfs-ganesha nfs-ganesha-ceph nfs-ganesha-rgw nfs-ganesha-rados-grace nfs-ganesha-rados-urls sssd-client dbus-daemon rpcbind" ]; then \
    echo "[ganesha]" > /etc/yum.repos.d/ganesha.repo ; \
echo "name=ganesha" >> /etc/yum.repos.d/ganesha.repo ; \
echo "baseurl=https://download.ceph.com/nfs-ganesha/rpm-V2.8-stable/$CEPH_VERSION/\$basearch/" >> /etc/yum.repos.d/ganesha.repo ; \
echo "gpgcheck=1" >> /etc/yum.repos.d/ganesha.repo ; \
echo "enabled=1" >> /etc/yum.repos.d/ganesha.repo  ; \
  fi ; \

mimic,centos,7

bash -c ' \
  if [ -n "nfs-ganesha nfs-ganesha-ceph nfs-ganesha-rgw nfs-ganesha-rados-grace nfs-ganesha-rados-urls sssd-client dbus-daemon rpcbind" ]; then \
    echo "[ganesha]" > /etc/yum.repos.d/ganesha.repo ; \
echo "name=ganesha" >> /etc/yum.repos.d/ganesha.repo ; \
echo "baseurl=https://download.ceph.com/nfs-ganesha/rpm-V2.7-stable/$CEPH_VERSION/\$basearch/" >> /etc/yum.repos.d/ganesha.repo ; \
echo "gpgcheck=1" >> /etc/yum.repos.d/ganesha.repo ; \
echo "enabled=1" >> /etc/yum.repos.d/ganesha.repo  ; \
  fi ; \

I'm pretty sure the option to build luminous is broken at this point, and it's also sufficiently old that it doesn't matter.

Comment on lines 0 to 1
../../../src/daemon-base/__GANESHA_PACKAGES__ No newline at end of file
../centos/__GANESHA_PACKAGES__
Copy link
Collaborator Author

@BlaineEXE BlaineEXE Nov 9, 2023

Choose a reason for hiding this comment

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

@guits this change makes it so the arm builds also pin to the same versions as the x86 ones. https://tracker.ceph.com/issues/63151

For context, there was a change to NFS-Ganesha that resulted in the inability for users to mount NFS volumes, and causing Rook's CI to fail. All the changes in this PR are after discussions with Kaleb and Frank to help make sure NFS updates aren't as likely to cause the kind of failure in the future.
See: ceph/ceph-csi#4251
And: rook/rook#13151 (comment)

Currently, NFS-Ganesha is broken for Rook users in V5.6. It should be working in v5.5 and lower, as well as in v5.7 and higher.

@adk3798
Copy link
Collaborator

adk3798 commented Nov 9, 2023

@BlaineEXE Do we actually need a ganesha version with the cmount_path fix in reef? The code in the ceph codebase to set the cmount_path in the exports isn't even merged in main. We had just tried to pin ganesha version in reef containers to 5.5-1 (#2168) to avoid https://tracker.ceph.com/issues/63151 and with that version plus a ceph codebase that doesn't mention the field, I'm not sure how it would show up at all. Using the official centos mirror over what we had I have no issue with, just wandering about the version choice in reef.

@BlaineEXE
Copy link
Collaborator Author

BlaineEXE commented Nov 9, 2023

@adk3798, I think it would be nice to get the cmount_path feature into Reef because it fixes a performance issue. However, Rook users have been using NFS-Ganesha with the CephFS FSAL for quite some time without reporting it, so I think it is probably okay to pin Reef to v5.5.

From Rook's standpoint, the thing I care about most is that the NFS-Ganesha version is not v5.6, which has a bugged cmount_path implementation that causes failures.

I think it's worth mentioning that the PR as it is right now does not change the behavior that @guits merged earlier today pinning the version to v5.5-1. [Update: it does make a change to ensure that the ARM build of the Reef image is also pinned to the same version]

However, Kaleb and Frank have advised me very clearly that the buildlogs.centos.org repos are not suitable for production. This PR as it is today is intended to fix that issue, and to codify NFS repos for upcoming stream9 builds.

@adk3798
Copy link
Collaborator

adk3798 commented Nov 9, 2023

@adk3798, I think it would be nice to get the cmount_path feature into Reef because it fixes a performance issue. However, Rook users have been using NFS-Ganesha with the CephFS FSAL for quite some time without reporting it, so I think it is probably okay to pin Reef to v5.5.

yeah, I think eventually we would want to bump the ganesha version in the ceph containers to 5.7. But at the moment, given the Ceph repo isn't yet setting the cmount path it in the exports and we're hitting the linked tracker with anything 5.5-2 or greater, I just wanted to pin it there for now and we can revisit after the next reef point release which is supposed to be quite soon.

From Rook's standpoint, the thing I care about most is that the NFS-Ganesha version is not v5.6, which has a bugged cmount_path implementation that causes failures.

I think it's worth mentioning that the PR as it is right now does not change the behavior that @guits merged earlier today pinning the version to v5.5-1.

Great, I just read "Note that this also updates the NFS packages to 5.7, which contains a critically-needed fix for cmount_path." from the original PR message and thought it would overwrite that change. If it pulls 5.5-1 but from the proper repo I have no issue with it.

However, Kaleb and Frank have advised me very clearly that the buildlogs.centos.org repos are not suitable for production. This PR as it is today is intended to fix that issue, and to codify NFS repos for upcoming stream9 builds.

That all sounds good.

@BlaineEXE
Copy link
Collaborator Author

Yeah. I started working on these changes before Guillaume pinned to v5.5-1, and it took me a little bit to figure out what the right thing to do is. I'll update the description to make sure it doesn't suggest any 5.7 changes.

Do not use buildlogs.centos.org as a repo. Those locations are not
guaranteed to be permanent storehouses for RPM builds, and they are
generally assumed to also have non-release versions as well.

Instead, use official Centos mirrors for nfs-ganesha packages starting
with ganesha v4. This commit also codifies the NFS repo for stream9,
which changes its URL structure significantly from stream8. Though
stream9 is not used yet, it will be coming soon, and it is important to
record the change in NFS repo that happens in stream9 alongside these
changes to make the upcoming transition easier.

To handle the additional complexity that now exists with ganesha repos
differing between stream8 and stream9, and also differing between Ceph
versions, this commit changes the old if,elsif,elsif,else approach to
setting the ganesha repo in the Dockerfile to one where __GANESHA_REPO__
is defined and can be overridden in Ceph version directories.

All builds using stream8 or stream9 begin with a base __GANESHA_REPO__
that allows the ceph version directory to specify __GANESHA_VER__. This
base __GANESHA_REPO__ should be suitable for main builds for the
forseeable future. It is also suitable for Ceph reef (which uses
__GANESHA_VER__ = 5) and Ceph quincy (which uses __GANESHA_VER__ = 4).
Ceph main builds obviously use the latest __GANESHA_VER__ = 5 value.

The above applies to the Ceph main, Reef, and Quincy builds which were
previously using buildlogs repos.

Ceph releases for Pacific and earlier include NFS-Ganesha packages that
are built alongside the Ceph RPMs which have specific API compatibility
options set. Therefore, __GANESHA_REPO__ is overridden for pacific and
lower builds and is set to the same content that was previously selected
via the if,elsif,elsif,else behavior.

Signed-off-by: Blaine Gardner <blaine.gardner@ibm.com>
@BlaineEXE
Copy link
Collaborator Author

@guits is the failure here unexpected?

Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in two weeks if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Apr 10, 2024
Copy link

This pull request has been automatically closed due to inactivity. Please re-open if these changes are still required.

@github-actions github-actions bot closed this Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants