Pull roles from ansible galaxy before running a playbook #19079

carbonin · 2019-07-30T16:12:50Z

This PR installs any roles defined in the requirements.yml file into the roles directory next to the requested playbook right before the playbook is run. It also gives ansible-runner some more information about the playbook context so it knows to use the roles we pull down rather than looking for them in the system roles path (where it will not find them).

Previously we specified the playbook as an absolute path rather than copying the playbook into the ansible runner project directory. Using this option will also allow us to install roles in the checked out repo directory rather than having to copy the playbook into the runner context directory and installing the roles there.

This is where AWX looks for the requirements.yml file ref: https://github.com/ansible/awx/blob/128fa8947ac620add275a15cb07577178745a849/awx/playbooks/project_update.yml#L146-L148 We still support the requirements in the base content directory because that's where we expect it to be for plugins, but if both exist we prefer the runtime case (in the roles directory) rather than the case we expect only at build time.

NickLaMuro

So, in one of your commits, you say this:

We still support the requirements in the base content directory because that's where we expect it to be for plugins

Are "plugins" in this case stuff defined for manageiq-content, or something else? I guess I am not sure where this previous functionality was used, and am only guessing. I assume since we are favoring the roles_dir_file, that is probably for the best.

Otherwise, I think I am good with everything else you have done here.

lib/ansible/runner.rb

Fryguy · 2019-07-30T19:33:38Z

@carbonin You have trailing whitespace issues (seems like blank lines)...can you fix?

carbonin · 2019-07-30T19:35:44Z

@carbonin You have trailing whitespace issues (seems like blank lines)...can you fix?

I think I fixed that @Fryguy bot commented about an hour ago with no issues (it was stuck for about an hour previously)

NickLaMuro

I think the 📖 below is correct, but please check me on that. Given it is, though, I think this needs some changes (should be minor though)

lib/ansible/runner.rb

carbonin · 2019-07-31T17:22:41Z

Marking this as WIP while I make the changes @NickLaMuro requested.

This is needed because the roles directory location is located relative to the playbook, not always the root of the repo. This will allow us to handle a case where a repo is configured with the following directory structure: azure_servers/ roles/ azure_stuffs/ requirements.yml azure_playbook.yml aws_servers/ roles/ s3_stuffs/ ec2_config/ aws_playbook.yml s3_playbook.yml localhost/ requirements.yml localhost_playbook.yml Where previously we would not have installed any roles because we wouldn't have found a /roles directory at the top-level of the repo directory. Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1734904

carbonin · 2019-07-31T20:30:44Z

This also fixes https://bugzilla.redhat.com/show_bug.cgi?id=1734904

miq-bot · 2019-07-31T20:31:00Z

Checked commits carbonin/manageiq@40a2244~...6bfcc74 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
4 files checked, 0 offenses detected
Everything looks fine. ⭐

carbonin · 2019-07-31T20:35:18Z

The BZ @pemcg opened references https://www.ansible.com/blog/using-ansible-and-ansible-tower-with-shared-roles which mentions the following about how Tower (and by extension, AWX) expects repos to be structured:

we strongly recommend to stick to the directory name roles, sitting in the root of your project directory. Do not choose another name for it or move it to some subdirectory.

Personally, I like the solution we have here so we can support multiple roles directories in the same repo, but it still feels like we may be opening ourselves up to a small corner case issue where someone might expect the situation I mentioned above to work. Thoughts?

@pemcg can you give any insight into how these repos tend to be structured?

pemcg · 2019-08-01T13:04:36Z

@carbonin in my experience most repos used for Tower/embedded Ansible do have this relatively flat structure with a single roles directory at the root of the project directory. I'm not even sure things work as expected if this convention is not used. @branic might have more 'real world' experience with this.

carbonin · 2019-08-01T13:45:13Z

@pemcg I guess the real question for this patch is if a repo is set up such that there is one roles directory in the repo root. Would we expect those roles to be available to a playbook in a sub-directory?

We're operating currently on the answer being "no".

carbonin · 2019-08-01T13:50:09Z

Ansible best practices seem to be that the roles directory be next to the playbook being executed based on https://docs.ansible.com/ansible/latest/user_guide/playbooks_reuse_roles.html#role-search-path, but as I linked before, Tower explicitly requests them to be at the root of the repository.

pemcg · 2019-08-01T13:54:40Z

@carbonin I would also expect that to be "no".

I think it's reasonable for the behaviour of embedded Ansible in this regard to match that of Tower as that's what users are accustomed to. So any role defined in the roles directory in the project root should only be usable by playbooks also in the root.

Fryguy · 2019-08-01T16:11:34Z

lib/ansible/content.rb

+        local_file     = path.join('requirements.yml')
+        roles_dir_file = roles_dir.join('requirements.yml')
+
+        roles_dir_file.exist? ? roles_dir_file : local_file


@agrare This code is technically a shim for the fact that provider plugins have the requirements.yml at a different location. I'd like to get those all consistent, then this code can be simplified.

Okay, looks like only nuage and lenovo have a content/ansible_runner/requirements.yml

I assume it should be in content/ansible_runner/roles/requirements.yml ?

Yeah, @agrare

ManageIQ/manageiq-providers-nuage#185
ManageIQ/manageiq-providers-lenovo#276

Fryguy · 2019-08-01T16:13:30Z

lib/ansible/runner.rb

@@ -216,6 +216,7 @@ def run_via_cli(hosts, credentials, env_vars, extra_vars, tags: nil, ansible_run
        params = runner_params(base_dir, ansible_runner_method, playbook_or_role_args, verbosity)

        begin
+          fetch_galaxy_roles(playbook_or_role_args)


@carbonin This may change the nature of provider-based playbooks where they were expecting a one-time galaxy install at build time, and now it's that plus all the time.

Fryguy

I'm good with this as is (aside from my question about the nature change for existing provider-based playbooks). I think that the subdirectory question we were going to handle separately, and the current implementation doesn't prevent us from doing that.

branic · 2019-08-01T17:52:33Z

@pemcg @carbonin

In general I've seen the roles directory next to the playbooks in a flat structure. However, I have occasionally seen a more complex structure where playbooks are in one directory and then roles are in a subdirectory of a different directory and an ansible.cfg file is used to set the roles_path

ansible.cfg
playbooks/
    playbook1.yml
    playbook2.yml
common/
    roles/
        role1/
            tasks/
                main.yml
        role2/
            tasks/
                main.yml

I don't think we necessarily need to support random directory structures as long as long its documented what the expected and supported structure is.

NickLaMuro · 2019-08-01T18:05:12Z

I don't think we necessarily need to support random directory structures as long as long its documented what the expected and supported structure is.

@branic I wouldn't say we aren't supporting the above, we just aren't going to auto detect something like this, and I don't think that Ansible Tower / awx did either. You should still be able to do this by configuring your playbook run to look for roles in a non-standard location via extra_vars or ENV variables with either though (if I am not mistaken)

cc @pemcg @carbonin

NickLaMuro

Oops, I think I was the hold out here, and Nick has addressed my previous comment:

https://github.com/ManageIQ/manageiq/pull/19079/files#r309016454

So 🐑 🇮🇹 !

Pull roles from ansible galaxy before running a playbook (cherry picked from commit e783031) https://bugzilla.redhat.com/show_bug.cgi?id=1734904

simaishi · 2019-08-01T22:37:39Z

Ivanchuk backport details:

$ git log -1
commit 1990eb4358ac3c20ceccf45166da17966c6b4c9f
Author: Jason Frey <[email protected]>
Date:   Thu Aug 1 15:51:48 2019 -0400

    Merge pull request #19079 from carbonin/pull_roles_from_ansible_galaxy
    
    Pull roles from ansible galaxy before running a playbook
    
    (cherry picked from commit e783031e055ccfffda09dc1b1f2c51a557710b95)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1734904

carbonin added enhancement core changelog/yes core/embedded ansible ivanchuk/yes labels Jul 30, 2019

carbonin requested a review from NickLaMuro July 30, 2019 16:12

carbonin assigned Fryguy Jul 30, 2019

carbonin added 3 commits July 30, 2019 13:40

Fetch roles from ansible galaxy after cloning the playbook repo

c603ac6

Allow strings or pathnames in Ansible::Content

2aba2be

carbonin force-pushed the pull_roles_from_ansible_galaxy branch from 1f07a71 to 2aba2be Compare July 30, 2019 17:40

NickLaMuro reviewed Jul 30, 2019

View reviewed changes

Fryguy reviewed Jul 30, 2019

View reviewed changes

lib/ansible/runner.rb Show resolved Hide resolved

NickLaMuro suggested changes Jul 31, 2019

View reviewed changes

lib/ansible/runner.rb Show resolved Hide resolved

carbonin added the wip label Jul 31, 2019

carbonin changed the title ~~Pull roles from ansible galaxy on repo clone~~ [WIP] Pull roles from ansible galaxy on repo clone Jul 31, 2019

carbonin changed the title ~~[WIP] Pull roles from ansible galaxy on repo clone~~ Pull roles from ansible galaxy on repo clone Jul 31, 2019

carbonin removed the wip label Jul 31, 2019

carbonin changed the title ~~Pull roles from ansible galaxy on repo clone~~ Pull roles from ansible galaxy before running a playbook Jul 31, 2019

Fryguy reviewed Aug 1, 2019

View reviewed changes

Fryguy approved these changes Aug 1, 2019

View reviewed changes

NickLaMuro approved these changes Aug 1, 2019

View reviewed changes

Fryguy merged commit e783031 into ManageIQ:master Aug 1, 2019

Fryguy added this to the Sprint 117 Ending Aug 5, 2019 milestone Aug 1, 2019

carbonin deleted the pull_roles_from_ansible_galaxy branch August 1, 2019 19:59

simaishi added ivanchuk/backported and removed ivanchuk/yes labels Aug 1, 2019

NickLaMuro mentioned this pull request Aug 2, 2019

[EmbeddedAnsible] Handle nested playbooks #19089

Merged

carbonin mentioned this pull request Aug 2, 2019

[RFC] EmbeddedAnsible with ansible-runner-based implementation ManageIQ/manageiq-design#45

Closed

Fryguy mentioned this pull request Aug 9, 2019

Remove Ansible::Content requirements.yml location shim #19131

Merged

dmetzger57 mentioned this pull request Jul 17, 2020

Embedded Ansible - Add support for ansible.cfg files in user repositories ManageIQ/manageiq-automation_engine#447

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pull roles from ansible galaxy before running a playbook #19079

Pull roles from ansible galaxy before running a playbook #19079

carbonin commented Jul 30, 2019 •

edited

Loading

NickLaMuro left a comment

Fryguy commented Jul 30, 2019

carbonin commented Jul 30, 2019 •

edited

Loading

NickLaMuro left a comment

carbonin commented Jul 31, 2019

carbonin commented Jul 31, 2019

miq-bot commented Jul 31, 2019

carbonin commented Jul 31, 2019

pemcg commented Aug 1, 2019

carbonin commented Aug 1, 2019

carbonin commented Aug 1, 2019

pemcg commented Aug 1, 2019

Fryguy Aug 1, 2019

agrare Aug 1, 2019

agrare Aug 1, 2019

carbonin Aug 1, 2019

agrare Aug 9, 2019

Fryguy Aug 9, 2019

Fryguy Aug 1, 2019

Fryguy left a comment

branic commented Aug 1, 2019

NickLaMuro commented Aug 1, 2019 •

edited

Loading

NickLaMuro left a comment

simaishi commented Aug 1, 2019

Pull roles from ansible galaxy before running a playbook #19079

Pull roles from ansible galaxy before running a playbook #19079

Conversation

carbonin commented Jul 30, 2019 • edited Loading

NickLaMuro left a comment

Choose a reason for hiding this comment

Fryguy commented Jul 30, 2019

carbonin commented Jul 30, 2019 • edited Loading

NickLaMuro left a comment

Choose a reason for hiding this comment

carbonin commented Jul 31, 2019

carbonin commented Jul 31, 2019

miq-bot commented Jul 31, 2019

carbonin commented Jul 31, 2019

pemcg commented Aug 1, 2019

carbonin commented Aug 1, 2019

carbonin commented Aug 1, 2019

pemcg commented Aug 1, 2019

Fryguy Aug 1, 2019

Choose a reason for hiding this comment

agrare Aug 1, 2019

Choose a reason for hiding this comment

agrare Aug 1, 2019

Choose a reason for hiding this comment

carbonin Aug 1, 2019

Choose a reason for hiding this comment

agrare Aug 9, 2019

Choose a reason for hiding this comment

Fryguy Aug 9, 2019

Choose a reason for hiding this comment

Fryguy Aug 1, 2019

Choose a reason for hiding this comment

Fryguy left a comment

Choose a reason for hiding this comment

branic commented Aug 1, 2019

NickLaMuro commented Aug 1, 2019 • edited Loading

NickLaMuro left a comment

Choose a reason for hiding this comment

simaishi commented Aug 1, 2019

carbonin commented Jul 30, 2019 •

edited

Loading

carbonin commented Jul 30, 2019 •

edited

Loading

NickLaMuro commented Aug 1, 2019 •

edited

Loading