Merge pull request #6186 from AlanCoding/scm_inv_stability

SCM Inventory task system and variables stability edits

awx/api/serializers.py

@@ -1603,6 +1603,7 @@ class InventorySourceSerializer(UnifiedJobTemplateSerializer, InventorySourceOpt
         update_on_launch = attrs.get('update_on_launch', self.instance and self.instance.update_on_launch)
         update_on_project_update = get_field_from_model_or_attrs('update_on_project_update')
         source = get_field_from_model_or_attrs('source')
+        overwrite_vars = get_field_from_model_or_attrs('overwrite_vars')
 
         if attrs.get('source_path', None) and source!='scm':
             raise serializers.ValidationError({"detail": _("Cannot set source_path if not SCM type.")})
@@ -1611,6 +1612,9 @@ class InventorySourceSerializer(UnifiedJobTemplateSerializer, InventorySourceOpt
         elif not self.instance and attrs.get('inventory', None) and InventorySource.objects.filter(
                 inventory=attrs.get('inventory', None), update_on_project_update=True, source='scm').exists():
             raise serializers.ValidationError({"detail": _("Inventory controlled by project-following SCM.")})
+        elif source=='scm' and not overwrite_vars:
+            raise serializers.ValidationError({"detail": _(
+                "SCM type sources must set `overwrite_vars` to `true` until a future Tower release.")})
 
         return super(InventorySourceSerializer, self).validate(attrs)
 

awx/main/management/commands/inventory_import.py

@@ -68,12 +68,17 @@ class AnsibleInventoryLoader(object):
 
     def __init__(self, source, group_filter_re=None, host_filter_re=None, is_custom=False):
         self.source = source
+        self.source_dir = functioning_dir(self.source)
         self.is_custom = is_custom
         self.tmp_private_dir = None
         self.method = 'ansible-inventory'
         self.group_filter_re = group_filter_re
         self.host_filter_re = host_filter_re
 
+        self.is_vendored_source = False
+        if self.source_dir == os.path.join(settings.BASE_DIR, 'plugins', 'inventory'):
+            self.is_vendored_source = True
+
     def build_env(self):
         # Use ansible venv if it's available and setup to use
         env = dict(os.environ.items())
@@ -93,9 +98,16 @@ class AnsibleInventoryLoader(object):
         for path in os.environ["PATH"].split(os.pathsep):
             potential_path = os.path.join(path.strip('"'), 'ansible-inventory')
             if os.path.isfile(potential_path) and os.access(potential_path, os.X_OK):
+                logger.debug('Using system install of ansible-inventory CLI: {}'.format(potential_path))
                 return [potential_path, '-i', self.source]
 
-        # ansible-inventory was not found, look for backported module
+        # Stopgap solution for group_vars, do not use backported module for official
+        # vendored cloud modules or custom scripts TODO: remove after Ansible 2.3 deprecation
+        if self.is_vendored_source or self.is_custom:
+            self.method = 'inventory script invocation'
+            return [self.source]
+
+        # ansible-inventory was not found, look for backported module TODO: remove after Ansible 2.3 deprecation
         abs_module_path = os.path.abspath(os.path.join(
             os.path.dirname(__file__), '..', '..', '..', 'plugins',
             'ansible_inventory', 'backport.py'))
@@ -103,10 +115,10 @@ class AnsibleInventoryLoader(object):
 
         if not os.path.exists(abs_module_path):
             raise ImproperlyConfigured('Can not find inventory module')
+        logger.debug('Using backported ansible-inventory module: {}'.format(abs_module_path))
         return [abs_module_path, '-i', self.source]
 
     def get_proot_args(self, cmd, env):
-        source_dir = functioning_dir(self.source)
         cwd = os.getcwd()
         if not check_proot_installed():
             raise RuntimeError("proot is not installed but is configured for use")
@@ -114,9 +126,9 @@ class AnsibleInventoryLoader(object):
         kwargs = {}
         if self.is_custom:
             # use source's tmp dir for proot, task manager will delete folder
-            logger.debug("Using provided directory '{}' for isolation.".format(source_dir))
+            logger.debug("Using provided directory '{}' for isolation.".format(self.source_dir))
-            kwargs['proot_temp_dir'] = source_dir
+            kwargs['proot_temp_dir'] = self.source_dir
-            cwd = source_dir
+            cwd = self.source_dir
         else:
             # we can not safely store tmp data in source dir or trust script contents
             if env['AWX_PRIVATE_DATA_DIR']:
@@ -147,11 +159,9 @@ class AnsibleInventoryLoader(object):
 
         if self.tmp_private_dir:
             shutil.rmtree(self.tmp_private_dir, True)
-        if proc.returncode != 0:
+        if proc.returncode != 0 or 'file not found' in stderr:
-            raise RuntimeError('%s failed (rc=%d) with output:\n%s' % (self.method, proc.returncode, stderr))
+            raise RuntimeError('%s failed (rc=%d) with stdout:\n%s\nstderr:\n%s' % (
-        elif 'file not found' in stderr:
+                self.method, proc.returncode, stdout, stderr))
-            # File not visible to inventory module due proot (exit code 0, Ansible behavior)
-            raise IOError('Inventory module failed to find source {} with output:\n{}.'.format(self.source, stderr))
 
         try:
             data = json.loads(stdout)

awx/main/tasks.py

@@ -687,7 +687,12 @@ class BaseTask(Task):
 
     def post_run_hook(self, instance, status, **kwargs):
         '''
-        Hook for any steps to run after job/task is complete.
+        Hook for any steps to run before job/task is marked as complete.
+        '''
+
+    def final_run_hook(self, instance, status, **kwargs):
+        '''
+        Hook for any steps to run after job/task is marked as complete.
         '''
 
     def run(self, pk, **kwargs):
@@ -778,10 +783,11 @@ class BaseTask(Task):
         if instance.cancel_flag:
             status = 'canceled'
 
+        self.post_run_hook(instance, status, **kwargs)
         instance = self.update_model(pk, status=status, result_traceback=tb,
                                      output_replacements=output_replacements,
                                      **extra_update_fields)
-        self.post_run_hook(instance, status, **kwargs)
+        self.final_run_hook(instance, status, **kwargs)
         instance.websocket_emit_status(status)
         if status != 'successful' and not hasattr(settings, 'CELERY_UNIT_TEST'):
             # Raising an exception will mark the job as 'failed' in celery
@@ -1152,11 +1158,8 @@ class RunJob(BaseTask):
                                                          ('project_update', local_project_sync.name, local_project_sync.id)))
                 raise
 
-    def post_run_hook(self, job, status, **kwargs):
+    def final_run_hook(self, job, status, **kwargs):
-        '''
+        super(RunJob, self).final_run_hook(job, status, **kwargs)
-        Hook for actions to run after job/task has completed.
-        '''
-        super(RunJob, self).post_run_hook(job, status, **kwargs)
         try:
             inventory = job.inventory
         except Inventory.DoesNotExist:
@@ -1443,15 +1446,19 @@ class RunProjectUpdate(BaseTask):
             if len(dependent_inventory_sources) > 0:
                 p.inventory_files = p.inventories
             p.save()
-        try:
+
-            os.remove(self.revision_path)
-        except Exception, e:
-            logger.error("Failed removing revision tmp file: {}".format(e))
         # Update any inventories that depend on this project
         if len(dependent_inventory_sources) > 0:
             if status == 'successful' and instance.launch_type != 'sync':
                 self._update_dependent_inventories(instance, dependent_inventory_sources)
 
+    def final_run_hook(self, instance, status, **kwargs):
+        super(RunProjectUpdate, self).final_run_hook(instance, status, **kwargs)
+        try:
+            os.remove(self.revision_path)
+        except Exception, e:
+            logger.error("Failed removing revision tmp file: {}".format(e))
+
 
 class RunInventoryUpdate(BaseTask):
 
@@ -1857,8 +1864,8 @@ class RunInventoryUpdate(BaseTask):
                                      ('project_update', local_project_sync.name, local_project_sync.id)))
                 raise
 
-    def post_run_hook(self, instance, status, **kwargs):
+    def final_run_hook(self, instance, status, **kwargs):
-        print("In post run hook")
+        print("In final run hook")
         if self.custom_dir_path:
             for p in self.custom_dir_path:
                 try:
@@ -2058,11 +2065,11 @@ class RunAdHocCommand(BaseTask):
         '''
         return getattr(settings, 'AWX_PROOT_ENABLED', False)
 
-    def post_run_hook(self, ad_hoc_command, status, **kwargs):
+    def final_run_hook(self, ad_hoc_command, status, **kwargs):
         '''
