From ca1d3e694c6390632c86e6dfcf01a2b3c6a67f0d Mon Sep 17 00:00:00 2001
From: Rishabh Dave <ridave@redhat.com>
Date: Sat, 4 Jan 2020 13:54:58 +0530
Subject: [PATCH] ceph-volume: delete devices.lvm.prepare.Prepare.get_lv

Since it calls get_lv_from_argument() internally and use get_first_lv()
instead and update tests.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
---
 .../ceph_volume/devices/lvm/prepare.py        | 52 +++++++++++--------
 .../tests/devices/lvm/test_prepare.py         | 16 ------
 2 files changed, 31 insertions(+), 37 deletions(-)

diff --git a/src/ceph-volume/ceph_volume/devices/lvm/prepare.py b/src/ceph-volume/ceph_volume/devices/lvm/prepare.py
index 2e07d15ec0a03..e91e6507df5a2 100644
--- a/src/ceph-volume/ceph_volume/devices/lvm/prepare.py
+++ b/src/ceph-volume/ceph_volume/devices/lvm/prepare.py
@@ -135,23 +135,6 @@ class Prepare(object):
             raise RuntimeError('unable to use device')
         return uuid
 
-    # TODO: get rid of this method?
-    def get_lv(self, argument):
-        """
-        Perform some parsing of the command-line value so that the process
-        can determine correctly if it got a device path or an lv.
-
-        :param argument: The command-line value that will need to be split to
-                         retrieve the actual lv
-        """
-        #TODO is this efficient?
-        try:
-            vg_name, lv_name = argument.split('/')
-        except (ValueError, AttributeError):
-            return None
-        return api.get_first_lv(filters={'lv_name': lv_name, 'vg_name':
-                                         vg_name})
-
     def setup_device(self, device_type, device_name, tags, size):
         """
         Check if ``device`` is an lv, if so, set the tags, making sure to
@@ -165,7 +148,14 @@ class Prepare(object):
             return '', '', tags
         tags['ceph.type'] = device_type
         tags['ceph.vdo'] = api.is_vdo(device_name)
-        lv = self.get_lv(device_name)
+
+        try:
+            vg_name, lv_name = device_name.split('/')
+            lv = api.get_first_lv(filters={'lv_name': lv_name,
+                                           'vg_name': vg_name})
+        except ValueError:
+            lv = None
+
         if lv:
             uuid = lv.lv_uuid
             path = lv.lv_path
@@ -245,7 +235,15 @@ class Prepare(object):
         """
         if args is not None:
             self.args = args
-        if api.is_ceph_device(self.get_lv(self.args.data)):
+
+        try:
+            vgname, lvname = self.args.data.split('/')
+            lv = api.get_first_lv(filters={'lv_name': lvname,
+                                           'vg_name': vgname})
+        except ValueError:
+            lv = None
+
+        if api.is_ceph_device(lv):
             logger.info("device {} is already used".format(self.args.data))
             raise RuntimeError("skipping {}, it is already prepared".format(self.args.data))
         try:
@@ -304,7 +302,13 @@ class Prepare(object):
             if not self.args.journal:
                 raise RuntimeError('--journal is required when using --filestore')
 
-            data_lv = self.get_lv(self.args.data)
+            try:
+                vg_name, lv_name = self.args.data.split('/')
+                data_lv = api.get_first_lv(filters={'lv_name': lv_name,
+                                                    'vg_name': vg_name})
+            except ValueError:
+                data_lv = None
+
             if not data_lv:
                 data_lv = self.prepare_data_device('data', osd_fsid)
 
@@ -329,7 +333,13 @@ class Prepare(object):
                 osd_fsid,
             )
         elif self.args.bluestore:
-            block_lv = self.get_lv(self.args.data)
+            try:
+                vg_name, lv_name = self.args.data.split('/')
+                block_lv = api.get_first_lv(filters={'lv_name': lv_name,
+                                                 'vg_name': vg_name})
+            except ValueError:
+                block_lv = None
+
             if not block_lv:
                 block_lv = self.prepare_data_device('block', osd_fsid)
 
diff --git a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_prepare.py b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_prepare.py
index 6a2c3fc72f8b4..dd902866884ba 100644
--- a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_prepare.py
+++ b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_prepare.py
@@ -1,5 +1,4 @@
 import pytest
-from ceph_volume.api import lvm as api
 from ceph_volume.devices import lvm
 from mock.mock import patch, Mock
 
@@ -116,21 +115,6 @@ class TestPrepare(object):
             expected = 'skipping {}, it is already prepared'.format('/dev/sdfoo')
             assert expected in str(error.value)
 
-class TestGetJournalLV(object):
-
-    @pytest.mark.parametrize('arg', ['', '///', None, '/dev/sda1'])
-    def test_no_journal_on_invalid_path(self, monkeypatch, arg):
-        monkeypatch.setattr(api, '_get_lv', lambda **kw: False)
-        prepare = lvm.prepare.Prepare([])
-        assert prepare.get_lv(arg) is None
-
-    def test_no_journal_lv_found(self, monkeypatch):
-        # patch it with 0 so we know we are getting to get_lv
-        monkeypatch.setattr(api, '_get_lv', lambda **kw: 0)
-        prepare = lvm.prepare.Prepare([])
-        assert prepare.get_lv('vg/lv') == 0
-
-
 class TestActivate(object):
 
     def test_main_spits_help_with_no_arguments(self, capsys):
-- 
2.39.5