cloud: ovirt: Various fixes for oVirt modules (#19141)

*  cloud: ovirt: fix various issues in ovirt modules

* cloud: ovirt: add support for nfs version

* cloud: ovirt: Fix facts documentation

* Add proper documentation fragmet
* Add proper argument_spec
* Fix return values

* cloud: ovirt: fix pep8

lib/ansible/modules/cloud/ovirt/ovirt_affinity_labels.py

@@ -1,4 +1,4 @@
-#!/usr/bin/pythonapi/
+#!/usr/bin/python
 # -*- coding: utf-8 -*-
 #
 # Copyright (c) 2016 Red Hat, Inc.

lib/ansible/modules/cloud/ovirt/ovirt_affinity_labels_facts.py

@@ -27,7 +27,7 @@ from ansible.module_utils.ovirt import (
     check_sdk,
     create_connection,
     get_dict_of_struct,
-    ovirt_full_argument_spec,
+    ovirt_facts_full_argument_spec,
 )
 
 
@@ -44,7 +44,7 @@ version_added: "2.3"
 description:
     - "Retrieve facts about one or more oVirt affinity labels."
 notes:
-    - "This module creates a new top-level C(affinity_labels) fact, which
+    - "This module creates a new top-level C(ovirt_affinity_labels) fact, which
        contains a list of affinity labels."
 options:
     name:
@@ -56,7 +56,7 @@ options:
     host:
       description:
         - "Name of the host, which affinity labels should be listed."
-extends_documentation_fragment: ovirt
+extends_documentation_fragment: ovirt_facts
 '''
 
 EXAMPLES = '''
@@ -93,7 +93,7 @@ EXAMPLES = '''
 '''
 
 RETURN = '''
-ovirt_vms:
+ovirt_affinity_labels:
     description: "List of dictionaries describing the affinity labels. Affinity labels attribues are mapped to dictionary keys,
                   all affinity labels attributes can be found at following url: https://ovirt.example.com/ovirt-engine/api/model#types/affinity_label."
     returned: On success.
@@ -102,7 +102,7 @@ ovirt_vms:
 
 
 def main():
-    argument_spec = ovirt_full_argument_spec(
+    argument_spec = ovirt_facts_full_argument_spec(
         name=dict(default=None),
         host=dict(default=None),
         vm=dict(default=None),
@@ -143,7 +143,7 @@ def main():
         module.exit_json(
             changed=False,
             ansible_facts=dict(
-                affinity_labels=[
+                ovirt_affinity_labels=[
                     get_dict_of_struct(
                         struct=l,
                         connection=connection,

lib/ansible/modules/cloud/ovirt/ovirt_auth.py

@@ -19,11 +19,16 @@
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 #
 
+import traceback
+
 try:
     import ovirtsdk4 as sdk
 except ImportError:
     pass
 
+from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.ovirt import check_sdk
+
 
 ANSIBLE_METADATA = {'status': ['preview'],
                     'supported_by': 'community',
@@ -222,13 +227,11 @@ def main():
             )
         )
     except Exception as e:
-        module.fail_json(msg="Error: %s" % e)
+        module.fail_json(msg=str(e), exception=traceback.format_exc())
     finally:
         # Close the connection, but don't revoke token
         connection.close(logout=state == 'absent')
 
 
-from ansible.module_utils.basic import *
-from ansible.module_utils.ovirt import *
 if __name__ == "__main__":
     main()

lib/ansible/modules/cloud/ovirt/ovirt_clusters.py

@@ -320,7 +320,7 @@ class ClustersModule(BaseModule):
 
     def _get_sched_policy(self):
         sched_policy = None
-        if self.param('serial_policy'):
+        if self.param('scheduling_policy'):
             sched_policies_service = self._connection.system_service().scheduling_policies_service()
             sched_policy = search_by_name(sched_policies_service, self.param('scheduling_policy'))
             if not sched_policy:
@@ -445,6 +445,8 @@ class ClustersModule(BaseModule):
         )
 
     def update_check(self, entity):
+        sched_policy = self._get_sched_policy()
+        migration_policy = getattr(entity.migration, 'policy', None)
         return (
             equal(self.param('comment'), entity.comment) and
             equal(self.param('description'), entity.description) and
@@ -470,10 +472,10 @@ class ClustersModule(BaseModule):
             equal(self.param('migration_bandwidth'), str(entity.migration.bandwidth.assignment_method)) and
             equal(self.param('migration_auto_converge'), str(entity.migration.auto_converge)) and
             equal(self.param('migration_compressed'), str(entity.migration.compressed)) and
-            equal(self.param('serial_policy'), str(entity.serial_number.policy)) and
+            equal(self.param('serial_policy'), str(getattr(entity.serial_number, 'policy', None))) and
-            equal(self.param('serial_policy_value'), entity.serial_number.value) and
+            equal(self.param('serial_policy_value'), getattr(entity.serial_number, 'value', None)) and
-            equal(self.param('scheduling_policy'), self._get_sched_policy().name) and
+            equal(self.param('scheduling_policy'), getattr(sched_policy, 'name', None)) and
-            equal(self._get_policy_id(), entity.migration.policy.id) and
+            equal(self._get_policy_id(), getattr(migration_policy, 'id', None)) and
             equal(self._get_memory_policy(), entity.memory_policy.over_commit.percent) and
             equal(self.__get_minor(self.param('compatibility_version')), self.__get_minor(entity.version)) and
             equal(self.__get_major(self.param('compatibility_version')), self.__get_major(entity.version)) and

lib/ansible/modules/cloud/ovirt/ovirt_clusters_facts.py

@@ -26,7 +26,7 @@ from ansible.module_utils.ovirt import (
     check_sdk,
     create_connection,
     get_dict_of_struct,
-    ovirt_full_argument_spec,
+    ovirt_facts_full_argument_spec,
 )
 
 
@@ -51,7 +51,7 @@ options:
         - "Search term which is accepted by oVirt search backend."
         - "For example to search cluster X from datacenter Y use following pattern:
            name=X and datacenter=Y"
-extends_documentation_fragment: ovirt
+extends_documentation_fragment: ovirt_facts
 '''
 
 EXAMPLES = '''
@@ -75,7 +75,7 @@ ovirt_clusters:
 
 
 def main():
-    argument_spec = ovirt_full_argument_spec(
+    argument_spec = ovirt_facts_full_argument_spec(
         pattern=dict(default='', required=False),
     )
     module = AnsibleModule(argument_spec)

lib/ansible/modules/cloud/ovirt/ovirt_datacenters_facts.py

@@ -26,7 +26,7 @@ from ansible.module_utils.ovirt import (
     check_sdk,
     create_connection,
     get_dict_of_struct,
-    ovirt_full_argument_spec,
+    ovirt_facts_full_argument_spec,
 )
 
 
@@ -50,7 +50,7 @@ options:
       description:
         - "Search term which is accepted by oVirt search backend."
         - "For example to search datacenter I(X) use following pattern: I(name=X)"
-extends_documentation_fragment: ovirt
+extends_documentation_fragment: ovirt_facts
 '''
 
 EXAMPLES = '''
@@ -74,7 +74,7 @@ ovirt_datacenters:
 
 
 def main():
-    argument_spec = ovirt_full_argument_spec(
+    argument_spec = ovirt_facts_full_argument_spec(
         pattern=dict(default='', required=False),
     )
     module = AnsibleModule(argument_spec)

lib/ansible/modules/cloud/ovirt/ovirt_disks.py

@@ -19,13 +19,24 @@
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 #
 
+import traceback
+
 try:
-    import ovirtsdk4 as sdk
     import ovirtsdk4.types as otypes
 except ImportError:
     pass
 
-from ansible.module_utils.ovirt import *
+from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.ovirt import (
+    BaseModule,
+    check_sdk,
+    check_params,
+    create_connection,
+    convert_to_bytes,
+    equal,
+    ovirt_full_argument_spec,
+    search_by_name,
+)
 
 
 ANSIBLE_METADATA = {'status': ['preview'],
@@ -151,7 +162,6 @@ disk_attachment:
 '''
 
 
-
 def _search_by_lun(disks_service, lun_id):
     """
     Find disk by LUN ID.
@@ -312,11 +322,10 @@ def main():
 
         module.exit_json(**ret)
     except Exception as e:
-        module.fail_json(msg=str(e))
+        module.fail_json(msg=str(e), exception=traceback.format_exc())
     finally:
         connection.close(logout=False)
 
 
-from ansible.module_utils.basic import *
 if __name__ == "__main__":
     main()

lib/ansible/modules/cloud/ovirt/ovirt_external_providers_facts.py

@@ -27,7 +27,7 @@ from ansible.module_utils.ovirt import (
     check_sdk,
     create_connection,
     get_dict_of_struct,
-    ovirt_full_argument_spec,
+    ovirt_facts_full_argument_spec,
 )
 
 
@@ -54,7 +54,7 @@ options:
     name:
         description:
             - "Name of the external provider, can be used as glob expression."
-extends_documentation_fragment: ovirt
+extends_documentation_fragment: ovirt_facts
 '''
 
 EXAMPLES = '''
@@ -105,7 +105,7 @@ def _external_provider_service(provider_type, system_service):
 
 
 def main():
-    argument_spec = ovirt_full_argument_spec(
+    argument_spec = ovirt_facts_full_argument_spec(
         name=dict(default=None, required=False),
         type=dict(
             default=None,

lib/ansible/modules/cloud/ovirt/ovirt_groups_facts.py

@@ -26,7 +26,7 @@ from ansible.module_utils.ovirt import (
     check_sdk,
     create_connection,
     get_dict_of_struct,
-    ovirt_full_argument_spec,
+    ovirt_facts_full_argument_spec,
 )
 
 
@@ -50,7 +50,7 @@ options:
       description:
         - "Search term which is accepted by oVirt search backend."
         - "For example to search group X use following pattern: name=X"
-extends_documentation_fragment: ovirt
+extends_documentation_fragment: ovirt_facts
 '''
 
 EXAMPLES = '''
@@ -74,7 +74,7 @@ ovirt_groups:
 
 
 def main():
-    argument_spec = ovirt_full_argument_spec(
+    argument_spec = ovirt_facts_full_argument_spec(
         pattern=dict(default='', required=False),
     )
     module = AnsibleModule(argument_spec)

lib/ansible/modules/cloud/ovirt/ovirt_host_networks.py

@@ -19,13 +19,24 @@
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 #
 
+import traceback
+
 try:
-    import ovirtsdk4 as sdk
     import ovirtsdk4.types as otypes
 except ImportError:
     pass
 
-from ansible.module_utils.ovirt import *
+from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.ovirt import (
+    BaseModule,
+    check_sdk,
+    create_connection,
+    equal,
+    get_dict_of_struct,
+    get_link_name,
+    ovirt_full_argument_spec,
+    search_by_name,
+)
 
 
 ANSIBLE_METADATA = {'status': ['preview'],
@@ -67,7 +78,7 @@ options:
             - "C(address) - IP address in case of I(static) boot protocol is used."
             - "C(prefix) - Routing prefix in case of I(static) boot protocol is used."
             - "C(gateway) - Gateway in case of I(static) boot protocol is used."
-            - "C(version) - IP version. Either v4 or v6."
+            - "C(version) - IP version. Either v4 or v6. Default is v4."
     labels:
         description:
             - "List of names of the network label to be assigned to bond or interface."
@@ -152,11 +163,11 @@ class HostNetworksModule(BaseModule):
     def build_entity(self):
         return otypes.Host()
 
-    def update_address(self, attachment, network):
+    def update_address(self, attachments_service, attachment, network):
         # Check if there is any change in address assignenmts and
         # update it if needed:
         for ip in attachment.ip_address_assignments:
-            if str(ip.ip.version) == network.get('version'):
+            if str(ip.ip.version) == network.get('version', 'v4'):
                 changed = False
                 if not equal(network.get('boot_protocol'), str(ip.assignment_method)):
                     ip.assignment_method = otypes.BootProtocol(network.get('boot_protocol'))
@@ -167,11 +178,12 @@ class HostNetworksModule(BaseModule):
                 if not equal(network.get('gateway'), ip.ip.gateway):
                     ip.ip.gateway = network.get('gateway')
                     changed = True
-                if not equal(network.get('prefix'), int(ip.ip.netmask)):
+                if not equal(network.get('prefix'), int(ip.ip.netmask) if ip.ip.netmask else None):
                     ip.ip.netmask = str(network.get('prefix'))
                     changed = True
 
                 if changed:
+                    if not self._module.check_mode:
                         attachments_service.service(attachment.id).update(attachment)
                     self.changed = True
                     break
@@ -214,7 +226,7 @@ class HostNetworksModule(BaseModule):
             if attachment is None:
                 return True
 
-            self.update_address(attachment, network)
+            self.update_address(attachments_service, attachment, network)
 
         return update
 
@@ -359,10 +371,10 @@ def main():
             'host_nic': get_dict_of_struct(nic),
         })
     except Exception as e:
-        module.fail_json(msg=str(e))
+        module.fail_json(msg=str(e), exception=traceback.format_exc())
     finally:
         connection.close(logout=False)
 
-from ansible.module_utils.basic import *
+
 if __name__ == "__main__":
     main()

lib/ansible/modules/cloud/ovirt/ovirt_host_pm.py

@@ -19,13 +19,22 @@
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 #
 
+import traceback
+
 try:
-    import ovirtsdk4 as sdk
     import ovirtsdk4.types as otypes
 except ImportError:
     pass
 
-from ansible.module_utils.ovirt import *
+from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.ovirt import (
+    BaseModule,
+    check_sdk,
+    create_connection,
+    equal,
+    ovirt_full_argument_spec,
+    search_by_name,
+)
 
 
 ANSIBLE_METADATA = {'status': ['preview'],
@@ -227,10 +236,10 @@ def main():
 
         module.exit_json(**ret)
     except Exception as e:
-        module.fail_json(msg=str(e))
+        module.fail_json(msg=str(e), exception=traceback.format_exc())
     finally:
         connection.close(logout=False)
 
-from ansible.module_utils.basic import *
+
 if __name__ == "__main__":
     main()

lib/ansible/modules/cloud/ovirt/ovirt_hosts.py

@@ -19,15 +19,24 @@
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 #
 
+import traceback
+
 try:
-    import ovirtsdk4 as sdk
     import ovirtsdk4.types as otypes
 
     from ovirtsdk4.types import HostStatus as hoststate
 except ImportError:
     pass
 
-from ansible.module_utils.ovirt import *
+from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.ovirt import (
+    BaseModule,
+    check_sdk,
+    create_connection,
+    equal,
+    ovirt_full_argument_spec,
+    wait,
+)
 
 
 ANSIBLE_METADATA = {'status': ['preview'],
@@ -200,7 +209,6 @@ def control_state(host_module):
     if host is None:
         return
 
-    state = host_module._module.params['state']
     host_service = host_module._service.service(host.id)
     if failed_state(host):
         raise Exception("Not possible to manage host '%s'." % host.name)
@@ -313,14 +321,12 @@ def main():
                 fence_type='restart',
             )
 
-
         module.exit_json(**ret)
     except Exception as e:
-        module.fail_json(msg=str(e))
+        module.fail_json(msg=str(e), exception=traceback.format_exc())
     finally:
         connection.close(logout=False)
 
 
-from ansible.module_utils.basic import *
 if __name__ == "__main__":
     main()

lib/ansible/modules/cloud/ovirt/ovirt_hosts_facts.py

@@ -19,12 +19,15 @@
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 #
 
-try:
+import traceback
-    import ovirtsdk4 as sdk
-except ImportError:
-    pass
 
-from ansible.module_utils.ovirt import *
+from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.ovirt import (
+    check_sdk,
+    create_connection,
+    get_dict_of_struct,
+    ovirt_full_argument_spec,
+)
 
 
 ANSIBLE_METADATA = {'status': ['preview'],
@@ -48,7 +51,7 @@ options:
         - "Search term which is accepted by oVirt search backend."
         - "For example to search host X from datacenter Y use following pattern:
            name=X and datacenter=Y"
-extends_documentation_fragment: ovirt
+extends_documentation_fragment: ovirt_facts
 '''
 
 EXAMPLES = '''
@@ -73,7 +76,7 @@ ovirt_hosts:
 
 
 def main():
-    argument_spec = ovirt_full_argument_spec(
+    argument_spec = ovirt_facts_full_argument_spec(
         pattern=dict(default='', required=False),
     )
     module = AnsibleModule(argument_spec)
@@ -97,8 +100,10 @@ def main():
             ),
         )
     except Exception as e:
-        module.fail_json(msg=str(e))
+        module.fail_json(msg=str(e), exception=traceback.format_exc())
+    finally:
+        connection.close(logout=False)
+
 
-from ansible.module_utils.basic import *
 if __name__ == '__main__':
     main()

lib/ansible/modules/cloud/ovirt/ovirt_networks.py

@@ -237,10 +237,11 @@ def main():
             ret = networks_module.create(entity=network)
 
             # Update clusters networks:
-            for param_cluster in module.params.get('clusters', []):
+            if module.params.get('clusters') is not None:
-                cluster = search_by_name(clusters_service, param_cluster.get('name', None))
+                for param_cluster in module.params.get('clusters'):
+                    cluster = search_by_name(clusters_service, param_cluster.get('name'))
                     if cluster is None:
-                    raise Exception("Cluster '%s' was not found." % cluster_name)
+                        raise Exception("Cluster '%s' was not found." % param_cluster.get('name'))
                     cluster_networks_service = clusters_service.service(cluster.id).networks_service()
                     cluster_networks_module = ClusterNetworksModule(
                         network_id=ret['id'],

lib/ansible/modules/cloud/ovirt/ovirt_networks_facts.py

@@ -26,7 +26,7 @@ from ansible.module_utils.ovirt import (
     check_sdk,
     create_connection,
     get_dict_of_struct,
-    ovirt_full_argument_spec,
+    ovirt_facts_full_argument_spec,
 )
 
 
@@ -50,7 +50,7 @@ options:
       description:
         - "Search term which is accepted by oVirt search backend."
         - "For example to search network starting with string vlan1 use: name=vlan1*"
-extends_documentation_fragment: ovirt
+extends_documentation_fragment: ovirt_facts
 '''
 
 
@@ -76,7 +76,7 @@ ovirt_networks:
 
 
 def main():
-    argument_spec = ovirt_full_argument_spec(
+    argument_spec = ovirt_facts_full_argument_spec(
         pattern=dict(default='', required=False),
     )
     module = AnsibleModule(argument_spec)

lib/ansible/modules/cloud/ovirt/ovirt_nics_facts.py

@@ -27,7 +27,7 @@ from ansible.module_utils.ovirt import (
     check_sdk,
     create_connection,
     get_dict_of_struct,
-    ovirt_full_argument_spec,
+    ovirt_facts_full_argument_spec,
     search_by_name,
 )
 
@@ -55,7 +55,7 @@ options:
     name:
         description:
             - "Name of the NIC, can be used as glob expression."
-extends_documentation_fragment: ovirt
+extends_documentation_fragment: ovirt_facts
 '''
 
 EXAMPLES = '''
@@ -80,7 +80,7 @@ ovirt_nics:
 
 
 def main():
-    argument_spec = ovirt_full_argument_spec(
+    argument_spec = ovirt_facts_full_argument_spec(
         vm=dict(required=True),
         name=dict(default=None),
     )

lib/ansible/modules/cloud/ovirt/ovirt_permissions_facts.py

@@ -31,7 +31,7 @@ from ansible.module_utils.ovirt import (
     check_sdk,
     create_connection,
     get_link_name,
-    ovirt_full_argument_spec,
+    ovirt_facts_full_argument_spec,
     search_by_name,
 )
 
@@ -67,7 +67,7 @@ options:
         description:
             - "Namespace of the authorization provider, where user/group resides."
         required: false
-extends_documentation_fragment: ovirt
+extends_documentation_fragment: ovirt_facts
 '''
 
 EXAMPLES = '''
@@ -106,7 +106,7 @@ def _permissions_service(connection, module):
 
 
 def main():
-    argument_spec = ovirt_full_argument_spec(
+    argument_spec = ovirt_facts_full_argument_spec(
         authz_name=dict(required=True, aliases=['domain']),
         user_name=dict(rdefault=None),
         group_name=dict(default=None),

lib/ansible/modules/cloud/ovirt/ovirt_quotas_facts.py

@@ -27,7 +27,7 @@ from ansible.module_utils.ovirt import (
     check_sdk,
     create_connection,
     get_dict_of_struct,
-    ovirt_full_argument_spec,
+    ovirt_facts_full_argument_spec,
     search_by_name,
 )
 
@@ -54,7 +54,7 @@ options:
     name:
         description:
             - "Name of the quota, can be used as glob expression."
-extends_documentation_fragment: ovirt
+extends_documentation_fragment: ovirt_facts
 '''
 
 EXAMPLES = '''
@@ -79,7 +79,7 @@ ovirt_quotas:
 
 
 def main():
-    argument_spec = ovirt_full_argument_spec(
+    argument_spec = ovirt_facts_full_argument_spec(
         datacenter=dict(required=True),
         name=dict(default=None),
     )

lib/ansible/modules/cloud/ovirt/ovirt_storage_domains.py

@@ -33,6 +33,7 @@ from ansible.module_utils.ovirt import (
     BaseModule,
     check_sdk,
     create_connection,
+    equal,
     ovirt_full_argument_spec,
     search_by_name,
     wait,
@@ -69,9 +70,11 @@ options:
     data_center:
         description:
             - "Data center name where storage domain should be attached."
+            - "This parameter isn't idempotent, it's not possible to change data center of storage domain."
     domain_function:
         description:
             - "Function of the storage domain."
+            - "This parameter isn't idempotent, it's not possible to change domain function of storage domain."
         choices: ['data', 'iso', 'export']
         default: 'data'
         aliases:  ['type']
@@ -83,40 +86,48 @@ options:
             - "Dictionary with values for NFS storage type:"
             - "C(address) - Address of the NFS server. E.g.: myserver.mydomain.com"
             - "C(path) - Path of the mount point. E.g.: /path/to/my/data"
+            - "C(version) - NFS version. One of: I(auto), I(v3), I(v4) or I(v4_1)."
+            - "C(timeout) - The time in tenths of a second to wait for a response before retrying NFS requests. Range 0 to 65535."
+            - "C(retrans) - The number of times to retry a request before attempting further recovery actions. Range 0 to 65535."
+            - "Note that these parameters are not idempotent."
     iscsi:
         description:
             - "Dictionary with values for iSCSI storage type:"
             - "C(address) - Address of the iSCSI storage server."
             - "C(port) - Port of the iSCSI storage server."
-            - "C(target) - iSCSI target."
+            - "C(target) - The target IQN for the storage device."
             - "C(lun_id) - LUN id."
-            - "C(username) - Username to be used to access storage server."
+            - "C(username) - A CHAP user name for logging into a target."
-            - "C(password) - Password of the user to be used to access storage server."
+            - "C(password) - A CHAP password for logging into a target."
+            - "Note that these parameters are not idempotent."
     posixfs:
         description:
             - "Dictionary with values for PosixFS storage type:"
             - "C(path) - Path of the mount point. E.g.: /path/to/my/data"
             - "C(vfs_type) - Virtual File System type."
             - "C(mount_options) - Option which will be passed when mounting storage."
+            - "Note that these parameters are not idempotent."
     glusterfs:
         description:
             - "Dictionary with values for GlusterFS storage type:"
             - "C(address) - Address of the NFS server. E.g.: myserver.mydomain.com"
             - "C(path) - Path of the mount point. E.g.: /path/to/my/data"
             - "C(mount_options) - Option which will be passed when mounting storage."
+            - "Note that these parameters are not idempotent."
     fcp:
         description:
             - "Dictionary with values for fibre channel storage type:"
             - "C(address) - Address of the fibre channel storage server."
             - "C(port) - Port of the fibre channel storage server."
             - "C(lun_id) - LUN id."
+            - "Note that these parameters are not idempotent."
     destroy:
         description:
-            - "If I(True) storage domain metadata won't be cleaned, and user have to clean them manually."
+            - "Logical remove of the storage domain. If I(true) retains the storage domain's data for import."
             - "This parameter is relevant only when C(state) is I(absent)."
     format:
         description:
-            - "If I(True) storage domain will be removed after removing it from oVirt."
+            - "If I(True) storage domain will be formatted after removing it from oVirt."
             - "This parameter is relevant only when C(state) is I(absent)."
 extends_documentation_fragment: ovirt
 '''
@@ -239,7 +250,12 @@ class StorageDomainModule(BaseModule):
                 vfs_type=storage.get('vfs_type'),
                 address=storage.get('address'),
                 path=storage.get('path'),
-            )
+                nfs_retrans=storage.get('retrans'),
+                nfs_timeo=storage.get('timeout'),
+                nfs_version=otypes.NfsVersion(
+                    storage.get('version')
+                ) if storage.get('version') else None,
+            ) if storage_type is not None else None
         )
 
     def _attached_sds_service(self):
@@ -325,6 +341,12 @@ class StorageDomainModule(BaseModule):
         self._service = self._attached_sds_service(storage_domain)
         self._maintenance(self._service, storage_domain)
 
+    def update_check(self, entity):
+        return (
+            equal(self._module.params['comment'], entity.comment) and
+            equal(self._module.params['description'], entity.description)
+        )
+
 
 def failed_state(sd):
     return sd.status in [sdstate.UNKNOWN, sdstate.INACTIVE]

lib/ansible/modules/cloud/ovirt/ovirt_storage_domains_facts.py

@@ -26,7 +26,7 @@ from ansible.module_utils.ovirt import (
     check_sdk,
     create_connection,
     get_dict_of_struct,
-    ovirt_full_argument_spec,
+    ovirt_facts_full_argument_spec,
 )
 
 
@@ -51,7 +51,7 @@ options:
         - "Search term which is accepted by oVirt search backend."
         - "For example to search storage domain X from datacenter Y use following pattern:
            name=X and datacenter=Y"
-extends_documentation_fragment: ovirt
+extends_documentation_fragment: ovirt_facts
 '''
 
 EXAMPLES = '''
@@ -76,7 +76,7 @@ ovirt_storage_domains:
 
 
 def main():
-    argument_spec = ovirt_full_argument_spec(
+    argument_spec = ovirt_facts_full_argument_spec(
         pattern=dict(default='', required=False),
     )
     module = AnsibleModule(argument_spec)

lib/ansible/modules/cloud/ovirt/ovirt_templates.py

@@ -1,4 +1,4 @@
-#!/usr/bin/pythonapi/
+#!/usr/bin/python
 # -*- coding: utf-8 -*-
 #
 # Copyright (c) 2016 Red Hat, Inc.

lib/ansible/modules/cloud/ovirt/ovirt_templates_facts.py

@@ -26,7 +26,7 @@ from ansible.module_utils.ovirt import (
     check_sdk,
     create_connection,
     get_dict_of_struct,
-    ovirt_full_argument_spec,
+    ovirt_facts_full_argument_spec,
 )
 
 
@@ -51,7 +51,7 @@ options:
         - "Search term which is accepted by oVirt search backend."
         - "For example to search template X from datacenter Y use following pattern:
            name=X and datacenter=Y"
-extends_documentation_fragment: ovirt
+extends_documentation_fragment: ovirt_facts
 '''
 
 EXAMPLES = '''
@@ -76,7 +76,7 @@ ovirt_templates:
 
 
 def main():
-    argument_spec = ovirt_full_argument_spec(
+    argument_spec = ovirt_facts_full_argument_spec(
         pattern=dict(default='', required=False),
     )
     module = AnsibleModule(argument_spec)

lib/ansible/modules/cloud/ovirt/ovirt_users_facts.py

@@ -26,7 +26,7 @@ from ansible.module_utils.ovirt import (
     check_sdk,
     create_connection,
     get_dict_of_struct,
-    ovirt_full_argument_spec,
+    ovirt_facts_full_argument_spec,
 )
 
 
@@ -50,7 +50,7 @@ options:
       description:
         - "Search term which is accepted by oVirt search backend."
         - "For example to search user X use following pattern: name=X"
-extends_documentation_fragment: ovirt
+extends_documentation_fragment: ovirt_facts
 '''
 
 EXAMPLES = '''
@@ -74,7 +74,7 @@ ovirt_users:
 
 
 def main():
-    argument_spec = ovirt_full_argument_spec(
+    argument_spec = ovirt_facts_full_argument_spec(
         pattern=dict(default='', required=False),
     )
     module = AnsibleModule(argument_spec)

lib/ansible/modules/cloud/ovirt/ovirt_vmpools_facts.py

@@ -26,7 +26,7 @@ from ansible.module_utils.ovirt import (
     check_sdk,
     create_connection,
     get_dict_of_struct,
-    ovirt_full_argument_spec,
+    ovirt_facts_full_argument_spec,
 )
 
 
@@ -50,7 +50,7 @@ options:
       description:
         - "Search term which is accepted by oVirt search backend."
         - "For example to search vmpool X: name=X"
-extends_documentation_fragment: ovirt
+extends_documentation_fragment: ovirt_facts
 '''
 
 EXAMPLES = '''
@@ -74,7 +74,7 @@ ovirt_vm_pools:
 
 
 def main():
-    argument_spec = ovirt_full_argument_spec(
+    argument_spec = ovirt_facts_full_argument_spec(
         pattern=dict(default='', required=False),
     )
     module = AnsibleModule(argument_spec)

lib/ansible/modules/cloud/ovirt/ovirt_vms.py

@@ -19,13 +19,26 @@
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 #
 
+import traceback
+
 try:
-    import ovirtsdk4 as sdk
     import ovirtsdk4.types as otypes
 except ImportError:
     pass
 
-from ansible.module_utils.ovirt import *
+from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.ovirt import (
+    BaseModule,
+    check_params,
+    check_sdk,
+    convert_to_bytes,
+    create_connection,
+    equal,
+    get_link_name,
+    ovirt_full_argument_spec,
+    search_by_name,
+    wait,
+)
 
 
 ANSIBLE_METADATA = {'status': ['preview'],
@@ -794,7 +807,8 @@ def main():
         if state == 'present' or state == 'running' or state == 'next_run':
             sysprep = module.params['sysprep']
             cloud_init = module.params['cloud_init']
-            cloud_init_nics = module.params['cloud_init_nics']
+            cloud_init_nics = module.params['cloud_init_nics'] or []
+            if cloud_init is not None:
                 cloud_init_nics.append(cloud_init)
 
             # In case VM don't exist, wait for VM DOWN state,
@@ -878,10 +892,10 @@ def main():
 
         module.exit_json(**ret)
     except Exception as e:
-        module.fail_json(msg=str(e))
+        module.fail_json(msg=str(e), exception=traceback.format_exc())
     finally:
         connection.close(logout=False)
 
-from ansible.module_utils.basic import *
+
 if __name__ == "__main__":
     main()

lib/ansible/modules/cloud/ovirt/ovirt_vms_facts.py

@@ -26,7 +26,7 @@ from ansible.module_utils.ovirt import (
     check_sdk,
     create_connection,
     get_dict_of_struct,
-    ovirt_full_argument_spec,
+    ovirt_facts_full_argument_spec,
 )
 
 
@@ -51,7 +51,7 @@ options:
         - "Search term which is accepted by oVirt search backend."
         - "For example to search VM X from cluster Y use following pattern:
            name=X and cluster=Y"
-extends_documentation_fragment: ovirt
+extends_documentation_fragment: ovirt_facts
 '''
 
 EXAMPLES = '''
@@ -76,7 +76,7 @@ ovirt_vms:
 
 
 def main():
-    argument_spec = ovirt_full_argument_spec(
+    argument_spec = ovirt_facts_full_argument_spec(
         pattern=dict(default='', required=False),
     )
     module = AnsibleModule(argument_spec)