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

ceph-volume: initial take on ceph-volume CLI tool #16632

Merged
merged 118 commits into from Aug 4, 2017

Conversation

Projects
None yet
7 participants
@alfredodeza
Contributor

alfredodeza commented Jul 27, 2017

There are no docs for this on this PR, since the code got moved from a separate repo, docs will need to migrate to ceph.git. This tool is very well documented.

  • creates the ceph-volume CLI
  • implements the ceph-volume lvm (prepare|activate|create) sub commands
  • adds a systemd service to handle osd activate at boot time
  • adds a separate CLI tool to handle systemd interactions (ceph-volume-systemd)

ceph-ansible support: ceph/ceph-ansible#1716
Pull Request tox tests: ceph/ceph-build#805
Docs that will get ported over: http://docs.ceph.com/ceph-volume/master/

@alfredodeza alfredodeza requested a review from liewegas Jul 27, 2017

@alfredodeza alfredodeza requested a review from andrewschoen Jul 31, 2017

@alfredodeza

This comment has been minimized.

Contributor

alfredodeza commented Jul 31, 2017

jenkins test ceph-volume tox

2 similar comments
@alfredodeza

This comment has been minimized.

Contributor

alfredodeza commented Jul 31, 2017

jenkins test ceph-volume tox

@alfredodeza

This comment has been minimized.

Contributor

alfredodeza commented Aug 1, 2017

jenkins test ceph-volume tox

Expected input is similar to::
['/path/to/ceph-volume-systemd', '<osd id>-<osd uuid>-<device type>']

This comment has been minimized.

@liewegas

liewegas Aug 1, 2017

Member

since we're going to be adding more device types in the future (e.g., gpt to support old ceph-disk style stuff, probably some nvme pci id to support SPDK kernel bypass weirdness), i wonder if we should instead make this all opaque, with a "$device_type-$opaqueid" id. It all just gets passed to ceph-volume anyway, so this function has no need to parse it. Then for other non-lvm stuff it can have whatever form ends up being necessary (not necessarily an osd id + uuid!).

I was originally imagining that we would have per-type systemd unit files, e.g. ceph-volume-lvm@whatever and ceph-volume-gpt@otherstuff. That's basically teh same thing, except the type is part of the unit file name instead of a piece of the id. I guess I ever so slightly prefer that style since it makes the id less mangley, but prepending it to the id for a generic ceph-volume@ unit works just fine to. Just as long as we don't codify it here and box ourselves into a corner...

This comment has been minimized.

@alfredodeza

alfredodeza Aug 1, 2017

Contributor

The current implementation doesn't expose anything. This executable lives in the same code base but it is a separate script for that same reason: it is not available in any of the ceph-volume sub-commands.

The ceph-volume-systemd script is not front-facing, and it is just there to accommodate for the parsing of the various other sub-systems. You are right that is kind of requiring 3 parameters, but a sub-command (e.g. nvme) could chose to add a noop.

As it is right now, this is directly consumable by any new tech that comes around, using the same script(s) and same systemd units with no modification. All one would need to do is to hook into the internal API and call the helpers to enable the units.

If we went the route of ceph-volume-gpt that would mean somehow somewhere another systemd unit would need to exist. With this approach it doesn't need anything else.

This comment has been minimized.

@liewegas

liewegas Aug 1, 2017

Member

What if the args aren't just unused, but aren't integers or uuids?

Why not just make it @$type-$stuff, and pass that through to 'ceph-volume $type trigger $stuff' (or similar), and drop all of the parsing code in here? Basically I think just switching $type to the beginning of the '-' separated list instead of the end makes it unnecessary to parse anything but the first -. The uuid/id parsing would then happen inside the main ceph-volume command with the lvm code which knows what to expect.

This will be harder to change later because all of the existing systemd units in the wild will have the old format, and this script will end up growing weird conditional parsing/logic for all the various $types. I think with this change it wouldn't have to change at all.

This comment has been minimized.

@alfredodeza

alfredodeza Aug 1, 2017

Contributor

Ok, I see where you are going. I agree here that it is useful to allow for extra/non-strict stuff. Since I was relying on correct argument passing to ceph-volume lvm activate I think it might be reasonable to pass a flag that indicates a "blob" is getting passed instead. Maybe something like:

ceph-volume@lvm-sha1-osd_id

Would end up calling:

ceph-volume lvm activate --systemd sha1-osd_id

In a similar way then, nvme could be:

ceph-volume@nvme-foo-100-bar-1

Would go then to:

ceph-volume nvme activate --systemd foo-100-bar-1

Does that sound flexible enough?

This comment has been minimized.

@liewegas

liewegas Aug 1, 2017

Member

Exactly! I might do a different verb than activate (e.g., 'systemd-trigger') just in case the argument parsing between the two modes of systemd and non-systemd operation is awkward, but you know better than I how the python libs behave.

This comment has been minimized.

@alfredodeza

alfredodeza Aug 1, 2017

Contributor

I guess that also sounds reasonable. To have a 'trigger' sub-command would mean the same as a flag like I was proposing but cleaner. So we would end up going from:

ceph-volume@nvme-foo-100-bar-1

To

ceph-volume nvme trigger foo-100-bar-1

This comment has been minimized.

@liewegas

This comment has been minimized.

@wjwithagen

wjwithagen Aug 1, 2017

Contributor

@alfredodeza @liewegas
At least take into consideration that there is life without systemd. :)
And those might need to do init/rc.d/.... stuff to get the OSDs started.

This comment has been minimized.

@alfredodeza

alfredodeza Aug 3, 2017

Contributor

@liewegas this has been addressed

@alfredodeza alfredodeza requested a review from b-ranto Aug 2, 2017

@@ -1021,6 +1027,7 @@ if [ $FIRST_ARG -ge 1 ] ; then
fi
if [ "X$CEPH_AUTO_RESTART_ON_UPGRADE" = "Xyes" ] ; then
/usr/bin/systemctl try-restart ceph-disk@\*.service > /dev/null 2>&1 || :
/usr/bin/systemctl try-restart ceph-volume@\*.service > /dev/null 2>&1 || :

This comment has been minimized.

@b-ranto

b-ranto Aug 2, 2017

Contributor

All the systemctl commands/macros support multiple arguments so you do not need to make a new call for ceph-volume*.

Other than that, the spec file changes lgtm.

This comment has been minimized.

@alfredodeza

alfredodeza Aug 3, 2017

Contributor

@b-ranto this has been updated

@alfredodeza

This comment has been minimized.

Contributor

alfredodeza commented Aug 2, 2017

jenkins test ceph-volume tox

@andrewschoen

I've been testing this branch with a functional test included in ceph/ceph-ansible#1716

Here's a link to a successful test run: https://jenkins.ceph.com/view/ceph-ansible/job/ceph-ansible-scenario/150/console

Here's the ouput that shows ceph-volume creating the OSD:

TASK [ceph-osd : use ceph-volume to create filestore osds with dedicated journals] ***
task path: /home/jenkins-build/build/workspace/ceph-ansible-scenario/roles/ceph-osd/tasks/scenarios/lvm.yml:2
changed: [osd0] => (item={'key': u'test_volume', 'value': u'/dev/sdb'}) => {
    "changed": true, 
    "cmd": [
        "ceph-volume", 
        "lvm", 
        "create", 
        "--filestore", 
        "--data", 
        "test_volume", 
        "--journal", 
        "/dev/sdb"
    ], 
    "delta": "0:00:18.427975", 
    "end": "2017-08-04 11:26:49.833672", 
    "item": {
        "key": "test_volume", 
        "value": "/dev/sdb"
    }, 
    "rc": 0, 
    "start": "2017-08-04 11:26:31.405697", 
    "warnings": []
}

STDOUT:

