Merge pull request #21685 from alfredodeza/wip-rm23874

ceph-volume  failed ceph-osd --mkfs command doesn't halt the OSD creation process

Reviewed-by: Andrew Schoen <aschoen@redhat.com>

src/ceph-volume/ceph_volume/tests/conftest.py

@@ -10,9 +10,15 @@ def __init__(self, *a, **kw):
         self.a = a
         self.kw = kw
         self.calls = []
+        self.return_values = kw.get('return_values', False)
+        self.always_returns = kw.get('always_returns', False)
 
     def __call__(self, *a, **kw):
         self.calls.append({'args': a, 'kwargs': kw})
+        if self.always_returns:
+            return self.always_returns
+        if self.return_values:
+            return self.return_values.pop()
 
 
 class Factory(object):
@@ -41,7 +47,7 @@ def fake_run(monkeypatch):
 
 @pytest.fixture
 def fake_call(monkeypatch):
-    fake_call = Capture()
+    fake_call = Capture(always_returns=([], [], 0))
     monkeypatch.setattr('ceph_volume.process.call', fake_call)
     return fake_call
 
@@ -51,10 +57,12 @@ def stub_call(monkeypatch):
     """
     Monkeypatches process.call, so that a caller can add behavior to the response
     """
-    def apply(return_value):
-        monkeypatch.setattr(
-            'ceph_volume.process.call',
-            lambda *a, **kw: return_value)
+    def apply(return_values):
+        if isinstance(return_values, tuple):
+            return_values = [return_values]
+        stubbed_call = Capture(return_values=return_values)
+        monkeypatch.setattr('ceph_volume.process.call', stubbed_call)
+        return stubbed_call
 
     return apply
 

src/ceph-volume/ceph_volume/tests/util/test_prepare.py

@@ -287,3 +287,60 @@ def test_normalize_strings_flags(self, flags):
     def test_normalize_strings_duplicate_flags(self, flags):
         result = sorted(prepare._normalize_mount_flags(flags, extras=['discard','rw']).split(','))
         assert ','.join(result) == 'auto,discard,exec,rw'
+
+
+class TestMkfsFilestore(object):
+
+    def test_non_zero_exit_status(self, stub_call, monkeypatch):
+        conf.cluster = 'ceph'
+        monkeypatch.setattr('ceph_volume.util.prepare.system.chown', lambda x: True)
+        stub_call(([], [], 1))
+        with pytest.raises(RuntimeError) as error:
+            prepare.osd_mkfs_filestore('1', 'asdf-1234', 'keyring')
+        assert "Command failed with exit code 1" in str(error)
+
+    def test_non_zero_exit_formats_command_correctly(self, stub_call, monkeypatch):
+        conf.cluster = 'ceph'
+        monkeypatch.setattr('ceph_volume.util.prepare.system.chown', lambda x: True)
+        stub_call(([], [], 1))
+        with pytest.raises(RuntimeError) as error:
+            prepare.osd_mkfs_filestore('1', 'asdf-1234', 'keyring')
+        expected = ' '.join([
+            'ceph-osd',
+            '--cluster',
+            'ceph',
+            '--osd-objectstore', 'filestore', '--mkfs',
+            '-i', '1', '--monmap', '/var/lib/ceph/osd/ceph-1/activate.monmap',
+            '--keyfile', '-', '--osd-data', '/var/lib/ceph/osd/ceph-1/',
+            '--osd-journal', '/var/lib/ceph/osd/ceph-1/journal',
+            '--osd-uuid', 'asdf-1234',
+            '--setuser', 'ceph', '--setgroup', 'ceph'])
+        assert expected in str(error)
+
+
+class TestMkfsBluestore(object):
+
+    def test_non_zero_exit_status(self, stub_call, monkeypatch):
+        conf.cluster = 'ceph'
+        monkeypatch.setattr('ceph_volume.util.prepare.system.chown', lambda x: True)
+        stub_call(([], [], 1))
+        with pytest.raises(RuntimeError) as error:
+            prepare.osd_mkfs_bluestore('1', 'asdf-1234', keyring='keyring')
+        assert "Command failed with exit code 1" in str(error)
+
+    def test_non_zero_exit_formats_command_correctly(self, stub_call, monkeypatch):
+        conf.cluster = 'ceph'
+        monkeypatch.setattr('ceph_volume.util.prepare.system.chown', lambda x: True)
+        stub_call(([], [], 1))
+        with pytest.raises(RuntimeError) as error:
+            prepare.osd_mkfs_bluestore('1', 'asdf-1234', keyring='keyring')
+        expected = ' '.join([
+            'ceph-osd',
+            '--cluster',
+            'ceph',
+            '--osd-objectstore', 'bluestore', '--mkfs',
+            '-i', '1', '--monmap', '/var/lib/ceph/osd/ceph-1/activate.monmap',
+            '--keyfile', '-', '--osd-data', '/var/lib/ceph/osd/ceph-1/',
+            '--osd-uuid', 'asdf-1234',
+            '--setuser', 'ceph', '--setgroup', 'ceph'])
+        assert expected in str(error)

src/ceph-volume/ceph_volume/util/prepare.py

@@ -322,7 +322,9 @@ def osd_mkfs_bluestore(osd_id, fsid, keyring=None, wal=False, db=False):
 
     command = base_command + supplementary_command
 
-    process.call(command, stdin=keyring, show_command=True)
+    _, _, returncode = process.call(command, stdin=keyring, show_command=True)
+    if returncode != 0:
+        raise RuntimeError('Command failed with exit code %s: %s' % (returncode, ' '.join(command)))
 
 
 def osd_mkfs_filestore(osd_id, fsid, keyring):
@@ -360,4 +362,8 @@ def osd_mkfs_filestore(osd_id, fsid, keyring):
         '--setuser', 'ceph',
         '--setgroup', 'ceph'
     ]
-    process.call(command, stdin=keyring, terminal_verbose=True, show_command=True)
+    _, _, returncode = process.call(
+        command, stdin=keyring, terminal_verbose=True, show_command=True
+    )
+    if returncode != 0:
+        raise RuntimeError('Command failed with exit code %s: %s' % (returncode, ' '.join(command)))