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.spec.in: fix handling of /var/run/ceph #3916

Merged
merged 1 commit into from Mar 25, 2015

Conversation

ktdreyer
Copy link
Member

@ktdreyer ktdreyer commented Mar 9, 2015

We want the RPM to install this. %ghost won't actually install it.

The purpose of this change is 1) simplicity and 2) gain the ability to automatically the permissions of the directory once Ceph gains unprivileged user support.

CC'ing @BRANTO1 for review.

@loic-bot
Copy link

loic-bot commented Mar 9, 2015

SUCCESS: the output of run-make-check.sh on centos-7 for a1cff47 is http://paste2.org/PXnYpf56

:octocat: Sent from GH.

@dalgaaf
Copy link
Contributor

dalgaaf commented Mar 10, 2015

about the commit message: "...gain the ability to automatically the permissions ..." there seems to be a word missing (e.g. "... automatically set the permissions ...")

@ktdreyer ktdreyer force-pushed the wip-rpm-no-ghost-socket-dir branch from 9c6bece to 54fd4c2 Compare March 10, 2015 15:40
@ktdreyer
Copy link
Member Author

Great catch, thanks. I've changed "automatically the permissions" to "automatically set the permissions".

@loic-bot
Copy link

SUCCESS: the output of run-make-check.sh on centos-7 for ba836ef is http://paste2.org/c0w7yDfU

:octocat: Sent from GH.

@ktdreyer ktdreyer force-pushed the wip-rpm-no-ghost-socket-dir branch from 54fd4c2 to f4b4131 Compare March 10, 2015 18:43
@loic-bot
Copy link

SUCCESS: the output of run-make-check.sh on centos-7 for f00b84e is http://paste2.org/NhzZmULe

:octocat: Sent from GH.

@dalgaaf
Copy link
Contributor

dalgaaf commented Mar 11, 2015

Still not sure if this is the right way to do it. It's for example handled differently in openSUSE: https://en.opensuse.org/openSUSE:Systemd_packaging_guidelines#Creating_files_.2F_subdirectories_in_.2Fvar.2Frun_and_.2Frun

The SUSE Ceph package still uses in the %install dir:

%if 0%{?suse_version} >= 1310
%{__install} -d -m 0755 %{buildroot}/%{_tmpfilesdir}
%{__install} -m 0644 %{SOURCE3} %{buildroot}/%{_tmpfilesdir}/%{name}.conf
%else
mkdir -p $RPM_BUILD_ROOT%{_localstatedir}/run/ceph/
%endif

and in %files:

%if 0%{?suse_version} < 1310
%ghost %dir %{_localstatedir}/run/ceph/
%else
%dir %{_tmpfilesdir}/
%{_tmpfilesdir}/%{name}.conf
%endif

https://build.opensuse.org/package/view_file/filesystems:ceph/ceph/ceph.spec?expand=1

@ktdreyer
Copy link
Member Author

Thanks for the links. I see now that I did not catch up with the latest information regarding systemd here. /etc/tmpfiles.d is probably the correct way to handle /var/run/ceph creation on modern systems that have systemd: http://www.freedesktop.org/software/systemd/man/tmpfiles.d.html

On older systems that don't have systemd (ie. RHEL 6), /var/run is not tmpfs, so I think we should still list the /var/run/ceph directory in %files, and remove the %ghost directive there.

@dalgaaf , how does that sound?

@dalgaaf
Copy link
Contributor

dalgaaf commented Mar 13, 2015

I still wonder what the actual problem with the %ghost directive is. Did you check how other daemons handle the /var/run/$name in their spec files unter RHEL6 ?

@ktdreyer
Copy link
Member Author

The problem with %ghost is that RPM doesn't make the /var/run/ceph directory writable by non-root users. This causes a problem for RGW, which runs as Apache by default (on non-Ubuntu). For the RGW problem, see http://tracker.ceph.com/issues/9001

So the immediate issue is for RGW, and from what Sage has said, there's also work in progress to make Ceph itself run as a non-root user too.

Since we want Ceph (and RGW) to eventually stop running as root, it makes sense for us to stop using %ghost on the /var/run/foo locations so that they can be made writable by non-root users.

Regarding your question of how other daemons do this, I checked the mysql package on RHEL 6, and it has the following in %files:

%attr(0755,mysql,mysql) %dir /var/run/mysqld

There's no %ghost there. We'd want to implement the same thing for Ceph on RHEL 6.

On RHEL 7, the mariadb package has Source10: mariadb.tmpfiles.d that's installed as %{_tmpfilesdir}/%{name}.conf. It also specifies /var/run locations in %files as well:

%attr(0755,mysql,mysql) %dir %{_localstatedir}/run/mariadb
%attr(0755,mysql,mysql) %dir %{_localstatedir}/lib/mysql

If this sounds good to you, I can re-work this pull request to include the tmpfiles.d snippet on distros with systemd in addition to removing the %ghost directive.

@dalgaaf
Copy link
Contributor

dalgaaf commented Mar 15, 2015

I guess for now we can leave systemd out of the picture, we don't even package systemd atm to rpm. I take a separate look into it, we have to check which Fedora/SUSE products already use systemd.

Go ahead with the patch, but may use %attr(0755,root,root) %dir %{_localstatedir}/run/ceph. I will merge it then.

@ktdreyer
Copy link
Member Author

Thanks Danny! new patch coming up.

Prior to this commit, we didn't install /var/run/ceph as a normal
directory. We used the %ghost directive and created the directory with
a "mkdir" command in %post.

This was lacking in several ways:

  1) Simplicy: there is no need to use %ghost; other packages (eg.
     mariadb) simply use a normal %dir for their socket directory.

  2) RPM does not have control over the permissions of the /var/run/ceph
     directory. This does not interact well with "rpm -V". Moreover,
     once Ceph itself gets unprivileged user support, RPM itself won't
     be able to set the permissions of the directory for a (future)
     unprivileged UID.

  3) On distributions that use systemd as an init system, /var/run is a
     symlink to /run, which is tmpfs. This means that /var/run/ceph does
     not persist across reboots on those systems.

Remove the %ghost directive; it makes more sense for RPM to simply
install this directory like the rest of the %files.

Add a "_with_systemd" conditional so we know which distros use systemd
as their init system.  Add the /etc/tmpfiles.d/ceph.conf file on those
distros. See
http://www.freedesktop.org/software/systemd/man/tmpfiles.d.html

Signed-off-by: Ken Dreyer <kdreyer@redhat.com>
@ktdreyer ktdreyer force-pushed the wip-rpm-no-ghost-socket-dir branch from f4b4131 to 71a5090 Compare March 17, 2015 18:42
@ktdreyer ktdreyer changed the title ceph.spec.in: do not %ghost /var/run/ceph ceph.spec.in: fix handling of /var/run/ceph Mar 17, 2015
@loic-bot
Copy link

SUCCESS: the output of run-make-check.sh on centos-7 for 110d87e is http://paste2.org/DXw7jzH4

:octocat: Sent from GH.

dalgaaf added a commit that referenced this pull request Mar 25, 2015
ceph.spec.in: fix handling of /var/run/ceph

Reviewed-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
@dalgaaf dalgaaf merged commit 24aeb78 into master Mar 25, 2015
@dalgaaf dalgaaf deleted the wip-rpm-no-ghost-socket-dir branch March 25, 2015 16:27
@ktdreyer
Copy link
Member Author

welp, this broke master. The fix is in #4181

@ghost ghost added the build/ops label Feb 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants