From: David Schmitt <david.schmitt@puppet.com>
Date: Tue, 25 Oct 2016 10:21:35 +0000 (+0100)
Subject: Merge pull request #631 from wilson208/key-server-timeout-fix
X-Git-Tag: 2.4.0~18
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=68cdcf71a3c4cdc0d431719494a4572c28afd9c4;hp=c6a7f3d85faa78eba0ccdb52eaa386103de0c3fc;p=puppet-modules%2Fpuppetlabs-apt.git

Merge pull request #631 from wilson208/key-server-timeout-fix

[MODULES-3562] Implement retry for tests which require modules to pull key from keyserver
---

diff --git a/spec/acceptance/apt_key_provider_spec.rb b/spec/acceptance/apt_key_provider_spec.rb
index 274df5e..b4347e1 100644
--- a/spec/acceptance/apt_key_provider_spec.rb
+++ b/spec/acceptance/apt_key_provider_spec.rb
@@ -17,6 +17,10 @@ KEY_CHECK_COMMAND              = "apt-key adv --list-keys --with-colons --finger
 PUPPETLABS_KEY_CHECK_COMMAND   = "#{KEY_CHECK_COMMAND} #{PUPPETLABS_GPG_KEY_FINGERPRINT}"
 CENTOS_KEY_CHECK_COMMAND       = "#{KEY_CHECK_COMMAND} #{CENTOS_GPG_KEY_FINGERPRINT}"
 
+MAX_TIMEOUT_RETRY              = 3
+TIMEOUT_RETRY_WAIT             = 5
+TIMEOUT_ERROR_MATCHER    = /no valid OpenPGP data found/
+
 describe 'apt_key' do
   before(:each) do
     # Delete twice to make sure everything is cleaned
@@ -85,20 +89,25 @@ describe 'apt_key' do
         }
         EOS
 
-        # Install the key first
-        shell("apt-key adv --keyserver hkps.pool.sks-keyservers.net \
+        # Install the key first (retry because key pool may timeout)
+        retry_on_error_matching(MAX_TIMEOUT_RETRY, TIMEOUT_RETRY_WAIT, TIMEOUT_ERROR_MATCHER) do
+          shell("apt-key adv --keyserver hkps.pool.sks-keyservers.net \
               --recv-keys #{CENTOS_GPG_KEY_FINGERPRINT}")
+        end
         shell(CENTOS_KEY_CHECK_COMMAND)
 
         # Time to remove it using Puppet
         apply_manifest(pp, :catch_failures => true)
-        apply_manifest(pp, :catch_failures => true)
+        apply_manifest(pp, :catch_changes => true)
 
         shell(CENTOS_KEY_CHECK_COMMAND,
               :acceptable_exit_codes => [1])
 
-        shell("apt-key adv --keyserver hkps.pool.sks-keyservers.net \
-              --recv-keys #{CENTOS_GPG_KEY_FINGERPRINT}")
+        # Re-Install the key (retry because key pool may timeout)
+        retry_on_error_matching(MAX_TIMEOUT_RETRY, TIMEOUT_RETRY_WAIT, TIMEOUT_ERROR_MATCHER) do
+          shell("apt-key adv --keyserver hkps.pool.sks-keyservers.net \
+                --recv-keys #{CENTOS_GPG_KEY_FINGERPRINT}")
+        end
       end
     end
 
@@ -111,14 +120,17 @@ describe 'apt_key' do
         }
         EOS
 
-        # Install the key first
-        shell("apt-key adv --keyserver hkps.pool.sks-keyservers.net \
-              --recv-keys #{PUPPETLABS_GPG_KEY_LONG_ID}")
+        # Install the key first (retry because key pool may timeout)
+        retry_on_error_matching(MAX_TIMEOUT_RETRY, TIMEOUT_RETRY_WAIT, TIMEOUT_ERROR_MATCHER) do
+          shell("apt-key adv --keyserver hkps.pool.sks-keyservers.net \
+                --recv-keys #{PUPPETLABS_GPG_KEY_LONG_ID}")
+        end
+
         shell(PUPPETLABS_KEY_CHECK_COMMAND)
 
         # Time to remove it using Puppet
         apply_manifest(pp, :catch_failures => true)
-        apply_manifest(pp, :catch_failures => true)
+        apply_manifest(pp, :catch_changes => true)
 
         shell(PUPPETLABS_KEY_CHECK_COMMAND,
               :acceptable_exit_codes => [1])
