Fix compatibility of affinity groups in oVirt 4.0 and 4.1 (#21308)

* cloud: ovirt: exit if incompatible API/SDK version

* cloud: ovirt: fix affinity groups compatibility for oVirt 4.1

lib/ansible/module_utils/ovirt.py

@@ -110,6 +110,15 @@ def get_dict_of_struct(struct, connection=None, fetch_nested=False, attributes=N
     return res
 
 
+def engine_version(connection):
+    """
+    Return string representation of oVirt engine version.
+    """
+    engine_api = connection.system_service().get()
+    engine_version = engine_api.product_info.version
+    return '%s.%s' % (engine_version.major, engine_version.minor)
+
+
 def create_connection(auth):
     """
     Create a connection to Python SDK, from task `auth` parameter.
@@ -126,7 +135,7 @@ def create_connection(auth):
     :return: Python SDK connection
     """
 
-    return sdk.Connection(
+    connection = sdk.Connection(
         url=auth.get('url'),
         username=auth.get('username'),
         password=auth.get('password'),
@@ -135,6 +144,15 @@ def create_connection(auth):
         token=auth.get('token', None),
         kerberos=auth.get('kerberos', None),
     )
+    api_version = LooseVersion(engine_version(connection))
+    python_sdk_version = LooseVersion(sdk_version.VERSION)
+    if python_sdk_version < api_version:
+        raise Exception(
+            "Your SDK version is lower than engine version, please use same "
+            "version of the SDK as engine, or unexpected errors may appear."
+        )
+
+    return connection
 
 
 def convert_to_bytes(param):
@@ -391,13 +409,8 @@ def check_params(module):
         module.fail_json(msg='"name" or "id" is required')
 
 
-def engine_version(connection):
+def engine_supported(connection, version):
-    """
+    return LooseVersion(engine_version(connection)) >= LooseVersion(version)
-    Return string representation of oVirt engine version.
-    """
-    engine_api = connection.system_service().get()
-    engine_version = engine_api.product_info.version
-    return '%s.%s' % (engine_version.major, engine_version.minor)
 
 
 def check_support(version, connection, module, params):

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

@@ -145,7 +145,9 @@ from ansible.module_utils.ovirt import (
     check_sdk,
     check_support,
     create_connection,
+    get_id_by_name,
     equal,
+    engine_supported,
     ovirt_full_argument_spec,
     search_by_name,
 )
@@ -158,8 +160,35 @@ class AffinityGroupsModule(BaseModule):
         self._vm_ids = vm_ids
         self._host_ids = host_ids
 
+    def update_vms(self, affinity_group):
+        """
+        This method iterate via the affinity VM assignnments and datech the VMs
+        which should not be attached to affinity and attach VMs which should be
+        attached to affinity.
+        """
+        assigned_vms = self.assigned_vms(affinity_group)
+        to_remove = [vm for vm in assigned_vms if vm not in self._vm_ids]
+        to_add = [vm for vm in self._vm_ids if vm not in assigned_vms]
+        ag_service = self._service.group_service(affinity_group.id)
+        for vm in to_remove:
+            ag_service.vms_service().vm_service(vm).remove()
+        for vm in to_add:
+            # API return <action> element instead of VM element, so we
+            # need to WA this issue, for oVirt versions having this bug:
+            try:
+                ag_service.vms_service().add(otypes.Vm(id=vm))
+            except ValueError as ex:
+                if 'complete' not in str(ex):
+                    raise ex
+
+    def post_create(self, affinity_group):
+        self.update_vms(affinity_group)
+
+    def post_update(self, affinity_group):
+        self.update_vms(affinity_group)
+
     def build_entity(self):
-        return otypes.AffinityGroup(
+        affinity_group = otypes.AffinityGroup(
             name=self._module.params['name'],
             description=self._module.params['description'],
             positive=(
@@ -168,56 +197,78 @@ class AffinityGroupsModule(BaseModule):
             enforcing=(
                 self._module.params['vm_enforcing']
             ) if self._module.params['vm_enforcing'] is not None else None,
-            vms=[
-                otypes.Vm(id=vm_id) for vm_id in self._vm_ids
-            ] if self._vm_ids is not None else None,
-            hosts=[
-                otypes.Host(id=host_id) for host_id in self._host_ids
-            ] if self._host_ids is not None else None,
-            vms_rule=otypes.AffinityRule(
-                positive=(
-                    self._module.params['vm_rule'] == 'positive'
-                ) if self._module.params['vm_rule'] is not None else None,
-                enforcing=self._module.params['vm_enforcing'],
-                enabled=(
-                    self._module.params['vm_rule'] in ['negative', 'positive']
-                ) if self._module.params['vm_rule'] is not None else None,
-            ) if (
-                self._module.params['vm_enforcing'] is not None or
-                self._module.params['vm_rule'] is not None
-            ) else None,
-            hosts_rule=otypes.AffinityRule(
-                positive=(
-                    self._module.params['host_rule'] == 'positive'
-                ) if self._module.params['host_rule'] is not None else None,
-                enforcing=self._module.params['host_enforcing'],
-            ) if (
-                self._module.params['host_enforcing'] is not None or
-                self._module.params['host_rule'] is not None
-            ) else None,
         )
 
+        # Those attributes are Supported since 4.1:
+        if not engine_supported(self._connection, '4.1'):
+            return affinity_group
+
+        affinity_group.hosts_rule = otypes.AffinityRule(
+            positive=(
+                self.param('host_rule') == 'positive'
+            ) if self.param('host_rule') is not None else None,
+            enforcing=self.param('host_enforcing'),
+        ) if (
+            self.param('host_enforcing') is not None or
+            self.param('host_rule') is not None
+        ) else None
+
+        affinity_group.vms_rule = otypes.AffinityRule(
+            positive=(
+                self.param('vm_rule') == 'positive'
+            ) if self.param('vm_rule') is not None else None,
+            enforcing=self.param('vm_enforcing'),
+            enabled=(
+                self.param('vm_rule') in ['negative', 'positive']
+            ) if self.param('vm_rule') is not None else None,
+        ) if (
+            self.param('vm_enforcing') is not None or
+            self.param('vm_rule') is not None
+        ) else None
+
+        affinity_group.hosts = [
+            otypes.Host(id=host_id) for host_id in self._host_ids
+        ] if self._host_ids is not None else None
+
+        return affinity_group
+
+    def assigned_vms(self, affinity_group):
+        if getattr(affinity_group.vms, 'href', None):
+            return sorted([
+                vm.id for vm in self._connection.follow_link(affinity_group.vms)
+            ])
+        else:
+            return sorted([vm.id for vm in affinity_group.vms])
+
     def update_check(self, entity):
-        assigned_vms = sorted([vm.id for vm in entity.vms])
+        assigned_vms = self.assigned_vms(entity)
-        assigned_hosts = sorted([host.id for host in entity.hosts])
+        do_update = (
-
+            equal(self.param('description'), entity.description)
-        return (
+            and equal(self.param('vm_enforcing'), entity.enforcing)
-            equal(self._module.params.get('description'), entity.description) and
+            and equal(
-            equal(self._module.params.get('vm_enforcing'), entity.vms_rule.enforcing) and
+                self.param('vm_rule') == 'positive' if self.param('vm_rule') else None,
-            equal(self._module.params.get('host_enforcing'), entity.hosts_rule.enforcing) and
+                entity.positive
-            equal(self._module.params.get('vm_rule') == 'positive', entity.vms_rule.positive) and
+            )
-            equal(self._module.params.get('vm_rule') in ['negative', 'positive'], entity.vms_rule.enabled) and
+            and equal(self._vm_ids, assigned_vms)
-            equal(self._module.params.get('host_rule') == 'positive', entity.hosts_rule.positive) and
-            equal(self._vm_ids, assigned_vms) and
-            equal(self._host_ids, assigned_hosts)
         )
+        # Following attributes is supported since 4.1,
+        # so return if it doesn't exist:
+        if not engine_supported(self._connection, '4.1'):
+            return do_update
 
-
+        # Following is supported since 4.1:
-def _get_obj_id(obj_service, obj_name):
+        return do_update and (
-    obj = search_by_name(obj_service, obj_name)
+            equal(
-    if obj is None:
+                self.param('host_rule') == 'positive' if self.param('host_rule') else None,
-        raise Exception("Object '%s' was not found." % obj_name)
+                entity.hosts_rule.positive
-    return obj.id
+            )
+            and equal(self.param('host_enforcing'), entity.hosts_rule.enforcing)
+            and equal(
+                self.param('vm_rule') in ['negative', 'positive'] if self.param('vm_rule') else None,
+                entity.vms_rule.enabled
+            )
+            and equal(self._host_ids, sorted([host.id for host in entity.hosts]))
+        )
 
 
 def main():
@@ -241,23 +292,21 @@ def main():
         supports_check_mode=True,
     )
     check_sdk(module)
-    connection = create_connection(module.params.pop('auth'))
-
-    # Check if unsupported parameters were passed:
-    supported_41 = ('host_enforcing', 'host_rule', 'hosts')
-    if not check_support(
-        version='4.1',
-        connection=connection,
-        module=module,
-        params=supported_41,
-    ):
-        module.fail_json(
-            msg='Following parameters are supported since 4.1: {params}'.format(
-                params=supported_41,
-            )
-        )
-
     try:
+        connection = create_connection(module.params.pop('auth'))
+        # Check if unsupported parameters were passed:
+        supported_41 = ('host_enforcing', 'host_rule', 'hosts')
+        if not check_support(
+            version='4.1',
+            connection=connection,
+            module=module,
+            params=supported_41,
+        ):
+            module.fail_json(
+                msg='Following parameters are supported since 4.1: {params}'.format(
+                    params=supported_41,
+                )
+            )
         clusters_service = connection.system_service().clusters_service()
         vms_service = connection.system_service().vms_service()
         hosts_service = connection.system_service().hosts_service()
@@ -270,12 +319,12 @@ def main():
 
         # Fetch VM ids which should be assigned to affinity group:
         vm_ids = sorted([
-            _get_obj_id(vms_service, vm_name)
+            get_id_by_name(vms_service, vm_name)
             for vm_name in module.params['vms']
         ]) if module.params['vms'] is not None else None
         # Fetch host ids which should be assigned to affinity group:
         host_ids = sorted([
-            _get_obj_id(hosts_service, host_name)
+            get_id_by_name(hosts_service, host_name)
             for host_name in module.params['hosts']
         ]) if module.params['hosts'] is not None else None