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.

Sign up for GitHub

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

Jump to bottom

ceph.spec.in: /var/run/ceph fixes #5126

Merged
merged 2 commits into from Jul 9, 2015
Merged

ceph.spec.in: /var/run/ceph fixes #5126

merged 2 commits into from Jul 9, 2015

Conversation

smithfarm
Copy link
Contributor

On systemd distros we should not be creating /var/run/ceph at all because: (1) /var/run is symlinked or bind mounted to /run, and (2) /run is tmpfs (re-created at every boot).

See https://lists.fedoraproject.org/pipermail/devel/2011-March/150031.html

Plus some related specfile cleanups.

http://tracker.ceph.com/issues/12201 Fixes: #12201

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

@loic-bot
Copy link

loic-bot commented Jul 2, 2015

SUCCESS: http://jenkins.ceph.dachary.org/job/ceph/6199/

@smithfarm smithfarm changed the title ceph.spec.in: use /run instead of /var/run [DNM] ceph.spec.in: use /run instead of /var/run Jul 2, 2015
@smithfarm
Copy link
Contributor Author

hmm, not as simple as I thought - see https://en.opensuse.org/openSUSE:Systemd_packaging_guidelines#Creating_files_.2F_subdirectories_in_.2Fvar.2Frun_and_.2Frun

@smithfarm smithfarm changed the title [DNM] ceph.spec.in: use /run instead of /var/run ceph.spec.in: /var/run/ceph fixes Jul 2, 2015
@smithfarm
Copy link
Contributor Author

@ktdreyer @osynge pls review

@loic-bot
Copy link

loic-bot commented Jul 2, 2015

SUCCESS: http://jenkins.ceph.dachary.org/job/ceph/6200/

@loic-bot
Copy link

loic-bot commented Jul 2, 2015

SUCCESS: http://jenkins.ceph.dachary.org/job/ceph/6201/

osynge
osynge reviewed Jul 2, 2015
View reviewed changes
ceph.spec.in
@@ -584,7 +585,9 @@ mv $RPM_BUILD_ROOT/sbin/mount.fuse.ceph $RPM_BUILD_ROOT/usr/sbin/mount.fuse.ceph

#set up placeholder directories
mkdir -p $RPM_BUILD_ROOT%{_sysconfdir}/ceph
%if ! 0%{?_with_systemd}
Copy link

Choose a reason for hiding this comment

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

I would prefer you added a sysVinit variable to the spec file.

(This is some thing I have been thinking of doing we will see what ken thinks of this idea.)

@smithfarm
Copy link
Contributor Author

We will also need to call systemd-tmpfiles --create in %post but that is part of @osynge 's systemd mods

@loic-bot
Copy link

loic-bot commented Jul 2, 2015

FAILURE: http://jenkins.ceph.dachary.org/job/ceph/6203/

@loic-bot
Copy link

loic-bot commented Jul 2, 2015

FAILURE: http://jenkins.ceph.dachary.org/job/ceph/6205/

@smithfarm
Copy link
Contributor Author

@dachary The ubuntu make check bot failure says:

pip._vendor.requests.exceptions.ConnectionError: HTTPSConnectionPool(host='pypi.python.org', 
port=443): Max retries exceeded with url: /packages/source/d/distribute/distribute-0.7.3.zip (Caused 
by <class 'ConnectionResetError'>: [Errno 104] Connection reset by peer)

@ghost
Copy link

ghost commented Jul 2, 2015

@smithfarm I'll take a look, thanks for the ping

@smithfarm
Copy link
Contributor Author

Imagine, I even vaguely remember reading about the double %% in comments somewhere. Bad commit removed.

@smithfarm
Copy link
Contributor Author

On SUSE it's customary to remove the directories from the %files lists and to run
systemd-tmpfiles --create /usr/lib/tmpfiles.d/<file_name> during %post.

On Fedora, it's customary to just create the directories using the %files lists.

So it sounds like I need to change the conditional in this case.

@smithfarm
Copy link
Contributor Author

I thought the "-D" argument to install does this already?

I guess so - dropping this commit, too.

@loic-bot
Copy link

loic-bot commented Jul 2, 2015

SUCCESS: http://jenkins.ceph.dachary.org/job/ceph/6208/

@smithfarm
ceph.spec.in: fix _with_systemd conditional
4dd1e67 
Signed-off-by: Nathan Cutler <ncutler@suse.com>
@smithfarm
Copy link
Contributor Author

Modified _with_systemd conditional as per @ktdreyer suggestion.

As for the %if ! 0%{?_with_systemd}, would it be reasonable to write it this way:

%if (! 0%{?_with_systemd}) || 0%{?fedora}

or I'm thinking this

%if ! 0%{?suse_version}

would be even more straightforward?

@loic-bot
Copy link

loic-bot commented Jul 3, 2015

SUCCESS: http://jenkins.ceph.dachary.org/job/ceph/6222/

@smithfarm
ceph.spec.in: make /var/run/ceph conditional
1abaebd 
/var/run/ceph should only be installed on Fedora, RH/CentOS, and
all supported SUSE distros that do not have systemd (e.g. SLE11-SP3).

Signed-off-by: Nathan Cutler <ncutler@suse.com>
@smithfarm
Copy link
Contributor Author

@ktdreyer ping, fixed the conditional

@loic-bot
Copy link

loic-bot commented Jul 9, 2015

SUCCESS: http://jenkins.ceph.dachary.org/job/ceph/6312/

@ktdreyer
Copy link
Member

ktdreyer commented Jul 9, 2015

Looks good to me - @BRANTO1 care to review?

@b-ranto
Copy link
Contributor

b-ranto commented Jul 9, 2015

Sure, looks good to me, you can consider this

Reviewed-by: Boris Ranto <branto@redhat.com>

@ktdreyer ktdreyer merged commit 1abaebd into ceph:master Jul 9, 2015
ktdreyer added a commit that referenced this pull request Jul 9, 2015
@ktdreyer
Merge pull request #5126 from SUSE/wip-12201
90708bc 
ceph.spec.in: /var/run/ceph fixes

Reviewed-by: Ken Dreyer <kdreyer@redhat.com>
Reviewed-by: Boris Ranto <branto@redhat.com>
@smithfarm smithfarm deleted the wip-12201 branch July 9, 2015 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@ktdreyer ktdreyer

Labels
bug-fix build/ops
Projects
None yet
Milestone
No milestone
5 participants
@smithfarm @loic-bot @ktdreyer @b-ranto @osynge