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

jewel: build/ops: deb: Fix logrotate packaging #15428

Merged
merged 1 commit into from Aug 22, 2017

Conversation

Projects
None yet
2 participants
@smithfarm
Contributor

smithfarm commented Jun 2, 2017

@smithfarm smithfarm self-assigned this Jun 2, 2017

@smithfarm smithfarm added this to the jewel milestone Jun 2, 2017

@smithfarm

This comment has been minimized.

Show comment
Hide comment
@smithfarm

smithfarm Jun 2, 2017

Contributor

Needs to be tested using a technique similar to the one demonstrated by @tchaikov in #14600 (comment)

Contributor

smithfarm commented Jun 2, 2017

Needs to be tested using a technique similar to the one demonstrated by @tchaikov in #14600 (comment)

@smithfarm

This comment has been minimized.

Show comment
Hide comment

@smithfarm smithfarm added build/ops and removed core labels Jun 2, 2017

@smithfarm smithfarm requested a review from tchaikov Jun 2, 2017

@smithfarm

This comment has been minimized.

Show comment
Hide comment
@smithfarm

smithfarm Jun 2, 2017

Contributor

Tested with:

$ wget https://3.chacra.ceph.com/r/ceph/wip-20162-jewel/201c6c5b916466ef05246f9cbf01c5eb61a72328/ubuntu/xenial/flavors/default/pool/main/c/ceph/ceph-common_10.2.7-251-g201c6c5-1xenial_amd64.deb
$ wget https://3.chacra.ceph.com/r/ceph/wip-20162-jewel/201c6c5b916466ef05246f9cbf01c5eb61a72328/ubuntu/xenial/flavors/default/pool/main/c/ceph/ceph-base_10.2.7-251-g201c6c5-1xenial_amd64.deb
$ dpkg-deb -x ceph-common_10.2.7-251-g201c6c5-1xenial_amd64.deb c
$ dpkg-deb -x ceph-base_10.2.7-251-g201c6c5-1xenial_amd64.deb d
$ ls -l c/etc/logrotate.d/
ls: cannot access 'c/etc/logrotate.d/': No such file or directory
$ ls -l d/etc/logrotate.d/
total 4
-rw-r--r-- 1 ubuntu ubuntu 228 Jun  2 08:30 ceph-base.logrotate
Contributor

smithfarm commented Jun 2, 2017

Tested with:

$ wget https://3.chacra.ceph.com/r/ceph/wip-20162-jewel/201c6c5b916466ef05246f9cbf01c5eb61a72328/ubuntu/xenial/flavors/default/pool/main/c/ceph/ceph-common_10.2.7-251-g201c6c5-1xenial_amd64.deb
$ wget https://3.chacra.ceph.com/r/ceph/wip-20162-jewel/201c6c5b916466ef05246f9cbf01c5eb61a72328/ubuntu/xenial/flavors/default/pool/main/c/ceph/ceph-base_10.2.7-251-g201c6c5-1xenial_amd64.deb
$ dpkg-deb -x ceph-common_10.2.7-251-g201c6c5-1xenial_amd64.deb c
$ dpkg-deb -x ceph-base_10.2.7-251-g201c6c5-1xenial_amd64.deb d
$ ls -l c/etc/logrotate.d/
ls: cannot access 'c/etc/logrotate.d/': No such file or directory
$ ls -l d/etc/logrotate.d/
total 4
-rw-r--r-- 1 ubuntu ubuntu 228 Jun  2 08:30 ceph-base.logrotate

@smithfarm smithfarm changed the title from jewel: Two logrotate scripts to [DNM] jewel: Two logrotate scripts Jun 2, 2017

@smithfarm

This comment has been minimized.

Show comment
Hide comment
@smithfarm

smithfarm Jun 2, 2017

Contributor

Marking DNM until we get positive confirmation that this is really necessary. See http://tracker.ceph.com/issues/15569#note-10

Contributor

smithfarm commented Jun 2, 2017

Marking DNM until we get positive confirmation that this is really necessary. See http://tracker.ceph.com/issues/15569#note-10

@tchaikov

This comment has been minimized.

Show comment
Hide comment
@tchaikov

tchaikov Jun 15, 2017

Contributor

@smithfarm do you think we should backport #15567 as well?

Contributor

tchaikov commented Jun 15, 2017

@smithfarm do you think we should backport #15567 as well?

@smithfarm

This comment has been minimized.

Show comment
Hide comment
@smithfarm

smithfarm Jun 15, 2017

Contributor

@tchaikov The jewel debian packaging does not have anything fancy. In #15567 it says:

Backporting to Kraken should work fine, Jewel could do a manual move similar to what is proposed in #15428 , but to ceph-common?

So, if you agree, I'll s/ceph-base/ceph-common/g in this patch and call it done?

Contributor

smithfarm commented Jun 15, 2017

@tchaikov The jewel debian packaging does not have anything fancy. In #15567 it says:

Backporting to Kraken should work fine, Jewel could do a manual move similar to what is proposed in #15428 , but to ceph-common?

So, if you agree, I'll s/ceph-base/ceph-common/g in this patch and call it done?

@smithfarm smithfarm changed the title from [DNM] jewel: Two logrotate scripts to [DNM] jewel: build/ops: deb: Fix logrotate packaging Jun 15, 2017

@tchaikov

This comment has been minimized.

Show comment
Hide comment
@tchaikov

tchaikov Jun 15, 2017

Contributor

@smithfarm i agree, and thanks =)

Contributor

tchaikov commented Jun 15, 2017

@smithfarm i agree, and thanks =)

@smithfarm smithfarm changed the title from [DNM] jewel: build/ops: deb: Fix logrotate packaging to jewel: build/ops: deb: Fix logrotate packaging Jun 15, 2017

build/ops: deb: fix logrotate packaging
This minimal jewel-only fix is not cherry-picked from master because
the Debian packaging was refactored between jewel and master and the
master fix is totally different.

Fixes: http://tracker.ceph.com/issues/20316
Signed-off-by: Nathan Cutler <ncutler@suse.com>
@smithfarm

This comment has been minimized.

Show comment
Hide comment
@smithfarm

smithfarm Jun 15, 2017

Contributor

@tchaikov Restaged, redone, ready for review.

Contributor

smithfarm commented Jun 15, 2017

@tchaikov Restaged, redone, ready for review.

@@ -67,7 +67,7 @@ clean:
ltmain.sh missing
rm -f configure Makefile.in man/Makefile.in src/Makefile.in
rm -f src/acconfig.h.in
rm -f debian/ceph-base.ceph.init debian/radosgw.init debian/ceph.logrotate debian/radosgw.logrotate
rm -f debian/ceph-base.ceph.init debian/radosgw.init debian/ceph-common.logrotate

This comment has been minimized.

@smithfarm

smithfarm Jun 15, 2017

Contributor

@tchaikov When a current jewel user upgrades to a new version containing this change, is this the line that cleans up the old files? I wonder if we should retain debian/ceph.logrotate here.

@smithfarm

smithfarm Jun 15, 2017

Contributor

@tchaikov When a current jewel user upgrades to a new version containing this change, is this the line that cleans up the old files? I wonder if we should retain debian/ceph.logrotate here.

This comment has been minimized.

@tchaikov

tchaikov Jun 15, 2017

Contributor

no, these lines instruct how "make clean" is supposed to cleanup the package maintainer's building directory. it has nothing to do with the end user.

@tchaikov

tchaikov Jun 15, 2017

Contributor

no, these lines instruct how "make clean" is supposed to cleanup the package maintainer's building directory. it has nothing to do with the end user.

@smithfarm smithfarm merged commit c795241 into ceph:jewel Aug 22, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details

@smithfarm smithfarm deleted the smithfarm:wip-20162-jewel branch Aug 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment