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

Set subman cron attributes in spec file #15270

Merged
merged 1 commit into from May 25, 2017

Conversation

Projects
None yet
3 participants
@tserlin
Contributor

tserlin commented May 24, 2017

See downstream BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1265435

This should be backport into Jewel also.

@tserlin tserlin requested a review from ktdreyer May 24, 2017

@ktdreyer

This comment has been minimized.

Member

ktdreyer commented May 24, 2017

Would you please push this to https://github.com/ceph/ceph-ci so Jenkins builds RPMs and we ensure that it installs the file in the way you expect?

Would you please file a tracker.ceph.com ticket for this issue so we ensure it makes it into jewel? (We typically do not refer to RHBZs upstream.)

@tserlin

This comment has been minimized.

Contributor

tserlin commented May 24, 2017

I created a tracker ticket here: http://tracker.ceph.com/issues/20074

Shaman builds are still going.

@tserlin

This comment has been minimized.

Contributor

tserlin commented May 24, 2017

@tserlin

This comment has been minimized.

Contributor

tserlin commented May 25, 2017

OK, so in testing, I did the following in a RHEL 7.3 VM (the nature of the change won't work in CentOS)...

Checked out this PR branch and ran "make-dist" to create a tarball. Examining the ceph.spec file in the tarball shows my PR change.

I also did a rpmbuild with the "ceph-12.0.2-1579-g959fc7c.tar.bz2" tarball: rpmbuild -ba rpmbuild/SPECS/ceph.spec

Did a "yum localinstall ceph-osd-12.0.2-1579.g959fc7c.el7.x86_64.rpm" and its dependencies and the "subman" file in /etc/cron.hourly/ has the execute bits on.

@ktdreyer

This comment has been minimized.

Member

ktdreyer commented May 25, 2017

Thanks, that's great. Want to amend your commit to have the Fixes: line? Then we can track it more easily.

Fixes: http://tracker.ceph.com/issues/20074
Set subman cron attributes in spec file
Fixes: http://tracker.ceph.com/issues/20074

Signed-off-by: Thomas Serlin <tserlin@redhat.com>
@tserlin

This comment has been minimized.

Contributor

tserlin commented May 25, 2017

Ken, I amended the commit with "Fixes" line.

Another build was triggered, but I think this is ready to merge.

@tserlin

This comment has been minimized.

Contributor

tserlin commented May 25, 2017

@alfredodeza

This comment has been minimized.

Contributor

alfredodeza commented May 25, 2017

@tserlin @ktdreyer I am afraid this is not testable in our systems because the script will only be installed when built in/for RHEL if I understand this line correctly [0]:

%if 0%{?rhel} && ! 0%{?centos}
-DWITH_SUBMAN=ON \

[0] https://github.com/ceph/ceph/blob/master/ceph.spec.in#L836

@tserlin

This comment has been minimized.

Contributor

tserlin commented May 25, 2017

Right. That's why I had to test this branch in a RHEL VM to make sure it worked.

Once this is merged, we'll have the proper ceph.spec in our base tarball for downstream's next point release.

@ktdreyer

This comment has been minimized.

Member

ktdreyer commented May 25, 2017

Thanks @tserlin and @alfredodeza

@ktdreyer ktdreyer merged commit a9eb6ec into master May 25, 2017

2 of 3 checks passed

default Build finished.
Details
Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details

ktdreyer added a commit that referenced this pull request May 25, 2017

Merge pull request #15270 from ceph/wip-subman-cron-attr-tserlin
Set subman cron attributes in spec file

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

@ktdreyer ktdreyer deleted the wip-subman-cron-attr-tserlin branch May 25, 2017

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