File virt.network_define-allow-adding-ip-configurati... of Package salt

Overview Repositories Revisions Requests Users Attributes Meta

File virt.network_define-allow-adding-ip-configuration.patch of Package salt (Revision b98af0adedef74d61715abd98c45f2c6)

Currently displaying revision b98af0adedef74d61715abd98c45f2c6 , Show latest

From 83fa82b713ac92371b1a6c1cf2b909fc440d30d0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9dric=20Bosdonnat?= <cbosdonnat@suse.com>
Date: Tue, 13 Aug 2019 12:26:59 +0200
Subject: [PATCH] virt.network_define allow adding IP configuration

If using virt.network_define with nat network type, then libvirt
complains about missing IP configuration. Allow setting it in both the
virt.network_define module and the virt.network_running state.

Add virt.pool_deleted state

The new virt.pool_deleted state takes care of removing a virtual storage
pool and optionally the contained volumes.

Fix virt._gen_pool_xml when all source parameters are None

When all the source_* parameters of _gen_pool_xml are None, no <source>
element should be generated. Instead we had the following:

Rather than filling a source structure with empty members, directly pass
a None for the source as expected by the jinja template.

Add virt.pool_update function

Add a function that allows changing a virtual storage pool. By using the
test=True parameter, this function can be used to check if a change
would be done.

virt.pool_running state improvements

With this commit the virt.pool_running state is capable to update an
existing pool before ensuring it is running.

This also adds support for the test parameter.

Fix virt.pool_define documentation

virt.pool_define example are mentioning a uuid property while this one
should be named value. The code was right, but not the doc.

Add source_initiator parameter in virt.pool_define

In order to define iscsi-direct virtual storage pools, the use needs to
be able to provide the initiator IQN. Add a parameter for it in:
  * virt.pool_define module function
  * virt.pool_update module function
  * virt.pool_running state

virt pool secret have no type attribute

According to libvirt schemas and doc only the volume secrets have a
type.

virt: create and update pool secret's password for the user

Libvirt stores RBD and iSCSI passwords in secrets. Add a password field
in the source_auth parameter of virt.pool_define and virt.pool_update to
set the password value rather than reuse an already defined libvirt
secret.

virt.pool_define: remove potential leading / in CIFS source path

libvirt needs the CIFS source path not to start with a /. Let's remove
them since this could be a common user mistake.

Add virt.pool_capabilities function

Not all storage backends are supported for a given libvirt hypervisor. For the user
to know what is supported and what is not expose the recently added
libvirt function providing pool capabilities.

For older libvirt version, let's craft data that are reasonable by
adding a computed flag to warn these data may not be 100% accurate.
---
 salt/modules/virt.py                      | 497 +++++++++++++++++++++++++++++-
 salt/states/virt.py                       | 279 +++++++++++++++--
 salt/templates/virt/libvirt_network.jinja |  13 +-
 salt/templates/virt/libvirt_pool.jinja    |  13 +-
 salt/templates/virt/libvirt_secret.jinja  |  12 +
 tests/unit/modules/test_virt.py           | 455 ++++++++++++++++++++++++++-
 tests/unit/states/test_virt.py            | 400 +++++++++++++++++++-----
 7 files changed, 1543 insertions(+), 126 deletions(-)
 create mode 100644 salt/templates/virt/libvirt_secret.jinja

diff --git a/salt/modules/virt.py b/salt/modules/virt.py
index 0f62856f5c..44c7e78ef0 100644
--- a/salt/modules/virt.py
+++ b/salt/modules/virt.py
@@ -74,6 +74,7 @@ The calls not using the libvirt connection setup are:
 
 # Import python libs
 from __future__ import absolute_import, print_function, unicode_literals
+import base64
 import copy
 import os
 import re
@@ -111,6 +112,7 @@ from salt.utils.virt import check_remote, download_remote
 from salt.exceptions import CommandExecutionError, SaltInvocationError
 from salt.ext import six
 from salt.ext.six.moves import range  # pylint: disable=import-error,redefined-builtin
+from salt._compat import ipaddress
 
 log = logging.getLogger(__name__)
 
@@ -662,7 +664,8 @@ def _gen_net_xml(name,
                  bridge,
                  forward,
                  vport,
-                 tag=None):
+                 tag=None,
+                 ip_configs=None):
     '''
     Generate the XML string to define a libvirt network
     '''
@@ -672,6 +675,10 @@ def _gen_net_xml(name,
         'forward': forward,
         'vport': vport,
         'tag': tag,
+        'ip_configs': [{
+            'address': ipaddress.ip_network(config['cidr']),
+            'dhcp_ranges': config.get('dhcp_ranges', []),
+        } for config in ip_configs or []],
     }
     fn_ = 'libvirt_network.jinja'
     try:
@@ -692,24 +699,31 @@ def _gen_pool_xml(name,
                   source_hosts=None,
                   source_auth=None,
                   source_name=None,
-                  source_format=None):
+                  source_format=None,
+                  source_initiator=None):
     '''
     Generate the XML string to define a libvirt storage pool
     '''
     hosts = [host.split(':') for host in source_hosts or []]
