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

mimic: ceph-volume: extend batch #29243

Merged
merged 26 commits into from Jul 29, 2019

Conversation

@jan--f
Copy link
Member

jan--f commented Jul 24, 2019

This is the backport for the batch extension to support explicit device lists. Also included are a few commits that haven't been backported and were needed for clean cherry-picks.

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

andrewschoen and others added 19 commits Nov 27, 2018
In the batch command use extents instead of size when creating lvs. This
gives a more precise size and avoids rounding errors.

Signed-off-by: Andrew Schoen <aschoen@redhat.com>
(cherry picked from commit add9f88)

 Conflicts:
	src/ceph-volume/ceph_volume/devices/lvm/strategies/bluestore.py
        remove json import; pick ours and add self.use_large_block_db = False

(cherry picked from commit 83e71842d8cd77b7d45efdd344bd8fac31078745)
We should show the user what the size of the device will be after lvm
creates a pv out of it. This way there isn't a discrepency between the
sizes that are reported to the user and what is actually created.

Signed-off-by: Andrew Schoen <aschoen@redhat.com>
(cherry picked from commit 071e7ce)
The self.use_large_block_db property was never getting set because
the block in compute was never called as block_db_size was reset in
validate if it was 0. We needed to set self.use_large_block_db in
validate instead of compute.

Signed-off-by: Andrew Schoen <aschoen@redhat.com>
(cherry picked from commit fdfb79b)
… size

We know with a mixed type scenario the device used for data will be used
at 100% capacity. This means we do not need to be explict when asking
for the size of the data lvs, which avoids rounding errors with very
small device sizes.

Signed-off-by: Andrew Schoen <aschoen@redhat.com>
(cherry picked from commit 0023961)
This will validate the devices on the size that lvm will allow to be
used, not the raw physical size of the device.

Signed-off-by: Andrew Schoen <aschoen@redhat.com>
(cherry picked from commit aaa915c)
Moving the arg parsing to __init__ allows to get the help message with
running as root.

Signed-off-by: Jan Fajerski <jfajerski@suse.com>
(cherry picked from commit a771781)
This commit refactors lvm strategies to:
a) change the constructor to receive usage specific device lists
b) add classmethod with_auto_devices of automatic splitting by the
rotational flag (keep backwards compatible behavior)
c) rename ssds and hdds member variables to wal_devs and block_devs
d) add db_devs member variable

Signed-off-by: Jan Fajerski <jfajerski@suse.com>
(cherry picked from commit 60dec8b)
Keep filtered_devices as a member variable of the batch object and pass
this to the report functionality instead of having the report functions
retrieve filtered_devices. Also keep selected strategy as a batch member
variable to avoid re-retrival in e.g. the report methods.

Signed-off-by: Jan Fajerski <jfajerski@suse.com>
(cherry picked from commit dc2c4c5)
Signed-off-by: Jan Fajerski <jfajerski@suse.com>
(cherry picked from commit 6dbf43e)
This aims at reducing confusion when reading the code that is shared
between the bluestore and filestore strategies.

Signed-off-by: Jan Fajerski <jfajerski@suse.com>
(cherry picked from commit 029414e)
…et_devices method.

Signed-off-by: Volker Theile <vtheile@suse.com>
(cherry picked from commit 19a3818)
Signed-off-by: Jan Fajerski <jfajerski@suse.com>
(cherry picked from commit 5465712)

 Conflicts:
	src/ceph-volume/ceph_volume/tests/devices/lvm/strategies/test_bluestore.py
	src/ceph-volume/ceph_volume/tests/devices/lvm/strategies/test_filestore.py
        pick with_auto_devices and error.value changes
Signed-off-by: Jan Fajerski <jfajerski@suse.com>
Signed-off-by: Jan Fajerski <jfajerski@suse.com>
Signed-off-by: Jan Fajerski <jfajerski@suse.com>

 Conflicts:
	src/ceph-volume/ceph_volume/tests/devices/lvm/strategies/test_bluestore.py
        Pick test addition and error.value change
Also add test case for a MixedStrategy with unusable data devices.

Signed-off-by: Jan Fajerski <jfajerski@suse.com>

 Conflicts:
	src/ceph-volume/ceph_volume/tests/devices/lvm/strategies/test_filestore.py
        Pick test addition and error-value change
This is so tests can continue to set sys_api['size'] and the code
can retrieve that as Device.lvm_size

Signed-off-by: Andrew Schoen <aschoen@redhat.com>
(cherry picked from commit 9e2175c)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1666822

Signed-off-by: Andrew Schoen <aschoen@redhat.com>
(cherry picked from commit 8ebff47)
(cherry picked from commit aa6da41)
The current help of batch was showing:

rotational drivesare passed in DEVICES

This commit fixes it and now we have:

rotational drives are passed in DEVICES

Tremendous fix, I know.

Signed-off-by: Sébastien Han <seb@redhat.com>
(cherry picked from commit a151e11)
@jan--f

This comment has been minimized.

Copy link
Member Author

jan--f commented Jul 24, 2019

jenkins test ceph-volume batch all

Signed-off-by: Jan Fajerski <jfajerski@suse.com>
(cherry picked from commit c330a47)
@jan--f

This comment has been minimized.

Copy link
Member Author

jan--f commented Jul 24, 2019

jenkins test ceph-volume batch all

@sebastian-philipp sebastian-philipp added this to the mimic milestone Jul 24, 2019
andrewschoen added 2 commits Dec 5, 2018
This will give us the size lvm should report because it takes into
account the 1GB physical extent size we set.

Signed-off-by: Andrew Schoen <aschoen@redhat.com>
(cherry picked from commit 5197c16)
Signed-off-by: Andrew Schoen <aschoen@redhat.com>
(cherry picked from commit 01185fb)
@jan--f

This comment has been minimized.

Copy link
Member Author

jan--f commented Jul 24, 2019

jenkins test ceph-volume batch all

@alfredodeza

This comment has been minimized.

Copy link
Contributor

alfredodeza commented Jul 24, 2019

@jan--f this doesn't have a link to a master PR because it is grouping a few commits that weren't together right?

It is OK if you want to pursue the backporting this way, but in the past we've seen that it was problematic to track what went where if backports weren't done in order. That is: every PR in master has a corresponding backport to other branches.

Again, it is OK if you want to do it this way, I am just pointing out that it might get painful to track commits back to their original PR if they are grouped.

@andrewschoen maybe you have some other thoughts on this? (my comments should not prevent this from moving forward)

Signed-off-by: Jan Fajerski <jfajerski@suse.com>
(cherry picked from commit bea35c6)
@jan--f

This comment has been minimized.

Copy link
Member Author

jan--f commented Jul 24, 2019

jenkins test ceph-volume batch all

@jan--f

This comment has been minimized.

Copy link
Member Author

jan--f commented Jul 24, 2019

@alfredodeza yeah, sady backporting was somewhat neglected for a while and its tough to gather all backports. Trying to get to working state on this branch for now, will see how to resolve this more cleanly.

Copy link
Contributor

alfredodeza left a comment

This should get merged if tests are passing, it is too large for a detailed review... entirely possible that we can miss something, so leaning on tests here

Copy link
Contributor

andrewschoen left a comment

This is also fine with me to get things caught up. It looks like you may have missed commits that added the *_explicit tests though maybe?

@jan--f

This comment has been minimized.

Copy link
Member Author

jan--f commented Jul 26, 2019

hmm seeing failing tests on master too....looking into it.

@alfredodeza

This comment has been minimized.

Copy link
Contributor

alfredodeza commented Jul 26, 2019

this is odd, something may have changed for the NVMe setup. The way this works is by creating a sparse file and using the NVME fabric tooling to create a device recognized by the Kernel. If you look at the output, we do even list it to check if it came up alright:

/dev/nvme0n1     afed1d952161a36e     Linux                                    1          16.11  GB /  16.11  GB    512   B +  0 B   3.10.0-9

What I think is happening here is that /dev/nvme0n1 exists but /dev/nvme1n1 does not. We create two IIRC, and I am not sure what is missing here (or if anything changed to add/remove /dev/nvme1n1)

Signed-off-by: Andrew Schoen <aschoen@redhat.com>
(cherry picked from commit 934c32a)
This is to enable testing of the --block-db-size flag

Signed-off-by: Andrew Schoen <aschoen@redhat.com>
(cherry picked from commit eab962c)
Signed-off-by: Andrew Schoen <aschoen@redhat.com>

Resolves: bz#1650306
(cherry picked from commit 893b61b)
@jan--f

This comment has been minimized.

Copy link
Member Author

jan--f commented Jul 29, 2019

jenkins test ceph-volume batch all

@jan--f

This comment has been minimized.

Copy link
Member Author

jan--f commented Jul 29, 2019

k, found the missing test backports. Thx for your pointers guys.

@jan--f jan--f merged commit 69dd20e into ceph:mimic Jul 29, 2019
20 checks passed
20 checks passed
Docs: build check OK - docs built
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
ceph-volume batch testing centos7-bluestore-mixed_type ceph-volume batch centos7-bluestore-mixed_type OK
Details
ceph-volume batch testing centos7-bluestore-mixed_type_dmcrypt ceph-volume batch centos7-bluestore-mixed_type_dmcrypt OK
Details
ceph-volume batch testing centos7-bluestore-mixed_type_dmcrypt_explicit ceph-volume batch centos7-bluestore-mixed_type_dmcrypt_explicit OK
Details
ceph-volume batch testing centos7-bluestore-mixed_type_explicit ceph-volume batch centos7-bluestore-mixed_type_explicit OK
Details
ceph-volume batch testing centos7-bluestore-single_type ceph-volume batch centos7-bluestore-single_type OK
Details
ceph-volume batch testing centos7-bluestore-single_type_dmcrypt ceph-volume batch centos7-bluestore-single_type_dmcrypt OK
Details
ceph-volume batch testing centos7-filestore-mixed_type ceph-volume batch centos7-filestore-mixed_type OK
Details
ceph-volume batch testing centos7-filestore-mixed_type_dmcrypt ceph-volume batch centos7-filestore-mixed_type_dmcrypt OK
Details
ceph-volume batch testing centos7-filestore-mixed_type_dmcrypt_explicit ceph-volume batch centos7-filestore-mixed_type_dmcrypt_explicit OK
Details
ceph-volume batch testing centos7-filestore-mixed_type_explicit ceph-volume batch centos7-filestore-mixed_type_explicit OK
Details
ceph-volume batch testing centos7-filestore-single_type ceph-volume batch centos7-filestore-single_type OK
Details
ceph-volume batch testing centos7-filestore-single_type_dmcrypt ceph-volume batch centos7-filestore-single_type_dmcrypt OK
Details
ceph-volume batch testing xenial-bluestore-single_type ceph-volume batch xenial-bluestore-single_type OK
Details
ceph-volume batch testing xenial-bluestore-single_type_dmcrypt ceph-volume batch xenial-bluestore-single_type_dmcrypt OK
Details
ceph-volume batch testing xenial-filestore-single_type ceph-volume batch xenial-filestore-single_type OK
Details
ceph-volume batch testing xenial-filestore-single_type_dmcrypt ceph-volume batch xenial-filestore-single_type_dmcrypt OK
Details
make check make check succeeded
Details
jan--f added a commit to jan--f/ceph that referenced this pull request Aug 6, 2019
This fixes a backported test that was not fixed on a backport. Original
fix: ceph#28948. Backport:
ceph#29243.

Signed-off-by: Jan Fajerski <jfajerski@suse.com>
@smithfarm smithfarm changed the title mimic: c-v extend batch mimic: ceph-volume: extend batch Aug 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.