From cedf281c7349daeec3ad85f12c75a47b1caf55ad Mon Sep 17 00:00:00 2001 From: Matthew Booth Date: Fri, 17 Nov 2017 13:44:27 +0000 Subject: [PATCH] Make close on cryptsetup volumes idempotent 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: I9f52f89b8466d03699cfd5c0e32c672c934cd6fb --- os_brick/encryptors/cryptsetup.py | 10 ++++++---- os_brick/tests/encryptors/test_cryptsetup.py | 4 ++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/os_brick/encryptors/cryptsetup.py b/os_brick/encryptors/cryptsetup.py index f866b56b8..ba4574cd0 100644 --- a/os_brick/encryptors/cryptsetup.py +++ b/os_brick/encryptors/cryptsetup.py @@ -169,11 +169,13 @@ class CryptsetupEncryptor(base.VolumeEncryptor): def _close_volume(self, **kwargs): """Closes the device (effectively removes the dm-crypt mapping).""" LOG.debug("closing encrypted volume %s", self.dev_path) - # cryptsetup returns 4 when attempting to destroy a non-active - # dm-crypt device. We are going to ignore this error code to make - # nova deleting that instance successfully. + # NOTE(mdbooth): remove 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', 'remove', 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) def detach_volume(self, **kwargs): diff --git a/os_brick/tests/encryptors/test_cryptsetup.py b/os_brick/tests/encryptors/test_cryptsetup.py index e2f1ac991..27443e448 100644 --- a/os_brick/tests/encryptors/test_cryptsetup.py +++ b/os_brick/tests/encryptors/test_cryptsetup.py @@ -88,7 +88,7 @@ class CryptsetupEncryptorTestCase(test_base.VolumeEncryptorTestCase): mock_execute.assert_has_calls([ mock.call('cryptsetup', 'remove', self.dev_name, root_helper=self.root_helper, - run_as_root=True, check_exit_code=True), + run_as_root=True, check_exit_code=[0, 4]), ]) @mock.patch('os_brick.executor.Executor._execute') @@ -98,7 +98,7 @@ class CryptsetupEncryptorTestCase(test_base.VolumeEncryptorTestCase): mock_execute.assert_has_calls([ mock.call('cryptsetup', 'remove', self.dev_name, root_helper=self.root_helper, - run_as_root=True, check_exit_code=True), + run_as_root=True, check_exit_code=[0, 4]), ]) def test_init_volume_encryption_not_supported(self):