From 40a2244434982b05c73ce13c83e68b4ed26ee54a Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Tue, 30 Jul 2019 10:55:15 -0400 Subject: [PATCH 1/5] Use the --project-dir option of ansible-runner for playbooks 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. --- lib/ansible/runner.rb | 6 ++++++ spec/lib/ansible/runner_spec.rb | 10 +++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/ansible/runner.rb b/lib/ansible/runner.rb index 7bd976e963d..e11e4188af4 100644 --- a/lib/ansible/runner.rb +++ b/lib/ansible/runner.rb @@ -254,6 +254,12 @@ def runner_params(base_dir, ansible_runner_method, playbook_or_role_args, verbos runner_args[:role_skip_facts] = nil if runner_args.delete(:role_skip_facts) runner_args[:ident] = "result" + playbook = runner_args.delete(:playbook) + if playbook + runner_args[:playbook] = File.basename(playbook) + runner_args[:project_dir] = File.dirname(playbook) + end + if verbosity.to_i > 0 v_flag = "-#{"v" * verbosity.to_i.clamp(1, 5)}" runner_args[v_flag] = nil diff --git a/spec/lib/ansible/runner_spec.rb b/spec/lib/ansible/runner_spec.rb index 47e973d93c2..aa70fe233dc 100644 --- a/spec/lib/ansible/runner_spec.rb +++ b/spec/lib/ansible/runner_spec.rb @@ -21,7 +21,7 @@ expect(method).to eq("run") expect(json).to eq(:json) - expect(args).to eq(:ident => "result", :playbook => playbook) + expect(args).to eq(:ident => "result", :playbook => "playbook", :project_dir => "/path/to/my") hosts = File.read(File.join(dir, "inventory", "hosts")) expect(hosts).to eq("localhost") @@ -44,7 +44,7 @@ expect(method).to eq("run") expect(json).to eq(:json) - expect(args).to eq(:ident => "result", :playbook => playbook) + expect(args).to eq(:ident => "result", :playbook => "playbook", :project_dir => "/path/to/my") hosts = File.read(File.join(dir, "inventory", "hosts")) expect(hosts).to eq("localhost") @@ -64,7 +64,7 @@ expect(command).to eq("ansible-runner") _method, _dir, _json, args = options[:params] - expect(args).to eq(:ident => "result", :playbook => playbook, "-vvvvv" => nil) + expect(args).to eq(:ident => "result", :playbook => "playbook", :project_dir => "/path/to/my", "-vvvvv" => nil) end.and_return(result) described_class.run(env_vars, extra_vars, playbook, :verbosity => 6) @@ -96,7 +96,7 @@ expect(method).to eq("run") expect(json).to eq(:json) - expect(args).to eq(:ident => "result", :playbook => playbook) + expect(args).to eq(:ident => "result", :playbook => "playbook", :project_dir => "/path/to/my") hosts = File.read(File.join(dir, "inventory", "hosts")) expect(hosts).to eq("localhost") @@ -126,7 +126,7 @@ expect(method).to eq("start") expect(json).to eq(:json) - expect(args).to eq(:ident => "result", :playbook => playbook) + expect(args).to eq(:ident => "result", :playbook => "playbook", :project_dir => "/path/to/my") hosts = File.read(File.join(dir, "inventory", "hosts")) expect(hosts).to eq("localhost") From e347b0b15c3b8cd72dd984edfbe7249ff7efc8b6 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Tue, 30 Jul 2019 11:25:12 -0400 Subject: [PATCH 2/5] Allow Ansible::Content to pull requirements from the roles directory 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. --- lib/ansible/content.rb | 20 +++++++++-- spec/lib/ansible/content_spec.rb | 60 ++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 3 deletions(-) create mode 100644 spec/lib/ansible/content_spec.rb diff --git a/lib/ansible/content.rb b/lib/ansible/content.rb index 3b4f68af561..1e83b7d3126 100644 --- a/lib/ansible/content.rb +++ b/lib/ansible/content.rb @@ -9,10 +9,9 @@ def initialize(path) end def fetch_galaxy_roles - roles_path = path.join('roles') - role_file = path.join('requirements.yml') + return true unless requirements_file.exist? - params = ["install", :roles_path= => roles_path, :role_file= => role_file] + params = ["install", :roles_path= => roles_dir, :role_file= => requirements_file] AwesomeSpawn.run!("ansible-galaxy", :params => params) end @@ -24,5 +23,20 @@ def self.consolidate_plugin_playbooks(dir = PLUGIN_CONTENT_DIR) FileUtils.cp_r(Dir.glob("#{content.path}/*"), dir) end end + + private + + def roles_dir + @roles_dir ||= path.join('roles') + end + + def requirements_file + @requirements_file ||= begin + local_file = path.join('requirements.yml') + roles_dir_file = roles_dir.join('requirements.yml') + + roles_dir_file.exist? ? roles_dir_file : local_file + end + end end end diff --git a/spec/lib/ansible/content_spec.rb b/spec/lib/ansible/content_spec.rb new file mode 100644 index 00000000000..9d84c95843d --- /dev/null +++ b/spec/lib/ansible/content_spec.rb @@ -0,0 +1,60 @@ +RSpec.describe Ansible::Content do + let(:content_dir) { Pathname.new(Dir.mktmpdir) } + let(:roles_dir) { content_dir.join("roles") } + let(:local_requirements) { content_dir.join("requirements.yml") } + let(:roles_requirements) { content_dir.join("roles", "requirements.yml") } + + subject { described_class.new(content_dir) } + + after { FileUtils.rm_rf(content_dir) } + + describe "#fetch_galaxy_roles" do + it "doesn't run anything if there is no requirements file" do + expect(AwesomeSpawn).not_to receive(:run!) + + subject.fetch_galaxy_roles + end + + it "runs ansible-galaxy using the local requirements file" do + FileUtils.touch(local_requirements) + + expected_params = [ + "install", + :roles_path= => roles_dir, + :role_file= => local_requirements + ] + expect(AwesomeSpawn).to receive(:run!).with("ansible-galaxy", :params => expected_params) + + subject.fetch_galaxy_roles + end + + it "runs ansible-runner using the roles requirements file" do + FileUtils.mkdir(roles_dir) + FileUtils.touch(roles_requirements) + + expected_params = [ + "install", + :roles_path= => roles_dir, + :role_file= => roles_requirements + ] + expect(AwesomeSpawn).to receive(:run!).with("ansible-galaxy", :params => expected_params) + + subject.fetch_galaxy_roles + end + + it "prefers the roles requirements file" do + FileUtils.mkdir(roles_dir) + FileUtils.touch(roles_requirements) + FileUtils.touch(local_requirements) + + expected_params = [ + "install", + :roles_path= => roles_dir, + :role_file= => roles_requirements + ] + expect(AwesomeSpawn).to receive(:run!).with("ansible-galaxy", :params => expected_params) + + subject.fetch_galaxy_roles + end + end +end From c603ac645acf23522875511c9ae1faddf0f365b8 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Tue, 30 Jul 2019 12:05:58 -0400 Subject: [PATCH 3/5] Fetch roles from ansible galaxy after cloning the playbook repo --- .../automation_manager/configuration_script_source.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/manageiq/providers/embedded_ansible/automation_manager/configuration_script_source.rb b/app/models/manageiq/providers/embedded_ansible/automation_manager/configuration_script_source.rb index e126aab9d29..89a8853e470 100644 --- a/app/models/manageiq/providers/embedded_ansible/automation_manager/configuration_script_source.rb +++ b/app/models/manageiq/providers/embedded_ansible/automation_manager/configuration_script_source.rb @@ -94,6 +94,7 @@ def playbooks_in_git_repository def checkout_git_repository(target_directory) git_repository.update_repo git_repository.checkout(scm_branch, target_directory) + Ansible::Content.new(target_directory).fetch_galaxy_roles end ERROR_MAX_SIZE = 50.kilobytes From 2aba2be2e07ab1bb72661bab12363a955b5f35c8 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Tue, 30 Jul 2019 12:09:19 -0400 Subject: [PATCH 4/5] Allow strings or pathnames in Ansible::Content --- lib/ansible/content.rb | 2 +- spec/lib/ansible/content_spec.rb | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/ansible/content.rb b/lib/ansible/content.rb index 1e83b7d3126..662572e71c8 100644 --- a/lib/ansible/content.rb +++ b/lib/ansible/content.rb @@ -5,7 +5,7 @@ class Content attr_accessor :path def initialize(path) - @path = path + @path = Pathname.new(path) end def fetch_galaxy_roles diff --git a/spec/lib/ansible/content_spec.rb b/spec/lib/ansible/content_spec.rb index 9d84c95843d..9f8e2f9ab4d 100644 --- a/spec/lib/ansible/content_spec.rb +++ b/spec/lib/ansible/content_spec.rb @@ -56,5 +56,18 @@ subject.fetch_galaxy_roles end + + it "works with a string path" do + FileUtils.touch(local_requirements) + + expected_params = [ + "install", + :roles_path= => roles_dir, + :role_file= => local_requirements + ] + expect(AwesomeSpawn).to receive(:run!).with("ansible-galaxy", :params => expected_params) + + described_class.new(content_dir.to_s).fetch_galaxy_roles + end end end From 6bfcc74e23e42a11bd6b263e24c6b161b9309dd2 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Wed, 31 Jul 2019 16:24:05 -0400 Subject: [PATCH 5/5] Move logic to fetch galaxy roles to Ansible::Runner 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 --- .../configuration_script_source.rb | 1 - lib/ansible/runner.rb | 8 ++++++++ spec/lib/ansible/runner_spec.rb | 14 ++++++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/app/models/manageiq/providers/embedded_ansible/automation_manager/configuration_script_source.rb b/app/models/manageiq/providers/embedded_ansible/automation_manager/configuration_script_source.rb index 89a8853e470..e126aab9d29 100644 --- a/app/models/manageiq/providers/embedded_ansible/automation_manager/configuration_script_source.rb +++ b/app/models/manageiq/providers/embedded_ansible/automation_manager/configuration_script_source.rb @@ -94,7 +94,6 @@ def playbooks_in_git_repository def checkout_git_repository(target_directory) git_repository.update_repo git_repository.checkout(scm_branch, target_directory) - Ansible::Content.new(target_directory).fetch_galaxy_roles end ERROR_MAX_SIZE = 50.kilobytes diff --git a/lib/ansible/runner.rb b/lib/ansible/runner.rb index e11e4188af4..f88b640abd9 100644 --- a/lib/ansible/runner.rb +++ b/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) result = AwesomeSpawn.run("ansible-runner", :env => env_vars_hash, :params => params) res = response(base_dir, ansible_runner_method, result) ensure @@ -292,6 +293,13 @@ def validate_params!(env_vars, extra_vars, tags, ansible_runner_method, playbook raise ArgumentError, errors.join("; ") if errors.any? end + def fetch_galaxy_roles(playbook_or_role_args) + return unless playbook_or_role_args[:playbook] + + playbook_dir = File.dirname(playbook_or_role_args[:playbook]) + Ansible::Content.new(playbook_dir).fetch_galaxy_roles + end + def credentials_info(credentials, base_dir) command_line = {} env_vars = {} diff --git a/spec/lib/ansible/runner_spec.rb b/spec/lib/ansible/runner_spec.rb index aa70fe233dc..0f7675da640 100644 --- a/spec/lib/ansible/runner_spec.rb +++ b/spec/lib/ansible/runner_spec.rb @@ -32,6 +32,8 @@ expect(File.exist?(File.join(dir, "env", "cmdline"))).to be_falsey end.and_return(result) + expect_galaxy_roles_fetched + described_class.run(env_vars, extra_vars, playbook) end @@ -56,6 +58,8 @@ expect(cmdline).to eq("--tags #{tags}") end.and_return(result) + expect_galaxy_roles_fetched + described_class.run(env_vars, extra_vars, playbook, :tags => tags) end @@ -105,6 +109,8 @@ expect(extravars).to eq("name" => "john's server", "ansible_connection" => "local") end.and_return(result) + expect_galaxy_roles_fetched + described_class.run(env_vars, extra_vars, playbook) end end @@ -137,6 +143,8 @@ expect(File.exist?(File.join(dir, "env", "cmdline"))).to be_falsey end.and_return(result) + expect_galaxy_roles_fetched + runner_result = described_class.run_async(env_vars, extra_vars, playbook) expect(runner_result).kind_of?(Ansible::Runner::ResponseAsync) end @@ -257,4 +265,10 @@ expect(MiqQueue.first.zone).to eq(zone.name) end end + + def expect_galaxy_roles_fetched + content_double = instance_double(Ansible::Content) + expect(Ansible::Content).to receive(:new).with("/path/to/my").and_return(content_double) + expect(content_double).to receive(:fetch_galaxy_roles) + end end