mirror of
https://github.com/saltstack/salt.git
synced 2025-04-17 10:10:20 +00:00
virt: handle build differently depending on the pool type
Some libvirt storage pools backends don't implement the build function. In thoses cases the virt.pool_defined and virt.pool_running states must not run the virt.pool_build() function. There are also cases where build only works once. For example when creating a LVM VG or a disk partition pool, if the VG or partition is already present build will fail. Fixes issue #56454
This commit is contained in:
Cédric Bosdonnat
Daniel Wozniak
parent
9b3072c992
commit
f19bcf49c2
3 changed files with 126 additions and 24 deletions
1
changelog/56454.fixed
Normal file
1
changelog/56454.fixed
Normal file
|
|
@ -0,0 +1 @@
|
|||
Better handle virt.pool_rebuild in virt.pool_running and virt.pool_defined states
|
||||
|
|
@ -16,6 +16,7 @@ for the generation and signing of certificates for systems running libvirt:
|
|||
from __future__ import absolute_import, print_function, unicode_literals
|
||||
|
||||
import fnmatch
|
||||
import logging
|
||||
import os
|
||||
|
||||
# Import Salt libs
|
||||
|
|
@ -38,6 +39,8 @@ except ImportError:
|
|||
|
||||
__virtualname__ = "virt"
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def __virtual__():
|
||||
"""
|
||||
|
|
@ -1090,6 +1093,10 @@ def network_running(
|
|||
return ret
|
||||
|
||||
|
||||
# Some of the libvirt storage drivers do not support the build action
|
||||
BUILDABLE_POOL_TYPES = {"disk", "fs", "netfs", "dir", "logical", "vstorage", "zfs"}
|
||||
|
||||
|
||||
def pool_defined(
|
||||
name,
|
||||
ptype=None,
|
||||
|
|
@ -1204,14 +1211,24 @@ def pool_defined(
|
|||
|
||||
action = ""
|
||||
if info[name]["state"] != "running":
|
||||
if not __opts__["test"]:
|
||||
__salt__["virt.pool_build"](
|
||||
name,
|
||||
connection=connection,
|
||||
username=username,
|
||||
password=password,
|
||||
)
|
||||
action = ", built"
|
||||
if ptype in BUILDABLE_POOL_TYPES:
|
||||
if not __opts__["test"]:
|
||||
# Storage pools build like disk or logical will fail if the disk or LV group
|
||||
# was already existing. Since we can't easily figure that out, just log the
|
||||
# possible libvirt error.
|
||||
try:
|
||||
__salt__["virt.pool_build"](
|
||||
name,
|
||||
connection=connection,
|
||||
username=username,
|
||||
password=password,
|
||||
)
|
||||
except libvirt.libvirtError as err:
|
||||
log.warning(
|
||||
"Failed to build libvirt storage pool: %s",
|
||||
err.get_error_message(),
|
||||
)
|
||||
action = ", built"
|
||||
|
||||
action = (
|
||||
"{}, autostart flag changed".format(action)
|
||||
|
|
@ -1247,9 +1264,22 @@ def pool_defined(
|
|||
password=password,
|
||||
)
|
||||
|
||||
__salt__["virt.pool_build"](
|
||||
name, connection=connection, username=username, password=password
|
||||
)
|
||||
if ptype in BUILDABLE_POOL_TYPES:
|
||||
# Storage pools build like disk or logical will fail if the disk or LV group
|
||||
# was already existing. Since we can't easily figure that out, just log the
|
||||
# possible libvirt error.
|
||||
try:
|
||||
__salt__["virt.pool_build"](
|
||||
name,
|
||||
connection=connection,
|
||||
username=username,
|
||||
password=password,
|
||||
)
|
||||
except libvirt.libvirtError as err:
|
||||
log.warning(
|
||||
"Failed to build libvirt storage pool: %s",
|
||||
err.get_error_message(),
|
||||
)
|
||||
if needs_autostart:
|
||||
ret["changes"][name] = "Pool defined, marked for autostart"
|
||||
ret["comment"] = "Pool {0} defined, marked for autostart".format(name)
|
||||
|
|
@ -1358,7 +1388,7 @@ def pool_running(
|
|||
is_running = info.get(name, {}).get("state", "stopped") == "running"
|
||||
if is_running:
|
||||
if updated:
|
||||
action = "built, restarted"
|
||||
action = "restarted"
|
||||
if not __opts__["test"]:
|
||||
__salt__["virt.pool_stop"](
|
||||
name,
|
||||
|
|
@ -1366,13 +1396,16 @@ def pool_running(
|
|||
username=username,
|
||||
password=password,
|
||||
)
|
||||
if not __opts__["test"]:
|
||||
__salt__["virt.pool_build"](
|
||||
name,
|
||||
connection=connection,
|
||||
username=username,
|
||||
password=password,
|
||||
)
|
||||
# if the disk or LV group is already existing build will fail (issue #56454)
|
||||
if ptype in BUILDABLE_POOL_TYPES - {"disk", "logical"}:
|
||||
if not __opts__["test"]:
|
||||
__salt__["virt.pool_build"](
|
||||
name,
|
||||
connection=connection,
|
||||
username=username,
|
||||
password=password,
|
||||
)
|
||||
action = "built, {}".format(action)
|
||||
else:
|
||||
action = "already running"
|
||||
result = True
|
||||
|
|
|
|||
|
|
@ -1978,6 +1978,72 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
|
|||
password="secret",
|
||||
)
|
||||
|
||||
# Define a pool that doesn't handle build
|
||||
for mock in mocks:
|
||||
mocks[mock].reset_mock()
|
||||
with patch.dict(
|
||||
virt.__salt__,
|
||||
{ # pylint: disable=no-member
|
||||
"virt.pool_info": MagicMock(
|
||||
side_effect=[
|
||||
{},
|
||||
{"mypool": {"state": "stopped", "autostart": True}},
|
||||
]
|
||||
),
|
||||
"virt.pool_define": mocks["define"],
|
||||
"virt.pool_build": mocks["build"],
|
||||
"virt.pool_set_autostart": mocks["autostart"],
|
||||
},
|
||||
):
|
||||
ret.update(
|
||||
{
|
||||
"changes": {"mypool": "Pool defined, marked for autostart"},
|
||||
"comment": "Pool mypool defined, marked for autostart",
|
||||
}
|
||||
)
|
||||
self.assertDictEqual(
|
||||
virt.pool_defined(
|
||||
"mypool",
|
||||
ptype="rbd",
|
||||
source={
|
||||
"name": "libvirt-pool",
|
||||
"hosts": ["ses2.tf.local", "ses3.tf.local"],
|
||||
"auth": {
|
||||
"username": "libvirt",
|
||||
"password": "AQAz+PRdtquBBRAASMv7nlMZYfxIyLw3St65Xw==",
|
||||
},
|
||||
},
|
||||
autostart=True,
|
||||
),
|
||||
ret,
|
||||
)
|
||||
mocks["define"].assert_called_with(
|
||||
"mypool",
|
||||
ptype="rbd",
|
||||
target=None,
|
||||
permissions=None,
|
||||
source_devices=None,
|
||||
source_dir=None,
|
||||
source_adapter=None,
|
||||
source_hosts=["ses2.tf.local", "ses3.tf.local"],
|
||||
source_auth={
|
||||
"username": "libvirt",
|
||||
"password": "AQAz+PRdtquBBRAASMv7nlMZYfxIyLw3St65Xw==",
|
||||
},
|
||||
source_name="libvirt-pool",
|
||||
source_format=None,
|
||||
source_initiator=None,
|
||||
start=False,
|
||||
transient=False,
|
||||
connection=None,
|
||||
username=None,
|
||||
password=None,
|
||||
)
|
||||
mocks["autostart"].assert_called_with(
|
||||
"mypool", state="on", connection=None, username=None, password=None,
|
||||
)
|
||||
mocks["build"].assert_not_called()
|
||||
|
||||
mocks["update"] = MagicMock(return_value=False)
|
||||
for mock in mocks:
|
||||
mocks[mock].reset_mock()
|
||||
|
|
@ -2027,6 +2093,9 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
|
|||
for mock in mocks:
|
||||
mocks[mock].reset_mock()
|
||||
mocks["update"] = MagicMock(return_value=True)
|
||||
mocks["build"] = MagicMock(
|
||||
side_effect=self.mock_libvirt.libvirtError("Existing VG")
|
||||
)
|
||||
with patch.dict(
|
||||
virt.__salt__,
|
||||
{ # pylint: disable=no-member
|
||||
|
|
@ -2130,6 +2199,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
|
|||
),
|
||||
ret,
|
||||
)
|
||||
mocks["build"].assert_not_called()
|
||||
mocks["update"].assert_called_with(
|
||||
"mypool",
|
||||
ptype="logical",
|
||||
|
|
@ -2477,8 +2547,8 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
|
|||
):
|
||||
ret.update(
|
||||
{
|
||||
"changes": {"mypool": "Pool updated, built, restarted"},
|
||||
"comment": "Pool mypool updated, built, restarted",
|
||||
"changes": {"mypool": "Pool updated, restarted"},
|
||||
"comment": "Pool mypool updated, restarted",
|
||||
"result": True,
|
||||
}
|
||||
)
|
||||
|
|
@ -2504,9 +2574,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
|
|||
mocks["start"].assert_called_with(
|
||||
"mypool", connection=None, username=None, password=None
|
||||
)
|
||||
mocks["build"].assert_called_with(
|
||||
"mypool", connection=None, username=None, password=None
|
||||
)
|
||||
mocks["build"].assert_not_called()
|
||||
mocks["update"].assert_called_with(
|
||||
"mypool",
|
||||
ptype="logical",
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue