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

rpm: drop support for unsupported distros #8720

Merged
merged 15 commits into from May 4, 2016

Conversation

Projects
None yet
4 participants
@smithfarm
Copy link
Contributor

commented Apr 23, 2016

Replaces #6225 which was targeting infernalis.

Fixes: http://tracker.ceph.com/issues/13445
References: http://tracker.ceph.com/issues/15486

@smithfarm smithfarm force-pushed the wip-13445 branch 3 times, most recently from d67f5fe to 9a04296 Apr 23, 2016

@smithfarm

This comment has been minimized.

Copy link
Contributor Author

commented Apr 24, 2016

Passes gitbuilder.

@ktdreyer

This comment has been minimized.

Copy link
Member

commented Apr 25, 2016

Really great!

Reviewed-by: Ken Dreyer <kdreyer@redhat.com>

@branto1 care to review?

restorecon -R %{_bindir}/ceph-mon > /dev/null 2>&1; \
restorecon -R %{_bindir}/ceph-osd > /dev/null 2>&1; \
restorecon -R %{_bindir}/ceph-mds > /dev/null 2>&1; \
restorecon -R %{_bindir}/radosgw > /dev/null 2>&1; \
restorecon -R /etc/rc\.d/init\.d/ceph > /dev/null 2>&1; \
restorecon -R /etc/rc\.d/init\.d/radosgw > /dev/null 2>&1; \

This comment has been minimized.

Copy link
@b-ranto

b-ranto Apr 26, 2016

Contributor

Here, we can drop the rc.d/init.d lines as well as they contain the deprecated init scripts.

/usr/bin/systemctl --no-reload disable $SERVICE > /dev/null 2>&1 || :
/usr/bin/systemctl stop $SERVICE > /dev/null 2>&1 || :
%{_bindir}/systemctl --no-reload disable $SERVICE > /dev/null 2>&1 || :
%{_bindir}/systemctl stop $SERVICE > /dev/null 2>&1 || :

This comment has been minimized.

Copy link
@b-ranto

b-ranto Apr 26, 2016

Contributor

Here (and in several other chunks), I'm not completely sure about this. People usually use the hard-coded paths in post scripts although I'm not sure what is the reason for that.

@b-ranto

This comment has been minimized.

Copy link
Contributor

commented Apr 26, 2016

Mostly lgtm although I'm not sure about the last two patches -- I commented on the next to last one and I know nothing about the resource agents so that is why I'm not sure about the last one.

@smithfarm

This comment has been minimized.

Copy link
Contributor Author

commented Apr 26, 2016

@branto1 The purpose of the OCF commit is to finish the job explained in http://tracker.ceph.com/issues/14828

@smithfarm smithfarm force-pushed the wip-13445 branch from 9a04296 to c660f8f Apr 26, 2016

@smithfarm

This comment has been minimized.

Copy link
Contributor Author

commented Apr 26, 2016

Changelog:

  • rebased
  • added References: line to "Drop ceph Resource Agent" commit
  • added a commit dropping the sysvinit lines from relabel_files() (per @branto1)
@smithfarm

This comment has been minimized.

Copy link
Contributor Author

commented Apr 26, 2016

@branto1 Can you find out what the reason is for writing out the path in the scriptlets, instead of using the macro?

Not using a macro - in even one place - would seem to defeat the purpose of using the macro in the first place.

@b-ranto

This comment has been minimized.

Copy link
Contributor

commented Apr 26, 2016

@smithfarm: I think the idea for not using the macros in rpm scripts is that we do not know where the other binaries were installed -- it is not guaranteed that they were built with the same set of macros as we are being built. This can be especially true for things that are not standard in the distro -- you might want to create your own build of ceph and put it in e.g. /usr/local/. If you overrode the macros, your rpm scripts would constantly fail.

You can see e.g. in examples in [1] that they don't use these macros for binaries that are outside of the scope of the package and they do not use them there on purpose.

[1] https://fedoraproject.org/wiki/Packaging:Scriptlets

@smithfarm

This comment has been minimized.

Copy link
Contributor Author

commented Apr 26, 2016

@branto1 Good point! I added a commit - please take a look.

@smithfarm

This comment has been minimized.

Copy link
Contributor Author

commented Apr 26, 2016

@b-ranto

This comment has been minimized.

Copy link
Contributor

commented Apr 27, 2016

@smithfarm: That line should be fine. AFAIK, the general rule of thumb is to use hard-coded paths for things outside the scope of the package and macros for all the files that are handled by the package (i.e. appear in the file lists). The missing full path for ln binary should also be quite ok, especially for things like 'ln' that are standard on all distros -- these scriptlets get evaluated by bash, I'd not be too concerned that there are any distros out there that don't have 'ln' (or 'mkdir') in PATH.

@smithfarm

This comment has been minimized.

Copy link
Contributor Author

commented Apr 28, 2016

Changelog:

  • rebase
  • s/ceph.spec.in:/rpm:/ in all commit messages
  • new commit "rpm: do a full make check when --with-tests is given"
  • reworked the "replace literal paths with RPM macros" commit to really apply the rule described by @branto1
  • revised "drop dead conditionals" commit to drop "%if 0%{?el6}" part of junit conditional
  • re-pushed to gitbuilder

@smithfarm smithfarm force-pushed the wip-13445 branch from 9551715 to 5004936 Apr 28, 2016

@smithfarm

This comment has been minimized.

Copy link
Contributor Author

commented Apr 28, 2016

Changelog:

  • added Debian bits to "Drop ceph Resource Agent" commit
@smithfarm

This comment has been minimized.

Copy link
Contributor Author

commented Apr 28, 2016

I'm going to split the "Drop ceph Resource Agent" commit off into a separate PR.

@smithfarm

This comment has been minimized.

Copy link
Contributor Author

commented Apr 28, 2016

Changelog:

  • moved the Resource Agent commit to #8818

@liewegas This is another try at #6225 . . . since Jewel dropped support for sysvinit, would it be OK to flag this for backport to Jewel?

@ktdreyer

This comment has been minimized.

Copy link
Member

commented Apr 28, 2016

I'm 👍 for backporting this to Jewel. It will help to make future cherry-picks easier if the packaging is aligned (particularly this early in Jewel's life).

@liewegas

This comment has been minimized.

Copy link
Member

commented Apr 28, 2016

Sounds ok to me! The only sysvinit we still have to worry about is debian; nothing I know of on the rpm side of the house.

@b-ranto

This comment has been minimized.

Copy link
Contributor

commented May 2, 2016

I've just re-checked the whole patch-set (full diff). Great work, +1 for merging this and back-porting it to jewel.

@liewegas

This comment has been minimized.

Copy link
Member

commented May 4, 2016

This conflicts with the removal of the rm -f lines from the test ec plugins. I thinkt eh conflict resolution is pretty trivial, but I think it'll be easier to backport if it's rebased first, then merged.

smithfarm added some commits Oct 11, 2015

rpm: drop python_sitelib/sitearch conditional
This conditional was required to support older versions of RHEL/CentOS that are
no longer supported in infernalis and above.

Signed-off-by: Nathan Cutler <ncutler@suse.com>
rpm: drop _with_systemd
Ceph versions jewel and above only support systemd.

Signed-off-by: Nathan Cutler <ncutler@suse.com>
rpm: drop python-argparse dependency
This was only necessary for older (now unsupported) distro versions.

Signed-off-by: Nathan Cutler <ncutler@suse.com>
rpm: drop dead conditionals
This commit drops conditionals that no longer serve any purpose, since
jewel and above do not support the distro versions they are checking for.

Signed-off-by: Nathan Cutler <ncutler@suse.com>
rpm: drop init-ceph.in-fedora.patch
Signed-off-by: Nathan Cutler <ncutler@suse.com>
rpm: move unified python-sphinx build dependency
Now that the python-sphinx build dependency is unified, move it
to the proper section of the spec file.

Signed-off-by: Nathan Cutler <ncutler@suse.com>
rpm: move boost-random dependency to appropriate section
Signed-off-by: Nathan Cutler <ncutler@suse.com>
rpm: drop sysvinit-specific dependencies
Obsolete in jewel and above.

Signed-off-by: Nathan Cutler <ncutler@suse.com>
rpm: put dependencies in alphabetical order
Signed-off-by: Nathan Cutler <ncutler@suse.com>
rpm: put /sbin/ldconfig into -p
This saves approximately 1 second per invocation.

Signed-off-by: Nathan Cutler <ncutler@suse.com>

@smithfarm smithfarm force-pushed the wip-13445 branch from f85570b to df1ed01 May 4, 2016

smithfarm added some commits Oct 11, 2015

rpm: global replace $RPM_BUILD_ROOT with %{buildroot}
Signed-off-by: Nathan Cutler <ncutler@suse.com>
rpm: drop udev/95-ceph-osd-alt.rules
This udev rules file was needed on older RHEL platforms, which are
unsupported as of jewel.

Signed-off-by: Nathan Cutler <ncutler@suse.com>
rpm: replace literal paths with RPM macros
The only place we should write out literal paths is in the RPM scriptlets,
and there only for things that are not installed by this package.

Signed-off-by: Nathan Cutler <ncutler@suse.com>
rpm: drop sysvinit bits from relabel_files function
Signed-off-by: Nathan Cutler <ncutler@suse.com>
rpm: do a full make check when --with-tests is given
The check-local target tests the CLI tools only.

Signed-off-by: Nathan Cutler <ncutler@suse.com>

@smithfarm smithfarm force-pushed the wip-13445 branch from df1ed01 to e151480 May 4, 2016

@smithfarm

This comment has been minimized.

Copy link
Contributor Author

commented May 4, 2016

Changelog

  • rebased to ceph/master
  • remove rm -f lines from the test ec plugins
  • found 6 or so instances of $RPM_BUILD_ROOT that I had missed; changed them to %{buildroot}
@smithfarm

This comment has been minimized.

Copy link
Contributor Author

commented May 4, 2016

Will merge after gitbuilder confirmation.

@smithfarm smithfarm merged commit ac64388 into master May 4, 2016

2 checks passed

Signed-off-by all commits in this PR are signed
Details
default Build finished.
Details

@smithfarm smithfarm deleted the wip-13445 branch May 5, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.