Merge pull request #631 from wilson208/key-server-timeout-fix
authorDavid Schmitt <david.schmitt@puppet.com>
Tue, 25 Oct 2016 10:21:35 +0000 (11:21 +0100)
committerGitHub <noreply@github.com>
Tue, 25 Oct 2016 10:21:35 +0000 (11:21 +0100)
[MODULES-3562] Implement retry for tests which require modules to pull key from keyserver

spec/acceptance/apt_key_provider_spec.rb
spec/acceptance/apt_spec.rb
spec/acceptance/class_spec.rb
spec/spec_helper_acceptance.rb

index 274df5e3bd477f7b64c3dcea29f44002c85859b6..b4347e19c5f9b1952e9015fb28aff1b00e48ccb7 100644 (file)
@@ -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
 
index 8a9d4ca6eb23e1210f277371ca7b0a2327613343..52415f5afaecb4e0cfe28fbe2af689c3ffad3f77 100644 (file)
@@ -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
index f228e4c4565f7b8d825b52ca83a421d2c007c142..1539a0dd125a5860d347df09daf91b5c7a49caa3 100644 (file)
@@ -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
index 409ce68b299d2ff4b302ab2e46726d26643017d0..a5ce06fc45815156e2d32c61377f97d04de21701 100644 (file)
@@ -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__), '..'))