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

selinux: Do parallel relabel on package install #14871

Merged
merged 3 commits into from May 31, 2017

Conversation

Projects
None yet
4 participants
@b-ranto
Contributor

b-ranto commented Apr 28, 2017

We can take advantage of ceph-disk fix subcommand when doing a package
install. We will keep using the differential fixfiles command otherwise.

We also need to add relabel for /usr/bin/ daemons so that we could use
this.

Signed-off-by: Boris Ranto branto@redhat.com

This should help when we are upgrading from a non-SELinux enabled ceph version to a SELinux enabled ceph version since we will be doing relabel in parallel per OSD.

@smithfarm smithfarm added the build/ops label Apr 28, 2017

@b-ranto b-ranto requested review from ktdreyer and smithfarm May 15, 2017

@b-ranto

This comment has been minimized.

Contributor

b-ranto commented May 15, 2017

@ktdreyer @smithfarm could you review? The idea here is quite simple -- if we are doing full relabel we might as well use the ceph-disk fix for that to relabel in parallel per OSD.

@ktdreyer

This comment has been minimized.

Member

ktdreyer commented May 15, 2017

So fixfiles runs in parallel but ceph-disk --selinux does not?

@b-ranto

This comment has been minimized.

Contributor

b-ranto commented May 15, 2017

@ktdreyer the other way around, ceph-disk fix runs restorecon in parallel per osd but fixfiles does not, it is a post script so if $1 = 1 it is a first installation of the package, not an update (correct me if I am wrong, here)

@b-ranto b-ranto requested a review from tchaikov May 25, 2017

@b-ranto

This comment has been minimized.

Contributor

b-ranto commented May 25, 2017

Could you review/merge? This is especially helpful when people are upgrading from a non-SELinux enabled ceph version to an SELinux enabled one and they are running with SELinux enabled (be it in Permissive or Enforcing mode).

@smithfarm smithfarm requested a review from May 25, 2017

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented May 25, 2017

I have looked at this, and it seems OK to me, but don't know enough about the subject to approve the PR.

@ghost

ghost approved these changes May 25, 2017

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented May 25, 2017

@b-ranto Run it through a ceph-disk suite?

@smithfarm smithfarm added the needs-qa label May 25, 2017

@ghost

This comment has been minimized.

ghost commented May 25, 2017

@smithfarm there are no tests covering that part of the code

@ktdreyer

This comment has been minimized.

Member

ktdreyer commented May 25, 2017

Would you please file a http://tracker.ceph.com/ ticket for this to ensure it gets into jewel?

@b-ranto

This comment has been minimized.

Contributor

b-ranto commented May 25, 2017

@smithfarm The only test we have for that part of the code is run by the build itself.

@ktdreyer done, I have also updated the commit messages to include the Fixes line

@ktdreyer

Untested, but LGTM! thanks

@smithfarm smithfarm removed the needs-qa label May 25, 2017

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented May 25, 2017

Reviewed-by: Loic Dachary <ldachary@redhat.com>
Reviewed-by: Ken Dreyer <kdreyer@redhat.com>
@smithfarm

This comment has been minimized.

Contributor

smithfarm commented May 25, 2017

@b-ranto ceph-disk unit test failure?

@b-ranto

This comment has been minimized.

Contributor

b-ranto commented May 26, 2017

@smithfarm Yeah, I had to update the unit test because now, the code is testing for presence of the files to restorecon/chown and if ceph is not installed on the machine doing the build, it won't do any restorecon/chown so the assert looking for these fails.

@b-ranto

This comment has been minimized.

Contributor

b-ranto commented May 26, 2017

QE raised a very good point that ceph-disk binary is not available on monitors (i.e. without ceph-osd package installed). We are already shipping the ceph-disk library in ceph-base so moving ceph-disk to ceph-base seems like a reasonable next step. I have rebased the branch and addressed that in the latest commit. Re-review the commit please.

@ktdreyer ktdreyer requested a review from alfredodeza May 26, 2017

@ktdreyer

This comment has been minimized.

Member

ktdreyer commented May 26, 2017

Using ceph-disk to relabel monitors feels really weird to me

# Relabel the files
# Use ceph-disk fix for first package install and fixfiles otherwise
if [ "$1" = "1" ]; then
/usr/sbin/ceph-disk fix --selinux

This comment has been minimized.

@alfredodeza

alfredodeza May 26, 2017

Contributor

@b-ranto it doesn't look like ceph-disk does too much to make this happen, would it be possible to just run the same shell command here instead? Or what was the difficulty you saw with that approach?

This comment has been minimized.

@b-ranto

b-ranto May 27, 2017

Contributor

The ceph-disk command does the relabel in parallel per osd which can speed up the relabel a lot as the OSDs usually reside on different disks.

@b-ranto

This comment has been minimized.

Contributor

b-ranto commented May 27, 2017

@ktdreyer it relabels all of ceph data including mon/mds/rgw/... data, it won't speed up the relabel on monitors (it runs the same commands) but making it conditional on the installed packages just does not feel right to me

b-ranto added some commits Apr 28, 2017

selinux: Do parallel relabel on package install
We can take advantage of ceph-disk fix subcommand when doing a package
install. We will keep using the differential fixfiles command otherwise.

We also need to add relabel for /usr/bin/ daemons so that we could use
this.

Fixes: http://tracker.ceph.com/issues/20077

Signed-off-by: Boris Ranto <branto@redhat.com>
ceph-disk: Fix the file ownership, skip missing
This commit fixes the file ownership for the /usr/bin/ and /etc/ceph
files and skips missing files as some of the files that we do specify
now can be missing from the system (not installed, e.f. radosgw).

Fixes: http://tracker.ceph.com/issues/20077

Signed-off-by: Boris Ranto <branto@redhat.com>
rpm: Move ceph-disk to ceph-base
The SELinux package now requires the ceph-disk binary but that one was
part of the ceph-osd package. The ceph-disk python library is already
packaged in ceph-base so moving ceph-disk to ceph-base seems like a
reasonable next step.

Signed-off-by: Boris Ranto <branto@redhat.com>
@b-ranto

This comment has been minimized.

Contributor

b-ranto commented May 31, 2017

jenkins retest this please

@b-ranto b-ranto merged commit f086499 into master May 31, 2017

3 checks passed

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

@b-ranto b-ranto deleted the wip-selinux-optimize branch May 31, 2017

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