-        Hook for actions to run after ad hoc command has completed.
+        Hook for actions to run after ad hoc command is marked as completed.
         '''
-        super(RunAdHocCommand, self).post_run_hook(ad_hoc_command, status, **kwargs)
+        super(RunAdHocCommand, self).final_run_hook(ad_hoc_command, status, **kwargs)
 
 
 class RunSystemJob(BaseTask):

awx/main/tests/unit/test_tasks.py

@@ -224,7 +224,7 @@ class TestJobExecution:
         self.task.run_pexpect = mock.Mock(return_value=['successful', 0])
 
         # ignore pre-run and post-run hooks, they complicate testing in a variety of ways
-        self.task.pre_run_hook = self.task.post_run_hook = mock.Mock()
+        self.task.pre_run_hook = self.task.post_run_hook = self.task.final_run_hook = mock.Mock()
 
     def teardown_method(self, method):
         for p in self.patches:

docs/scm_inventory.md

@@ -1,4 +1,4 @@
-# SCM Flat File Inventory
+# SCM Inventory
 
 Users can create inventory sources that use content in the source tree of
 a project as an Ansible inventory file.
@@ -17,14 +17,29 @@ Additionally:
 
  - `source_vars` - if these are set on a "file" type inventory source
    then they will be passed to the environment vars when running
+ - `update_on_project_update` - if set, a project update of the source
+   project will automatically update this inventory source as a side effect
 
-A user should not be able to update this inventory source via through
+If `update_on_project_update` is not set, then they can manually update
-the endpoint `/inventory_sources/N/update/`. Instead, they should update
+just the inventory source with a POST to its update endpoint,
-the linked project.
+`/inventory_sources/N/update/`.
 
-An update of the project automatically triggers an inventory update within
+If `update_on_project_update` is set, the POST to the inventory source's
-the proper context. An update _of the project_ is scheduled immediately
+update endpoint will trigger an update of the source project, which may,
-after creation of the inventory source.
+in turn, trigger an update of the inventory source.
+Also, with this flag set, an update _of the project_ is
+scheduled immediately after creation of the inventory source.
+Also, if this flag is set, no inventory updates will be triggered
+_unless the scm revision of the project changes_.
+
+### RBAC
+
+User needs `admin` role to the project in order to use it as a source
+project for inventory (this entails permission to run arbitrary scripts).
+To update the project, they need `update` permission to the project,
+even if the update is done indirectly.
+
+### Inventory File Suggestions
 
 The project should show a listing of suggested inventory locations, at the
 endpoint `/projects/N/inventories/`, but this is not a comprehensive list of
@@ -32,6 +47,7 @@ all paths that could be used as an Ansible inventory because of the wide
 range of inclusion criteria. The list will also max out at 50 entries.
 The user should be allowed to specify a location manually in the UI.
 This listing should be refreshed to latest SCM info on a project update.
+
 If no inventory sources use a project as an SCM inventory source, then
 the inventory listing may not be refreshed on update.
 
@@ -47,7 +63,7 @@ update the project.
 > Any Inventory Ansible supports should be supported by this feature
 
 This is accomplished by making use of the `ansible-inventory` command.
-the inventory import tower-manage command will check for the existnce
+the inventory import tower-manage command will check for the existence
 of `ansible-inventory` and if it is not present, it will call a backported
 version of it. The backport is maintained as its own GPL3 licensed
 repository.
@@ -83,9 +99,9 @@ standard use.
 
 ## Update-on-launch
 
-This type of inventory source will not allow the `update_on_launch` field
+If the SCM inventory source is configured to follow the project updates,
-to be set to True. This is because of concerns related to the task
+the `update_on_launch` field can not to be set to True. This is because
-manager job dependency tree.
+of concerns related to the task manager job dependency tree.
 
 We should document the alternatives for a user to accomplish the same thing
 through in a different way.
@@ -110,8 +126,3 @@ until the inventory update is finished.
 
 Note that a failed inventory update does not mark the project as failed.
 
-## Lazy inventory updates
-
-It should also be noted that not every project update will trigger a
-corresponding inventory update. If the project revision has not changed
-and the inventory has not been edited, the inventory update will not fire.