ceph-volume: fix lvm migrate without args

When running the `lvm migrate` subcommand without any args then the
ceph-volume command fails with a stack trace.

Traceback (most recent call last):
  File "/usr/lib/python3.6/site-packages/ceph_volume/decorators.py", line 59, in newfunc
    return f(*a, **kw)
  File "/usr/lib/python3.6/site-packages/ceph_volume/main.py", line 151, in main
    terminal.dispatch(self.mapper, subcommand_args)
  File "/usr/lib/python3.6/site-packages/ceph_volume/terminal.py", line 194, in dispatch
    instance.main()
  File "/usr/lib/python3.6/site-packages/ceph_volume/devices/lvm/main.py", line 46, in main
    terminal.dispatch(self.mapper, self.argv)
  File "/usr/lib/python3.6/site-packages/ceph_volume/terminal.py", line 194, in dispatch
    instance.main()
  File "/usr/lib/python3.6/site-packages/ceph_volume/devices/lvm/migrate.py", line 520, in main
    self.migrate_osd()
  File "/usr/lib/python3.6/site-packages/ceph_volume/decorators.py", line 16, in is_root
    return func(*a, **kw)
  File "/usr/lib/python3.6/site-packages/ceph_volume/devices/lvm/migrate.py", line 403, in migrate_osd
    if self.args.osd_id:
AttributeError: 'Migrate' object has no attribute 'args'

That's because we're exiting the parse_argv function but we continue to
execute the migrate_osd function. We should instead exit from the main function.

This update the parsing argument to have the same code than new-db and
new-wal classes.
Now the parsing is done in the make_parser function but the argv testing is
done in the main function allowing to exit the program and displaying the
help message when no arguments are provided.

Fixes: https://tracker.ceph.com/issues/51811
Signed-off-by: Dimitri Savineau <dsavinea@redhat.com>
(cherry picked from commit 98de3306127cb758e5517a322f6da9e636f91036)

diff --git a/src/ceph-volume/ceph_volume/devices/lvm/migrate.py b/src/ceph-volume/ceph_volume/devices/lvm/migrate.py
index 872fe3dd31b37cfd79c9570efd5a4888d2894c52..886b9f7b4fd6b1a6e54a6b71be50358d33482f15 100644 (file)
--- a/src/ceph-volume/ceph_volume/devices/lvm/migrate.py
+++ b/src/ceph-volume/ceph_volume/devices/lvm/migrate.py
@@ -436,7 +436,46 @@ class Migrate(object):
                 devices,
                 target_lv)
 
-    def parse_argv(self):
+    def make_parser(self, prog, sub_command_help):
+        parser = argparse.ArgumentParser(
+            prog=prog,
+            formatter_class=argparse.RawDescriptionHelpFormatter,
+            description=sub_command_help,
+        )
+
+        parser.add_argument(
+            '--osd-id',
+            required=True,
+            help='Specify an OSD ID to detect associated devices for zapping',
+        )
+
+        parser.add_argument(
+            '--osd-fsid',
+            required=True,
+            help='Specify an OSD FSID to detect associated devices for zapping',
+        )
+        parser.add_argument(
+            '--target',
+            required=True,
+            help='Specify target Logical Volume (LV) to migrate data to',
+        )
+        parser.add_argument(
+            '--from',
+            nargs='*',
+            dest='from_',
+            required=True,
+            choices=['data', 'db', 'wal'],
+            help='Copy BlueFS data from DB device',
+        )
+        parser.add_argument(
+            '--no-systemd',
+            dest='no_systemd',
+            action='store_true',
+            help='Skip checking OSD systemd unit',
+        )
+        return parser
+
+    def main(self):
         sub_command_help = dedent("""
         Moves BlueFS data from source volume(s) to the target one, source
         volumes (except the main (i.e. data or block) one) are removed on
@@ -479,50 +518,15 @@ class Migrate(object):
             ceph-volume lvm migrate --osd-id 1 --osd-fsid <uuid> --from db wal --target vgname/data
 
         """)
-        parser = argparse.ArgumentParser(
-            prog='ceph-volume lvm migrate',
-            formatter_class=argparse.RawDescriptionHelpFormatter,
-            description=sub_command_help,
-        )
 
-        parser.add_argument(
-            '--osd-id',
-            required=True,
-            help='Specify an OSD ID to detect associated devices for zapping',
-        )
-
-        parser.add_argument(
-            '--osd-fsid',
-            required=True,
-            help='Specify an OSD FSID to detect associated devices for zapping',
-        )
-        parser.add_argument(
-            '--target',
-            required=True,
-            help='Specify target Logical Volume (LV) to migrate data to',
-        )
-        parser.add_argument(
-            '--from',
-            nargs='*',
-            dest='from_',
-            required=True,
-            choices=['data', 'db', 'wal'],
-            help='Copy BlueFS data from DB device',
-        )
-        parser.add_argument(
-            '--no-systemd',
-            dest='no_systemd',
-            action='store_true',
-            help='Skip checking OSD systemd unit',
-        )
+        parser = self.make_parser('ceph-volume lvm migrate', sub_command_help)
 
         if len(self.argv) == 0:
             print(sub_command_help)
             return
+
         self.args = parser.parse_args(self.argv)
 
-    def main(self):
-        self.parse_argv()
         self.migrate_osd()
 
 class NewVolume(object):

diff --git a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_migrate.py b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_migrate.py
index 8b28e6054c043c17142b6adb4a961758d799de6b..6c20273507a4a782d296183abeda60fe97faddf3 100644 (file)
--- a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_migrate.py
+++ b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_migrate.py
@@ -1060,24 +1060,28 @@ class TestMigrate(object):
             lambda osd_id, osd_fsid: devices)
 
 
-        m = migrate.Migrate(argv=[
+        argv = [
             '--osd-id', '2',
             '--osd-fsid', '55BD4219-16A7-4037-BC20-0F158EFCC83D',
             '--from', 'data', 'wal',
-            '--target', 'vgname/new_wal'])
-        m.parse_argv()
+            '--target', 'vgname/new_wal'
+        ]
+        m = migrate.Migrate(argv=argv)
+        m.args = m.make_parser('ceph-volume lvm migation', 'help').parse_args(argv)
         res_devices = m.get_source_devices(devices)
 
         assert 2 == len(res_devices)
         assert devices[0] == res_devices[0]
         assert devices[2] == res_devices[1]
 
-        m = migrate.Migrate(argv=[
+        argv = [
             '--osd-id', '2',
             '--osd-fsid', '55BD4219-16A7-4037-BC20-0F158EFCC83D',
             '--from', 'db', 'wal', 'data',
-            '--target', 'vgname/new_wal'])
-        m.parse_argv()
+            '--target', 'vgname/new_wal'
+        ]
+        m = migrate.Migrate(argv=argv)
+        m.args = m.make_parser('ceph-volume lvm migation', 'help').parse_args(argv)
         res_devices = m.get_source_devices(devices)
 
         assert 3 == len(res_devices)
@@ -1086,6 +1090,56 @@ class TestMigrate(object):
         assert devices[2] == res_devices[2]
 
 
+    def test_migrate_without_args(self, capsys):
+        help_msg = """
+Moves BlueFS data from source volume(s) to the target one, source
+volumes (except the main (i.e. data or block) one) are removed on
+success. LVM volumes are permitted for Target only, both already
+attached or new logical one. In the latter case it is attached to OSD
+replacing one of the source devices. Following replacement rules apply
+(in the order of precedence, stop on the first match):
+* if source list has DB volume - target device replaces it.
+* if source list has WAL volume - target device replace it.
+* if source list has slow volume only - operation is not permitted,
+  requires explicit allocation via new-db/new-wal command.
+
+Example calls for supported scenarios:
+
+  Moves BlueFS data from main device to LV already attached as DB:
+
+    ceph-volume lvm migrate --osd-id 1 --osd-fsid <uuid> --from data --target vgname/db
+
+  Moves BlueFS data from shared main device to LV which will be attached
+   as a new DB:
+
+    ceph-volume lvm migrate --osd-id 1 --osd-fsid <uuid> --from data --target vgname/new_db
+
+  Moves BlueFS data from DB device to new LV, DB is replaced:
+
+    ceph-volume lvm migrate --osd-id 1 --osd-fsid <uuid> --from db --target vgname/new_db
+
+  Moves BlueFS data from main and DB devices to new LV, DB is replaced:
+
+    ceph-volume lvm migrate --osd-id 1 --osd-fsid <uuid> --from data db --target vgname/new_db
+
+  Moves BlueFS data from main, DB and WAL devices to new LV, WAL is
+   removed and DB is replaced:
+
+    ceph-volume lvm migrate --osd-id 1 --osd-fsid <uuid> --from data db wal --target vgname/new_db
+
+  Moves BlueFS data from main, DB and WAL devices to main device, WAL
+   and DB are removed:
+
+    ceph-volume lvm migrate --osd-id 1 --osd-fsid <uuid> --from db wal --target vgname/data
+
+"""
+        m = migrate.Migrate(argv=[])
+        m.main()
+        stdout, stderr = capsys.readouterr()
+        assert help_msg in stdout
+        assert not stderr
+
+
     @patch('os.getuid')
     def test_migrate_data_db_to_new_db(self, m_getuid, monkeypatch):
         m_getuid.return_value = 0