From faaec4f9bcf84810faeed96bc8b0d012f69a5cda Mon Sep 17 00:00:00 2001 From: "kirby@puppetlabs.com" Date: Tue, 3 Jul 2018 11:24:05 -0700 Subject: [PATCH] (POOLER-124) Fix evaluation of max_tries This commit updates usage of max_tries to ensure that the increment of the try value happens after evaluation of whether max_tries has been exceeded. Without this change max_tries doesn't work as advertised. Additionally, checking looks to see if try is greater than or equal to so the try count exceeding max_tries would also be detected. --- lib/vmpooler/providers/vsphere.rb | 4 ++-- spec/unit/providers/vsphere_spec.rb | 9 +-------- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/lib/vmpooler/providers/vsphere.rb b/lib/vmpooler/providers/vsphere.rb index 5d241c6..a522008 100644 --- a/lib/vmpooler/providers/vsphere.rb +++ b/lib/vmpooler/providers/vsphere.rb @@ -407,10 +407,10 @@ module Vmpooler metrics.increment('connect.open') return connection rescue => err - try += 1 metrics.increment('connect.fail') - raise err if try == max_tries + raise err if try >= max_tries sleep(try * retry_factor) + try += 1 retry end end diff --git a/spec/unit/providers/vsphere_spec.rb b/spec/unit/providers/vsphere_spec.rb index df618d3..115f593 100644 --- a/spec/unit/providers/vsphere_spec.rb +++ b/spec/unit/providers/vsphere_spec.rb @@ -897,13 +897,8 @@ EOT allow(subject).to receive(:sleep) end - it 'should raise an error' do - expect{subject.connect_to_vsphere}.to raise_error(RuntimeError,'MockError') - end - it 'should retry the connection attempt config.max_tries times' do - pending('Resolution of issue https://github.com/puppetlabs/vmpooler/issues/199') - expect(RbVmomi::VIM).to receive(:connect).exactly(config[:config]['max_tries']).times.and_raise(RuntimeError,'MockError') + expect(RbVmomi::VIM).to receive(:connect).exactly(config[:config]['max_tries']).times begin # Swallow any errors @@ -913,7 +908,6 @@ EOT end it 'should increment the connect.fail counter config.max_tries times' do - pending('Resolution of issue https://github.com/puppetlabs/vmpooler/issues/199') expect(metrics).to receive(:increment).with('connect.fail').exactly(config[:config]['max_tries']).times begin @@ -928,7 +922,6 @@ EOT ].each do |testcase| context "Configuration set for max_tries of #{testcase[:max_tries]} and retry_facter of #{testcase[:retry_factor]}" do it "should sleep #{testcase[:max_tries] - 1} times between attempts with increasing timeout" do - pending('Resolution of issue https://github.com/puppetlabs/vmpooler/issues/199') config[:config]['max_tries'] = testcase[:max_tries] config[:config]['retry_factor'] = testcase[:retry_factor]