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-disk: use '-' for feeding ceph cli with stdin #16362

Merged
merged 2 commits into from Jul 25, 2017

Conversation

Projects
None yet
3 participants
@tchaikov
Contributor

tchaikov commented Jul 17, 2017

No description provided.

@tchaikov tchaikov requested a review from Jul 17, 2017

2. journal partition, if co-located with data
3. block.db for BlueStore, if co-located with data
4. block.wal for BlueStore, if co-located with data
5. lockbox

This comment has been minimized.

@tchaikov

tchaikov Jul 17, 2017

Contributor

this change depends on #16247

@tchaikov tchaikov changed the title from ceph-disk: raise exception if cryptsetup fails to doc: add dev doc for ceph-disk Jul 19, 2017

@tchaikov tchaikov removed the cleanup label Jul 19, 2017

@tchaikov tchaikov changed the title from doc: add dev doc for ceph-disk to ceph-disk: use '-' for feeding ceph cli with stdin Jul 19, 2017

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jul 19, 2017

it's a follow-up of #15432.

@ghost

ghost approved these changes Jul 19, 2017

ceph-disk creates partitions for preparing a device for OSD deployment. The partition numbers are:
1. data partition

This comment has been minimized.

@alfredodeza

alfredodeza Jul 19, 2017

Contributor

in rst these can be just # and at rendering time they will get ported to numbers, so there is no need to keep knowing what number goes where

This comment has been minimized.

@tchaikov

tchaikov Jul 19, 2017

Contributor

no, this is on purpose. as the partition number is hard coded in the code.

partitions
==========
ceph-disk creates partitions for preparing a device for OSD deployment. The partition numbers are:

This comment has been minimized.

@alfredodeza

alfredodeza Jul 19, 2017

Contributor

"ceph-disk" should be normalized to have (or not) the double backticks. Here it doesn't have them, at the beginning it does.

lockbox
-------
Ceph-disk supports dmcrypt (device-mapper crypt), so the data and journal devices are encrypted using this machinary. The lockbox is introduced for holding the information regarding how the dmcrypt key are stored. Ceph-disk

This comment has been minimized.

@alfredodeza

alfredodeza Jul 19, 2017

Contributor

This ends with "Ceph-disk" but it doesn't read OK, did you miss out on something else in here?

This comment has been minimized.

@tchaikov

tchaikov Jul 19, 2017

Contributor

yes, i mixed two sentences into one in a bad manner! will fix it.

This comment has been minimized.

@tchaikov

tchaikov Jul 20, 2017

Contributor

@alfredodeza you might want to read

Ceph-disk creates ..., populates ...

lockbox
-------
Ceph-disk supports dmcrypt (device-mapper crypt), so the data and journal devices are encrypted using this machinary. The lockbox is introduced for holding the information regarding how the dmcrypt key are stored. Ceph-disk

This comment has been minimized.

@alfredodeza

alfredodeza Jul 19, 2017

Contributor

Doesn't look like you need to mention "machinery" here, so this would read:

so the data and journal devices are encrypted
``osd_dmcrypt_type``
:Description: this option specifies the mode in which ``cryptsetup`` works. It can be ``luks`` or ``plain``. It kicks in only if the ``--dmcrypy`` option is passed to ``ceph-disk``. See also `cryptsetup document <https://gitlab.com/cryptsetup/cryptsetup/wikis/DMCrypt#configuration-using-cryptsetup>`_ for more details.

This comment has been minimized.

@alfredodeza

alfredodeza Jul 19, 2017

Contributor

s/--dmcrypy/--dmcrypt/

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jul 21, 2017

the failed tests are related to health checking and monitor, and they are addressed by #16477. so i don't think they are relevant.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jul 21, 2017

retest this please.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jul 21, 2017

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jul 21, 2017

all runs in the above test failed, need to fix them first =(

@tchaikov tchaikov self-assigned this Jul 21, 2017

@tchaikov tchaikov requested a review from alfredodeza Jul 24, 2017

lockbox
-------
``Ceph-disk`` supports dmcrypt (device-mapper crypt). If dmcrypt is enabled, the partitions will be encrypted using this machinary. For each OSD device, a lockbox is introduced for holding the information regarding how the dmcrypt key is stored. To prepare a lockbox, ``ceph-disk``

This comment has been minimized.

@alfredodeza

alfredodeza Jul 24, 2017

Contributor

can we normalize on ``ceph-disk`` ? (all lower case with double backticks) It doesn't need to be capitalized at all

partitions
==========
``ceph-disk`` creates partitions for preparing a device for OSD deployment. Their partition numbers are hardcoded. For instance, data partition's partition number is always *1* :

This comment has been minimized.

@alfredodeza

alfredodeza Jul 24, 2017

Contributor

s/Their/The/

This comment has been minimized.

@tchaikov

tchaikov Jul 24, 2017

Contributor

please note, Their == The partitions'. but i will change if you insist.

tchaikov added some commits Jul 17, 2017

doc/dev/ceph-disk: add docs for ceph-disk
we might want to move part of it to user-facing docs.

Signed-off-by: Kefu Chai <kchai@redhat.com>
ceph-disk: s|/dev/fd/0|-|
3a4931b allows ceph cli to use '-' to denote stdin/stdout, so use it
instead of /dev/fd/0.

Signed-off-by: Kefu Chai <kchai@redhat.com>

@liewegas liewegas added this to the luminous milestone Jul 24, 2017

@tchaikov tchaikov merged commit 60d0c67 into ceph:master Jul 25, 2017

3 of 4 checks passed

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

@tchaikov tchaikov deleted the tchaikov:wip-ceph-disk branch Jul 25, 2017

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