@@ -188,12 +200,17 @@ zGioYMWgVePywFGaTV51/0uF9ymHHC7BDIcLgUWHdg/1B67jR5YQfzPJUqLhnylt
           }
         EOS
 
-        apply_manifest(pp, :catch_failures => true)
-        apply_manifest(pp, :catch_failures => true)
+        #Apply the manifest (Retry if timeout error is received from key pool)
+        retry_on_error_matching(MAX_TIMEOUT_RETRY, TIMEOUT_RETRY_WAIT, TIMEOUT_ERROR_MATCHER) do
+          apply_manifest(pp, :catch_failures => true)
+        end
+
+        apply_manifest(pp, :catch_changes => true)
         shell(PUPPETLABS_KEY_CHECK_COMMAND)
       end
     end
 
+
     context 'multiple keys' do
       it 'runs without errors' do
         pp = <<-EOS
@@ -448,8 +465,9 @@ FPfZDNCu/TXoqyJk7434jJrcHgPryzrHFBLfEmc=
 -----END PGP PUBLIC KEY BLOCK----- ",
           }
         EOS
+
         apply_manifest(pp, :catch_failures => true)
-        apply_manifest(pp, :catch_failures => true)
+        apply_manifest(pp, :catch_changes => true)
         shell(PUPPETLABS_KEY_CHECK_COMMAND)
       end
     end
@@ -482,8 +500,12 @@ FPfZDNCu/TXoqyJk7434jJrcHgPryzrHFBLfEmc=
         }
         EOS
 
-        apply_manifest(pp, :catch_failures => true)
-        apply_manifest(pp, :catch_failures => true)
+        #Apply the manifest (Retry if timeout error is received from key pool)
+        retry_on_error_matching(MAX_TIMEOUT_RETRY, TIMEOUT_RETRY_WAIT, TIMEOUT_ERROR_MATCHER) do
+          apply_manifest(pp, :catch_failures => true)
+        end
+
+        apply_manifest(pp, :catch_changes => true)
         shell(PUPPETLABS_KEY_CHECK_COMMAND)
       end
     end
@@ -498,8 +520,11 @@ FPfZDNCu/TXoqyJk7434jJrcHgPryzrHFBLfEmc=
         }
         EOS
 
-        apply_manifest(pp, :catch_failures => true)
-        apply_manifest(pp, :catch_failures => true)
+        retry_on_error_matching(MAX_TIMEOUT_RETRY, TIMEOUT_RETRY_WAIT, TIMEOUT_ERROR_MATCHER) do
+          apply_manifest(pp, :catch_failures => true)
+        end
+
+        apply_manifest(pp, :catch_changes => true)
         shell(PUPPETLABS_KEY_CHECK_COMMAND)
       end
     end
@@ -549,7 +574,7 @@ FPfZDNCu/TXoqyJk7434jJrcHgPryzrHFBLfEmc=
         EOS
 
         apply_manifest(pp, :catch_failures => true)
-        apply_manifest(pp, :catch_failures => true)
+        apply_manifest(pp, :catch_changes => true)
         shell(PUPPETLABS_KEY_CHECK_COMMAND)
       end
 
@@ -563,7 +588,7 @@ FPfZDNCu/TXoqyJk7434jJrcHgPryzrHFBLfEmc=
         EOS
 
         apply_manifest(pp, :catch_failures => true)
-        apply_manifest(pp, :catch_failures => true)
+        apply_manifest(pp, :catch_changes => true)
         shell(PUPPETLABS_KEY_CHECK_COMMAND)
       end
 
@@ -612,7 +637,7 @@ FPfZDNCu/TXoqyJk7434jJrcHgPryzrHFBLfEmc=
         EOS
 
         apply_manifest(pp, :catch_failures => true)
-        apply_manifest(pp, :catch_failures => true)
+        apply_manifest(pp, :catch_changes => true)
         shell(CENTOS_KEY_CHECK_COMMAND)
       end
 
@@ -656,7 +681,7 @@ FPfZDNCu/TXoqyJk7434jJrcHgPryzrHFBLfEmc=
         EOS
 
         apply_manifest(pp, :catch_failures => true)
-        apply_manifest(pp, :catch_failures => true)
+        apply_manifest(pp, :catch_changes => true)
         shell(PUPPETLABS_KEY_CHECK_COMMAND)
       end
 
@@ -670,7 +695,7 @@ FPfZDNCu/TXoqyJk7434jJrcHgPryzrHFBLfEmc=
         EOS
 
         apply_manifest(pp, :catch_failures => true)
