openstack/os-brick
Code Issues Proposed changes

Make close on luks volumes idempotent

Browse Source 
When recovering from the failure of a compute host, Nova can call
close on an encryptor whose state Nova can't be certain of, but which
hasn't been created. This change makes the close operation idempotent,
which allows recovery to be more robust.

Related-bug: #1724573
Change-Id: I31d72357c89db53a147c2d986a28c9c6870efad0
This commit is contained in:
Matthew Booth
2017-11-17 13:44:27 +00:00
parent e9422a730a
commit 657ba45375
2 changed files with 10 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
os_brick/encryptors/luks.py
View File

@@ -181,7 +181,12 @@ class LuksEncryptor(cryptsetup.CryptsetupEncryptor):
def _close_volume(self, **kwargs): def _close_volume(self, **kwargs):
"""Closes the device (effectively removes the dm-crypt mapping).""" """Closes the device (effectively removes the dm-crypt mapping)."""
LOG.debug("closing encrypted volume %s", self.dev_path) LOG.debug("closing encrypted volume %s", self.dev_path)
# NOTE(mdbooth): luksClose will return 4 (wrong device specified) if
# the device doesn't exist. We assume here that the caller hasn't
# specified the wrong device, and that it doesn't exist because it
# isn't open. We don't fail in this case in order to make this
# operation idempotent.
self._execute('cryptsetup', 'luksClose', self.dev_name, self._execute('cryptsetup', 'luksClose', self.dev_name,
run_as_root=True, check_exit_code=True, run_as_root=True, check_exit_code=[0, 4],
root_helper=self._root_helper, root_helper=self._root_helper,
attempts=3) attempts=3)

8
os_brick/tests/encryptors/test_luks.py
View File

@@ -169,7 +169,7 @@ class LuksEncryptorTestCase(test_cryptsetup.CryptsetupEncryptorTestCase):
mock_execute.assert_has_calls([ mock_execute.assert_has_calls([
mock.call('cryptsetup', 'luksClose', self.dev_name, mock.call('cryptsetup', 'luksClose', self.dev_name,
root_helper=self.root_helper, root_helper=self.root_helper,
attempts=3, run_as_root=True, check_exit_code=True), attempts=3, run_as_root=True, check_exit_code=[0, 4]),
]) ])
@mock.patch('os_brick.executor.Executor._execute') @mock.patch('os_brick.executor.Executor._execute')
@@ -179,7 +179,7 @@ class LuksEncryptorTestCase(test_cryptsetup.CryptsetupEncryptorTestCase):
mock_execute.assert_has_calls([ mock_execute.assert_has_calls([
mock.call('cryptsetup', 'luksClose', self.dev_name, mock.call('cryptsetup', 'luksClose', self.dev_name,
root_helper=self.root_helper, root_helper=self.root_helper,
attempts=3, run_as_root=True, check_exit_code=True), attempts=3, run_as_root=True, check_exit_code=[0, 4]),
]) ])
def test_get_mangled_passphrase(self): def test_get_mangled_passphrase(self):
@@ -224,7 +224,7 @@ class LuksEncryptorTestCase(test_cryptsetup.CryptsetupEncryptorTestCase):
check_exit_code=True), check_exit_code=True),
mock.call('cryptsetup', 'luksClose', self.dev_name, mock.call('cryptsetup', 'luksClose', self.dev_name,
root_helper=self.root_helper, run_as_root=True, root_helper=self.root_helper, run_as_root=True,
check_exit_code=True, attempts=3), check_exit_code=[0, 4], attempts=3),
mock.call('cryptsetup', 'luksAddKey', self.dev_path, mock.call('cryptsetup', 'luksAddKey', self.dev_path,
process_input=''.join([fake_key_mangled, process_input=''.join([fake_key_mangled,
'\n', fake_key, '\n', fake_key,
@@ -237,7 +237,7 @@ class LuksEncryptorTestCase(test_cryptsetup.CryptsetupEncryptorTestCase):
check_exit_code=True), check_exit_code=True),
mock.call('cryptsetup', 'luksClose', self.dev_name, mock.call('cryptsetup', 'luksClose', self.dev_name,
root_helper=self.root_helper, run_as_root=True, root_helper=self.root_helper, run_as_root=True,
check_exit_code=True, attempts=3), check_exit_code=[0, 4], attempts=3),
mock.call('cryptsetup', 'luksRemoveKey', self.dev_path, mock.call('cryptsetup', 'luksRemoveKey', self.dev_path,
process_input=fake_key_mangled, process_input=fake_key_mangled,
root_helper=self.root_helper, run_as_root=True, root_helper=self.root_helper, run_as_root=True,