From ee60a2fe4703d5c8f053d3473b7f3ed6008cc95d Mon Sep 17 00:00:00 2001 From: Josh Partlow Date: Sat, 12 Feb 2022 01:13:52 +0000 Subject: [PATCH] (GH-187) Fixes abs failing provision if inventory file exists I introduced this in cd72bb6. When the newly structured abs task calls lib/task_helper's get_inventory_hash() method and finds a pre-existing inventory_full_path file. It attempts to call PuppetLitmus::InventoryManipulation.inventory_hash_from_inventory_file() which fails, because that method is not a class method. The puzzling thing about this is why it ever worked, because I was wrong that #187 was pre-existing. The issue here is that the abs (the previous version), docker, docker_exp, and vagrant tasks all use 'include PuppetLitmus::InventoryManipulation' or 'include PuppetLitmus' (which includes InventoryManipulation) in the scope of global functions declared implicitly on the main Object. This causes the included module to be included in the base Object class, which effectively makes its methods available everywhere. So a call to PuppetLitmus::InventoryManipulation.inventory_hash_from_inventory_file() actually works, as would String.inventory_hash_from_inventory_file or anything else... When I 'fixed' the abs task to include PuppetLitmus::InventoryManipulation within the new ABSProvision class, I broke that global behavior but only for abs. The fix is straightforward now that I know what's going on. Removing the module prefix from the method call works for both the new abs task and the other tasks that still rely on the global behavior. In the abs task case, InventoryManipulation is included in the class, so the methods are available. For the other tasks, InventoryManipulation is still included by main, and the methods are available everywhere anyway. --- lib/task_helper.rb | 3 +-- spec/tasks/abs_spec.rb | 7 ------- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/lib/task_helper.rb b/lib/task_helper.rb index f9ddf68..d440659 100644 --- a/lib/task_helper.rb +++ b/lib/task_helper.rb @@ -7,8 +7,7 @@ def sanitise_inventory_location(location) def get_inventory_hash(inventory_full_path) if File.file?(inventory_full_path) - require 'puppet_litmus/inventory_manipulation' - PuppetLitmus::InventoryManipulation.inventory_hash_from_inventory_file(inventory_full_path) + inventory_hash_from_inventory_file(inventory_full_path) else { 'version' => 2, 'groups' => [{ 'name' => 'docker_nodes', 'targets' => [] }, { 'name' => 'ssh_nodes', 'targets' => [] }, { 'name' => 'winrm_nodes', 'targets' => [] }] } end diff --git a/spec/tasks/abs_spec.rb b/spec/tasks/abs_spec.rb index 1b82186..046f04a 100644 --- a/spec/tasks/abs_spec.rb +++ b/spec/tasks/abs_spec.rb @@ -110,13 +110,6 @@ def with_env(env_vars) end it 'provision with an existing inventory file' do - pending(<<~EOS) - XXX: (#187) It looks like there's an error hidden here in the way - lib/task_helper.rb get_inventory_hash() attempts to call - PuppetLitmus::InventoryManipulation.inventory_hash_from_inventory_file() - as though it were a class method. - EOS - stub_request(:post, 'https://abs-prod.k8s.infracore.puppet.net/api/v2/request') .to_return({ status: 202 }, { status: 200, body: response_body.to_json })