-    context = {
-        'name': name,
-        'ptype': ptype,
-        'target': {'path': target, 'permissions': permissions},
-        'source': {
+    source = None
+    if any([source_devices, source_dir, source_adapter, hosts, source_auth, source_name, source_format,
+            source_initiator]):
+        source = {
             'devices': source_devices or [],
-            'dir': source_dir,
+            'dir': source_dir if source_format != 'cifs' or not source_dir else source_dir.lstrip('/'),
             'adapter': source_adapter,
             'hosts': [{'name': host[0], 'port': host[1] if len(host) > 1 else None} for host in hosts],
             'auth': source_auth,
             'name': source_name,
-            'format': source_format
+            'format': source_format,
+            'initiator': source_initiator,
         }
+
+    context = {
+        'name': name,
+        'ptype': ptype,
+        'target': {'path': target, 'permissions': permissions},
+        'source': source
     }
     fn_ = 'libvirt_pool.jinja'
     try:
@@ -720,6 +734,24 @@ def _gen_pool_xml(name,
     return template.render(**context)
 
 
+def _gen_secret_xml(auth_type, usage, description):
+    '''
+    Generate a libvirt secret definition XML
+    '''
+    context = {
+        'type': auth_type,
+        'usage': usage,
+        'description': description,
+    }
+    fn_ = 'libvirt_secret.jinja'
+    try:
+        template = JINJA.get_template(fn_)
+    except jinja2.exceptions.TemplateNotFound:
+        log.error('Could not load template %s', fn_)
+        return ''
+    return template.render(**context)
+
+
 def _get_images_dir():
     '''
     Extract the images dir from the configuration. First attempts to
@@ -4425,7 +4457,12 @@ def cpu_baseline(full=False, migratable=False, out='libvirt', **kwargs):
     return cpu.toxml()
 
 
-def network_define(name, bridge, forward, **kwargs):
+def network_define(name,
+                   bridge,
+                   forward,
+                   ipv4_config=None,
+                   ipv6_config=None,
+                   **kwargs):
     '''
     Create libvirt network.
 
@@ -4436,10 +4473,38 @@ def network_define(name, bridge, forward, **kwargs):
     :param tag: Vlan tag
     :param autostart: Network autostart (default True)
     :param start: Network start (default True)
+    :param ipv4_config: IP v4 configuration
+        Dictionary describing the IP v4 setup like IP range and
+        a possible DHCP configuration. The structure is documented
+        in net-define-ip_.
+
+        ..versionadded:: Neon
+    :type ipv4_config: dict or None
+
+    :param ipv6_config: IP v6 configuration
+        Dictionary describing the IP v6 setup like IP range and
+        a possible DHCP configuration. The structure is documented
+        in net-define-ip_.
+
+        ..versionadded:: Neon
+    :type ipv6_config: dict or None
+
     :param connection: libvirt connection URI, overriding defaults
     :param username: username to connect with, overriding defaults
     :param password: password to connect with, overriding defaults
 
+    .. _net-define-ip:
+
+    ** IP configuration definition
+
+    Both the IPv4 and IPv6 configuration dictionaries can contain the following properties:
+
+    cidr
+        CIDR notation for the network. For example '192.168.124.0/24'
+
+    dhcp_ranges
+        A list of dictionary with ``'start'`` and ``'end'`` properties.
+
     CLI Example:
 
     .. code-block:: bash
@@ -4453,12 +4518,14 @@ def network_define(name, bridge, forward, **kwargs):
     tag = kwargs.get('tag', None)
     autostart = kwargs.get('autostart', True)
     starting = kwargs.get('start', True)
+
     net_xml = _gen_net_xml(
         name,
         bridge,
         forward,
         vport,
-        tag,
+        tag=tag,
+        ip_configs=[config for config in [ipv4_config, ipv6_config] if config],
     )
     try:
         conn.networkDefineXML(net_xml)
@@ -4669,12 +4736,171 @@ def network_set_autostart(name, state='on', **kwargs):
         conn.close()
 
 
+def _parse_pools_caps(doc):
+    '''
+    Parse libvirt pool capabilities XML
+    '''
+    def _parse_pool_caps(pool):
+        pool_caps = {
+            'name': pool.get('type'),
+            'supported': pool.get('supported', 'no') == 'yes'
+        }
+        for option_kind in ['pool', 'vol']:
+            options = {}
+            default_format_node = pool.find('{0}Options/defaultFormat'.format(option_kind))
+            if default_format_node is not None:
+                options['default_format'] = default_format_node.get('type')
+            options_enums = {enum.get('name'): [value.text for value in enum.findall('value')]
+                 for enum in pool.findall('{0}Options/enum'.format(option_kind))}
+            if options_enums:
+                options.update(options_enums)
+            if options:
+                if 'options' not in pool_caps:
+                    pool_caps['options'] = {}
+                kind = option_kind if option_kind is not 'vol' else 'volume'
+                pool_caps['options'][kind] = options
+        return pool_caps
+
+    return [_parse_pool_caps(pool) for pool in doc.findall('pool')]
+
+
+def pool_capabilities(**kwargs):
+    '''
+    Return the hypervisor connection storage pool capabilities.
+
+    The returned data are either directly extracted from libvirt or computed.
+    In the latter case some pool types could be listed as supported while they
+    are not. To distinguish between the two cases, check the value of the ``computed`` property.
+
+    :param connection: libvirt connection URI, overriding defaults
+    :param username: username to connect with, overriding defaults
+    :param password: password to connect with, overriding defaults
+
+    .. versionadded:: Neon
+
+    CLI Example:
+
+    .. code-block:: bash
+
+        salt '*' virt.pool_capabilities
+
+    '''
+    try:
+        conn = __get_conn(**kwargs)
+        has_pool_capabilities = bool(getattr(conn, 'getStoragePoolCapabilities', None))
+        if has_pool_capabilities:
+            caps = ElementTree.fromstring(conn.getStoragePoolCapabilities())
+            pool_types = _parse_pools_caps(caps)
+        else:
+            # Compute reasonable values
+            all_hypervisors = ['xen', 'kvm', 'bhyve']
+            images_formats = ['none', 'raw', 'dir', 'bochs', 'cloop', 'dmg', 'iso', 'vpc', 'vdi',
+                              'fat', 'vhd', 'ploop', 'cow', 'qcow', 'qcow2', 'qed', 'vmdk']
+            common_drivers = [
+                {
+                    'name': 'fs',
+                    'default_source_format': 'auto',
+                    'source_formats': ['auto', 'ext2', 'ext3', 'ext4', 'ufs', 'iso9660', 'udf', 'gfs', 'gfs2',
+                                       'vfat', 'hfs+', 'xfs', 'ocfs2'],
+                    'default_target_format': 'raw',
+                    'target_formats': images_formats
+                },
+                {
+                    'name': 'dir',
+                    'default_target_format': 'raw',
+                    'target_formats': images_formats
+                },
+                {'name': 'iscsi'},
+                {'name': 'scsi'},
+                {
+                    'name': 'logical',
+                    'default_source_format': 'lvm2',
+                    'source_formats': ['unknown', 'lvm2'],
+                },
+                {
+                    'name': 'netfs',
+                    'default_source_format': 'auto',
+                    'source_formats': ['auto', 'nfs', 'glusterfs', 'cifs'],
+                    'default_target_format': 'raw',
+                    'target_formats': images_formats
+                },
+                {
+                    'name': 'disk',
+                    'default_source_format': 'unknown',
+                    'source_formats': ['unknown', 'dos', 'dvh', 'gpt', 'mac', 'bsd', 'pc98', 'sun', 'lvm2'],
+                    'default_target_format': 'none',
+                    'target_formats': ['none', 'linux', 'fat16', 'fat32', 'linux-swap', 'linux-lvm',
+                                       'linux-raid', 'extended']
+                },
+                {'name': 'mpath'},
+                {
+                    'name': 'rbd',
+                    'default_target_format': 'raw',
+                    'target_formats': []
+                },
+                {
+                    'name': 'sheepdog',
+                    'version': 10000,
+                    'hypervisors': ['kvm'],
+                    'default_target_format': 'raw',
+                    'target_formats': images_formats
+                },
+                {
+                    'name': 'gluster',
+                    'version': 1002000,
+                    'hypervisors': ['kvm'],
+                    'default_target_format': 'raw',
+                    'target_formats': images_formats
+                },
+                {'name': 'zfs', 'version': 1002008, 'hypervisors': ['bhyve']},
+                {'name': 'iscsi-direct', 'version': 4007000, 'hypervisors': ['kvm', 'xen']}
+            ]
+
+            libvirt_version = conn.getLibVersion()
+            hypervisor = get_hypervisor()
+
+            def _get_backend_output(backend):
+                output = {
+                    'name': backend['name'],
+                    'supported': (not backend.get('version') or libvirt_version >= backend['version']) and
+                        hypervisor in backend.get('hypervisors', all_hypervisors),
+                    'options': {
+                        'pool': {
+                            'default_format': backend.get('default_source_format'),
+                            'sourceFormatType': backend.get('source_formats')
+                        },
+                        'volume': {
+                            'default_format': backend.get('default_target_format'),
+                            'targetFormatType': backend.get('target_formats')
+                        }
+                    }
+                }
+
+                # Cleanup the empty members to match the libvirt output
+                for option_kind in ['pool', 'volume']:
+                    if not [value for value in output['options'][option_kind].values() if value is not None]:
+                        del output['options'][option_kind]
+                if not output['options']:
+                    del output['options']
+
+                return output
+            pool_types = [_get_backend_output(backend) for backend in common_drivers]
+    finally:
+        conn.close()
+
+    return {
+        'computed': not has_pool_capabilities,
+        'pool_types': pool_types,
+    }
+
+
 def pool_define(name,
                 ptype,
                 target=None,
                 permissions=None,
                 source_devices=None,
                 source_dir=None,
+                source_initiator=None,
                 source_adapter=None,
                 source_hosts=None,
                 source_auth=None,
@@ -4705,6 +4931,10 @@ def pool_define(name,
     :param source_dir:
         Path to the source directory for pools of type ``dir``, ``netfs`` or ``gluster``.
         (Default: ``None``)
+    :param source_initiator:
+        Initiator IQN for libiscsi-direct pool types. (Default: ``None``)
+
+        .. versionadded:: neon
     :param source_adapter:
         SCSI source definition. The value is a dictionary with ``type``, ``name``, ``parent``,
         ``managed``, ``parent_wwnn``, ``parent_wwpn``, ``parent_fabric_wwn``, ``wwnn``, ``wwpn``
@@ -4736,7 +4966,7 @@ def pool_define(name,
                 'username': 'admin',
                 'secret': {
                     'type': 'uuid',
-                    'uuid': '2ec115d7-3a88-3ceb-bc12-0ac909a6fd87'
+                    'value': '2ec115d7-3a88-3ceb-bc12-0ac909a6fd87'
                 }
             }
 
@@ -4747,10 +4977,14 @@ def pool_define(name,
                 'username': 'myname',
                 'secret': {
                     'type': 'usage',
-                    'uuid': 'mycluster_myname'
+                    'value': 'mycluster_myname'
                 }
             }
 
+        Since neon, instead the source authentication can only contain ``username``
+        and ``password`` properties. In this case the libvirt secret will be defined and used.
+        For Ceph authentications a base64 encoded key is expected.
+
     :param source_name:
         Identifier of name-based sources.
     :param source_format:
@@ -4803,6 +5037,8 @@ def pool_define(name,
     .. versionadded:: 2019.2.0
     '''
     conn = __get_conn(**kwargs)
+    auth = _pool_set_secret(conn, ptype, name, source_auth)
+
     pool_xml = _gen_pool_xml(
         name,
         ptype,
@@ -4812,9 +5048,10 @@ def pool_define(name,
         source_dir=source_dir,
         source_adapter=source_adapter,
         source_hosts=source_hosts,
-        source_auth=source_auth,
+        source_auth=auth,
         source_name=source_name,
-        source_format=source_format
+        source_format=source_format,
+        source_initiator=source_initiator
     )
     try:
         if transient:
@@ -4832,6 +5069,236 @@ def pool_define(name,
     return True
 
 
+def _pool_set_secret(conn, pool_type, pool_name, source_auth, uuid=None, usage=None, test=False):
+    secret_types = {
+        'rbd': 'ceph',
+        'iscsi': 'chap',
+        'iscsi-direct': 'chap'
+    }
+    secret_type = secret_types.get(pool_type)
+    auth = source_auth
+    if source_auth and 'username' in source_auth and 'password' in source_auth:
+        if secret_type:
+            # Get the previously defined secret if any
+            secret = None
+            if usage:
+                usage_type = libvirt.VIR_SECRET_USAGE_TYPE_CEPH if secret_type == 'ceph' \
+                                else libvirt.VIR_SECRET_USAGE_TYPE_ISCSI
+                secret = conn.secretLookupByUsage(usage_type, usage)
+            elif uuid:
+                secret = conn.secretLookupByUUIDString(uuid)
+
+            # Create secret if needed
+            if not secret:
+                description = 'Passphrase for {} pool created by Salt'.format(pool_name)
+                if not usage:
+                    usage = 'pool_{}'.format(pool_name)
+                secret_xml = _gen_secret_xml(secret_type, usage, description)
+                if not test:
+                    secret = conn.secretDefineXML(secret_xml)
+
+            # Assign the password to it
+            password = auth['password']
+            if pool_type == 'rbd':
+                # RBD password are already base64-encoded, but libvirt will base64-encode them later
+                password = base64.b64decode(salt.utils.stringutils.to_bytes(password))
+            if not test:
+                secret.setValue(password)
+
+            # update auth with secret reference
+            auth['type'] = secret_type
+            auth['secret'] = {
+                'type': 'uuid' if uuid else 'usage',
+                'value': uuid if uuid else usage,
+            }
+    return auth
+
+
+def pool_update(name,
+                ptype,
+                target=None,
+                permissions=None,
+                source_devices=None,
+                source_dir=None,
+                source_initiator=None,
+                source_adapter=None,
+                source_hosts=None,
+                source_auth=None,
+                source_name=None,
+                source_format=None,
+                test=False,
+                **kwargs):
+    '''
+    Update a libvirt storage pool if needed.
+    If called with test=True, this is also reporting whether an update would be performed.
+
+    :param name: Pool name
+    :param ptype:
+        Pool type. See `libvirt documentation <https://libvirt.org/storage.html>`_  for the
+        possible values.
+    :param target: Pool full path target
+    :param permissions:
+        Permissions to set on the target folder. This is mostly used for filesystem-based
+        pool types. See :ref:`pool-define-permissions` for more details on this structure.
+    :param source_devices:
+        List of source devices for pools backed by physical devices. (Default: ``None``)
+
+        Each item in the list is a dictionary with ``path`` and optionally ``part_separator``
+        keys. The path is the qualified name for iSCSI devices.
+
+        Report to `this libvirt page <https://libvirt.org/formatstorage.html#StoragePool>`_
+        for more informations on the use of ``part_separator``
+    :param source_dir:
+        Path to the source directory for pools of type ``dir``, ``netfs`` or ``gluster``.
+        (Default: ``None``)
+    :param source_initiator:
+        Initiator IQN for libiscsi-direct pool types. (Default: ``None``)
+
+        .. versionadded:: neon
+    :param source_adapter:
+        SCSI source definition. The value is a dictionary with ``type``, ``name``, ``parent``,
+        ``managed``, ``parent_wwnn``, ``parent_wwpn``, ``parent_fabric_wwn``, ``wwnn``, ``wwpn``
+        and ``parent_address`` keys.
+
+        The ``parent_address`` value is a dictionary with ``unique_id`` and ``address`` keys.
+        The address represents a PCI address and is itself a dictionary with ``domain``, ``bus``,
+        ``slot`` and ``function`` properties.
+        Report to `this libvirt page <https://libvirt.org/formatstorage.html#StoragePool>`_
+        for the meaning and possible values of these properties.
+    :param source_hosts:
+        List of source for pools backed by storage from remote servers. Each item is the hostname
+        optionally followed by the port separated by a colon. (Default: ``None``)
+    :param source_auth:
+        Source authentication details. (Default: ``None``)
+
+        The value is a dictionary with ``type``, ``username`` and ``secret`` keys. The type
+        can be one of ``ceph`` for Ceph RBD or ``chap`` for iSCSI sources.
+
+        The ``secret`` value links to a libvirt secret object. It is a dictionary with
+        ``type`` and ``value`` keys. The type value can be either ``uuid`` or ``usage``.
+
+        Examples:
+
+        .. code-block:: python
+
+            source_auth={
+                'type': 'ceph',
+                'username': 'admin',
+                'secret': {
+                    'type': 'uuid',
+                    'uuid': '2ec115d7-3a88-3ceb-bc12-0ac909a6fd87'
+                }
+            }
+
+        .. code-block:: python
+
+            source_auth={
+                'type': 'chap',
+                'username': 'myname',
+                'secret': {
+                    'type': 'usage',
+                    'uuid': 'mycluster_myname'
+                }
+            }
+
+        Since neon, instead the source authentication can only contain ``username``
+        and ``password`` properties. In this case the libvirt secret will be defined and used.
+        For Ceph authentications a base64 encoded key is expected.
+
+    :param source_name:
+        Identifier of name-based sources.
+    :param source_format:
+        String representing the source format. The possible values are depending on the
+        source type. See `libvirt documentation <https://libvirt.org/storage.html>`_ for
+        the possible values.
+    :param test: run in dry-run mode if set to True
+    :param connection: libvirt connection URI, overriding defaults
+    :param username: username to connect with, overriding defaults
+    :param password: password to connect with, overriding defaults
+
+    .. rubric:: Example:
+
+    Local folder pool:
+
+    .. code-block:: bash
+
+        salt '*' virt.pool_update somepool dir target=/srv/mypool \
+                                  permissions="{'mode': '0744' 'ower': 107, 'group': 107 }"
+
+    CIFS backed pool:
+
+    .. code-block:: bash
+
+        salt '*' virt.pool_update myshare netfs source_format=cifs \
+                                  source_dir=samba_share source_hosts="['example.com']" target=/mnt/cifs
+
+    .. versionadded:: neon
+    '''
+    # Get the current definition to compare the two
+    conn = __get_conn(**kwargs)
+    needs_update = False
+    try:
+        pool = conn.storagePoolLookupByName(name)
+        old_xml = ElementTree.fromstring(pool.XMLDesc())
+
+        # If we have username and password in source_auth generate a new secret
+        # Or change the value of the existing one
+        secret_node = old_xml.find('source/auth/secret')
+        usage = secret_node.get('usage') if secret_node is not None else None
+        uuid = secret_node.get('uuid') if secret_node is not None else None
+        auth = _pool_set_secret(conn, ptype, name, source_auth, uuid=uuid, usage=usage, test=test)
+
+        # Compute new definition
+        new_xml = ElementTree.fromstring(_gen_pool_xml(
+            name,
+            ptype,
+            target,
+            permissions=permissions,
+            source_devices=source_devices,
+            source_dir=source_dir,
+            source_initiator=source_initiator,
+            source_adapter=source_adapter,
+            source_hosts=source_hosts,
+            source_auth=auth,
+            source_name=source_name,
+            source_format=source_format
+        ))
+
+        # Copy over the uuid, capacity, allocation, available elements
+        elements_to_copy = ['available', 'allocation', 'capacity', 'uuid']
+        for to_copy in elements_to_copy:
+            element = old_xml.find(to_copy)
+            new_xml.insert(1, element)
+
+        # Filter out spaces and empty elements like <source/> since those would mislead the comparison
+        def visit_xml(node, fn):
+            fn(node)
+            for child in node:
+                visit_xml(child, fn)
+
+        def space_stripper(node):
+            if node.tail is not None:
+                node.tail = node.tail.strip(' \t\n')
+            if node.text is not None:
+                node.text = node.text.strip(' \t\n')
+
+        visit_xml(old_xml, space_stripper)
+        visit_xml(new_xml, space_stripper)
+
+        def empty_node_remover(node):
+            for child in node:
+                if not child.tail and not child.text and not child.items() and not child:
+                    node.remove(child)
+        visit_xml(old_xml, empty_node_remover)
+
+        needs_update = ElementTree.tostring(old_xml) != ElementTree.tostring(new_xml)
+        if needs_update and not test:
+            conn.storagePoolDefineXML(salt.utils.stringutils.to_str(ElementTree.tostring(new_xml)))
+    finally:
+        conn.close()
+    return needs_update
+
+
 def list_pools(**kwargs):
     '''
     List all storage pools.
diff --git a/salt/states/virt.py b/salt/states/virt.py
index c700cae849..d1c9191a29 100644
--- a/salt/states/virt.py
+++ b/salt/states/virt.py
@@ -658,6 +658,8 @@ def network_running(name,
                     forward,
                     vport=None,
                     tag=None,
+                    ipv4_config=None,
+                    ipv6_config=None,
                     autostart=True,
                     connection=None,
                     username=None,
@@ -665,6 +667,25 @@ def network_running(name,
     '''
     Defines and starts a new network with specified arguments.
 
+    :param bridge: Bridge name
+    :param forward: Forward mode(bridge, router, nat)
+    :param vport: Virtualport type (Default: ``'None'``)
+    :param tag: Vlan tag (Default: ``'None'``)
+    :param ipv4_config:
+        IPv4 network configuration. See the :py:func`virt.network_define
+        <salt.modules.virt.network_define>` function corresponding parameter documentation
+        for more details on this dictionary.
+        (Default: None).
+
+        .. versionadded:: neon
+    :param ipv6_config:
+        IPv6 network configuration. See the :py:func`virt.network_define
+        <salt.modules.virt.network_define>` function corresponding parameter documentation
+        for more details on this dictionary.
+        (Default: None).
+
+        .. versionadded:: neon
+    :param autostart: Network autostart (default ``'True'``)
     :param connection: libvirt connection URI, overriding defaults
 
         .. versionadded:: 2019.2.0
@@ -690,6 +711,21 @@ def network_running(name,
             - tag: 180
             - autostart: True
 
+    .. code-block:: yaml
+
+        network_name:
+          virt.network_define:
+            - bridge: natted
+            - forward: nat
+            - ipv4_config:
+                cidr: 192.168.42.0/24
+                dhcp_ranges:
+                  - start: 192.168.42.10
+                    end: 192.168.42.25
+                  - start: 192.168.42.100
+                    end: 192.168.42.150
+            - autostart: True
+
     '''
     ret = {'name': name,
            'changes': {},
@@ -712,6 +748,8 @@ def network_running(name,
                                             forward,
                                             vport=vport,
                                             tag=tag,
+                                            ipv4_config=ipv4_config,
+                                            ipv6_config=ipv6_config,
                                             autostart=autostart,
                                             start=True,
                                             connection=connection,
@@ -784,57 +822,230 @@ def pool_running(name,
     '''
     ret = {'name': name,
            'changes': {},
-           'result': True,
+           'result': True if not __opts__['test'] else None,
            'comment': ''
            }
 
     try:
         info = __salt__['virt.pool_info'](name, connection=connection, username=username, password=password)
+        needs_autostart = False
         if info:
-            if info[name]['state'] == 'running':
-                ret['comment'] = 'Pool {0} exists and is running'.format(name)
+            needs_autostart = info[name]['autostart'] and not autostart or not info[name]['autostart'] and autostart
+
+            # Update can happen for both running and stopped pools
+            needs_update = __salt__['virt.pool_update'](name,
+                                                        ptype=ptype,
+                                                        target=target,
+                                                        permissions=permissions,
+                                                        source_devices=(source or {}).get('devices'),
+                                                        source_dir=(source or {}).get('dir'),
+                                                        source_initiator=(source or {}).get('initiator'),
+                                                        source_adapter=(source or {}).get('adapter'),
+                                                        source_hosts=(source or {}).get('hosts'),
+                                                        source_auth=(source or {}).get('auth'),
+                                                        source_name=(source or {}).get('name'),
+                                                        source_format=(source or {}).get('format'),
+                                                        test=True,
+                                                        connection=connection,
+                                                        username=username,
+                                                        password=password)
+            if needs_update:
+                if not __opts__['test']:
+                    __salt__['virt.pool_update'](name,
+                                                 ptype=ptype,
+                                                 target=target,
+                                                 permissions=permissions,
+                                                 source_devices=(source or {}).get('devices'),
+                                                 source_dir=(source or {}).get('dir'),
+                                                 source_initiator=(source or {}).get('initiator'),
+                                                 source_adapter=(source or {}).get('adapter'),
+                                                 source_hosts=(source or {}).get('hosts'),
+                                                 source_auth=(source or {}).get('auth'),
+                                                 source_name=(source or {}).get('name'),
+                                                 source_format=(source or {}).get('format'),
+                                                 connection=connection,
+                                                 username=username,
+                                                 password=password)
+
+                action = "started"
+                if info[name]['state'] == 'running':
+                    action = "restarted"
+                    if not __opts__['test']:
+                        __salt__['virt.pool_stop'](name, connection=connection, username=username, password=password)
+
+                if not __opts__['test']:
+                    __salt__['virt.pool_build'](name, connection=connection, username=username, password=password)
+                    __salt__['virt.pool_start'](name, connection=connection, username=username, password=password)
+
+                autostart_str = ', autostart flag changed' if needs_autostart else ''
+                ret['changes'][name] = 'Pool updated, built{0} and {1}'.format(autostart_str, action)
+                ret['comment'] = 'Pool {0} updated, built{1} and {2}'.format(name, autostart_str, action)
+
             else:
-                __salt__['virt.pool_start'](name, connection=connection, username=username, password=password)
-                ret['changes'][name] = 'Pool started'
-                ret['comment'] = 'Pool {0} started'.format(name)
+                if info[name]['state'] == 'running':
+                    ret['comment'] = 'Pool {0} unchanged and is running'.format(name)
+                    ret['result'] = True
+                else:
+                    ret['changes'][name] = 'Pool started'
+                    ret['comment'] = 'Pool {0} started'.format(name)
+                    if not __opts__['test']:
+                        __salt__['virt.pool_start'](name, connection=connection, username=username, password=password)
         else:
-            __salt__['virt.pool_define'](name,
-                                         ptype=ptype,
-                                         target=target,
-                                         permissions=permissions,
-                                         source_devices=(source or {}).get('devices', None),
-                                         source_dir=(source or {}).get('dir', None),
-                                         source_adapter=(source or {}).get('adapter', None),
-                                         source_hosts=(source or {}).get('hosts', None),
-                                         source_auth=(source or {}).get('auth', None),
-                                         source_name=(source or {}).get('name', None),
-                                         source_format=(source or {}).get('format', None),
-                                         transient=transient,
-                                         start=False,
-                                         connection=connection,
-                                         username=username,
-                                         password=password)
-            if autostart:
+            needs_autostart = autostart
+            if not __opts__['test']:
+                __salt__['virt.pool_define'](name,
+                                             ptype=ptype,
+                                             target=target,
+                                             permissions=permissions,
+                                             source_devices=(source or {}).get('devices'),
+                                             source_dir=(source or {}).get('dir'),
+                                             source_initiator=(source or {}).get('initiator'),
+                                             source_adapter=(source or {}).get('adapter'),
+                                             source_hosts=(source or {}).get('hosts'),
+                                             source_auth=(source or {}).get('auth'),
+                                             source_name=(source or {}).get('name'),
+                                             source_format=(source or {}).get('format'),
+                                             transient=transient,
+                                             start=False,
+                                             connection=connection,
+                                             username=username,
+                                             password=password)
+
+                __salt__['virt.pool_build'](name,
+                                            connection=connection,
+                                            username=username,
+                                            password=password)
+
+                __salt__['virt.pool_start'](name,
+                                            connection=connection,
+                                            username=username,
+                                            password=password)
+            if needs_autostart:
+                ret['changes'][name] = 'Pool defined, started and marked for autostart'
+                ret['comment'] = 'Pool {0} defined, started and marked for autostart'.format(name)
+            else:
+                ret['changes'][name] = 'Pool defined and started'
+                ret['comment'] = 'Pool {0} defined and started'.format(name)
+
+        if needs_autostart:
+            if not __opts__['test']:
                 __salt__['virt.pool_set_autostart'](name,
                                                     state='on' if autostart else 'off',
                                                     connection=connection,
                                                     username=username,
                                                     password=password)
+    except libvirt.libvirtError as err:
+        ret['comment'] = err.get_error_message()
+        ret['result'] = False
+
+    return ret
+
 
-            __salt__['virt.pool_build'](name,
-                                        connection=connection,
-                                        username=username,
-                                        password=password)
+def pool_deleted(name,
+                 purge=False,
+                 connection=None,
+                 username=None,
+                 password=None):
+    '''
+    Deletes a virtual storage pool.
+
+    :param name: the name of the pool to delete.
+    :param purge:
+        if ``True``, the volumes contained in the pool will be deleted as well as the pool itself.
+        Note that these will be lost for ever. If ``False`` the pool will simply be undefined.
+        (Default: ``False``)
+    :param connection: libvirt connection URI, overriding defaults
+    :param username: username to connect with, overriding defaults
+    :param password: password to connect with, overriding defaults
 
-            __salt__['virt.pool_start'](name,
-                                        connection=connection,
-                                        username=username,
-                                        password=password)
+    In order to be purged a storage pool needs to be running to get the list of volumes to delete.
+
+    Some libvirt storage drivers may not implement deleting, those actions are implemented on a
+    best effort idea. In any case check the result's comment property to see if any of the action
+    was unsupported.
+
+    .. code-block::yaml
+
+        pool_name:
+          uyuni_virt.pool_deleted:
+            - purge: True
+
+    .. versionadded:: Neon
+    '''
+    ret = {'name': name, 'changes': {}, 'result': True, 'comment': ''}
 
-            ret['changes'][name] = 'Pool defined and started'
-            ret['comment'] = 'Pool {0} defined and started'.format(name)
+    try:
+        info = __salt__['virt.pool_info'](name, connection=connection, username=username, password=password)
+        if info:
+            ret['changes']['stopped'] = False
+            ret['changes']['deleted'] = False
+            ret['changes']['undefined'] = False
+            ret['changes']['deleted_volumes'] = []
+            unsupported = []
+
+            if info[name]['state'] == 'running':
+                if purge:
+                    unsupported_volume_delete = ['iscsi', 'iscsi-direct', 'mpath', 'scsi']
+                    if info[name]['type'] not in unsupported_volume_delete:
+                        __salt__['virt.pool_refresh'](name,
+                                                      connection=connection,
+                                                      username=username,
+                                                      password=password)
+                        volumes = __salt__['virt.pool_list_volumes'](name,
+                                                                     connection=connection,
+                                                                     username=username,
+                                                                     password=password)
+                        for volume in volumes:
+                            # Not supported for iSCSI and SCSI drivers
+                            deleted = __opts__['test']
+                            if not __opts__['test']:
+                                deleted = __salt__['virt.volume_delete'](name,
+                                                                         volume,
+                                                                         connection=connection,
+                                                                         username=username,
+                                                                         password=password)
+                            if deleted:
+                                ret['changes']['deleted_volumes'].append(volume)
+                    else:
+                        unsupported.append('deleting volume')
+
+                if not __opts__['test']:
+                    ret['changes']['stopped'] = __salt__['virt.pool_stop'](name,
+                                                                           connection=connection,
+                                                                           username=username,
+                                                                           password=password)
+                else:
+                    ret['changes']['stopped'] = True
+
+                if purge:
+                    supported_pool_delete = ['dir', 'fs', 'netfs', 'logical', 'vstorage', 'zfs']
+                    if info[name]['type'] in supported_pool_delete:
+                        if not __opts__['test']:
+                            ret['changes']['deleted'] = __salt__['virt.pool_delete'](name,
+                                                                                     connection=connection,
+                                                                                     username=username,
+                                                                                     password=password)
+                        else:
+                            ret['changes']['deleted'] = True
+                    else:
+                        unsupported.append('deleting pool')
+
+            if not __opts__['test']:
+                ret['changes']['undefined'] = __salt__['virt.pool_undefine'](name,
+                                                                             connection=connection,
+                                                                             username=username,
+                                                                             password=password)
+            else:
+                ret['changes']['undefined'] = True
+                ret['result'] = None
+
+            if unsupported:
+                ret['comment'] = 'Unsupported actions for pool of type "{0}": {1}'.format(info[name]['type'],
+                                                                                          ', '.join(unsupported))
+        else:
+            ret['comment'] = 'Storage pool could not be found: {0}'.format(name)
     except libvirt.libvirtError as err:
-        ret['comment'] = err.get_error_message()
+        ret['comment'] = 'Failed deleting pool: {0}'.format(err.get_error_message())
         ret['result'] = False
 
     return ret
diff --git a/salt/templates/virt/libvirt_network.jinja b/salt/templates/virt/libvirt_network.jinja
index d0db99cad8..2f11e64559 100644
--- a/salt/templates/virt/libvirt_network.jinja
+++ b/salt/templates/virt/libvirt_network.jinja
@@ -6,4 +6,15 @@
   <vlan>
     <tag id='{{ tag }}'/>
   </vlan>{% endif %}
-</network>
\ No newline at end of file
+  {% for ip_config in ip_configs %}
+  <ip family='ipv{{ ip_config.address.version }}'
+      address='{{ ip_config.address.network_address }}'
+      prefix='{{ ip_config.address.prefixlen }}'>
+    <dhcp>
+      {% for range in ip_config.dhcp_ranges %}
+      <range start='{{ range.start }}' end='{{ range.end }}' />
+      {% endfor %}
+    </dhcp>
+  </ip>
+  {% endfor %}
+</network>
diff --git a/salt/templates/virt/libvirt_pool.jinja b/salt/templates/virt/libvirt_pool.jinja
index 58c82f7177..515702cf46 100644
--- a/salt/templates/virt/libvirt_pool.jinja
+++ b/salt/templates/virt/libvirt_pool.jinja
@@ -18,7 +18,7 @@
   {% endif %}
   {% if source %}
   <source>
-    {% if ptype in ['fs', 'logical', 'disk', 'iscsi', 'zfs', 'vstorage'] %}
+    {% if ptype in ['fs', 'logical', 'disk', 'iscsi', 'zfs', 'vstorage', 'iscsi-direct'] %}
     {% for device in source.devices %}
     <device path='{{ device.path }}'
             {% if 'part_separator' in device %}part_separator='{{ device.part_separator }}'{% endif %}/>
@@ -43,14 +43,14 @@
          {% endif %}
     </adapter>
     {% endif %}
-    {% if ptype in ['netfs', 'iscsi', 'rbd', 'sheepdog', 'gluster'] and source.hosts %}
+    {% if ptype in ['netfs', 'iscsi', 'rbd', 'sheepdog', 'gluster', 'iscsi-direct'] and source.hosts %}
     {% for host in source.hosts %}
     <host name='{{ host.name }}' {% if host.port %}port='{{ host.port }}'{% endif %}/>
     {% endfor %}
     {% endif %}
-    {% if ptype in ['iscsi', 'rbd'] and source.auth %}
+    {% if ptype in ['iscsi', 'rbd', 'iscsi-direct'] and source.auth %}
     <auth type='{{source.auth.type}}' username='{{ source.auth.username }}'>
-      <secret type='{{ source.auth.secret.type }}' {{ source.auth.secret.type }}='{{source.auth.secret.value}}'/>
+      <secret {{ source.auth.secret.type }}='{{source.auth.secret.value}}'/>
     </auth>
     {% endif %}
     {% if ptype in ['logical', 'rbd', 'sheepdog', 'gluster'] and source.name %}
@@ -59,6 +59,11 @@
     {% if ptype in ['fs', 'netfs', 'logical', 'disk'] and source.format %}
     <format type='{{ source.format }}'/>
     {% endif %}
+    {% if ptype == 'iscsi-direct' and source.initiator %}
+    <initiator>
+      <iqn name='{{ source.initiator }}'/>
+    </initiator>
+    {% endif %}
   </source>
   {% endif %}
 </pool>
diff --git a/salt/templates/virt/libvirt_secret.jinja b/salt/templates/virt/libvirt_secret.jinja
new file mode 100644
index 0000000000..41c6dd811a
--- /dev/null
+++ b/salt/templates/virt/libvirt_secret.jinja
@@ -0,0 +1,12 @@
+<secret ephemeral='no' private='yes'>
+    <description>{{ description }}</description>
+    {% if type == 'chap' %}
+    <usage type='iscsi'>
+        <target>{{ usage }}</target>
+    </usage>
+    {% elif type == 'ceph' %}
+    <usage type='ceph'>
+        <name>{{ usage }}</name>
+    </usage>
+    {% endif %}
+</secret>
diff --git a/tests/unit/modules/test_virt.py b/tests/unit/modules/test_virt.py
index 4bdb933a2d..698e1922fc 100644
--- a/tests/unit/modules/test_virt.py
+++ b/tests/unit/modules/test_virt.py
@@ -2016,6 +2016,32 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         self.assertEqual(root.find('forward').attrib['mode'], 'bridge')
         self.assertEqual(root.find('virtualport').attrib['type'], 'openvswitch')
 
+    def test_network_nat(self):
+        '''
+        Test virt._get_net_xml() in a nat setup
+        '''
+        xml_data = virt._gen_net_xml('network', 'main', 'nat', None, ip_configs=[
+            {
+                'cidr': '192.168.2.0/24',
+                'dhcp_ranges': [
+                    {'start': '192.168.2.10', 'end': '192.168.2.25'},
+                    {'start': '192.168.2.110', 'end': '192.168.2.125'},
+                ]
+            }
+        ])
+        root = ET.fromstring(xml_data)
+        self.assertEqual(root.find('name').text, 'network')
+        self.assertEqual(root.find('bridge').attrib['name'], 'main')
+        self.assertEqual(root.find('forward').attrib['mode'], 'nat')
+        self.assertEqual(root.find("./ip[@address='192.168.2.0']").attrib['prefix'], '24')
+        self.assertEqual(root.find("./ip[@address='192.168.2.0']").attrib['family'], 'ipv4')
+        self.assertEqual(
+                root.find("./ip[@address='192.168.2.0']/dhcp/range[@start='192.168.2.10']").attrib['end'],
+                '192.168.2.25')
+        self.assertEqual(
+                root.find("./ip[@address='192.168.2.0']/dhcp/range[@start='192.168.2.110']").attrib['end'],
+                '192.168.2.125')
+
     def test_domain_capabilities(self):
         '''
         Test the virt.domain_capabilities parsing
@@ -2468,7 +2494,6 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         self.assertEqual(root.findall('source/host')[1].attrib['port'], '69')
         self.assertEqual(root.find('source/auth').attrib['type'], 'ceph')
         self.assertEqual(root.find('source/auth').attrib['username'], 'admin')
-        self.assertEqual(root.find('source/auth/secret').attrib['type'], 'uuid')
         self.assertEqual(root.find('source/auth/secret').attrib['uuid'], 'someuuid')
 
     def test_pool_with_netfs(self):
@@ -2506,6 +2531,114 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         self.assertEqual(root.find('source/host').attrib['name'], 'nfs.host')
         self.assertEqual(root.find('source/auth'), None)
 
+    def test_pool_with_iscsi_direct(self):
+        '''
+        Test virt._gen_pool_xml() with a iscsi-direct source
+        '''
+        xml_data = virt._gen_pool_xml('pool',
+                                      'iscsi-direct',
+                                      source_hosts=['iscsi.example.com'],
+                                      source_devices=[{'path': 'iqn.2013-06.com.example:iscsi-pool'}],
+                                      source_initiator='iqn.2013-06.com.example:iscsi-initiator')
+        root = ET.fromstring(xml_data)
+        self.assertEqual(root.find('name').text, 'pool')
+        self.assertEqual(root.attrib['type'], 'iscsi-direct')
+        self.assertEqual(root.find('target'), None)
+        self.assertEqual(root.find('source/device').attrib['path'], 'iqn.2013-06.com.example:iscsi-pool')
+        self.assertEqual(root.findall('source/host')[0].attrib['name'], 'iscsi.example.com')
+        self.assertEqual(root.find('source/initiator/iqn').attrib['name'], 'iqn.2013-06.com.example:iscsi-initiator')
+
+    def test_pool_define(self):
+        '''
+        Test virt.pool_define()
+        '''
+        mock_pool = MagicMock()
+        mock_secret = MagicMock()
+        mock_secret_define = MagicMock(return_value=mock_secret)
+        self.mock_conn.secretDefineXML = mock_secret_define
+        self.mock_conn.storagePoolCreateXML = MagicMock(return_value=mock_pool)
+        self.mock_conn.storagePoolDefineXML = MagicMock(return_value=mock_pool)
+
+        mocks = [mock_pool, mock_secret, mock_secret_define, self.mock_conn.storagePoolCreateXML,
+                self.mock_conn.secretDefineXML, self.mock_conn.storagePoolDefineXML]
+
+        # Test case with already defined secret and permanent pool
+        self.assertTrue(virt.pool_define('default',
+                                         'rbd',
+                                         source_hosts=['one.example.com', 'two.example.com'],
+                                         source_name='rbdvol',
+                                         source_auth={
+                                            'type': 'ceph',
+                                            'username': 'admin',
+                                            'secret': {
+                                                'type': 'uuid',
+                                                'value': 'someuuid'
+                                            }
+                                         }))
+        self.mock_conn.storagePoolDefineXML.assert_called_once()
+        self.mock_conn.storagePoolCreateXML.assert_not_called()
+        mock_pool.create.assert_called_once()
+        mock_secret_define.assert_not_called()
+
+        # Test case with Ceph secret to be defined and transient pool
+        for mock in mocks:
+            mock.reset_mock()
+        self.assertTrue(virt.pool_define('default',
+                                         'rbd',
+                                         transient=True,
+                                         source_hosts=['one.example.com', 'two.example.com'],
+                                         source_name='rbdvol',
+                                         source_auth={
+                                            'username': 'admin',
+                                            'password': 'c2VjcmV0'
+                                         }))
+        self.mock_conn.storagePoolDefineXML.assert_not_called()
+
+        pool_xml = self.mock_conn.storagePoolCreateXML.call_args[0][0]
+        root = ET.fromstring(pool_xml)
+        self.assertEqual(root.find('source/auth').attrib['type'], 'ceph')
+        self.assertEqual(root.find('source/auth').attrib['username'], 'admin')
+        self.assertEqual(root.find('source/auth/secret').attrib['usage'], 'pool_default')
+        mock_pool.create.assert_not_called()
+        mock_secret.setValue.assert_called_once_with(b'secret')
+
+        secret_xml = mock_secret_define.call_args[0][0]
+        root = ET.fromstring(secret_xml)
+        self.assertEqual(root.find('usage/name').text, 'pool_default')
+        self.assertEqual(root.find('usage').attrib['type'], 'ceph')
+        self.assertEqual(root.attrib['private'], 'yes')
+        self.assertEqual(root.find('description').text, 'Passphrase for default pool created by Salt')
+
+        # Test case with iscsi secret not starting
+        for mock in mocks:
+            mock.reset_mock()
+        self.assertTrue(virt.pool_define('default',
+                                         'iscsi',
+                                         target='/dev/disk/by-path',
+                                         source_hosts=['iscsi.example.com'],
+                                         source_devices=[{'path': 'iqn.2013-06.com.example:iscsi-pool'}],
+                                         source_auth={
+                                            'username': 'admin',
+                                            'password': 'secret'
+                                         },
+                                         start=False))
+        self.mock_conn.storagePoolCreateXML.assert_not_called()
+
+        pool_xml = self.mock_conn.storagePoolDefineXML.call_args[0][0]
+        root = ET.fromstring(pool_xml)
+        self.assertEqual(root.find('source/auth').attrib['type'], 'chap')
+        self.assertEqual(root.find('source/auth').attrib['username'], 'admin')
+        self.assertEqual(root.find('source/auth/secret').attrib['usage'], 'pool_default')
+        mock_pool.create.assert_not_called()
+        mock_secret.setValue.assert_called_once_with('secret')
+
+        secret_xml = mock_secret_define.call_args[0][0]
+        root = ET.fromstring(secret_xml)
+        self.assertEqual(root.find('usage/target').text, 'pool_default')
+        self.assertEqual(root.find('usage').attrib['type'], 'iscsi')
+        self.assertEqual(root.attrib['private'], 'yes')
+        self.assertEqual(root.find('description').text, 'Passphrase for default pool created by Salt')
+
     def test_list_pools(self):
         '''
         Test virt.list_pools()
@@ -3135,3 +3268,323 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
 
         isxen_mock.return_value = True
         self.assertEqual('xen', virt.get_hypervisor())
+
+    def test_pool_update(self):
+        '''
+        Test the pool_update function
+        '''
+        current_xml = '''<pool type='dir'>
+          <name>default</name>
+          <uuid>20fbe05c-ab40-418a-9afa-136d512f0ede</uuid>
+          <capacity unit='bytes'>1999421108224</capacity>
+          <allocation unit='bytes'>713207042048</allocation>
+          <available unit='bytes'>1286214066176</available>
+          <source>
+          </source>
+          <target>
+            <path>/path/to/pool</path>
+            <permissions>
+              <mode>0775</mode>
+              <owner>0</owner>
+              <group>100</group>
+            </permissions>
+          </target>
+        </pool>'''
+
+        expected_xml = '<pool type="netfs">' \
+                       '<name>default</name>' \
+                         '<uuid>20fbe05c-ab40-418a-9afa-136d512f0ede</uuid>' \
+                         '<capacity unit="bytes">1999421108224</capacity>' \
+                         '<allocation unit="bytes">713207042048</allocation>' \
+                         '<available unit="bytes">1286214066176</available>' \
+                         '<target>' \
+                           '<path>/mnt/cifs</path>' \
+                           '<permissions>' \
+                             '<mode>0774</mode>' \
+                             '<owner>1234</owner>' \
+                             '<group>123</group>' \
+                           '</permissions>' \
+                         '</target>' \
+                         '<source>' \
+                           '<dir path="samba_share" />' \
+                           '<host name="one.example.com" />' \
+                           '<host name="two.example.com" />' \
+                           '<format type="cifs" />' \
+                         '</source>' \
+                       '</pool>'
+
+        mocked_pool = MagicMock()
+        mocked_pool.XMLDesc = MagicMock(return_value=current_xml)
+        self.mock_conn.storagePoolLookupByName = MagicMock(return_value=mocked_pool)
+        self.mock_conn.storagePoolDefineXML = MagicMock()
+
+        self.assertTrue(
+            virt.pool_update('default',
+                             'netfs',
+                             target='/mnt/cifs',
+                             permissions={'mode': '0774', 'owner': '1234', 'group': '123'},
+                             source_format='cifs',
+                             source_dir='samba_share',
+                             source_hosts=['one.example.com', 'two.example.com']))
+        self.mock_conn.storagePoolDefineXML.assert_called_once_with(expected_xml)
+
+    def test_pool_update_nochange(self):
+        '''
+        Test the pool_update function when no change is needed
+        '''
+
+        current_xml = '''<pool type='dir'>
+          <name>default</name>
+          <uuid>20fbe05c-ab40-418a-9afa-136d512f0ede</uuid>
+          <capacity unit='bytes'>1999421108224</capacity>
+          <allocation unit='bytes'>713207042048</allocation>
+          <available unit='bytes'>1286214066176</available>
+          <source>
+          </source>
+          <target>
+            <path>/path/to/pool</path>
+            <permissions>
+              <mode>0775</mode>
+              <owner>0</owner>
+              <group>100</group>
+            </permissions>
+          </target>
+        </pool>'''
+
+        mocked_pool = MagicMock()
+        mocked_pool.XMLDesc = MagicMock(return_value=current_xml)
+        self.mock_conn.storagePoolLookupByName = MagicMock(return_value=mocked_pool)
+        self.mock_conn.storagePoolDefineXML = MagicMock()
+
+        self.assertFalse(
+            virt.pool_update('default',
+                             'dir',
+                             target='/path/to/pool',
+                             permissions={'mode': '0775', 'owner': '0', 'group': '100'},
+                             test=True))
+        self.mock_conn.storagePoolDefineXML.assert_not_called()
+
+    def test_pool_update_password(self):
+        '''
+        Test the pool_update function, where the password only is changed
+        '''
+        current_xml = '''<pool type='rbd'>
+          <name>default</name>
+          <uuid>20fbe05c-ab40-418a-9afa-136d512f0ede</uuid>
+          <capacity unit='bytes'>1999421108224</capacity>
+          <allocation unit='bytes'>713207042048</allocation>
+          <available unit='bytes'>1286214066176</available>
+          <source>
+            <name>iscsi-images</name>
+            <host name='ses4.tf.local'/>
+            <host name='ses5.tf.local'/>
+            <auth username='libvirt' type='ceph'>
+              <secret uuid='14e9a0f1-8fbf-4097-b816-5b094c182212'/>
+            </auth>
+          </source>
+        </pool>'''
+
+        expected_xml = '<pool type="rbd">' \
+                       '<name>default</name>' \
+                         '<uuid>20fbe05c-ab40-418a-9afa-136d512f0ede</uuid>' \
+                         '<capacity unit="bytes">1999421108224</capacity>' \
+                         '<allocation unit="bytes">713207042048</allocation>' \
+                         '<available unit="bytes">1286214066176</available>' \
+                         '<source>' \
+                           '<host name="ses4.tf.local" />' \
+                           '<host name="ses5.tf.local" />' \
+                           '<auth type="ceph" username="libvirt">' \
+                             '<secret uuid="14e9a0f1-8fbf-4097-b816-5b094c182212" />' \
+                           '</auth>' \
+                           '<name>iscsi-images</name>' \
+                         '</source>' \
+                       '</pool>'
+
+        mock_secret = MagicMock()
+        self.mock_conn.secretLookupByUUIDString = MagicMock(return_value=mock_secret)
+
+        mocked_pool = MagicMock()
+        mocked_pool.XMLDesc = MagicMock(return_value=current_xml)
+        self.mock_conn.storagePoolLookupByName = MagicMock(return_value=mocked_pool)
+        self.mock_conn.storagePoolDefineXML = MagicMock()
+
+        self.assertTrue(
+            virt.pool_update('default',
+                             'rbd',
+                             source_name='iscsi-images',
+                             source_hosts=['ses4.tf.local', 'ses5.tf.local'],
+                             source_auth={'username': 'libvirt',
+                                          'password': 'c2VjcmV0'}))
+        self.mock_conn.storagePoolDefineXML.assert_called_once_with(expected_xml)
+        mock_secret.setValue.assert_called_once_with(b'secret')
+
+    def test_pool_update_password_create(self):
+        '''
+        Test the pool_update function, where the password only is changed
+        '''
+        current_xml = '''<pool type='rbd'>
+          <name>default</name>
+          <uuid>20fbe05c-ab40-418a-9afa-136d512f0ede</uuid>
+          <capacity unit='bytes'>1999421108224</capacity>
+          <allocation unit='bytes'>713207042048</allocation>
+          <available unit='bytes'>1286214066176</available>
+          <source>
+            <name>iscsi-images</name>
+            <host name='ses4.tf.local'/>
+            <host name='ses5.tf.local'/>
+          </source>
+        </pool>'''
+
+        expected_xml = '<pool type="rbd">' \
+                       '<name>default</name>' \
+                         '<uuid>20fbe05c-ab40-418a-9afa-136d512f0ede</uuid>' \
+                         '<capacity unit="bytes">1999421108224</capacity>' \
+                         '<allocation unit="bytes">713207042048</allocation>' \
+                         '<available unit="bytes">1286214066176</available>' \
+                         '<source>' \
+                           '<host name="ses4.tf.local" />' \
+                           '<host name="ses5.tf.local" />' \
+                           '<auth type="ceph" username="libvirt">' \
+                             '<secret usage="pool_default" />' \
+                           '</auth>' \
+                           '<name>iscsi-images</name>' \
+                         '</source>' \
+                       '</pool>'
+
+        mock_secret = MagicMock()
+        self.mock_conn.secretDefineXML = MagicMock(return_value=mock_secret)
+
+        mocked_pool = MagicMock()
+        mocked_pool.XMLDesc = MagicMock(return_value=current_xml)
+        self.mock_conn.storagePoolLookupByName = MagicMock(return_value=mocked_pool)
+        self.mock_conn.storagePoolDefineXML = MagicMock()
+
+        self.assertTrue(
+            virt.pool_update('default',
+                             'rbd',
+                             source_name='iscsi-images',
+                             source_hosts=['ses4.tf.local', 'ses5.tf.local'],
+                             source_auth={'username': 'libvirt',
+                                          'password': 'c2VjcmV0'}))
+        self.mock_conn.storagePoolDefineXML.assert_called_once_with(expected_xml)
+        mock_secret.setValue.assert_called_once_with(b'secret')
+    
+    def test_pool_capabilities(self):
+        '''
+        Test virt.pool_capabilities where libvirt has the pool-capabilities feature
+        '''
+        xml_caps = '''
+<storagepoolCapabilities>
+  <pool type='disk' supported='yes'>
+    <poolOptions>
+      <defaultFormat type='unknown'/>
+      <enum name='sourceFormatType'>
+        <value>unknown</value>
+        <value>dos</value>
+        <value>dvh</value>
+      </enum>
+    </poolOptions>
+    <volOptions>
+      <defaultFormat type='none'/>
+      <enum name='targetFormatType'>
+        <value>none</value>
+        <value>linux</value>
+      </enum>
+    </volOptions>
+  </pool>
+  <pool type='iscsi' supported='yes'>
+  </pool>
+  <pool type='rbd' supported='yes'>
+    <volOptions>
+      <defaultFormat type='raw'/>
+      <enum name='targetFormatType'>
+      </enum>
+    </volOptions>
+  </pool>
+  <pool type='sheepdog' supported='no'>
+  </pool>
+</storagepoolCapabilities>
+        '''
+        self.mock_conn.getStoragePoolCapabilities = MagicMock(return_value=xml_caps)
+
+        actual = virt.pool_capabilities()
+        self.assertEqual({
+            'computed': False,
+            'pool_types': [{
+                'name': 'disk',
+                'supported': True,
+                'options': {
+                    'pool': {
+                        'default_format': 'unknown',
+                        'sourceFormatType': ['unknown', 'dos', 'dvh']
+                    },
+                    'volume': {
+                        'default_format': 'none',
+                        'targetFormatType': ['none', 'linux']
+                    }
+                }
+            },
+            {
+                'name': 'iscsi',
+                'supported': True,
+            },
+            {
+                'name': 'rbd',
+                'supported': True,
+                'options': {
+                    'volume': {
+                        'default_format': 'raw',
+                        'targetFormatType': []
+                    }
+                }
+            },
+            {
+                'name': 'sheepdog',
+                'supported': False,
+            },
+        ]}, actual)
+
+    @patch('salt.modules.virt.get_hypervisor', return_value='kvm')
+    def test_pool_capabilities_computed(self, mock_get_hypervisor):
+        '''
+        Test virt.pool_capabilities where libvirt doesn't have the pool-capabilities feature
+        '''
+        self.mock_conn.getLibVersion = MagicMock(return_value=4006000)
+        del self.mock_conn.getStoragePoolCapabilities
+
+        actual = virt.pool_capabilities()
+
+        self.assertTrue(actual['computed'])
+        backends = actual['pool_types']
+
+        # libvirt version matching check
+        self.assertFalse([backend for backend in backends if backend['name'] == 'iscsi-direct'][0]['supported'])
+        self.assertTrue([backend for backend in backends if backend['name'] == 'gluster'][0]['supported'])
+        self.assertFalse([backend for backend in backends if backend['name'] == 'zfs'][0]['supported'])
+
+        # test case matching other hypervisors
+        mock_get_hypervisor.return_value = 'xen'
+        backends = virt.pool_capabilities()['pool_types']
+        self.assertFalse([backend for backend in backends if backend['name'] == 'gluster'][0]['supported'])
+
+        mock_get_hypervisor.return_value = 'bhyve'
+        backends = virt.pool_capabilities()['pool_types']
+        self.assertFalse([backend for backend in backends if backend['name'] == 'gluster'][0]['supported'])
+        self.assertTrue([backend for backend in backends if backend['name'] == 'zfs'][0]['supported'])
+
+        # Test options output
+        self.assertNotIn('options', [backend for backend in backends if backend['name'] == 'iscsi'][0])
+        self.assertNotIn('pool', [backend for backend in backends if backend['name'] == 'dir'][0]['options'])
+        self.assertNotIn('volume', [backend for backend in backends if backend['name'] == 'logical'][0]['options'])
+        self.assertEqual({
+                'pool': {
+                    'default_format': 'auto',
+                    'sourceFormatType': ['auto', 'nfs', 'glusterfs', 'cifs']
+                },
+                'volume': {
+                    'default_format': 'raw',
+                    'targetFormatType': ['none', 'raw', 'dir', 'bochs', 'cloop', 'dmg', 'iso', 'vpc', 'vdi',
+                          'fat', 'vhd', 'ploop', 'cow', 'qcow', 'qcow2', 'qed', 'vmdk']
+                }
+            },
+            [backend for backend in backends if backend['name'] == 'netfs'][0]['options'])
diff --git a/tests/unit/states/test_virt.py b/tests/unit/states/test_virt.py
index 109faf5fba..334c33b7d0 100644
--- a/tests/unit/states/test_virt.py
+++ b/tests/unit/states/test_virt.py
@@ -619,6 +619,19 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                                                       'bridge',
                                                       vport='openvswitch',
                                                       tag=180,
+                                                      ipv4_config={
+                                                        'cidr': '192.168.2.0/24',
+                                                        'dhcp_ranges': [
+                                                            {'start': '192.168.2.10', 'end': '192.168.2.25'},
+                                                            {'start': '192.168.2.110', 'end': '192.168.2.125'},
+                                                        ]
+                                                      },
+                                                      ipv6_config={
+                                                        'cidr': '2001:db8:ca2:2::1/64',
+                                                        'dhcp_ranges': [
+                                                            {'start': '2001:db8:ca2:1::10', 'end': '2001:db8:ca2::1f'},
+                                                        ]
+                                                      },
                                                       autostart=False,
                                                       connection='myconnection',
                                                       username='user',
@@ -630,6 +643,19 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                                            tag=180,
                                            autostart=False,
                                            start=True,
+                                           ipv4_config={
+                                             'cidr': '192.168.2.0/24',
+                                             'dhcp_ranges': [
+                                                 {'start': '192.168.2.10', 'end': '192.168.2.25'},
+                                                 {'start': '192.168.2.110', 'end': '192.168.2.125'},
+                                             ]
+                                           },
+                                           ipv6_config={
+                                             'cidr': '2001:db8:ca2:2::1/64',
+                                             'dhcp_ranges': [
+                                                 {'start': '2001:db8:ca2:1::10', 'end': '2001:db8:ca2::1f'},
+                                             ]
+                                           },
                                            connection='myconnection',
                                            username='user',
                                            password='secret')
@@ -668,92 +694,324 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
         pool_running state test cases.
         '''
         ret = {'name': 'mypool', 'changes': {}, 'result': True, 'comment': ''}
-        mocks = {mock: MagicMock(return_value=True) for mock in ['define', 'autostart', 'build', 'start']}
-        with patch.dict(virt.__salt__, {  # pylint: disable=no-member
-                    'virt.pool_info': MagicMock(return_value={}),
-                    'virt.pool_define': mocks['define'],
-                    'virt.pool_build': mocks['build'],
-                    'virt.pool_start': mocks['start'],
-                    'virt.pool_set_autostart': mocks['autostart']
-                }):
-            ret.update({'changes': {'mypool': 'Pool defined and started'},
-                        'comment': 'Pool mypool defined and started'})
-            self.assertDictEqual(virt.pool_running('mypool',
+        mocks = {mock: MagicMock(return_value=True) for mock in ['define', 'autostart', 'build', 'start', 'stop']}
+        with patch.dict(virt.__opts__, {'test': False}):
+            with patch.dict(virt.__salt__, {  # pylint: disable=no-member
+                        'virt.pool_info': MagicMock(return_value={}),
+                        'virt.pool_define': mocks['define'],
+                        'virt.pool_build': mocks['build'],
+                        'virt.pool_start': mocks['start'],
+                        'virt.pool_set_autostart': mocks['autostart']
+                    }):
+                ret.update({'changes': {'mypool': 'Pool defined, started and marked for autostart'},
+                            'comment': 'Pool mypool defined, started and marked for autostart'})
+                self.assertDictEqual(virt.pool_running('mypool',
+                                                       ptype='logical',
+                                                       target='/dev/base',
+                                                       permissions={'mode': '0770',
+                                                                    'owner': 1000,
+                                                                    'group': 100,
+                                                                    'label': 'seclabel'},
+                                                       source={'devices': [{'path': '/dev/sda'}]},
+                                                       transient=True,
+                                                       autostart=True,
+                                                       connection='myconnection',
+                                                       username='user',
+                                                       password='secret'), ret)
+                mocks['define'].assert_called_with('mypool',
                                                    ptype='logical',
                                                    target='/dev/base',
                                                    permissions={'mode': '0770',
                                                                 'owner': 1000,
                                                                 'group': 100,
                                                                 'label': 'seclabel'},
-                                                   source={'devices': [{'path': '/dev/sda'}]},
+                                                   source_devices=[{'path': '/dev/sda'}],
+                                                   source_dir=None,
+                                                   source_adapter=None,
+                                                   source_hosts=None,
+                                                   source_auth=None,
+                                                   source_name=None,
+                                                   source_format=None,
+                                                   source_initiator=None,
                                                    transient=True,
-                                                   autostart=True,
+                                                   start=False,
                                                    connection='myconnection',
                                                    username='user',
-                                                   password='secret'), ret)
-            mocks['define'].assert_called_with('mypool',
-                                               ptype='logical',
-                                               target='/dev/base',
-                                               permissions={'mode': '0770',
-                                                            'owner': 1000,
-                                                            'group': 100,
-                                                            'label': 'seclabel'},
-                                               source_devices=[{'path': '/dev/sda'}],
-                                               source_dir=None,
-                                               source_adapter=None,
-                                               source_hosts=None,
-                                               source_auth=None,
-                                               source_name=None,
-                                               source_format=None,
-                                               transient=True,
-                                               start=False,
-                                               connection='myconnection',
-                                               username='user',
-                                               password='secret')
-            mocks['autostart'].assert_called_with('mypool',
-                                                  state='on',
+                                                   password='secret')
+                mocks['autostart'].assert_called_with('mypool',
+                                                      state='on',
+                                                      connection='myconnection',
+                                                      username='user',
+                                                      password='secret')
+                mocks['build'].assert_called_with('mypool',
+                                                  connection='myconnection',
+                                                  username='user',
+                                                  password='secret')
+                mocks['start'].assert_called_with('mypool',
                                                   connection='myconnection',
                                                   username='user',
                                                   password='secret')
-            mocks['build'].assert_called_with('mypool',
-                                              connection='myconnection',
-                                              username='user',
-                                              password='secret')
-            mocks['start'].assert_called_with('mypool',
-                                              connection='myconnection',
-                                              username='user',
-                                              password='secret')
 
-        with patch.dict(virt.__salt__, {  # pylint: disable=no-member
-                    'virt.pool_info': MagicMock(return_value={'mypool': {'state': 'running'}}),
-                }):
-            ret.update({'changes': {}, 'comment': 'Pool mypool exists and is running'})
-            self.assertDictEqual(virt.pool_running('mypool',
+            mocks['update'] = MagicMock(return_value=False)
+            with patch.dict(virt.__salt__, {  # pylint: disable=no-member
+                        'virt.pool_info': MagicMock(return_value={'mypool': {'state': 'running', 'autostart': True}}),
+                        'virt.pool_update': MagicMock(return_value=False),
+                    }):
+                ret.update({'changes': {}, 'comment': 'Pool mypool unchanged and is running'})
+                self.assertDictEqual(virt.pool_running('mypool',
+                                                       ptype='logical',
+                                                       target='/dev/base',
+                                                       source={'devices': [{'path': '/dev/sda'}]}), ret)
+
+            for mock in mocks:
+                mocks[mock].reset_mock()
+            with patch.dict(virt.__salt__, {  # pylint: disable=no-member
+                        'virt.pool_info': MagicMock(return_value={'mypool': {'state': 'stopped', 'autostart': True}}),
+                        'virt.pool_update': mocks['update'],
+                        'virt.pool_build': mocks['build'],
+                        'virt.pool_start': mocks['start']
+                    }):
+                ret.update({'changes': {'mypool': 'Pool started'}, 'comment': 'Pool mypool started'})
+                self.assertDictEqual(virt.pool_running('mypool',
+                                                       ptype='logical',
+                                                       target='/dev/base',
+                                                       source={'devices': [{'path': '/dev/sda'}]}), ret)
+                mocks['start'].assert_called_with('mypool', connection=None, username=None, password=None)
+                mocks['build'].assert_not_called()
+
+            with patch.dict(virt.__salt__, {  # pylint: disable=no-member
+                        'virt.pool_info': MagicMock(return_value={}),
+                        'virt.pool_define': MagicMock(side_effect=self.mock_libvirt.libvirtError('Some error'))
+                    }):
+                ret.update({'changes': {}, 'comment': 'Some error', 'result': False})
+                self.assertDictEqual(virt.pool_running('mypool',
+                                                       ptype='logical',
+                                                       target='/dev/base',
+                                                       source={'devices': [{'path': '/dev/sda'}]}), ret)
+
+            # Test case with update and autostart change on stopped pool
+            for mock in mocks:
+                mocks[mock].reset_mock()
+            mocks['update'] = MagicMock(return_value=True)
+            with patch.dict(virt.__salt__, {  # pylint: disable=no-member
+                        'virt.pool_info': MagicMock(return_value={'mypool': {'state': 'stopped', 'autostart': True}}),
+                        'virt.pool_update': mocks['update'],
+                        'virt.pool_set_autostart': mocks['autostart'],
+                        'virt.pool_build': mocks['build'],
+                        'virt.pool_start': mocks['start']
+                    }):
+                ret.update({'changes': {'mypool': 'Pool updated, built, autostart flag changed and started'},
+                            'comment': 'Pool mypool updated, built, autostart flag changed and started',
+                            'result': True})
+                self.assertDictEqual(virt.pool_running('mypool',
+                                                       ptype='logical',
+                                                       target='/dev/base',
+                                                       autostart=False,
+                                                       permissions={'mode': '0770',
+                                                                    'owner': 1000,
+                                                                    'group': 100,
+                                                                    'label': 'seclabel'},
+                                                       source={'devices': [{'path': '/dev/sda'}]}), ret)
+                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['autostart'].assert_called_with('mypool', state='off',
+                                                      connection=None, username=None, password=None)
+                mocks['update'].assert_called_with('mypool',
                                                    ptype='logical',
                                                    target='/dev/base',
-                                                   source={'devices': [{'path': '/dev/sda'}]}), ret)
-
-        for mock in mocks:
-            mocks[mock].reset_mock()
-        with patch.dict(virt.__salt__, {  # pylint: disable=no-member
-                    'virt.pool_info': MagicMock(return_value={'mypool': {'state': 'stopped'}}),
-                    'virt.pool_build': mocks['build'],
-                    'virt.pool_start': mocks['start']
-                }):
-            ret.update({'changes': {'mypool': 'Pool started'}, 'comment': 'Pool mypool started'})
-            self.assertDictEqual(virt.pool_running('mypool',
+                                                   permissions={'mode': '0770',
+                                                                'owner': 1000,
+                                                                'group': 100,
+                                                                'label': 'seclabel'},
+                                                   source_devices=[{'path': '/dev/sda'}],
+                                                   source_dir=None,
+                                                   source_adapter=None,
+                                                   source_hosts=None,
+                                                   source_auth=None,
+                                                   source_name=None,
+                                                   source_format=None,
+                                                   source_initiator=None,
+                                                   connection=None,
+                                                   username=None,
+                                                   password=None)
+
+            # test case with update and no autostart change on running pool
+            for mock in mocks:
+                mocks[mock].reset_mock()
+            with patch.dict(virt.__salt__, {  # pylint: disable=no-member
+                        'virt.pool_info': MagicMock(return_value={'mypool': {'state': 'running', 'autostart': False}}),
+                        'virt.pool_update': mocks['update'],
+                        'virt.pool_build': mocks['build'],
+                        'virt.pool_start': mocks['start'],
+                        'virt.pool_stop': mocks['stop']
+                    }):
+                ret.update({'changes': {'mypool': 'Pool updated, built and restarted'},
+                            'comment': 'Pool mypool updated, built and restarted',
+                            'result': True})
+                self.assertDictEqual(virt.pool_running('mypool',
+                                                       ptype='logical',
+                                                       target='/dev/base',
+                                                       autostart=False,
+                                                       permissions={'mode': '0770',
+                                                                    'owner': 1000,
+                                                                    'group': 100,
+                                                                    'label': 'seclabel'},
+                                                       source={'devices': [{'path': '/dev/sda'}]}), ret)
+                mocks['stop'].assert_called_with('mypool', connection=None, username=None, password=None)
+                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['update'].assert_called_with('mypool',
                                                    ptype='logical',
                                                    target='/dev/base',
-                                                   source={'devices': [{'path': '/dev/sda'}]}), ret)
-            mocks['start'].assert_called_with('mypool', connection=None, username=None, password=None)
-            mocks['build'].assert_not_called()
-
-        with patch.dict(virt.__salt__, {  # pylint: disable=no-member
-                    'virt.pool_info': MagicMock(return_value={}),
-                    'virt.pool_define': MagicMock(side_effect=self.mock_libvirt.libvirtError('Some error'))
+                                                   permissions={'mode': '0770',
+                                                                'owner': 1000,
+                                                                'group': 100,
+                                                                'label': 'seclabel'},
+                                                   source_devices=[{'path': '/dev/sda'}],
+                                                   source_dir=None,
+                                                   source_adapter=None,
+                                                   source_hosts=None,
+                                                   source_auth=None,
+                                                   source_name=None,
+                                                   source_format=None,
+                                                   source_initiator=None,
+                                                   connection=None,
+                                                   username=None,
+                                                   password=None)
+
+        with patch.dict(virt.__opts__, {'test': True}):
+            # test case with test=True and no change
+            with patch.dict(virt.__salt__, {  # pylint: disable=no-member
+                        'virt.pool_info': MagicMock(return_value={'mypool': {'state': 'running', 'autostart': True}}),
+                        'virt.pool_update': MagicMock(return_value=False),
+                    }):
+                ret.update({'changes': {}, 'comment': 'Pool mypool unchanged and is running',
+                            'result': True})
+                self.assertDictEqual(virt.pool_running('mypool',
+                                                       ptype='logical',
+                                                       target='/dev/base',
+                                                       source={'devices': [{'path': '/dev/sda'}]}), ret)
+
+            # test case with test=True and started
+            for mock in mocks:
+                mocks[mock].reset_mock()
+            mocks['update'] = MagicMock(return_value=False)
+            with patch.dict(virt.__salt__, {  # pylint: disable=no-member
+                        'virt.pool_info': MagicMock(return_value={'mypool': {'state': 'stopped', 'autostart': True}}),
+                        'virt.pool_update': mocks['update']
+                    }):
+                ret.update({'changes': {'mypool': 'Pool started'},
+                            'comment': 'Pool mypool started',
+                            'result': None})
+                self.assertDictEqual(virt.pool_running('mypool',
+                                                       ptype='logical',
+                                                       target='/dev/base',
+                                                       source={'devices': [{'path': '/dev/sda'}]}), ret)
+
+    def test_pool_deleted(self):
+        '''
+        Test the pool_deleted state
+        '''
+        # purge=False test case, stopped pool
+        with patch.dict(virt.__salt__, {
+                    'virt.pool_info': MagicMock(return_value={'test01': {'state': 'stopped', 'type': 'dir'}}),
+                    'virt.pool_undefine': MagicMock(return_value=True)
                 }):
-            ret.update({'changes': {}, 'comment': 'Some error', 'result': False})
-            self.assertDictEqual(virt.pool_running('mypool',
-                                                   ptype='logical',
-                                                   target='/dev/base',
-                                                   source={'devices': [{'path': '/dev/sda'}]}), ret)
+            expected = {
+                'name': 'test01',
+                'changes': {
+                    'stopped': False,
+                    'deleted_volumes': [],
+                    'deleted': False,
+                    'undefined': True,
+                },
+                'result': True,
+                'comment': '',
+            }
+
+            with patch.dict(virt.__opts__, {'test': False}):
+                self.assertDictEqual(expected, virt.pool_deleted('test01'))
+
+            with patch.dict(virt.__opts__, {'test': True}):
+                expected['result'] = None
+                self.assertDictEqual(expected, virt.pool_deleted('test01'))
+
+        # purge=False test case
+        with patch.dict(virt.__salt__, {
+                    'virt.pool_info': MagicMock(return_value={'test01': {'state': 'running', 'type': 'dir'}}),
+                    'virt.pool_undefine': MagicMock(return_value=True),
+                    'virt.pool_stop': MagicMock(return_value=True)
+                }):
+            expected = {
+                'name': 'test01',
+                'changes': {
+                    'stopped': True,
+                    'deleted_volumes': [],
+                    'deleted': False,
+                    'undefined': True,
+                },
+                'result': True,
+                'comment': '',
+            }
+
+            with patch.dict(virt.__opts__, {'test': False}):
+                self.assertDictEqual(expected, virt.pool_deleted('test01'))
+
+            with patch.dict(virt.__opts__, {'test': True}):
+                expected['result'] = None
+                self.assertDictEqual(expected, virt.pool_deleted('test01'))
+
+        # purge=True test case
+
+        with patch.dict(virt.__salt__, {
+                    'virt.pool_info': MagicMock(return_value={'test01': {'state': 'running', 'type': 'dir'}}),
+                    'virt.pool_list_volumes': MagicMock(return_value=['vm01.qcow2', 'vm02.qcow2']),
+                    'virt.pool_refresh': MagicMock(return_value=True),
+                    'virt.volume_delete': MagicMock(return_value=True),
+                    'virt.pool_stop': MagicMock(return_value=True),
+                    'virt.pool_delete': MagicMock(return_value=True),
+                    'virt.pool_undefine': MagicMock(return_value=True)
+                }):
+            expected = {
+                'name': 'test01',
+                'changes': {
+                    'stopped': True,
+                    'deleted_volumes': ['vm01.qcow2', 'vm02.qcow2'],
+                    'deleted': True,
+                    'undefined': True,
+                },
+                'result': True,
+                'comment': '',
+            }
+
+            with patch.dict(virt.__opts__, {'test': False}):
+                self.assertDictEqual(expected, virt.pool_deleted('test01', purge=True))
+
+            with patch.dict(virt.__opts__, {'test': True}):
+                expected['result'] = None
+                self.assertDictEqual(expected, virt.pool_deleted('test01', purge=True))
+
+        # Case of backend not unsupporting delete operations
+        with patch.dict(virt.__salt__, {
+                    'virt.pool_info': MagicMock(return_value={'test01': {'state': 'running', 'type': 'iscsi'}}),
+                    'virt.pool_stop': MagicMock(return_value=True),
+                    'virt.pool_undefine': MagicMock(return_value=True)
+                }):
+            expected = {
+                'name': 'test01',
+                'changes': {
+                    'stopped': True,
+                    'deleted_volumes': [],
+                    'deleted': False,
+                    'undefined': True,
+                },
+                'result': True,
+                'comment': 'Unsupported actions for pool of type "iscsi": deleting volume, deleting pool',
+            }
+
+            with patch.dict(virt.__opts__, {'test': False}):
+                self.assertDictEqual(expected, virt.pool_deleted('test01', purge=True))
+
+            with patch.dict(virt.__opts__, {'test': True}):
+                expected['result'] = None
+                self.assertDictEqual(expected, virt.pool_deleted('test01', purge=True))
-- 
2.16.4

Places

File virt.network_define-allow-adding-ip-configuration.patch of Package salt (Revision b98af0adedef74d61715abd98c45f2c6)

Places