-        apply_manifest(pp, :catch_failures => true)
+        apply_manifest(pp, :catch_changes => true)
         shell(PUPPETLABS_KEY_CHECK_COMMAND)
       end
 
@@ -723,7 +748,7 @@ FPfZDNCu/TXoqyJk7434jJrcHgPryzrHFBLfEmc=
         EOS
 
         apply_manifest(pp, :catch_failures => true)
-        apply_manifest(pp, :catch_failures => true)
+        apply_manifest(pp, :catch_changes => true)
         shell(PUPPETLABS_KEY_CHECK_COMMAND)
       end
     end
@@ -780,7 +805,7 @@ FPfZDNCu/TXoqyJk7434jJrcHgPryzrHFBLfEmc=
         EOS
 
         apply_manifest(pp, :catch_failures => true)
-        apply_manifest(pp, :catch_failures => true)
+        apply_manifest(pp, :catch_changes => true)
         shell(PUPPETLABS_KEY_CHECK_COMMAND)
       end
     end
@@ -798,7 +823,7 @@ FPfZDNCu/TXoqyJk7434jJrcHgPryzrHFBLfEmc=
         EOS
 
         apply_manifest(pp, :catch_failures => true)
-        apply_manifest(pp, :catch_failures => true)
+        apply_manifest(pp, :catch_changes => true)
       end
     end
 
diff --git a/spec/acceptance/apt_spec.rb b/spec/acceptance/apt_spec.rb
index 8a9d4ca..52415f5 100644
--- a/spec/acceptance/apt_spec.rb
+++ b/spec/acceptance/apt_spec.rb
@@ -1,5 +1,9 @@
 require 'spec_helper_acceptance'
 
+MAX_TIMEOUT_RETRY              = 3
+TIMEOUT_RETRY_WAIT             = 5
+TIMEOUT_ERROR_MATCHER    = /no valid OpenPGP data found/
+
 describe 'apt class' do
 
   context 'reset' do
@@ -42,7 +46,11 @@ describe 'apt class' do
       }
       EOS
 
-      apply_manifest(pp, :catch_failures => true)
+      #Apply the manifest (Retry if timeout error is received from key pool)
+      retry_on_error_matching(MAX_TIMEOUT_RETRY, TIMEOUT_RETRY_WAIT, TIMEOUT_ERROR_MATCHER) do
+        apply_manifest(pp, :catch_failures => true)
+      end
+
       apply_manifest(pp, :catch_failures => true)
     end
     it 'should still work' do
diff --git a/spec/acceptance/class_spec.rb b/spec/acceptance/class_spec.rb
index f228e4c..1539a0d 100644
--- a/spec/acceptance/class_spec.rb
+++ b/spec/acceptance/class_spec.rb
@@ -11,7 +11,7 @@ describe 'apt class' do
 
       # Run it twice and test for idempotency
       apply_manifest(pp, :catch_failures => true)
-      apply_manifest(pp, :catch_failures => true)
+      apply_manifest(pp, :catch_changes => true)
     end
   end
 end
diff --git a/spec/spec_helper_acceptance.rb b/spec/spec_helper_acceptance.rb
index 409ce68..a5ce06f 100644
--- a/spec/spec_helper_acceptance.rb
+++ b/spec/spec_helper_acceptance.rb
@@ -5,6 +5,32 @@ run_puppet_install_helper
 
 UNSUPPORTED_PLATFORMS = ['RedHat','Suse','windows','AIX','Solaris']
 
+# This method allows a block to be passed in and if an exception is raised
+# that matches the 'error_matcher' matcher, the block will wait a set number
+# of seconds before retrying.
+# Params:
+# - max_retry_count - Max number of retries
+# - retry_wait_interval_secs - Number of seconds to wait before retry
+# - error_matcher - Matcher which the exception raised must match to allow retry
+# Example Usage:
+# retry_on_error_matching(3, 5, /OpenGPG Error/) do
+#   apply_manifest(pp, :catch_failures => true)
+# end
+def retry_on_error_matching(max_retry_count = 3, retry_wait_interval_secs = 5, error_matcher = nil)
+  try = 0
+  begin
+    try += 1
+    yield
+  rescue Exception => e
+    if try < max_retry_count && (error_matcher == nil || e.message =~ error_matcher)
+      sleep retry_wait_interval_secs
+      retry
+    else
+      raise
+    end
+  end
+end
+
 RSpec.configure do |c|
   # Project root
   proj_root = File.expand_path(File.join(File.dirname(__FILE__), '..'))