Skip to content

scripts/build/ceph.spec.in: fix rhel version checks#66772

Merged
ronen-fr merged 1 commit intoceph:mainfrom
ronen-fr:wip-rf-gtsversion
Jan 8, 2026
Merged

scripts/build/ceph.spec.in: fix rhel version checks#66772
ronen-fr merged 1 commit intoceph:mainfrom
ronen-fr:wip-rf-gtsversion

Conversation

@ronen-fr
Copy link
Contributor

@ronen-fr ronen-fr commented Jan 1, 2026

Fixing multiple instances in this file where the RHEL version
is checked - without properly ensuring that the OS is indeed RHEL.

0%{?rhel} is only defined on RHEL systems, and is '0' otherwise.
That resulted, for example, in Fedora 43 having 'gts_version'
incorrectly set to '13'.

Edit (post merge): fixes https://tracker.ceph.com/issues/74368

@ronen-fr
Copy link
Contributor Author

ronen-fr commented Jan 1, 2026

Should solve most of what #66736 tried to solve.

@ronen-fr
Copy link
Contributor Author

ronen-fr commented Jan 1, 2026

Tested on a clean container. This solves most of Fedora-43 installation issues
(up to the pip failure on rstcheck)

@ronen-fr ronen-fr marked this pull request as ready for review January 1, 2026 15:08
Copy link
Contributor

@tchaikov tchaikov left a comment

Choose a reason for hiding this comment

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

lgtm. this change follows the existing pattern in ceph.spec.in. but i am afraid it is not the only place where we failed to check 0%{?rhel} before comparing it with a number. @ronen-fr hi Ronen, could you help verify if we should fix

%if 0%{?rhel} <= 8
in a similar way?

@ronen-fr
Copy link
Contributor Author

ronen-fr commented Jan 1, 2026

@tchaikov

lgtm. this change follows the existing pattern in ceph.spec.in. but i am afraid it is not the only place where we failed to check 0%{?rhel} before comparing it with a number. @ronen-fr hi Ronen, could you help verify if we should fix

%if 0%{?rhel} <= 8

in a similar way?

Thanks. Checking.

Update: fixed 3 additional lines. Can you take a look at line 38, and verify that this is the logic we want (for Fedora)?
Thanks a lot

Copy link
Contributor

@tchaikov tchaikov left a comment

Choose a reason for hiding this comment

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

please squash the changes into a single commit. and add the explanation on why we need them, and their impact to the built artifacts.

@ronen-fr ronen-fr changed the title scripts/build/ceph.spec.in: fix rhel version check scripts/build/ceph.spec.in: fix rhel version checks Jan 2, 2026
@ronen-fr
Copy link
Contributor Author

ronen-fr commented Jan 2, 2026

@tchaikov: thanks for your help, Kefu.

@ronen-fr ronen-fr requested a review from idryomov January 2, 2026 06:27
@ronen-fr ronen-fr added the needs-tentacle-backport PRs that need tentacle back-port label Jan 2, 2026
@idryomov
Copy link
Contributor

idryomov commented Jan 2, 2026

jenkins test make check

@idryomov
Copy link
Contributor

idryomov commented Jan 2, 2026

jenkins test make check arm64

@idryomov
Copy link
Contributor

idryomov commented Jan 2, 2026

jenkins test windows

1 similar comment
@ronen-fr
Copy link
Contributor Author

ronen-fr commented Jan 3, 2026

jenkins test windows

@ronen-fr
Copy link
Contributor Author

ronen-fr commented Jan 3, 2026

jenkins test make check

@ronen-fr
Copy link
Contributor Author

ronen-fr commented Jan 3, 2026

jenkins test windows

@tchaikov
Copy link
Contributor

tchaikov commented Jan 4, 2026

the windows test failure is tracked by https://tracker.ceph.com/issues/74308

cc @djgalloway hi David, in case you are interested.

@tchaikov
Copy link
Contributor

tchaikov commented Jan 5, 2026

jenkins test windows

1 similar comment
@ronen-fr
Copy link
Contributor Author

ronen-fr commented Jan 5, 2026

jenkins test windows

@ronen-fr
Copy link
Contributor Author

ronen-fr commented Jan 6, 2026

@idryomov - do you see any reason to not merge this now? It can't make Fedora 43 installation worse than it is now, and I do not think Teuthology tests this more than the CI pipeline. What is your recommendation?
Thanks

@idryomov
Copy link
Contributor

idryomov commented Jan 6, 2026

do you see any reason to not merge this now? It can't make Fedora 43 installation worse than it is now, and I do not think Teuthology tests this more than the CI pipeline. What is your recommendation?

... but it can break installation on other RPM-based OSes, at least in theory ;) I'd recommend at least getting the RPM packages built in Shaman -- make check doesn't do that.

@ronen-fr
Copy link
Contributor Author

ronen-fr commented Jan 6, 2026

I'd recommend at least getting the RPM packages built in Shaman

Sure. I think I did that, but will verify that it was with the latest version. Thanks

Fixing multiple instances in this file where
the RHEL version is checked - without properly
ensuring that the OS is indeed RHEL.

0%{?rhel} is only defined on RHEL systems, and
is '0' otherwise. That resulted, for example, in
Fedora 43 having 'gts_version' incorrectly
set to '13'.

Signed-off-by: Ronen Friedman <rfriedma@redhat.com>
@ronen-fr
Copy link
Contributor Author

ronen-fr commented Jan 7, 2026

jenkins test windows

@ronen-fr
Copy link
Contributor Author

ronen-fr commented Jan 8, 2026

Following Ilya's comment above, and as "Shaman" builds were OK, I will be merging this PR now.

@ronen-fr ronen-fr merged commit 533f2c7 into ceph:main Jan 8, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build/ops needs-tentacle-backport PRs that need tentacle back-port

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants