virt: pool secret should be undefined in pool_undefine not pool_delete

Got confused between pool_delete and pool_undefine. Delete only deletes
the data and is not implemented for remote volume pools like iSCSI or
RBD... the secret needs to be removed when removing the libvirt
definition of the pool instead.
This commit is contained in:
Cédric Bosdonnat 2020-05-19 10:37:03 +02:00 committed by Daniel Wozniak
parent 9c3f6166c9
commit 9b3072c992
2 changed files with 21 additions and 27 deletions

View file

@ -6267,6 +6267,22 @@ def pool_undefine(name, **kwargs):
conn = __get_conn(**kwargs)
try:
pool = conn.storagePoolLookupByName(name)
desc = ElementTree.fromstring(pool.XMLDesc())
# Is there a secret that we generated and would need to be removed?
# Don't remove the other secrets
auth_node = desc.find("source/auth")
if auth_node is not None:
auth_types = {
"ceph": libvirt.VIR_SECRET_USAGE_TYPE_CEPH,
"iscsi": libvirt.VIR_SECRET_USAGE_TYPE_ISCSI,
}
secret_type = auth_types[auth_node.get("type")]
secret_usage = auth_node.find("secret").get("usage")
if secret_type and "pool_{}".format(name) == secret_usage:
secret = conn.secretLookupByUsage(secret_type, secret_usage)
secret.undefine()
return not bool(pool.undefine())
finally:
conn.close()
@ -6292,22 +6308,6 @@ def pool_delete(name, **kwargs):
conn = __get_conn(**kwargs)
try:
pool = conn.storagePoolLookupByName(name)
desc = ElementTree.fromstring(pool.XMLDesc())
# Is there a secret that we generated and would need to be removed?
# Don't remove the other secrets
auth_node = desc.find("source/auth")
if auth_node is not None:
auth_types = {
"ceph": libvirt.VIR_SECRET_USAGE_TYPE_CEPH,
"iscsi": libvirt.VIR_SECRET_USAGE_TYPE_ISCSI,
}
secret_type = auth_types[auth_node.get("type")]
secret_usage = auth_node.find("secret").get("usage")
if secret_type and "pool_{}".format(name) == secret_usage:
secret = conn.secretLookupByUsage(secret_type, secret_usage)
secret.undefine()
return not bool(pool.delete(libvirt.VIR_STORAGE_POOL_DELETE_NORMAL))
finally:
conn.close()

View file

@ -4226,7 +4226,6 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
"""
mock_pool = MagicMock()
mock_pool.delete = MagicMock(return_value=0)
mock_pool.XMLDesc.return_value = "<pool type='dir'/>"
self.mock_conn.storagePoolLookupByName = MagicMock(return_value=mock_pool)
res = virt.pool_delete("test-pool")
@ -4240,12 +4239,12 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
self.mock_libvirt.VIR_STORAGE_POOL_DELETE_NORMAL
)
def test_pool_delete_secret(self):
def test_pool_undefine_secret(self):
"""
Test virt.pool_delete function where the pool has a secret
Test virt.pool_undefine function where the pool has a secret
"""
mock_pool = MagicMock()
mock_pool.delete = MagicMock(return_value=0)
mock_pool.undefine = MagicMock(return_value=0)
mock_pool.XMLDesc.return_value = """
<pool type='rbd'>
<name>test-ses</name>
@ -4262,16 +4261,11 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
mock_undefine = MagicMock(return_value=0)
self.mock_conn.secretLookupByUsage.return_value.undefine = mock_undefine
res = virt.pool_delete("test-ses")
res = virt.pool_undefine("test-ses")
self.assertTrue(res)
self.mock_conn.storagePoolLookupByName.assert_called_once_with("test-ses")
# Shouldn't be called with another parameter so far since those are not implemented
# and thus throwing exceptions.
mock_pool.delete.assert_called_once_with(
self.mock_libvirt.VIR_STORAGE_POOL_DELETE_NORMAL
)
mock_pool.undefine.assert_called_once_with()
self.mock_conn.secretLookupByUsage.assert_called_once_with(
self.mock_libvirt.VIR_SECRET_USAGE_TYPE_CEPH, "pool_test-ses"