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.

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

rpm: unconditionally set ceph user's primary group to ceph (SUSE) #9106

Merged
merged 1 commit into from
Jun 1, 2016

Conversation

smithfarm
Copy link
Contributor

@smithfarm smithfarm commented May 12, 2016

This commit brings the user/group creation into greater semantic alignment
with the Debian packaging, but only for SUSE.

Fixes: http://tracker.ceph.com/issues/15869
Signed-off-by: Nathan Cutler ncutler@suse.com

@smithfarm
Copy link
Contributor Author

Note that in Debian there is an unconditional usermod - see https://github.com/ceph/ceph/blob/v10.2.0/debian/ceph-common.postinst#L59-L65

This commit brings the user/group creation into greater semantic alignment
with the Debian packaging.

Fixes: http://tracker.ceph.com/issues/15869
Signed-off-by: Nathan Cutler <ncutler@suse.com>
@smithfarm smithfarm changed the title rpm: set pre-existing ceph user's gid to match ceph group rpm: unconditionally set ceph user's primary group to ceph (SUSE) May 12, 2016
@smithfarm
Copy link
Contributor Author

smithfarm commented May 12, 2016

@ktdreyer @BRANTO1 I'm a little worried about the RH user/group creation semantics - especially the -o which could result in multiple "ceph" users and groups (with different uids/gids) existing on the system at the same time. (Really? It sounds weird, but that's my reading of the useradd and groupadd manpages.)

@b-ranto
Copy link
Contributor

b-ranto commented May 12, 2016

The -o flag comes from ed0cd42 by @dalgaaf. I suppose it is intentional to create the user/group with the default system uid/gid no matter if the user already exists?

@smithfarm
Copy link
Contributor Author

smithfarm commented May 13, 2016

The -o flag comes from ed0cd42 by @dalgaaf. I suppose it is intentional to create the user/group with the default system uid/gid no matter if the user already exists?

Yes, that was my assumption, too, but intentional or not I don't think it's good for the ceph packaging to create a second "ceph" user with a differing UID. Do you?

Also, the behavior is inconsistent across the supported distros - Debian and SUSE packaging do not use -o

@smithfarm
Copy link
Contributor Author

smithfarm commented May 13, 2016

I created http://tracker.ceph.com/issues/15876 to track the RH -o issue.

@b-ranto
Copy link
Contributor

b-ranto commented May 13, 2016

I'm not sure, here. It depends on how the underlying ceph daemons handle the case of multiple ceph users/groups. AFAICR, it was ok (well, even better for scenarios like disk-swapping) for us to use the -o option when the user changes were created.

Nevertheless, the underlying daemons should be able to handle this case as we might be on a system that created ceph user/group before it was a system user/group and once the system user/group was introduced, the system would have two ceph user.

@smithfarm
Copy link
Contributor Author

Nevertheless, the underlying daemons should be able to handle this case as we might be on a system that created ceph user/group before it was a system user/group and once the system user/group was introduced, the system would have two ceph user.

Yes, but only in RH/CentOS/Fedora. The Debian/Ubuntu and SUSE packaging takes care not to create a second ceph user.

@b-ranto
Copy link
Contributor

b-ranto commented May 19, 2016

Yes, but only in RH/CentOS/Fedora. The Debian/Ubuntu and SUSE packaging takes care not to create a second ceph user.

I would not be that sure about it. What happens if you have your own ceph user, upgrade the machine and the new system package that creates a ceph user with system (static) uid comes up? I doubt it will modify or delete the existing user.

@smithfarm
Copy link
Contributor Author

@BRANTO1 It does modify the existing user to ensure its primary group is "ceph". Here's what happens in Debian/Ubuntu: https://github.com/ceph/ceph/blob/master/debian/ceph-common.postinst#L39-L65

What happens in SUSE is the subject matter of this PR. . .

@smithfarm
Copy link
Contributor Author

Essentially, the daemons cannot assume that the user "ceph" will be a system user.

@b-ranto
Copy link
Contributor

b-ranto commented May 20, 2016

The daemons don't (afaik, they get the UID from packaging), the users might (especially when hot-swapping disks). I guess that was the reason for forcing non-unique users, there.

Anyway, I originally thought that when you add a new system/static UID to the packaging (package setup in fedora/rhel), it will be automatically created after upgrade. I'm not so sure about it any more, though.

@smithfarm
Copy link
Contributor Author

@ktdreyer Pinging in case you'd like to review. If there are no objections, I will merge.

@ktdreyer
Copy link
Member

ktdreyer commented Jun 1, 2016

👍 LGTM

@smithfarm smithfarm merged commit 048aa3f into ceph:master Jun 1, 2016
@smithfarm smithfarm deleted the wip-15869 branch June 1, 2016 16:14
@smithfarm
Copy link
Contributor Author

Note this PR introduced a regression (trailing whitespace) which is fixed by #10707

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants