[EmbeddedAnsible] Handle nested playbooks #19089

NickLaMuro · 2019-08-01T19:40:35Z

As mentioned by:

https://bugzilla.redhat.com/show_bug.cgi?id=1734446

We currently don't support nested playbooks with EmbeddedAnsible using AnsibleRunner. This was an oversight that Ansible Tower / awx gave us for free, so this implements what those two were doing in the past.

Of note, we are saving the full relative file path for the name now in the ConfigurationScriptPayload record. This is what was done previously when doing a refresh, but worth noting.

Links

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1734446
awx links:

Steps for Testing/QA

Need to test this with an actual playbook run still...

NickLaMuro · 2019-08-01T19:42:56Z

@miq-bot add_label core, embedded ansible, bug

@Fryguy @carbonin please review

...models/manageiq/providers/embedded_ansible/automation_manager/configuration_script_source.rb

lib/git_worktree.rb

...s/manageiq/providers/embedded_ansible/automation_manager/configuration_script_source_spec.rb

spec/support/fake_ansible_repo.rb

Fryguy · 2019-08-01T21:00:45Z

I'm good with this testing style and FakeAnsibleRepo helper class. It seems like we could even reuse this helper elsewhere. I just don't like the spec + String -> gsub | YAML.parse -> Hash when we can just do spec + Hash.

NickLaMuro · 2019-08-05T18:55:41Z

Still working on trying out some of the other fixes for this PR (supporting $ANSIBLE_VAULT, and adjusting walk_blobs and such), but pushed up the changes for a non-yaml form of the helper. I left the previous version in a new branch for posterity:

master...NickLaMuro:well_at_least_I_thought_it_was_cool

And will continue working on the other changes after I break for lunch.

spec/support/fake_ansible_repo.rb

carbonin · 2019-08-08T13:40:55Z

@Fryguy is this ready to go outside of the typo? Also @NickLaMuro can you add the BZ link to at least one commit (or ideally all of them)?

With the current GitRepository#entries method, the results returned are only the top level files. In this patch a new method, `GitWorktree#blob_list`, was added to list only files (no directory entires) for a given ref (or the current one). Using this in place of `.entries` allows us to find nested playbooks properly. Partially fixes https://bugzilla.redhat.com/show_bug.cgi?id=1734446

NickLaMuro · 2019-08-08T15:39:40Z

@Fryguy @carbonin This should be good for a final look. As part of the rebase, I should have addressed any outstanding comments and feedback, and as a result the logic has changed a bit. Please do take a look and let me know if I botched anything as a result.

spec/support/fake_ansible_repo.rb

...models/manageiq/providers/embedded_ansible/automation_manager/configuration_script_source.rb

Fryguy

LGTM...up to you if you care about those 2 comments above.

When syncing playbooks from the GitRepository, make sure we are only pulling in files that match a playbook extension (yaml), and have the "looks of a playbook". Basically, they should include one of the following lines: "- hosts: ..." "- import_playbook: ..." "- include:" (sans-quotes) So if there is a `hosts` line, or we are importing another playbook or role, it is a playbook. Logic for this commit borrowed from AWX: https://github.com/ansible/awx/blob/128fa894/awx/main/utils/ansible.py#L17 https://github.com/ansible/awx/blob/128fa894/awx/main/utils/ansible.py#L39-L66 Partially fixes https://bugzilla.redhat.com/show_bug.cgi?id=1734446

Any file which it's content starts with `"$ANSIBLE_VAULT"` is considered a "playbook" by AWX: https://github.com/ansible/awx/blob/1242ee2b6584f32cbc4267145ee0c0c6bc845049/awx/main/utils/ansible.py#L59 So we are going to replicate that as well... Partially fixes https://bugzilla.redhat.com/show_bug.cgi?id=1734446

Role directories, and others, shouldn't be included with the list of executable playbooks for EmbeddedAnsible, so this filters them out. Logic for `.playbook_dir?` pulled from awx: https://github.com/ansible/awx/blob/128fa894/awx/main/utils/ansible.py#L21-L36 Partially fixes https://bugzilla.redhat.com/show_bug.cgi?id=1734446

Playbooks shouldn't be in hidden directories, or considered when they are a hidden file either. Partially fixes https://bugzilla.redhat.com/show_bug.cgi?id=1734446

miq-bot · 2019-08-08T19:20:56Z

Checked commits NickLaMuro/manageiq@652105e~...efd8050 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. 🏆

…d-playbooks [EmbeddedAnsible] Handle nested playbooks (cherry picked from commit 1d47fd7) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1734446

simaishi · 2019-08-08T21:55:06Z

Ivanchuk backport details:

$ git log -1
commit cd0cdd9a65674e135dcbbcdfdca929f98fa0ccba
Author: Nick Carboni <[email protected]>
Date:   Thu Aug 8 17:12:43 2019 -0400

    Merge pull request #19089 from NickLaMuro/embedded-ansible-find-nested-playbooks
    
    [EmbeddedAnsible] Handle nested playbooks
    
    (cherry picked from commit 1d47fd71df6ce000d27c557aac80a75340592a26)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1734446

simaishi · 2019-08-09T12:39:24Z

@NickLaMuro Travis is failing in ~~manageiq-providers-vmware~~ multiple repos, please take a look.
https://travis-ci.org/ManageIQ/manageiq-providers-vmware/jobs/569665969

An error occurred while loading ./spec/manageiq/spec/models/manageiq/providers/embedded_ansible/automation_manager/configuration_script_source_spec.rb.
Failure/Error: require 'support/fake_ansible_repo'
LoadError:
  cannot load such file -- support/fake_ansible_repo
# ./spec/manageiq/spec/models/manageiq/providers/embedded_ansible/automation_manager/configuration_script_source_spec.rb:1:in `<top (required)>'
Run options: include {:providers_common=>true}

miq-bot added bug core core/embedded ansible labels Aug 1, 2019