From 533c3464671b2d457a6cda9455057774971eb998 Mon Sep 17 00:00:00 2001
From: Takashi Kajinami <kajinamit@oss.nttdata.com>
Date: Tue, 1 Oct 2024 21:19:58 +0900
Subject: [PATCH] Extend command timeout

In some deployments with huge number of resources, some commands
(especially list commands) may take long time. Extend the timeout to
avoid giving up commands too early.

Also extend the whole timeout and sleep between command executions,
to avoid too many requests within timeouts.

Change-Id: I3610340067d416dc7be94696f2fba921fa015eb9
---
 lib/puppet/provider/openstack.rb     | 17 ++++++++---------
 spec/unit/provider/openstack_spec.rb | 10 +++++-----
 2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/lib/puppet/provider/openstack.rb b/lib/puppet/provider/openstack.rb
index 5c261e3d..c9439932 100644
--- a/lib/puppet/provider/openstack.rb
+++ b/lib/puppet/provider/openstack.rb
@@ -14,11 +14,9 @@ class Puppet::Provider::Openstack < Puppet::Provider
   commands :openstack_command => 'openstack'
 
   @@no_retry_actions = %w(create remove delete)
-  @@command_timeout  = 40
-  # Fails on the 5th retry for a max of 212s (~3.5min) before total
-  # failure.
-  @@request_timeout  = 170
-  @@retry_sleep      = 3
+  @@command_timeout  = 90
+  @@request_timeout  = 300
+  @@retry_sleep      = 10
   class << self
     [:no_retry_actions, :request_timeout, :retry_sleep].each do |m|
       define_method m do
@@ -89,8 +87,8 @@ class Puppet::Provider::Openstack < Puppet::Provider
 
     Puppet::Util.withenv(env) do
       rv = nil
-      end_time = current_time + request_timeout
       start_time = current_time
+      end_time = start_time + request_timeout
       retry_count = 0
       loop do
         begin
@@ -129,9 +127,10 @@ class Puppet::Provider::Openstack < Puppet::Provider
         rescue Puppet::ExecutionFailure => exception
           raise Puppet::Error::OpenstackUnauthorizedError, 'Could not authenticate' if exception.message =~ /HTTP 40[13]/
           raise Puppet::Error::OpenstackUnauthorizedError, 'Could not authenticate' if exception.message.match(/Missing value \S* required for auth plugin/)
-          if current_time > end_time
+          remaining_time = end_time - current_time
+          if remaining_time < 0
             error_message = exception.message
-            error_message += " (tried #{retry_count}, for a total of #{end_time - start_time } seconds)"
+            error_message += " (tried #{retry_count}, for a total of #{end_time - start_time} seconds)"
             raise(Puppet::ExecutionFailure, error_message)
           end
 
@@ -142,7 +141,7 @@ class Puppet::Provider::Openstack < Puppet::Provider
               raise exception if exception.message.match(nr)
             end
           end
-          debug "Non-fatal error: '#{exception.message}'. Retrying for #{end_time - current_time} more seconds"
+          debug "Non-fatal error: '#{exception.message}'. Retrying for #{remaining_time} more seconds"
           sleep retry_sleep
           retry_count += 1
           retry
diff --git a/spec/unit/provider/openstack_spec.rb b/spec/unit/provider/openstack_spec.rb
index 81b636aa..30999a76 100644
--- a/spec/unit/provider/openstack_spec.rb
+++ b/spec/unit/provider/openstack_spec.rb
@@ -111,7 +111,7 @@ name="test"
               lambda { |*args| raise Puppet::ExecutionFailure, 'Unable to establish connection' },
               lambda { |*args| return list_data }
             )
-        expect(provider.class).to receive(:sleep).with(3).and_return(nil)
+        expect(provider.class).to receive(:sleep).with(10).and_return(nil)
         response = Puppet::Provider::Openstack.request('project', 'list', ['--long'])
         expect(response.first[:description]).to eq 'Test tenant'
       end
@@ -119,10 +119,10 @@ name="test"
       it 'fails after the timeout and redacts' do
         expect(provider.class).to receive(:execute)
             .and_raise(Puppet::ExecutionFailure, "Execution of 'openstack user create foo --password secret' returned 1: command failed")
-            .exactly(3).times
+            .exactly(6).times
         allow(provider.class).to receive(:sleep)
         allow(provider.class).to receive(:current_time)
-            .and_return(0, 10, 10, 20, 20, 200, 200)
+            .and_return(0, 10, 20, 100, 200, 300, 400)
         expect do
           Puppet::Provider::Openstack.request('project', 'list', ['--long'])
         end.to raise_error Puppet::ExecutionFailure, /Execution of \'openstack user create foo --password \[redacted secret\]\' returned 1/
@@ -132,10 +132,10 @@ name="test"
         expect(provider.class).to receive(:openstack)
             .with('project', 'list', '--quiet', '--format', 'csv', ['--long'])
             .and_raise(Puppet::ExecutionFailure, 'Unable to establish connection')
-            .exactly(3).times
+            .exactly(6).times
         allow(provider.class).to receive(:sleep)
         allow(provider.class).to receive(:current_time)
-            .and_return(0, 10, 10, 20, 20, 200, 200)
+            .and_return(0, 10, 20, 100, 200, 300, 400)
         expect do
           Puppet::Provider::Openstack.request('project', 'list', ['--long'])
         end.to raise_error Puppet::ExecutionFailure, /Unable to establish connection/