From 23efcc4cc0b12c791aa45a224273e40db48dc04a Mon Sep 17 00:00:00 2001
From: Samuel Beaulieu <samuel+14401380@puppet.com>
Date: Thu, 23 Dec 2021 10:58:38 -0600
Subject: [PATCH 1/2] (maint) Fix EXTRA_CONFIG merge behavior Before this
 change if an extra config file had new keys they would get merged to the main
 config but if it contained an existing key, like 'providers' it would
 overwrite the original config. Adding a library 'deep_merge' to do a more
 natural merge, where existing keys get sub-elements added together and arrays
 are combined like for the pool configuration. Adding spec tests around
 EXTRA_CONFIG as they were missing, by adding and testing two new
 extra_config.yaml fixture files

---
 lib/vmpooler.rb                  |  3 ++-
 spec/fixtures/extra_config1.yaml |  4 ++++
 spec/fixtures/extra_config2.yaml | 11 +++++++++++
 spec/unit/vmpooler_spec.rb       | 22 +++++++++++++++++++++-
 vmpooler.gemspec                 |  1 +
 5 files changed, 39 insertions(+), 2 deletions(-)
 create mode 100644 spec/fixtures/extra_config1.yaml
 create mode 100644 spec/fixtures/extra_config2.yaml

diff --git a/lib/vmpooler.rb b/lib/vmpooler.rb
index 8b86fdb..c68e4ef 100644
--- a/lib/vmpooler.rb
+++ b/lib/vmpooler.rb
@@ -3,6 +3,7 @@
 module Vmpooler
   require 'concurrent'
   require 'date'
+  require 'deep_merge'
   require 'json'
   require 'net/ldap'
   require 'open-uri'
@@ -42,7 +43,7 @@ module Vmpooler
         extra_configs = parsed_config[:config]['extra_config'].split(',')
         extra_configs.each do |config|
           extra_config = YAML.load_file(config)
-          parsed_config.merge!(extra_config)
+          parsed_config.deep_merge(extra_config)
         end
       end
     end
diff --git a/spec/fixtures/extra_config1.yaml b/spec/fixtures/extra_config1.yaml
new file mode 100644
index 0000000..d19523c
--- /dev/null
+++ b/spec/fixtures/extra_config1.yaml
@@ -0,0 +1,4 @@
+---
+:providers:
+  :alice:
+    foo: "foo"
\ No newline at end of file
diff --git a/spec/fixtures/extra_config2.yaml b/spec/fixtures/extra_config2.yaml
new file mode 100644
index 0000000..e9f30d8
--- /dev/null
+++ b/spec/fixtures/extra_config2.yaml
@@ -0,0 +1,11 @@
+---
+:providers:
+  :bob:
+    foo: "foo_bob"
+    bar: "bar"
+
+:pools:
+  - name: 'pool05'
+    size: 5
+    provider: dummy
+    ready_ttl: 5
\ No newline at end of file
diff --git a/spec/unit/vmpooler_spec.rb b/spec/unit/vmpooler_spec.rb
index 19aed91..84d43ed 100644
--- a/spec/unit/vmpooler_spec.rb
+++ b/spec/unit/vmpooler_spec.rb
@@ -45,10 +45,30 @@ describe 'Vmpooler' do
     end
 
     context 'when config file is set' do
-      it 'should use the file' do
+      before(:each) do
         ENV['VMPOOLER_CONFIG_FILE'] = config_file
+      end
+      it 'should use the file' do
         expect(Vmpooler.config[:pools]).to eq(config[:pools])
       end
+      it 'merges one extra file, results in two providers' do
+        ENV['EXTRA_CONFIG'] = File.join(fixtures_dir, 'extra_config1.yaml')
+        expect(Vmpooler.config[:providers].keys).to include(:dummy)
+        expect(Vmpooler.config[:providers].keys).to include(:alice)
+      end
+      it 'merges two extra file, results in three providers and an extra pool' do
+        extra1 = File.join(fixtures_dir, 'extra_config1.yaml')
+        extra2 = File.join(fixtures_dir, 'extra_config2.yaml')
+        ENV['EXTRA_CONFIG'] = "#{extra1},#{extra2}"
+        expect(Vmpooler.config[:providers].keys).to include(:dummy)
+        expect(Vmpooler.config[:providers].keys).to include(:alice)
+        expect(Vmpooler.config[:providers].keys).to include(:bob)
+        merged_pools = [{"name"=>"pool03", "provider"=>"dummy", "ready_ttl"=>5, "size"=>5},
+                        {"name"=>"pool04", "provider"=>"dummy", "ready_ttl"=>5, "size"=>5},
+                        {"name"=>"pool05", "provider"=>"dummy", "ready_ttl"=>5, "size"=>5}]
+        expect(Vmpooler.config[:pools]).to eq(merged_pools)
+        expect(Vmpooler.config[:config]).not_to be_nil #merge does not deleted existing keys
+      end
     end
   end
 end
diff --git a/vmpooler.gemspec b/vmpooler.gemspec
index 1f3299e..144ae0b 100644
--- a/vmpooler.gemspec
+++ b/vmpooler.gemspec
@@ -19,6 +19,7 @@ Gem::Specification.new do |s|
   s.require_paths = ["lib"]
   s.add_dependency 'concurrent-ruby', '~> 1.1'
   s.add_dependency 'connection_pool', '~> 2.2'
+  s.add_dependency 'deep_merge', '~> 1.2'
   s.add_dependency 'net-ldap', '~> 0.16'
   s.add_dependency 'nokogiri', '~> 1.10'
   s.add_dependency 'opentelemetry-exporter-jaeger', '= 0.20.1'

From 3a508a3afb54a163d241023b3e191d471faecde7 Mon Sep 17 00:00:00 2001
From: Samuel Beaulieu <samuel+14401380@puppet.com>
Date: Thu, 23 Dec 2021 14:08:18 -0600
Subject: [PATCH 2/2] (maint) do not raise an error when base provider
 create_template_delta_disks called

---
 lib/vmpooler/providers/base.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/vmpooler/providers/base.rb b/lib/vmpooler/providers/base.rb
index 25d39ef..1147aa8 100644
--- a/lib/vmpooler/providers/base.rb
+++ b/lib/vmpooler/providers/base.rb
@@ -242,7 +242,7 @@ module Vmpooler
         # returns
         #   nil when successful. Raises error when encountered
         def create_template_delta_disks(_pool)
-          raise("#{self.class.name} does not implement create_template_delta_disks")
+          puts("#{self.class.name} does not implement create_template_delta_disks")
         end
 
         # inputs