Running command: ceph-authtool --gen-print-key
Running command: ceph --cluster ceph --name client.bootstrap-osd --keyring /var/lib/ceph/bootstrap-osd/ceph.keyring -i - osd new 95fec51d-719a-4d08-bf55-c71ac94c5043
Running command: sudo vgs --reportformat=json
Running command: sudo lvs -o lv_tags,lv_path,lv_name,vg_name --reportformat=json
Running command: sudo vgs --reportformat=json
Running command: sudo lvs -o lv_tags,lv_path,lv_name,vg_name --reportformat=json
Running command: sudo lvchange --addtag ceph.osd_id=0 /dev/test_group/test_volume
Running command: sudo lvchange --addtag ceph.osd_fsid=95fec51d-719a-4d08-bf55-c71ac94c5043 /dev/test_group/test_volume
Running command: sudo lvchange --addtag ceph.type=data /dev/test_group/test_volume
Running command: sudo lvchange --addtag ceph.cluster_fsid=2f4ad268-1440-4b49-9b84-d6f97655e738 /dev/test_group/test_volume
Running command: sudo lvchange --addtag ceph.journal_device=/dev/sdb /dev/test_group/test_volume
Running command: sudo lvchange --addtag ceph.data_device=/dev/test_group/test_volume /dev/test_group/test_volume
Running command: sudo lvs -o lv_tags,lv_path,lv_name,vg_name --reportformat=json
Running command: ceph-authtool --gen-print-key
Running command: sudo mkfs -t xfs -f -i size=2048 /dev/test_group/test_volume
 stdout: meta-data=/dev/test_group/test_volume isize=2048   agcount=4, agsize=3276544 blks
         =                       sectsz=512   attr=2, projid32bit=1
         =                       crc=1        finobt=0, sparse=0
data     =                       bsize=4096   blocks=13106176, imaxpct=25
         =                       sunit=0      swidth=0 blks
naming   =version 2              bsize=4096   ascii-ci=0 ftype=1
log      =internal log           bsize=4096   blocks=6399, version=2
         =                       sectsz=512   sunit=0 blks, lazy-count=1
realtime =none                   extsz=4096   blocks=0, rtextents=0
Running command: sudo mount -t xfs -o noatime,inode64 /dev/test_group/test_volume /var/lib/ceph/osd/ceph-0
Running command: sudo ln -s /dev/sdb /var/lib/ceph/osd/ceph-0/journal
Running command: sudo ceph --cluster ceph --name client.bootstrap-osd --keyring /var/lib/ceph/bootstrap-osd/ceph.keyring mon getmap -o /var/lib/ceph/osd/ceph-0/activate.monmap
 stderr: got monmap epoch 1
Running command: chown -R ceph:ceph /dev/sdb
Running command: chown -R ceph:ceph /var/lib/ceph/osd/ceph-0/
Running command: sudo ceph-osd --cluster ceph --mkfs -i 0 --monmap /var/lib/ceph/osd/ceph-0/activate.monmap --osd-data /var/lib/ceph/osd/ceph-0/ --osd-journal /var/lib/ceph/osd/ceph-0/journal --osd-uuid 95fec51d-719a-4d08-bf55-c71ac94c5043 --setuser ceph --setgroup ceph
 stderr: 2017-08-04 11:26:43.953089 7fbb30058d00 -1 journal check: ondisk fsid 00000000-0000-0000-0000-000000000000 doesn't match expected 95fec51d-719a-4d08-bf55-c71ac94c5043, invalid (someone else's?) journal
 stderr: 2017-08-04 11:26:44.313094 7fbb30058d00 -1 read_settings error reading settings: (2) No such file or directory
Running command: ceph-authtool /var/lib/ceph/osd/ceph-0/keyring --create-keyring --name osd.0 --add-key AQDnWYRZaA+SHBAALkbgD0T9nYPpXj8ovWtRcw==
 stdout: creating /var/lib/ceph/osd/ceph-0/keyring
added entity osd.0 auth auth(auid = 18446744073709551615 key=AQDnWYRZaA+SHBAALkbgD0T9nYPpXj8ovWtRcw== with 0 caps)
Running command: chown -R ceph:ceph /var/lib/ceph/osd/ceph-0/keyring
Running command: sudo lvs -o lv_tags,lv_path,lv_name,vg_name --reportformat=json
Running command: sudo mount -v /dev/test_group/test_volume /var/lib/ceph/osd/ceph-0
 stderr: mount: /dev/mapper/test_group-test_volume is already mounted or /var/lib/ceph/osd/ceph-0 busy
 stderr: /dev/mapper/test_group-test_volume is already mounted on /var/lib/ceph/osd/ceph-0
Running command: chown -R ceph:ceph /dev/sdb
Running command: sudo systemctl enable ceph-volume@lvm-0-95fec51d-719a-4d08-bf55-c71ac94c5043
 stderr: Created symlink from /etc/systemd/system/multi-user.target.wants/ceph-volume@lvm-0-95fec51d-719a-4d08-bf55-c71ac94c5043.service to /usr/lib/systemd/system/ceph-volume@.service.
Running command: sudo systemctl start ceph-osd@0

alfredodeza added some commits Jun 16, 2017

ceph-volume: rename: initial take on renaming to ceph-volume
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: config: default to 'info' verbosity, set the config dict
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: log: create a utility for setting up logging
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: log: be more robust, report back to config
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: decorators: catch exceptions, disable on debug
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: terminal: easier terminal reporting utils
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: main: initial take on main
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: process: module for running system commands
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: systemd: create systemd entry point script
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: log: notify to stdout if it is not possible to write to …
…the log location

Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: main: no need to pass the config to log setup
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: terminal: use raw instead of write
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: main: use subhelp to parse lvm's help
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: decorators: add a check for super user privileges
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: systemd: move the exceptions to the new exception module
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: exceptions: create a module for all exceptions
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: lvm: initial take on tag api
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: exceptions: create a MultipleLV error
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: main: remove extra new lines when generating help
Signed-off-by: Alfredo Deza <adeza@redhat.com>

alfredodeza added some commits Jul 25, 2017

ceph-volume: util: use osd new for creating osds
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: lvm: activate should not use the bootstrap-osd to auth a…
…dd the keyring

Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: lvm remove the creation of the osd so early in the prepa…
…re process

Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: lvm consume the JSON secrets for preparing
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: util prepare should pass -i to consume stdin
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: systemd: add helpers for the ceph-volume systemd unit
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: prevent missing conf values for logging config
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: systemd script should pass pre-configured log args
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: devices chown the journal when activating
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: systemd should retry several times to activate a device
Allows environment variables to tweak the retries and intervals,
defaulting to 30 tries at 5 second intervals.

Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume lvm.activate remove unused import for activate utils
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume remove activate utilities, not needed with 'osd new'
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: tests add an lvm trigger for the systemd argument parsing
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: tests update argument parsing for systemd
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: lvm: add a trigger sub-command to parent parser
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: lvm add the actual trigger subcommand handler
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: systemd script should handle type-data format as argument
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume: decorators should call the wrapped func
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume lvm activate should check for root
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume lvm create should check for root
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume lvm prepare should check for root
Signed-off-by: Alfredo Deza <adeza@redhat.com>
ceph-volume lvm trigger should check for root
Signed-off-by: Alfredo Deza <adeza@redhat.com>

@liewegas liewegas changed the title from [ceph-volume] initial take on create the ceph-volume CLI tool to ceph-volume: initial take on create the ceph-volume CLI tool Aug 4, 2017

@liewegas liewegas added this to the luminous milestone Aug 4, 2017

@liewegas liewegas changed the title from ceph-volume: initial take on create the ceph-volume CLI tool to ceph-volume: initial take on ceph-volume CLI tool Aug 4, 2017

@liewegas liewegas merged commit 9d95f1a into master Aug 4, 2017

2 of 4 checks passed

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

@liewegas liewegas deleted the wip-volume branch Aug 4, 2017

@smithfarm

This comment has been minimized.

Contributor

smithfarm commented Aug 4, 2017

This PR introduced an RPM packaging regression: http://tracker.ceph.com/issues/20915

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