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

ceph-releases/ALL/centos/daemon-base: change hardcoded nfs-ganesha ve… #2153

Merged
merged 1 commit into from
Sep 8, 2023

Conversation

tserlin
Copy link
Collaborator

@tserlin tserlin commented Sep 6, 2023

…rsion

Change hard-coded nfs-ganesha version V5.3.2 to V5.5.

We are currently pinning the daily nfs-ganesha builds to V5.5 in https://jenkins.ceph.com/job/nfs-ganesha

…rsion

Change hard-coded nfs-ganesha version V5.3.2 to V5.5.

We are currently pinning the daily nfs-ganesha builds to V5.5 in https://jenkins.ceph.com/job/nfs-ganesha
Copy link
Contributor

@ljflores ljflores left a comment

Choose a reason for hiding this comment

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

What about reef, quincy, and pacific? We should decide on that too.

Also, @guits had refactord the nfs-ganesha repo with this commit:
b86898a

But I had to revert it in #2151 since it caused a regression in our container scripts (see more details about the regression in https://pad.ceph.com/p/16.2.14-struggles).

@guits mentioned he wants to reintroduce this change; @guits do you agree it could be handled in a subsequent PR?

@@ -3,7 +3,7 @@ yum install -y jq && \
bash -c ' \
if [ -n "__GANESHA_PACKAGES__" ]; then \
if [[ "${CEPH_VERSION}" == master || "${CEPH_VERSION}" == main ]]; then \
curl -s -L "https://shaman.ceph.com/api/repos/nfs-ganesha/V5.3.2/latest/centos/__ENV_[DISTRO_VERSION]__/flavors/ceph_main/repo?arch=$(arch)" -o /etc/yum.repos.d/ganesha.repo ; \
curl -s -L "https://shaman.ceph.com/api/repos/nfs-ganesha/V5.5/latest/centos/__ENV_[DISTRO_VERSION]__/flavors/ceph_main/repo?arch=$(arch)" -o /etc/yum.repos.d/ganesha.repo ; \
Copy link
Collaborator

@guits guits Sep 7, 2023

Choose a reason for hiding this comment

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

@ljflores I think we should test ceph devel builds with nfs-ganesha devel builds

Copy link
Contributor

Choose a reason for hiding this comment

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

@kalebskeithley thoughts?

@guits in that case, we'd have to add something to the nfs-ganesha Jenkins job so it periodically builds "next" as well as V5.5: https://github.com/ceph/ceph-build/blob/main/nfs-ganesha/config/definitions/nfs-ganesha.yml

Choose a reason for hiding this comment

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

'next' is the bleeding edge devel branch. It's not really suitable, IMO, to be used. For anything

If you use a 'next' branch build and something doesn't work we may or may not make it a priority to fix.

I strongly suggest you use only released/tagged VX.Y versions, e.g. V5.5, V5.6, etc. V5.Y is perfectly fine for pacific, quincy, reef, and beyond.

I don't understand the comment that you need to build 'next' occasionally. Why?

Copy link
Contributor

Choose a reason for hiding this comment

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

In that case, let's stick with V5.5.

Copy link
Contributor

@ljflores ljflores left a comment

Choose a reason for hiding this comment

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

LGTM, except we need to ensure that V5.5 is also being used for reef, quincy, and pacific. I'm not sure if further changes need to be made to the script for that to happen.

@tserlin

@tserlin
Copy link
Collaborator Author

tserlin commented Sep 7, 2023

LGTM, except we need to ensure that V5.5 is also being used for reef, quincy, and pacific. I'm not sure if further changes need to be made to the script for that to happen.

@tserlin

Are you okay with merging this change first and seeing if we get a good build from it? Then I'll get a PR for the others.

@ljflores
Copy link
Contributor

ljflores commented Sep 8, 2023

Sure

@ljflores ljflores merged commit 82d8690 into ceph:main Sep 8, 2023
6 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants