From: Dimitri Savineau Date: Fri, 3 Sep 2021 14:45:18 +0000 (-0400) Subject: ceph-volume: fix lvm migrate without args X-Git-Tag: v15.2.15~38^2 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=d15401c581a4c943d9119ccc1d9977a670af0b3b;p=ceph.git 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 (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 872fe3dd31b37..886b9f7b4fd6b 100644 --- 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 --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 8b28e6054c043..6c20273507a4a 100644 --- 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 --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 --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 --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 --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 --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 --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