cephadm: don't add "ceph-volume lvm activate" for adopted simple OSDs#36463
Merged
sebastian-philipp merged 1 commit intoceph:masterfrom Aug 5, 2020
Merged
cephadm: don't add "ceph-volume lvm activate" for adopted simple OSDs#36463sebastian-philipp merged 1 commit intoceph:masterfrom
sebastian-philipp merged 1 commit intoceph:masterfrom
Conversation
This changes the logic in deploy_daemon_units() to add either `chown` calls for simple (ceph-disk style) OSDs, or to add `ceph-volume lvm activate` calls for LVM OSDs, rather than always adding both. When I was working on ceph#34703, I'd originally added an "osd_simple" flag to figure out what type of OSD was being adopted/deployed, but passing that around was kinda ugly, so was removed from that PR. This time around I'm checking if /etc/ceph/osd/$OSD_ID-$OSD_FSID.json.adopted-by-cephadm exists, which seems pretty safe IMO. My only concern with this method is: what happens if someone adopts a simple OSD, then later wants to migrate it to LVM. Presumably that's a destroy and recreate, keeping the same OSD ID? If that's true, then the JSON file probably still exists, so the subsequent create will do the wrong thing, i.e. will add `chown` calls, not `ceph-volume lvm activate` calls. Any/all feedback appreciated... Fixes: https://tracker.ceph.com/issues/46833 Signed-off-by: Tim Serong <tserong@suse.com>
Contributor
|
This bug was introduced with f9e5dd5 |
sebastian-philipp
approved these changes
Aug 5, 2020
ceph-volume lvm activate for adopted simple OSDs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This changes the logic in deploy_daemon_units() to add either
chowncalls for simple (ceph-disk style) OSDs, or to addceph-volume lvm activatecalls for LVM OSDs, rather than always adding both. When I was working on #34703, I'd originally added an "osd_simple" flag to figure out what type of OSD was being adopted/deployed, but passing that around was kinda ugly, so was removed from that PR. This time around I'm checking if /etc/ceph/osd/$OSD_ID-$OSD_FSID.json.adopted-by-cephadm exists, which seems pretty safe IMO. My only concern with this method is: what happens if someone adopts a simple OSD, then later wants to migrate it to LVM. Presumably that's a destroy and recreate, keeping the same OSD ID? If that's true, then the JSON file probably still exists, so the subsequent create will do the wrong thing, i.e. will addchowncalls, notceph-volume lvm activatecalls. Any/all feedback appreciated...Fixes: https://tracker.ceph.com/issues/46833
Signed-off-by: Tim Serong tserong@suse.com