Increase performance of 'icinga2_object' (#389)

Author: Donien

changelogs/fragments/enhance-icinga2-objects.yml

@@ -0,0 +1,22 @@
+major_changes:
+  - |
+    The performance of the action plugin :code:`icinga2_object` has been greatly improved.
+    Instead of writing individual objects to files and later merging them,
+    they are instead now merged in memory on a per destination basis.
+    This means that configuration files no longer have to be assembled after the fact.
+
+    This also drops the :code:`order` parameter previously used to define the order in which
+    objects are written if they belong to the same destination file.
+    The new behavior only changes the order in the files but does not change the end result.
+
+    A performance gain of up to 80% has been seen in testing.
+
+known_issues:
+  - |
+    With the changes in :code:`icinga2_object` arises a problem.
+    The prior directory structure within :code:`icinga2_fragments_path` (default: :code:`/var/tmp/icinga/`) does not fit the new approach for writing configuration files.
+    Some paths that would become directories before are now treated as files.
+    If the old directory structure is present on a remote host, deployment with the new method will most likely fail due to this.
+
+    If the execution of :code:`icinga2_object` fails, deleting :code:`icinga2_fragments_path` should fix the problem.
+    This, however, is a manual step that needs to be done.

plugins/action/icinga2_object.py

@@ -4,6 +4,7 @@
 from ansible.errors import AnsibleError
 from ansible.plugins.action import ActionBase
 from ansible.utils.vars import merge_hash
+from ansible.utils.display import Display
 from ansible_collections.netways.icinga.plugins.module_utils.parse import Icinga2Parser
 
 
@@ -13,111 +14,133 @@ def run(self, tmp=None, task_vars=None):
 
         result = super(ActionModule, self).run(tmp, task_vars)
 
-        args = dict()
-        args = self._task.args.copy()
-        args = merge_hash(args.pop('args', {}), args)
-        object_type = args.pop('type', None)
-
-        if object_type not in task_vars['icinga2_object_types']:
-            raise AnsibleError('unknown Icinga object type: %s' % object_type)
-
-        #
-        # distribute to object type as module (name: icinga2_type)
-        #
-        obj = dict()
-        obj = self._execute_module(
-            module_name='icinga2_'+object_type.lower(),
-            module_args=args,
-            task_vars=task_vars,
-            tmp=tmp
-        )
-
-        if 'failed' in obj:
-            raise AnsibleError('Call to module failed: %s' % obj['msg'])
-        if 'skipped' in obj and obj['skipped']:
-            raise AnsibleError('Call to module was skipped: %s' % obj['msg'])
-
-        #
-        # file path handling for assemble
-        #
-        path = task_vars['icinga2_fragments_path'] + '/' + obj['file'] + '/'
-        file_fragment = path + obj['order'] + '_' + object_type.lower() + '-' + obj['name']
-
-        if obj['state'] != 'absent':
-            file_args = dict()
-            file_args['state'] = 'directory'
-            file_args['path'] = path
-            file_module = self._execute_module(
-                module_name='file',
-                module_args=file_args,
-                task_vars=task_vars,
-                tmp=tmp
+        arguments = dict()
+        arguments = self._task.args.copy()
+
+        # Create dict to bundle objects that will end up in the same file
+        destinations = dict()
+
+        # Deprecate order key
+        display = Display()
+        if [x for x in arguments['objects'] if 'order' in x]:
+            display.deprecated(
+                'The \'order\' parameter for different object types is deprecated. It no longer has any effect.'
             )
-            result = merge_hash(result, file_module)
 
-            varlist = list()  # list of variables from 'apply for'
+        for args in arguments['objects']:
+            args = merge_hash(args.pop('args', {}), args)
+            object_type = args.pop('type', None)
+
+            if object_type not in task_vars['icinga2_object_types']:
+                raise AnsibleError('unknown Icinga object type: %s' % object_type)
 
             #
-            # quoting of object name?
+            # distribute to object type as module (name: icinga2_type)
             #
-            if obj['name'] not in task_vars['icinga2_combined_constants']:
-                object_name = '"' + obj['name'] + '"'
-            else:
-                object_name = obj['name']
+            obj = dict()
+            obj = self._execute_module(
+                module_name='icinga2_'+object_type.lower(),
+                module_args=args,
+                task_vars=task_vars,
+                tmp=tmp
+            )
+
+            if 'failed' in obj:
+                raise AnsibleError('Call to module failed: %s' % obj['msg'])
+            if 'skipped' in obj and obj['skipped']:
+                raise AnsibleError('Call to module was skipped: %s' % obj['msg'])
 
             #
-            # apply rule?
+            # file path handling for assemble
             #
-            if 'apply' in obj and obj['apply'] and not obj['args']['assign']:
-                raise AnsibleError('Apply rule %s is missing the assign rule.' % obj['name'])
-            if 'apply' in obj and obj['apply']:
-                object_content = 'apply ' + object_type
-                if 'apply_target' in obj and obj['apply_target']:
-                    object_content += ' ' + object_name + ' to ' + obj['apply_target']
-                elif 'apply_for' in obj and obj['apply_for']:
-                    object_content += ' for (' + obj['apply_for'] + ') '
-                    r = re.search(r'^(.+)\s+in\s+', obj['apply_for'])
-                    if r:
-                        tmp = r.group(1).strip()
-                        r = re.search(r'^(.+)=>(.+)$', tmp)
+            path = task_vars['icinga2_fragments_path'] + '/' + obj['file']
+            if path not in destinations:
+                destinations[path] = list()
+
+            if obj['state'] != 'absent':
+                varlist = list()  # list of variables from 'apply for'
+
+                #
+                # quoting of object name?
+                #
+                if obj['name'] not in task_vars['icinga2_combined_constants']:
+                    object_name = '"' + obj['name'] + '"'
+                else:
+                    object_name = obj['name']
+
+                #
+                # apply rule?
+                #
+                if 'apply' in obj and obj['apply'] and not obj['args']['assign']:
+                    raise AnsibleError('Apply rule %s is missing the assign rule.' % obj['name'])
+                if 'apply' in obj and obj['apply']:
+                    object_content = 'apply ' + object_type
+                    if 'apply_target' in obj and obj['apply_target']:
+                        object_content += ' ' + object_name + ' to ' + obj['apply_target']
+                    elif 'apply_for' in obj and obj['apply_for']:
+                        object_content += ' for (' + obj['apply_for'] + ') '
+                        r = re.search(r'^(.+)\s+in\s+', obj['apply_for'])
                         if r:
-                            varlist.extend([r.group(1).strip(), r.group(2).strip()])
-                        else:
-                            varlist.append(tmp)
+                            tmp = r.group(1).strip()
+                            r = re.search(r'^(.+)=>(.+)$', tmp)
+                            if r:
+                                varlist.extend([r.group(1).strip(), r.group(2).strip()])
+                            else:
+                                varlist.append(tmp)
+                    else:
+                        object_content += ' ' + object_name
+                #
+                # template?
+                #
+                elif 'template' in obj and obj['template']:
+                    object_content = 'template ' + object_type + ' ' + object_name
+                #
+                # object
+                #
                 else:
-                    object_content += ' ' + object_name
-            #
-            # template?
-            #
-            elif 'template' in obj and obj['template']:
-                object_content = 'template ' + object_type + ' ' + object_name
-            #
-            # object
-            #
-            else:
-                object_content = 'object ' + object_type + ' ' + object_name
-            object_content += ' {\n'
+                    object_content = 'object ' + object_type + ' ' + object_name
+                object_content += ' {\n'
 
-            #
-            # imports?
-            #
-            if 'imports' in obj:
-                for item in obj['imports']:
-                    object_content += '  import "' + str(item) + '"\n'
-                object_content += '\n'
+                #
+                # imports?
+                #
+                if 'imports' in obj:
+                    for item in obj['imports']:
+                        object_content += '  import "' + str(item) + '"\n'
+                    object_content += '\n'
+
+                #
+                # parser
+                #
+                object_content += Icinga2Parser().parse(obj['args'], list(task_vars['icinga2_combined_constants'].keys())+task_vars['icinga2_reserved']+varlist+list(obj['args'].keys()), 2) + '}\n'
+                destinations[path] += [object_content]
+
+
+
+        for destination, objects in destinations.items():
+            # Remove duplicate entries and sort list to ensure idempotency
+            objects = list(set(objects))
+            objects.sort()
+
+            config_string = '\n\n'.join(objects)
+
+            file_args = dict()
+            file_args['state'] = 'directory'
+            file_args['path'] = '/'.join(destination.split('/')[:-1])
+            file_module = self._execute_module(
+                module_name='file',
+                module_args=file_args,
+                task_vars=task_vars,
+                tmp=tmp
+            )
+            result = merge_hash(result, file_module)
+
+            varlist = list()  # list of variables from 'apply for'
 
-            #
-            # parser
-            #
-            object_content += Icinga2Parser().parse(
-                obj['args'],
-                list(task_vars['icinga2_combined_constants'].keys()) + task_vars['icinga2_reserved'] + varlist + list(obj['args'].keys()),
-                2
-            ) + '}\n'
             copy_action = self._task.copy()
             copy_action.args = dict()
-            copy_action.args['dest'] = file_fragment
-            copy_action.args['content'] = object_content
+            copy_action.args['dest'] = destination
+            copy_action.args['content'] = config_string
 
             copy_action = self._shared_loader_obj.action_loader.get(
                 'copy',
@@ -130,19 +153,6 @@ def run(self, tmp=None, task_vars=None):
                 )
 
             result = merge_hash(result, copy_action.run(task_vars=task_vars))
-        else:
-            # remove file if does not belong to a feature
-            if 'features-available' not in path:
-                file_args = dict()
-                file_args['state'] = 'absent'
-                file_args['path'] = file_fragment
-                file_module = self._execute_module(
-                    module_name='file',
-                    module_args=file_args,
-                    task_vars=task_vars,
-                    tmp=tmp
-                )
-                result = merge_hash(result, file_module)
-            result['dest'] = file_fragment
 
+        result['destinations'] = list(destinations.keys())
         return result

roles/icinga2/defaults/main.yml

@@ -14,6 +14,7 @@ icinga2_features:
   - name: checker
   - name: notification
   - name: mainlog
+icinga2_objects: []
 icinga2_remote_objects: []
 _icinga2_custom_conf_paths: []
 icinga2_config_host: "{{ ansible_fqdn }}"