logrotate.conf: fixes for systemd #5086
Conversation
smithfarm
force-pushed
the
wip-12173
branch
2 times, most recently
from
2f1e039
to
6366b18
Compare
|
@smithfarm I know nothing of systemd and probably not the best person to review what looks like a very fine commit. |
ghost
removed their assignment
|
I can't fathom why the test is failing in this way. I can see that the test is calling "ceph-disk activate" which then (apparently) calls ceph-osd, which returns -15: At that point, the trail goes cold for me. |
|
running it through the make check bot one more time |
|
also running make check locally in EL7 docker container |
| @@ -8,6 +8,10 @@ | |||
| service ceph reload >/dev/null | |||
| elif which invoke-rc.d > /dev/null 2>&1 && [ -x `which invoke-rc.d` ]; then | |||
| invoke-rc.d ceph reload >/dev/null | |||
| elif which systemctl > /dev/null 2>&1 && [ -x `which systemctl` ]; then | |||
| if systemctl is-active 'ceph-*' | grep -i active > /dev/null 2>&1 ; then | |||
| systemctl reload 'ceph-*' >/dev/null | |||
There was a problem hiding this comment.
Do we care about the exit code of systemctl reload here? If not we could tack on "|| : . (just looking at what other packages do)
There was a problem hiding this comment.
Do you mean systemctl reload 'ceph-*' || : instead of systemctl reload 'ceph-*' >/dev/null ?
There was a problem hiding this comment.
or systemctl reload 'ceph-*' >/dev/null || : ?
There was a problem hiding this comment.
OK, now I see that the point is to ensure that we don't leave $? set to false for no good reason. So the latter. Will do.
|
Looks good to me! @dachary mind confirming that the make check bot failure isn't related to this change? |
|
I ran make check in a CentOS 7 docker container and |
Before this patch, the command 'logrotate -f /etc/logrotate.d/ceph' was generating an error "Failed to reload ceph.target: Job type reload is not applicable for unit ceph.target". Before we issue systemctl reload, check that there is at least one active ceph-* service. (The hyphen is significant.) Since we use grep, make the grep package a dependency. http://tracker.ceph.com/issues/12173 Fixes: ceph#12173 Signed-off-by: Tim Serong <tserong@suse.com> Signed-off-by: Lars Marowsky-Bree <lmb@suse.com> Signed-off-by: Nathan Cutler <ncutler@suse.com>
|
Local |
|
Ok, let's merge this and assume the make check bot failure is spurious :) |
logrotate.conf: fixes for systemd Reviewed-by: Ken Dreyer <kdreyer@redhat.com>
Before this patch, the command 'logrotate -f /etc/logrotate.d/ceph'
was generating an error "Failed to reload ceph.target: Job type reload is not
applicable for unit ceph.target".
Before we issue systemctl reload, check that there is at least
one active ceph-* service.
Since we use grep, make the grep package a dependency.
http://tracker.ceph.com/issues/12173 Fixes: #12173
Signed-off-by: Tim Serong tserong@suse.com
Signed-off-by: Lars Marowsky-Bree lmb@suse.com
Signed-off-by: Nathan Cutler ncutler@suse.com