From d401ef21ef8bfd6bb3e9df0c079132b85331a89f Mon Sep 17 00:00:00 2001 From: Guillaume Abrioux Date: Wed, 26 Jul 2023 13:55:52 +0000 Subject: [PATCH] ceph-volume: fix and clean up unit tests - update unit test devices/raw/test_list.py: e5e4296 missed the corresponding unit test update. This commit updates it. - fix unit tests when run as non-root user: When unit tests are run as non-root user, it fails for many reasons such as 'permission denied' etc... This commit addresses that by adding missing fixture `is_root` and using pyfakefs when it's needed. - drop test_writer_uses_log_on_unicodeerror test: This test is broken for a while. Since we don't make test against py2, let's drop it. - test_arg_validators cleanup: This makes use of `self.validator` defined in `setup_method()` rather than redefining a new object in each tests. Also, this migrates test_files_are_missing to pyfakefs - test_migrate cleanup: This makes use of `is_root` fixture insead of using mock.patch on `os.getuid` - test_activate cleanup: - removes the fixture monkeypatch from test_no_data_uuid() given that it's not used in this test. - remove the fixture is_root from test_activate_all() - simple.TestActivate.test_no_data_uuid fix: This fixes the following issue: ``` ________________________ TestActivate.test_no_data_uuid ________________________ self = factory = , is_root = None capture = fake_filesystem = def test_no_data_uuid(self, factory, is_root, capture, fake_filesystem): fake_filesystem.create_file('/tmp/json-config', contents='{}') args = factory(osd_id='0', osd_fsid='1234', json_config='/tmp/json-config') with pytest.raises(RuntimeError): > activate.Activate([]).activate(args) /home/jenkins-build/build/workspace/ceph-volume-pr/src/ceph-volume/ceph_volume/tests/devices/simple/test_activate.py:12: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ a = (, ) kw = {} @wraps(func) def is_root(*a, **kw): if not os.getuid() == 0 and not os.environ.get('CEPH_VOLUME_SKIP_NEEDS_ROOT', False): > raise exceptions.SuperUserError() E ceph_volume.exceptions.SuperUserError: This command needs to be executed with sudo or as root /home/jenkins-build/build/workspace/ceph-volume-pr/src/ceph-volume/ceph_volume/decorators.py:15: SuperUserError ``` Even though we use the fixture `is_root`, it doesn't seem to work. Using @patch() instead fixes this issue. - address 'PytestRemovedIn8Warning' messages: pretty self-explanatory: ``` PytestRemovedIn8Warning: Support for nose tests is deprecated and will be removed in a future release. ``` ``` To remove this warning, rename it to `setup_method(self)` ``` so this commit renames some `def setup(self)` to `def setup_method(self)` as suggested. - add missing unit tests: This adds some unit test updates missed by commits 0985e201342 and bd5e1a83495 Signed-off-by: Guillaume Abrioux --- src/ceph-volume/ceph_volume/tests/conftest.py | 9 +++ .../tests/devices/lvm/test_activate.py | 10 +-- .../tests/devices/lvm/test_migrate.py | 52 ++++----------- .../tests/devices/lvm/test_prepare.py | 4 +- .../tests/devices/raw/test_list.py | 66 +++++++++++++++---- .../tests/devices/raw/test_prepare.py | 9 ++- .../tests/devices/simple/test_activate.py | 6 +- .../tests/objectstore/test_lvmbluestore.py | 12 +++- .../tests/objectstore/test_rawbluestore.py | 5 +- .../ceph_volume/tests/systemd/test_main.py | 6 +- .../ceph_volume/tests/test_main.py | 6 +- .../ceph_volume/tests/test_terminal.py | 10 --- .../tests/util/test_arg_validators.py | 18 +++-- .../ceph_volume/tests/util/test_disk.py | 42 +++++++----- .../ceph_volume/tests/util/test_encryption.py | 6 +- .../ceph_volume/tests/util/test_prepare.py | 2 +- 16 files changed, 150 insertions(+), 113 deletions(-) diff --git a/src/ceph-volume/ceph_volume/tests/conftest.py b/src/ceph-volume/ceph_volume/tests/conftest.py index 4a6820852ef..fb43da7ab22 100644 --- a/src/ceph-volume/ceph_volume/tests/conftest.py +++ b/src/ceph-volume/ceph_volume/tests/conftest.py @@ -256,6 +256,13 @@ def is_root(monkeypatch): """ monkeypatch.setattr('os.getuid', lambda: 0) +@pytest.fixture +def is_non_root(monkeypatch): + """ + Patch ``os.getuid()`` so that ceph-volume's decorators that ensure a user + is not root. + """ + monkeypatch.setattr('os.getuid', lambda: 100) @pytest.fixture def tmpfile(tmpdir): @@ -380,6 +387,8 @@ def fake_filesystem(fs): fs.create_dir('/sys/block/sda/slaves') fs.create_dir('/sys/block/sda/queue') fs.create_dir('/sys/block/rbd0') + fs.create_dir('/var/log/ceph') + fs.create_dir('/tmp/osdpath') yield fs @pytest.fixture diff --git a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_activate.py b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_activate.py index 6f38097a1ab..b44071026ad 100644 --- a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_activate.py +++ b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_activate.py @@ -63,7 +63,7 @@ class TestActivate(object): a.objectstore.activate() assert result.value.args[0] == 'Please provide both osd_id and osd_fsid' - def test_bluestore_no_systemd(self, is_root, monkeypatch, capture): + def test_bluestore_no_systemd(self, m_create_key, is_root, monkeypatch, capture): monkeypatch.setattr('ceph_volume.configuration.load', lambda: None) fake_enable = Capture() fake_start_osd = Capture() @@ -88,7 +88,7 @@ class TestActivate(object): assert fake_enable.calls == [] assert fake_start_osd.calls == [] - def test_bluestore_systemd(self, is_root, monkeypatch, capture): + def test_bluestore_systemd(self, m_create_key, is_root, monkeypatch, capture): monkeypatch.setattr('ceph_volume.configuration.load', lambda: None) fake_enable = Capture() fake_start_osd = Capture() @@ -114,7 +114,7 @@ class TestActivate(object): assert fake_enable.calls != [] assert fake_start_osd.calls != [] - def test_bluestore_no_systemd_autodetect(self, is_root, monkeypatch, capture): + def test_bluestore_no_systemd_autodetect(self, m_create_key, is_root, monkeypatch, capture): monkeypatch.setattr('ceph_volume.configuration.load', lambda: None) fake_enable = Capture() fake_start_osd = Capture() @@ -140,7 +140,7 @@ class TestActivate(object): assert fake_enable.calls == [] assert fake_start_osd.calls == [] - def test_bluestore_systemd_autodetect(self, is_root, monkeypatch, capture): + def test_bluestore_systemd_autodetect(self, m_create_key, is_root, monkeypatch, capture): monkeypatch.setattr('ceph_volume.configuration.load', lambda: None) fake_enable = Capture() fake_start_osd = Capture() @@ -242,7 +242,7 @@ class TestActivateAll(object): m_activate.assert_has_calls(calls) @patch('ceph_volume.objectstore.lvmbluestore.LvmBlueStore.activate') - def test_detects_osds_to_activate_no_systemd(self, m_activate, is_root, monkeypatch): + def test_detects_osds_to_activate_no_systemd(self, m_activate, m_create_key, is_root, monkeypatch): monkeypatch.setattr('ceph_volume.objectstore.lvmbluestore.direct_report', lambda: direct_report) args = ['--all', '--no-systemd', '--bluestore'] a = activate.Activate(args) 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 13f4acb075c..072d4f1ef35 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 @@ -524,8 +524,7 @@ class TestNew(object): def mock_prepare_dmcrypt(self, *args, **kwargs): return '/dev/mapper/' + kwargs['mapping'] - @patch('os.getuid', return_value=1) - def test_newdb_non_root(self, m_getuid): + def test_newdb_non_root(self, is_non_root): with pytest.raises(Exception) as error: migrate.NewDB(argv=[ '--osd-id', '1', @@ -534,9 +533,7 @@ class TestNew(object): expected = 'This command needs to be executed with sudo or as root' assert expected in str(error.value) - @patch('os.getuid') - def test_newdb_not_target_lvm(self, m_getuid, capsys): - m_getuid.return_value = 0 + def test_newdb_not_target_lvm(self, is_root, capsys): with pytest.raises(SystemExit) as error: migrate.NewDB(argv=[ '--osd-id', '1', @@ -549,10 +546,7 @@ class TestNew(object): assert expected in stderr - @patch('os.getuid') - def test_newdb_already_in_use(self, m_getuid, monkeypatch, capsys): - m_getuid.return_value = 0 - + def test_newdb_already_in_use(self, is_root, monkeypatch, capsys): self.mock_volume = api.Volume(lv_name='volume1', lv_uuid='y', vg_name='vg', @@ -571,10 +565,7 @@ class TestNew(object): expected = 'Target Logical Volume is already used by ceph: vgname/new_db' assert expected in stderr - @patch('os.getuid') - def test_newdb(self, m_getuid, monkeypatch, capsys): - m_getuid.return_value = 0 - + def test_newdb(self, is_root, monkeypatch, capsys): source_tags = \ 'ceph.osd_id=0,ceph.type=data,ceph.osd_fsid=1234,'\ 'ceph.wal_uuid=wal_uuid,ceph.db_device=/dbdevice' @@ -819,10 +810,7 @@ class TestNew(object): '--dev-target', '/dev/VolGroup/target_volume', '--command', 'bluefs-bdev-new-db'] - @patch('os.getuid') - def test_newwal(self, m_getuid, monkeypatch, capsys): - m_getuid.return_value = 0 - + def test_newwal(self, is_root, monkeypatch, capsys): source_tags = \ 'ceph.osd_id=0,ceph.type=data,ceph.osd_fsid=1234' @@ -1226,9 +1214,7 @@ Example calls for supported scenarios: assert not stderr - @patch('os.getuid') - def test_migrate_data_db_to_new_db(self, m_getuid, monkeypatch): - m_getuid.return_value = 0 + def test_migrate_data_db_to_new_db(self, is_root, monkeypatch): source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \ 'ceph.cluster_name=ceph,ceph.db_uuid=dbuuid,ceph.db_device=db_dev' @@ -1600,10 +1586,7 @@ Example calls for supported scenarios: '--devs-source', '/var/lib/ceph/osd/ceph-2/block', '--devs-source', '/var/lib/ceph/osd/ceph-2/block.db'] - @patch('os.getuid') - def test_migrate_data_db_to_new_db_skip_wal(self, m_getuid, monkeypatch): - m_getuid.return_value = 0 - + def test_migrate_data_db_to_new_db_skip_wal(self, is_root, monkeypatch): source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \ 'ceph.cluster_name=ceph,ceph.db_uuid=dbuuid,ceph.db_device=db_dev' source_db_tags = 'ceph.osd_id=2,ceph.type=db,ceph.osd_fsid=1234,' \ @@ -1722,10 +1705,7 @@ Example calls for supported scenarios: '--devs-source', '/var/lib/ceph/osd/ceph-2/block', '--devs-source', '/var/lib/ceph/osd/ceph-2/block.db'] - @patch('os.getuid') - def test_migrate_data_db_wal_to_new_db(self, m_getuid, monkeypatch): - m_getuid.return_value = 0 - + def test_migrate_data_db_wal_to_new_db(self, is_root, monkeypatch): source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \ 'ceph.cluster_name=ceph,ceph.db_uuid=dbuuid,ceph.db_device=db_dev,' \ 'ceph.wal_uuid=waluuid,ceph.wal_device=wal_dev' @@ -1996,7 +1976,6 @@ Example calls for supported scenarios: monkeypatch, capsys): m_getuid.return_value = 0 - source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \ 'ceph.cluster_name=ceph,ceph.db_uuid=dbuuid,ceph.db_device=db_dev' source_db_tags = 'ceph.osd_id=2,ceph.type=db,ceph.osd_fsid=1234,' \ @@ -2058,13 +2037,10 @@ Example calls for supported scenarios: ' please use new-db or new-wal command before.' assert expected in stderr - @patch('os.getuid') def test_dont_migrate_db_to_wal(self, - m_getuid, + is_root, monkeypatch, capsys): - m_getuid.return_value = 0 - source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \ 'ceph.cluster_name=ceph,ceph.db_uuid=dbuuid,ceph.db_device=db_dev,' \ 'ceph.wal_uuid=waluuid,ceph.wal_device=wal_dev' @@ -2134,13 +2110,10 @@ Example calls for supported scenarios: expected = 'Migrate to WAL is not supported' assert expected in stderr - @patch('os.getuid') def test_migrate_data_db_to_db(self, - m_getuid, + is_root, monkeypatch, capsys): - m_getuid.return_value = 0 - source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \ 'ceph.cluster_name=ceph,ceph.db_uuid=dbuuid,ceph.db_device=db_dev,' \ 'ceph.wal_uuid=waluuid,ceph.wal_device=wal_dev' @@ -2361,13 +2334,10 @@ Example calls for supported scenarios: '--command', 'bluefs-bdev-migrate', '--devs-source', '/var/lib/ceph/osd/ceph-2/block'] - @patch('os.getuid') def test_migrate_data_wal_to_db(self, - m_getuid, + is_root, monkeypatch, capsys): - m_getuid.return_value = 0 - source_tags = 'ceph.osd_id=2,ceph.type=data,ceph.osd_fsid=1234,' \ 'ceph.cluster_name=ceph,ceph.db_uuid=dbuuid,ceph.db_device=db_dev,' \ 'ceph.wal_uuid=waluuid,ceph.wal_device=wal_dev' 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 7f51896ea8f..ec301d6eb93 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 @@ -39,7 +39,7 @@ class TestPrepareDevice(object): @patch('ceph_volume.util.prepare.create_key', return_value='fake-secret') class TestGetClusterFsid(object): - def setup(self): + def setup_method(self): self.p = lvm.prepare.Prepare([]) def test_fsid_is_passed_in(self, m_create_key, factory): @@ -57,7 +57,7 @@ class TestGetClusterFsid(object): @patch('ceph_volume.util.prepare.create_key', return_value='fake-secret') class TestPrepare(object): - def setup(self): + def setup_method(self): self.p = lvm.prepare.Prepare([]) def test_main_spits_help_with_no_arguments(self, m_create_key, capsys): diff --git a/src/ceph-volume/ceph_volume/tests/devices/raw/test_list.py b/src/ceph-volume/ceph_volume/tests/devices/raw/test_list.py index 5ad501bab94..604fb4faa3e 100644 --- a/src/ceph-volume/ceph_volume/tests/devices/raw/test_list.py +++ b/src/ceph-volume/ceph_volume/tests/devices/raw/test_list.py @@ -44,23 +44,27 @@ def _devices_side_effect(): "/dev/sdb3": {}, "/dev/sdc": {}, "/dev/sdd": {}, + "/dev/sde": {}, + "/dev/sde1": {}, "/dev/mapper/ceph--osd--block--1": {}, "/dev/mapper/ceph--osd--block--2": {}, } def _lsblk_all_devices(abspath=True): return [ - {"NAME": "/dev/sda", "KNAME": "/dev/sda", "PKNAME": ""}, - {"NAME": "/dev/sda1", "KNAME": "/dev/sda1", "PKNAME": "/dev/sda"}, - {"NAME": "/dev/sda2", "KNAME": "/dev/sda2", "PKNAME": "/dev/sda"}, - {"NAME": "/dev/sda3", "KNAME": "/dev/sda3", "PKNAME": "/dev/sda"}, - {"NAME": "/dev/sdb", "KNAME": "/dev/sdb", "PKNAME": ""}, - {"NAME": "/dev/sdb2", "KNAME": "/dev/sdb2", "PKNAME": "/dev/sdb"}, - {"NAME": "/dev/sdb3", "KNAME": "/dev/sdb3", "PKNAME": "/dev/sdb"}, - {"NAME": "/dev/sdc", "KNAME": "/dev/sdc", "PKNAME": ""}, - {"NAME": "/dev/sdd", "KNAME": "/dev/sdd", "PKNAME": ""}, - {"NAME": "/dev/mapper/ceph--osd--block--1", "KNAME": "/dev/mapper/ceph--osd--block--1", "PKNAME": "/dev/sdd"}, - {"NAME": "/dev/mapper/ceph--osd--block--2", "KNAME": "/dev/mapper/ceph--osd--block--2", "PKNAME": "/dev/sdd"}, + {"NAME": "/dev/sda", "KNAME": "/dev/sda", "PKNAME": "", "TYPE": "disk"}, + {"NAME": "/dev/sda1", "KNAME": "/dev/sda1", "PKNAME": "/dev/sda", "TYPE": "part"}, + {"NAME": "/dev/sda2", "KNAME": "/dev/sda2", "PKNAME": "/dev/sda", "TYPE": "part"}, + {"NAME": "/dev/sda3", "KNAME": "/dev/sda3", "PKNAME": "/dev/sda", "TYPE": "part"}, + {"NAME": "/dev/sdb", "KNAME": "/dev/sdb", "PKNAME": "", "TYPE": "disk"}, + {"NAME": "/dev/sdb2", "KNAME": "/dev/sdb2", "PKNAME": "/dev/sdb", "TYPE": "part"}, + {"NAME": "/dev/sdb3", "KNAME": "/dev/sdb3", "PKNAME": "/dev/sdb", "TYPE": "part"}, + {"NAME": "/dev/sdc", "KNAME": "/dev/sdc", "PKNAME": "", "TYPE": "disk"}, + {"NAME": "/dev/sdd", "KNAME": "/dev/sdd", "PKNAME": "", "TYPE": "disk"}, + {"NAME": "/dev/sde", "KNAME": "/dev/sde", "PKNAME": "", "TYPE": "disk"}, + {"NAME": "/dev/sde1", "KNAME": "/dev/sde1", "PKNAME": "/dev/sde", "TYPE": "part"}, + {"NAME": "/dev/mapper/ceph--osd--block--1", "KNAME": "/dev/mapper/ceph--osd--block--1", "PKNAME": "/dev/sdd", "TYPE": "lvm"}, + {"NAME": "/dev/mapper/ceph--osd--block--2", "KNAME": "/dev/mapper/ceph--osd--block--2", "PKNAME": "/dev/sdd", "TYPE": "lvm"}, ] # dummy lsblk output for device with optional parent output @@ -116,6 +120,29 @@ def _bluestore_tool_label_output_sdb2(): } }''' +def _bluestore_tool_label_output_sde1(): + return '''{ + "/dev/sde1": { + "osd_uuid": "sde1-uuid", + "size": 214747316224, + "btime": "2023-07-26T13:20:19.509457+0000", + "description": "main", + "bfm_blocks": "268435456", + "bfm_blocks_per_key": "128", + "bfm_bytes_per_block": "4096", + "bfm_size": "214747316224", + "bluefs": "1", + "ceph_fsid": "sde1-fsid", + "kv_backend": "rocksdb", + "magic": "ceph osd volume v026", + "mkfs_done": "yes", + "osd_key": "AQCSHcFkUeLIMBAAjKqANkXafjvVISkXt6FGCA==", + "ready": "ready", + "require_osd_release": "16", + "whoami": "1" + } +}''' + def _bluestore_tool_label_output_dm_okay(): return '''{ "/dev/mapper/ceph--osd--block--1": { @@ -149,6 +176,8 @@ def _process_call_side_effect(command, **kw): return _lsblk_output(dev, parent="/dev/sdb"), '', 0 if dev == "/dev/sda" or dev == "/dev/sdb" or dev == "/dev/sdc" or dev == "/dev/sdd": return _lsblk_output(dev), '', 0 + if dev == "/dev/sde1": + return _lsblk_output(dev, parent="/dev/sde"), '', 0 if "mapper" in dev: return _lsblk_output(dev, parent="/dev/sdd"), '', 0 pytest.fail('dev {} needs behavior specified for it'.format(dev)) @@ -163,6 +192,8 @@ def _process_call_side_effect(command, **kw): if "/dev/sdb2" in command: # sdb2 is a phantom atari partition that appears to have some valid bluestore info return _bluestore_tool_label_output_sdb2(), '', 0 + if "/dev/sde1" in command: + return _bluestore_tool_label_output_sde1(), '', 0 if "/dev/mapper/ceph--osd--block--1" in command: # dm device 1 is a valid bluestore OSD (the other is corrupted/invalid) return _bluestore_tool_label_output_dm_okay(), '', 0 @@ -181,6 +212,10 @@ def _has_bluestore_label_side_effect(disk_path): return False # empty disk if disk_path == "/dev/sdd": return False # has LVM subdevices + if disk_path == "/dev/sde": + return False # has partitions, it means it shouldn't be an OSD + if disk_path == "/dev/sde1": + return True # is a valid OSD if disk_path == "/dev/mapper/ceph--osd--block--1": return True # good OSD if disk_path == "/dev/mapper/ceph--osd--block--2": @@ -209,13 +244,18 @@ class TestList(object): assert sdb['device'] == '/dev/sdb' assert sdb['ceph_fsid'] == 'sdb-fsid' assert sdb['type'] == 'bluestore' - lvm1 = result['lvm-1-uuid'] assert lvm1['osd_uuid'] == 'lvm-1-uuid' assert lvm1['osd_id'] == 2 assert lvm1['device'] == '/dev/mapper/ceph--osd--block--1' assert lvm1['ceph_fsid'] == 'lvm-1-fsid' assert lvm1['type'] == 'bluestore' + sde1 = result['sde1-uuid'] + assert sde1['osd_uuid'] == 'sde1-uuid' + assert sde1['osd_id'] == 1 + assert sde1['device'] == '/dev/sde1' + assert sde1['ceph_fsid'] == 'sde1-fsid' + assert sde1['type'] == 'bluestore' @patch('ceph_volume.util.device.disk.get_devices') @patch('ceph_volume.util.disk.has_bluestore_label') @@ -234,5 +274,5 @@ class TestList(object): patched_get_devices.side_effect = _devices_side_effect result = raw.list.List([]).generate() - assert len(result) == 3 + assert len(result) == 2 assert 'sdb-uuid' in result diff --git a/src/ceph-volume/ceph_volume/tests/devices/raw/test_prepare.py b/src/ceph-volume/ceph_volume/tests/devices/raw/test_prepare.py index 0802f923249..285bc8b5cdf 100644 --- a/src/ceph-volume/ceph_volume/tests/devices/raw/test_prepare.py +++ b/src/ceph-volume/ceph_volume/tests/devices/raw/test_prepare.py @@ -1,6 +1,6 @@ import pytest from ceph_volume.devices import raw -from mock.mock import patch +from mock.mock import patch, MagicMock from ceph_volume import objectstore class TestRaw(object): @@ -47,8 +47,13 @@ class TestPrepare(object): assert 'Path to bluestore block.wal block device' in stdout assert 'Enable device encryption via dm-crypt' in stdout + @patch('ceph_volume.util.arg_validators.set_dmcrypt_no_workqueue', return_value=MagicMock()) @patch('ceph_volume.util.arg_validators.ValidRawDevice.__call__') - def test_prepare_dmcrypt_no_secret_passed(self, m_valid_device, m_create_key, capsys): + def test_prepare_dmcrypt_no_secret_passed(self, + m_valid_device, + m_set_dmcrypt_no_workqueue, + m_create_key, + capsys): m_valid_device.return_value = '/dev/foo' with pytest.raises(SystemExit): raw.prepare.Prepare(argv=['--bluestore', '--data', '/dev/foo', '--dmcrypt']).main() diff --git a/src/ceph-volume/ceph_volume/tests/devices/simple/test_activate.py b/src/ceph-volume/ceph_volume/tests/devices/simple/test_activate.py index 152ac9b09e2..ae7e52e518b 100644 --- a/src/ceph-volume/ceph_volume/tests/devices/simple/test_activate.py +++ b/src/ceph-volume/ceph_volume/tests/devices/simple/test_activate.py @@ -1,11 +1,13 @@ import os import pytest from ceph_volume.devices.simple import activate +from mock.mock import patch class TestActivate(object): - def test_no_data_uuid(self, factory, is_root, monkeypatch, capture, fake_filesystem): + @patch('ceph_volume.decorators.os.getuid', return_value=0) + def test_no_data_uuid(self, m_getuid, factory, capture, fake_filesystem): fake_filesystem.create_file('/tmp/json-config', contents='{}') args = factory(osd_id='0', osd_fsid='1234', json_config='/tmp/json-config') with pytest.raises(RuntimeError): @@ -22,7 +24,7 @@ class TestActivate(object): stdout, stderr = capsys.readouterr() assert 'Activate OSDs by mounting devices previously configured' in stdout - def test_activate_all(self, is_root, monkeypatch): + def test_activate_all(self, monkeypatch): ''' make sure Activate calls activate for each file returned by glob ''' diff --git a/src/ceph-volume/ceph_volume/tests/objectstore/test_lvmbluestore.py b/src/ceph-volume/ceph_volume/tests/objectstore/test_lvmbluestore.py index 49872bb6a80..45fbd3005b6 100644 --- a/src/ceph-volume/ceph_volume/tests/objectstore/test_lvmbluestore.py +++ b/src/ceph-volume/ceph_volume/tests/objectstore/test_lvmbluestore.py @@ -152,7 +152,7 @@ class TestLvmBlueStore: @patch('ceph_volume.objectstore.baseobjectstore.BaseObjectStore.get_ptuuid', Mock(return_value='c6798f59-01')) @patch('ceph_volume.api.lvm.Volume.set_tags', MagicMock()) @patch('ceph_volume.api.lvm.get_single_lv') - def test_prepare(self, m_get_single_lv, factory): + def test_prepare(self, m_get_single_lv, is_root, factory): m_get_single_lv.return_value = Volume(lv_name='lv_foo', lv_path='/fake-path', vg_name='vg_foo', @@ -445,7 +445,9 @@ class TestLvmBlueStore: @patch('ceph_volume.systemd.systemctl.osd_is_active', return_value=False) def test_activate_all(self, + m_create_key, mock_lvm_direct_report, + is_root, factory, fake_run): args = factory(no_systemd=True) @@ -461,6 +463,8 @@ class TestLvmBlueStore: @patch('ceph_volume.systemd.systemctl.osd_is_active', return_value=False) def test_activate_all_no_osd_found(self, + m_create_key, + is_root, factory, fake_run, monkeypatch, @@ -476,6 +480,7 @@ class TestLvmBlueStore: @patch('ceph_volume.systemd.systemctl.osd_is_active', return_value=True) def test_activate_all_osd_is_active(self, mock_lvm_direct_report, + is_root, factory, fake_run): args = factory(no_systemd=False) @@ -487,6 +492,7 @@ class TestLvmBlueStore: @patch('ceph_volume.api.lvm.get_lvs') def test_activate_osd_id_and_fsid(self, m_get_lvs, + is_root, factory): args = factory(osd_id='1', osd_fsid='824f7edf', @@ -507,6 +513,7 @@ class TestLvmBlueStore: @patch('ceph_volume.api.lvm.get_lvs') def test_activate_not_osd_id_and_fsid(self, m_get_lvs, + is_root, factory): args = factory(no_systemd=True, osd_id=None, @@ -524,6 +531,7 @@ class TestLvmBlueStore: assert m_get_lvs.mock_calls == [call(tags={'ceph.osd_fsid': '824f7edf'})] def test_activate_osd_id_and_not_fsid(self, + is_root, factory): args = factory(no_systemd=True, osd_id='1', @@ -535,6 +543,7 @@ class TestLvmBlueStore: assert str(error.value) == 'could not activate osd.1, please provide the osd_fsid too' def test_activate_not_osd_id_and_not_fsid(self, + is_root, factory): args = factory(no_systemd=True, osd_id=None, @@ -548,6 +557,7 @@ class TestLvmBlueStore: @patch('ceph_volume.api.lvm.get_lvs') def test_activate_couldnt_find_osd(self, m_get_lvs, + is_root, factory): args = factory(osd_id='1', osd_fsid='824f7edf', diff --git a/src/ceph-volume/ceph_volume/tests/objectstore/test_rawbluestore.py b/src/ceph-volume/ceph_volume/tests/objectstore/test_rawbluestore.py index 4e9861b8b34..204dcdb6f2f 100644 --- a/src/ceph-volume/ceph_volume/tests/objectstore/test_rawbluestore.py +++ b/src/ceph-volume/ceph_volume/tests/objectstore/test_rawbluestore.py @@ -58,7 +58,7 @@ class TestRawBlueStore: @patch.dict('os.environ', {'CEPH_VOLUME_DMCRYPT_SECRET': 'dmcrypt-key'}) @patch('ceph_volume.objectstore.rawbluestore.prepare_utils.create_id') @patch('ceph_volume.objectstore.rawbluestore.system.generate_uuid') - def test_prepare(self, m_generate_uuid, m_create_id, factory): + def test_prepare(self, m_generate_uuid, m_create_id, is_root, factory): m_generate_uuid.return_value = 'fake-uuid' m_create_id.return_value = MagicMock() self.raw_bs.prepare_dmcrypt = MagicMock() @@ -117,6 +117,7 @@ class TestRawBlueStore: assert m_create_osd_path.mock_calls == [call('1', tmpfs=True)] def test_activate_raises_exception(self, + is_root, mock_raw_direct_report): with pytest.raises(RuntimeError) as error: self.raw_bs.activate([], @@ -126,6 +127,7 @@ class TestRawBlueStore: assert str(error.value) == 'did not find any matching OSD to activate' def test_activate_osd_id(self, + is_root, mock_raw_direct_report): self.raw_bs._activate = MagicMock() self.raw_bs.activate([], @@ -141,6 +143,7 @@ class TestRawBlueStore: tmpfs=True)] def test_activate_osd_fsid(self, + is_root, mock_raw_direct_report): self.raw_bs._activate = MagicMock() with pytest.raises(RuntimeError): diff --git a/src/ceph-volume/ceph_volume/tests/systemd/test_main.py b/src/ceph-volume/ceph_volume/tests/systemd/test_main.py index be13438f6fb..3156d50ddfa 100644 --- a/src/ceph-volume/ceph_volume/tests/systemd/test_main.py +++ b/src/ceph-volume/ceph_volume/tests/systemd/test_main.py @@ -31,15 +31,15 @@ class TestMain(object): def setup_method(self): conf.log_path = '/tmp/' - def test_no_arguments_parsing_error(self): + def test_no_arguments_parsing_error(self, fake_filesystem): with pytest.raises(RuntimeError): main(args=[]) - def test_parsing_suffix_error(self): + def test_parsing_suffix_error(self, fake_filesystem): with pytest.raises(exceptions.SuffixParsingError): main(args=['asdf']) - def test_correct_command(self, monkeypatch): + def test_correct_command(self, monkeypatch, fake_filesystem): run = Capture() monkeypatch.setattr(process, 'run', run) main(args=['ceph-volume-systemd', 'lvm-8715BEB4-15C5-49DE-BA6F-401086EC7B41-0' ]) diff --git a/src/ceph-volume/ceph_volume/tests/test_main.py b/src/ceph-volume/ceph_volume/tests/test_main.py index d03d405d553..65689bf4f3b 100644 --- a/src/ceph-volume/ceph_volume/tests/test_main.py +++ b/src/ceph-volume/ceph_volume/tests/test_main.py @@ -32,7 +32,7 @@ class TestVolume(object): assert '--cluster' in stdout assert '--log-path' in stdout - def test_log_ignoring_missing_ceph_conf(self, caplog): + def test_log_ignoring_missing_ceph_conf(self, caplog, fake_filesystem): with pytest.raises(SystemExit) as error: main.Volume(argv=['ceph-volume', '--cluster', 'barnacle', 'lvm', '--help']) # make sure we aren't causing an actual error @@ -41,7 +41,7 @@ class TestVolume(object): assert log.message == 'ignoring inability to load ceph.conf' assert log.levelname == 'WARNING' - def test_logs_current_command(self, caplog): + def test_logs_current_command(self, caplog, fake_filesystem): with pytest.raises(SystemExit) as error: main.Volume(argv=['ceph-volume', '--cluster', 'barnacle', 'lvm', '--help']) # make sure we aren't causing an actual error @@ -50,7 +50,7 @@ class TestVolume(object): assert log.message == 'Running command: ceph-volume --cluster barnacle lvm --help' assert log.levelname == 'INFO' - def test_logs_set_level_warning(self, caplog): + def test_logs_set_level_warning(self, caplog, fake_filesystem): with pytest.raises(SystemExit) as error: main.Volume(argv=['ceph-volume', '--log-level', 'warning', '--cluster', 'barnacle', 'lvm', '--help']) # make sure we aren't causing an actual error diff --git a/src/ceph-volume/ceph_volume/tests/test_terminal.py b/src/ceph-volume/ceph_volume/tests/test_terminal.py index e59a036baa8..3c420f15e19 100644 --- a/src/ceph-volume/ceph_volume/tests/test_terminal.py +++ b/src/ceph-volume/ceph_volume/tests/test_terminal.py @@ -131,13 +131,3 @@ class TestWriteUnicode(object): writer.seek(0) val = buffer.getvalue() assert self.octpus_and_squid_en.encode(encoding) in val - - def test_writer_uses_log_on_unicodeerror(self, stream, monkeypatch, capture): - - if sys.version_info > (3,): - pytest.skip("Something breaks inside of pytest's capsys") - monkeypatch.setattr(terminal.terminal_logger, 'info', capture) - buffer = io.BytesIO() - writer = stream(buffer, 'ascii') - terminal._Write(_writer=writer).raw(self.message) - assert self.octpus_and_squid_en in capture.calls[0]['args'][0] diff --git a/src/ceph-volume/ceph_volume/tests/util/test_arg_validators.py b/src/ceph-volume/ceph_volume/tests/util/test_arg_validators.py index c6349308ee7..abbf1d57f33 100644 --- a/src/ceph-volume/ceph_volume/tests/util/test_arg_validators.py +++ b/src/ceph-volume/ceph_volume/tests/util/test_arg_validators.py @@ -1,6 +1,5 @@ import argparse import pytest -import os from ceph_volume import exceptions, process from ceph_volume.util import arg_validators from mock.mock import patch, MagicMock @@ -12,23 +11,22 @@ class TestOSDPath(object): self.validator = arg_validators.OSDPath() def test_is_not_root(self, monkeypatch): - monkeypatch.setattr(os, 'getuid', lambda: 100) + monkeypatch.setattr('ceph_volume.decorators.os.getuid', lambda : 100) with pytest.raises(exceptions.SuperUserError): self.validator('') - def test_path_is_not_a_directory(self, is_root, monkeypatch, fake_filesystem): + def test_path_is_not_a_directory(self, monkeypatch, fake_filesystem): fake_file = fake_filesystem.create_file('/tmp/foo') + monkeypatch.setattr('ceph_volume.decorators.os.getuid', lambda : 0) monkeypatch.setattr(arg_validators.disk, 'is_partition', lambda x: False) - validator = arg_validators.OSDPath() with pytest.raises(argparse.ArgumentError): - validator(fake_file.path) + self.validator(fake_file.path) - def test_files_are_missing(self, is_root, tmpdir, monkeypatch): - tmppath = str(tmpdir) - monkeypatch.setattr(arg_validators.disk, 'is_partition', lambda x: False) - validator = arg_validators.OSDPath() + @patch('ceph_volume.decorators.os.getuid', return_value=0) + @patch('ceph_volume.util.arg_validators.disk.is_partition', return_value=False) + def test_files_are_missing(self, m_is_partition, m_getuid, fake_filesystem): with pytest.raises(argparse.ArgumentError) as error: - validator(tmppath) + self.validator('/tmp/osdpath') assert 'Required file (ceph_fsid) was not found in OSD' in str(error.value) diff --git a/src/ceph-volume/ceph_volume/tests/util/test_disk.py b/src/ceph-volume/ceph_volume/tests/util/test_disk.py index da2cedae1be..94f3d3566e6 100644 --- a/src/ceph-volume/ceph_volume/tests/util/test_disk.py +++ b/src/ceph-volume/ceph_volume/tests/util/test_disk.py @@ -260,64 +260,72 @@ class TestGetDevices(object): result = disk.get_devices(_sys_block_path=str(tmpdir)) assert result == {} - def test_sda_block_is_found(self, patched_get_block_devs_sysfs, fake_filesystem): + @patch('ceph_volume.util.disk.udevadm_property') + def test_sda_block_is_found(self, m_udev_adm_property, patched_get_block_devs_sysfs, fake_filesystem): sda_path = '/dev/sda' - patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk']] + patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk', sda_path]] result = disk.get_devices() assert len(result.keys()) == 1 assert result[sda_path]['human_readable_size'] == '0.00 B' assert result[sda_path]['model'] == '' assert result[sda_path]['partitions'] == {} - def test_sda_size(self, patched_get_block_devs_sysfs, fake_filesystem): + @patch('ceph_volume.util.disk.udevadm_property') + def test_sda_size(self, m_udev_adm_property, patched_get_block_devs_sysfs, fake_filesystem): sda_path = '/dev/sda' - patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk']] + patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk', sda_path]] fake_filesystem.create_file('/sys/block/sda/size', contents = '1024') result = disk.get_devices() assert list(result.keys()) == [sda_path] assert result[sda_path]['human_readable_size'] == '512.00 KB' - def test_sda_sectorsize_fallsback(self, patched_get_block_devs_sysfs, fake_filesystem): + @patch('ceph_volume.util.disk.udevadm_property') + def test_sda_sectorsize_fallsback(self, m_udev_adm_property, patched_get_block_devs_sysfs, fake_filesystem): # if no sectorsize, it will use queue/hw_sector_size sda_path = '/dev/sda' - patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk']] + patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk', sda_path]] fake_filesystem.create_file('/sys/block/sda/queue/hw_sector_size', contents = '1024') result = disk.get_devices() assert list(result.keys()) == [sda_path] assert result[sda_path]['sectorsize'] == '1024' - def test_sda_sectorsize_from_logical_block(self, patched_get_block_devs_sysfs, fake_filesystem): + @patch('ceph_volume.util.disk.udevadm_property') + def test_sda_sectorsize_from_logical_block(self, m_udev_adm_property, patched_get_block_devs_sysfs, fake_filesystem): sda_path = '/dev/sda' - patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk']] + patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk', sda_path]] fake_filesystem.create_file('/sys/block/sda/queue/logical_block_size', contents = '99') result = disk.get_devices() assert result[sda_path]['sectorsize'] == '99' - def test_sda_sectorsize_does_not_fallback(self, patched_get_block_devs_sysfs, fake_filesystem): + @patch('ceph_volume.util.disk.udevadm_property') + def test_sda_sectorsize_does_not_fallback(self, m_udev_adm_property, patched_get_block_devs_sysfs, fake_filesystem): sda_path = '/dev/sda' - patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk']] + patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk', sda_path]] fake_filesystem.create_file('/sys/block/sda/queue/logical_block_size', contents = '99') fake_filesystem.create_file('/sys/block/sda/queue/hw_sector_size', contents = '1024') result = disk.get_devices() assert result[sda_path]['sectorsize'] == '99' - def test_is_rotational(self, patched_get_block_devs_sysfs, fake_filesystem): + @patch('ceph_volume.util.disk.udevadm_property') + def test_is_rotational(self, m_udev_adm_property, patched_get_block_devs_sysfs, fake_filesystem): sda_path = '/dev/sda' - patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk']] + patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk', sda_path]] fake_filesystem.create_file('/sys/block/sda/queue/rotational', contents = '1') result = disk.get_devices() assert result[sda_path]['rotational'] == '1' - def test_is_ceph_rbd(self, patched_get_block_devs_sysfs, fake_filesystem): + @patch('ceph_volume.util.disk.udevadm_property') + def test_is_ceph_rbd(self, m_udev_adm_property, patched_get_block_devs_sysfs, fake_filesystem): rbd_path = '/dev/rbd0' - patched_get_block_devs_sysfs.return_value = [[rbd_path, rbd_path, 'disk']] + patched_get_block_devs_sysfs.return_value = [[rbd_path, rbd_path, 'disk', rbd_path]] result = disk.get_devices() assert rbd_path not in result - def test_actuator_device(self, patched_get_block_devs_sysfs, fake_filesystem): + @patch('ceph_volume.util.disk.udevadm_property') + def test_actuator_device(self, m_udev_adm_property, patched_get_block_devs_sysfs, fake_filesystem): sda_path = '/dev/sda' fake_actuator_nb = 2 - patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk']] + patched_get_block_devs_sysfs.return_value = [[sda_path, sda_path, 'disk', sda_path]] for actuator in range(0, fake_actuator_nb): fake_filesystem.create_dir(f'/sys/block/sda/queue/independent_access_ranges/{actuator}') result = disk.get_devices() @@ -544,7 +552,7 @@ class TestSizeSpecificFormatting(object): class TestAllowLoopDevsWarning(object): - def setup(self): + def setup_method(self): disk.AllowLoopDevices.allow = False disk.AllowLoopDevices.warned = False if os.environ.get('CEPH_VOLUME_ALLOW_LOOP_DEVICES'): diff --git a/src/ceph-volume/ceph_volume/tests/util/test_encryption.py b/src/ceph-volume/ceph_volume/tests/util/test_encryption.py index cd2ea8f187f..4a720241dd9 100644 --- a/src/ceph-volume/ceph_volume/tests/util/test_encryption.py +++ b/src/ceph-volume/ceph_volume/tests/util/test_encryption.py @@ -103,8 +103,9 @@ class TestLuksFormat(object): class TestLuksOpen(object): + @patch('ceph_volume.util.encryption.bypass_workqueue', return_value=False) @patch('ceph_volume.util.encryption.process.call') - def test_luks_open_command_with_default_size(self, m_call, conf_ceph_stub): + def test_luks_open_command_with_default_size(self, m_call, m_bypass_workqueue, conf_ceph_stub): conf_ceph_stub('[global]\nfsid=abcd') expected = [ 'cryptsetup', @@ -120,8 +121,9 @@ class TestLuksOpen(object): encryption.luks_open('abcd', '/dev/foo', '/dev/bar') assert m_call.call_args[0][0] == expected + @patch('ceph_volume.util.encryption.bypass_workqueue', return_value=False) @patch('ceph_volume.util.encryption.process.call') - def test_luks_open_command_with_custom_size(self, m_call, conf_ceph_stub): + def test_luks_open_command_with_custom_size(self, m_call, m_bypass_workqueue, conf_ceph_stub): conf_ceph_stub('[global]\nfsid=abcd\n[osd]\nosd_dmcrypt_key_size=256') expected = [ 'cryptsetup', diff --git a/src/ceph-volume/ceph_volume/tests/util/test_prepare.py b/src/ceph-volume/ceph_volume/tests/util/test_prepare.py index c45beb45c7a..4bda56581c4 100644 --- a/src/ceph-volume/ceph_volume/tests/util/test_prepare.py +++ b/src/ceph-volume/ceph_volume/tests/util/test_prepare.py @@ -119,7 +119,7 @@ class TestFormatDevice(object): class TestOsdMkfsBluestore(object): - def setup(self): + def setup_method(self): conf.cluster = 'ceph' def test_keyring_is_added(self, fake_call, monkeypatch): -- 2.39.5