Add combined roles/collection requirements on project sync #14081

john-westcott-iv · 2023-06-02T20:02:10Z

SUMMARY

Previously, during a project sync, we would run ansible-galaxy in the following ways.

For roles we would run ansible-galaxy roles install for roles/requirements.y[a]ml.
For collections we would run ansible-galaxy collections install for collections/requirements.y[a]ml or requirements.y[a]ml

With this PR we will continue to install roles from roles/requirements.y[a]ml and collections from collections/requirement.y[a]ml. But we will now now install both roles and collections from requirements.y[a]ml files IF:

You are on ansible >= 2.10
You have both collection and role syncing enabled

If someone was using a requirements.y[a]ml file for only collections this will still work because a collections file is a valid combined file.

However, if you were using a requirements.y[a]ml file for collections and have role syncing disabled, you will have to move your file into the collections directory or enable role syncing in order for this file to be used.

Replaces: #13404
Fixes: #12364

TESTING

Create a project linked to SCM with a requirements.y[a]ml file at its root, a roles/requirement.y[a]ml file and a collections/requirements.y[a]ml file.
Disable role and collection syncing and sync the project.... no tasks will be run.
Enable role syncing and sync the project.... only the roles/requirements.y[a]ml file will be run.
Disable role syncing and enable collection syncing.... only the collections/requirements.y[a]ml file will be run.
Enable both role and collection syncing... all three files will be run.

If you were to change your version of ansible to < 2.10 the requirements.y[a]ml file would no longer be processed.

ISSUE TYPE

New or Enhanced Feature

COMPONENT NAME

API

AWX VERSION

ADDITIONAL INFORMATION

gamuniz · 2023-06-06T15:08:30Z

awx/playbooks/project_update.yml

+      environment: "{{ additional_collections_env | combine(galaxy_task_env) | combine(paths) }}"
+      vars:
+        paths:
+          ANSIBLE_COLLECTIONS_PATHS: "{{ projects_root }}/.__awx_cache/{{ local_path }}/stage/requirements_collections"


pretty sure the plural here is going to be deprecated soon. Maybe we want to be forward thinking?

@gamuniz that's pre-existing. Sure, rename it, but do it in the right place.

Here, it is not making any sense why | combine(paths) is added. That is, why was the paths variable introduced at all? It appears to be the same as additional_collections_env. This is, again, duplicated with the use of --roles-path and --collections-paths CLI flags in specific commands (but I would not touch this right now). This also shouldn't come at higher precedence than the user's custom galaxy_task_env setting.

In, at least my version of ansible, ansible-galaxy install does not respect the --collections-paths:

$ ansible-galaxy install --collections-path . awx.awx usage: ansible-galaxy [-h] [--version] [-v] TYPE ... ansible-galaxy: error: unrecognized arguments: --collections-path

But it does respect the ANSIBLE_COLLECTIONS_PATHS environment variable and combine takes a dict so I just make the paths dict variable to do the combine to make the generic install work.

@gamuniz do you know where the deprecation message is? I see them both in the latest docs but don't see a deprecation warning on either. I could be looking in the wrong spot though.

AlanCoding · 2023-06-09T15:33:08Z

I expect this to heavily conflict with #14065, I'd like to compare and contrast the objectives of the two and make some decisions on desired behavior.

AlanCoding · 2023-06-26T14:18:33Z

From the other PR, I agree that req_file: "{{ req_candidates |first_found }}" is a better pattern than the with_fileglob looping mechanism. So my position is now that I'd like to accept both of these. I'm looking over this to see if there's anything else that needs to be done.

AlanCoding

I don't see a good reason for ANSIBLE_ROLES_PATH to be an exception here. We should combine this with the --roles-path usage in the other task. I'd say remove --roles-path and add the env var to additional_collections_env instead of this custom paths var.

I also want to note that ANSIBLE_COLLECTIONS_PATHS is duplicated and unnecessary. This should be removed, which results in the paths: var being completely removed. This will give better consistency and a smaller diff.

john-westcott-iv · 2023-06-28T17:16:28Z

@AlanCoding take a look at the latest changes and let me know if this meets your requirements.

AlanCoding · 2023-06-28T20:17:52Z

awx/playbooks/project_update.yml

@@ -232,20 +233,37 @@
    - name: Fetch galaxy collections from collections/requirements.(yml/yaml)
      ansible.builtin.command: >
        ansible-galaxy collection install -r {{ item }}
-        --collections-path {{ projects_root }}/.__awx_cache/{{ local_path }}/stage/requirements_collections


So I'm hoping that this was already unnecessary because additional_collections_env set the same thing via ANSIBLE_COLLECTIONS_PATHS

AlanCoding · 2023-06-28T20:18:58Z

awx/playbooks/project_update.yml

@@ -190,6 +190,8 @@
  name: Install content with ansible-galaxy command if necessary
  vars:
    galaxy_task_env:  # configure in settings
+      # This environment variable is used for install roles
+      ANSIBLE_ROLES_PATH: "{{ projects_root }}/.__awx_cache/{{ local_path }}/stage/requirements_roles"


This extra var is set here

awx/awx/main/tasks/jobs.py

Line 1281 in 86f6b54

'galaxy_task_env': settings.GALAXY_TASK_ENV,

and it doesn't merge, it just overwrites anything here. The only reason the variable was put here was to establish an expectation that the CLI ansible-playbook -e will define it.

AlanCoding · 2023-06-28T20:21:16Z

awx/playbooks/project_update.yml

      ansible.builtin.command: >
        ansible-galaxy role install -r {{ item }}
-        --roles-path {{ projects_root }}/.__awx_cache/{{ local_path }}/stage/requirements_roles


Here's how I'd do the puzzle here:

if you're taking out --roles-path here, fine, counter-balance by adding ANSIBLE_ROLES_PATH to the shared environment blob, which is additional_collections_env, and adding to to environment: for this task

when you do that, obviously it's not just collections paths you're setting (which is totally fine), so just rename it to something more general, as this defines paths for collections/roles/tmp

AlanCoding · 2023-06-29T15:47:20Z

awx/playbooks/project_update.yml

@@ -190,13 +190,15 @@
  name: Install content with ansible-galaxy command if necessary
  vars:
    galaxy_task_env:  # configure in settings
-    additional_collections_env:
+    additional_galaxy_env:
      # These environment variables are used for installing collections, in addition to galaxy_task_env
      # setting the collections paths silences warnings


rewrite this comment

setting the collections paths silences warnings

That comment is written with the assumption that the functional path is provided by --collections-path and you have changed that. This now needs to be aggressively clear that this variable provides the destination collections path to install requirements into.

AlanCoding · 2023-06-29T15:58:45Z

awx/playbooks/project_update.yml

+      with_fileglob:
+        - "{{ project_path | quote }}/requirements.yaml"
+        - "{{ project_path | quote }}/requirements.yml"
+      changed_when: "'Nothing to do.' not in galaxy_combined_result.stdout"


Could your "Nothing to do" criteria also be applied to the other 2 Galaxy commands?

Also, environment: may be shared between them all, maybe we could set those with a block:

Looks like we could do collections:

ansible-galaxy collection install awx.awx Starting galaxy collection install process Nothing to do. All requested collections are already installed. If you want to reinstall them, consider using `--force`.

But not the role install:

ansible-galaxy role install geerlingguy.jenkins Starting galaxy role install process [WARNING]: - geerlingguy.jenkins (4.3.0) is already installed - use --force to change version to unspecified

In addition to extracting environment I also made a var to shorten up the command line.

AlanCoding

Looks great 👍

thanks for adding more polish to this.

…4081)

github-actions bot added the component:api label Jun 2, 2023

john-westcott-iv requested review from AlanCoding and gamuniz June 2, 2023 20:54

gamuniz reviewed Jun 6, 2023

View reviewed changes

john-westcott-iv added community and removed community labels Jun 15, 2023

AlanCoding requested changes Jun 26, 2023

View reviewed changes

AlanCoding mentioned this pull request Jun 26, 2023

fix project_update role/collection install #14065

Merged

john-westcott-iv force-pushed the requirements branch from f23e7d4 to d431a8e Compare June 28, 2023 17:14

AlanCoding reviewed Jun 28, 2023

View reviewed changes

john-westcott-iv force-pushed the requirements branch from d431a8e to d6dd4eb Compare June 29, 2023 14:16

AlanCoding reviewed Jun 29, 2023

View reviewed changes

john-westcott-iv force-pushed the requirements branch from 79b420d to d6dd4eb Compare June 29, 2023 18:30

Allowing for combined roles/collection install during project sync

939bc47

john-westcott-iv force-pushed the requirements branch from d6dd4eb to 939bc47 Compare June 29, 2023 19:10

AlanCoding mentioned this pull request Jun 30, 2023

Clarify galaxy CLI --help about install locations ansible/ansible#81159

Closed

AlanCoding approved these changes Jun 30, 2023

View reviewed changes

jay-steurer self-assigned this Jul 3, 2023

jay-steurer approved these changes Jul 4, 2023

View reviewed changes

jay-steurer merged commit 3624fe2 into ansible:devel Jul 5, 2023

eric-zadara mentioned this pull request Jul 18, 2023

Return back chdir to project sync to support project-local roles/coll… #14258

Merged

djyasin pushed a commit to djyasin/awx that referenced this pull request Sep 16, 2024

Add combined roles/collection requirements on project sync (ansible#1…

acf4fb1

…4081)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add combined roles/collection requirements on project sync #14081

Add combined roles/collection requirements on project sync #14081

john-westcott-iv commented Jun 2, 2023

gamuniz Jun 6, 2023

AlanCoding Jun 9, 2023

john-westcott-iv Jun 28, 2023

john-westcott-iv Jun 28, 2023

AlanCoding commented Jun 9, 2023

AlanCoding commented Jun 26, 2023

AlanCoding left a comment

john-westcott-iv commented Jun 28, 2023

AlanCoding Jun 28, 2023

AlanCoding Jun 28, 2023

AlanCoding Jun 28, 2023

AlanCoding Jun 29, 2023

AlanCoding Jun 29, 2023

john-westcott-iv Jun 29, 2023

AlanCoding left a comment

Add combined roles/collection requirements on project sync #14081

Add combined roles/collection requirements on project sync #14081

Conversation

john-westcott-iv commented Jun 2, 2023

SUMMARY

TESTING

ISSUE TYPE

COMPONENT NAME

AWX VERSION

ADDITIONAL INFORMATION

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlanCoding commented Jun 9, 2023

AlanCoding commented Jun 26, 2023

AlanCoding left a comment

Choose a reason for hiding this comment

john-westcott-iv commented Jun 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlanCoding left a comment

Choose a reason for hiding this comment