Add exponential increase on timeout in #sshable? for services that do not respond in 8s

fog-core.gemspec

@@ -2,6 +2,7 @@
 lib = File.expand_path("../lib", __FILE__)
 $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)
 require "fog/core/version"
+require 'english'
 
 Gem::Specification.new do |spec|
   spec.name          = "fog-core"

lib/fog/compute/models/server.rb

@@ -3,6 +3,8 @@
 module Fog
   module Compute
     class Server < Fog::Model
+      INITIAL_SSHABLE_TIMEOUT = 8
+
       attr_writer :username, :private_key, :private_key_path, :public_key, :public_key_path, :ssh_port, :ssh_options
       # Sets the proc used to determine the IP Address used for ssh/scp interactions.
       # @example
@@ -87,10 +89,37 @@ def ssh(commands, options = {}, &blk)
       end
 
       def sshable?(options = {})
-        ready? && !ssh_ip_address.nil? && !!Timeout.timeout(8) { ssh("pwd", options) }
-      rescue SystemCallError, Net::SSH::AuthenticationFailed, Net::SSH::Disconnect, Timeout::Error
+        result = ready? && !ssh_ip_address.nil? && !!Timeout.timeout(sshable_timeout) { ssh("pwd", options) }
+        @sshable_timeout = nil
+        result
+      rescue SystemCallError
+        false
+      rescue Net::SSH::AuthenticationFailed, Net::SSH::Disconnect
+        @sshable_timeout = nil
+        false
+      rescue Timeout::Error
+        increase_sshable_timeout
         false
       end
+
+
+      private
+
+      def sshable_timeout
+        @sshable_timeout ||= increase_sshable_timeout
+      end
+
+      def increase_sshable_timeout
+        if defined?(@sshable_timeout) && @sshable_timeout
+          if @sshable_timeout >= 40
+            @sshable_timeout = 60
+          else
+            @sshable_timeout *= 1.5
+          end
+        else
+          @sshable_timeout = INITIAL_SSHABLE_TIMEOUT
+        end
+      end
     end
   end
 end

spec/compute/models/server_spec.rb

@@ -0,0 +1,234 @@
+require "spec_helper"
+require "fog/compute/models/server"
+
+describe Fog::Compute::Server do
+  before do
+    @server = Fog::Compute::Server.new
+  end
+
+  describe "#sshable?" do
+    before do
+      # I'm not sure why #sshable? depends on a method that's not defined except in implementing classes
+      def @server.ready?;end
+    end
+
+    describe "when the server is not ready" do
+      it "is false" do
+        @server.stub(:ready?, false) do
+          refute @server.sshable?
+        end
+      end
+    end
+
+    describe "when the server is ready" do
+      describe "when the ssh_ip_address is nil" do
+        it "is false" do
+          @server.stub(:ready?, true) do
+            @server.stub(:ssh_ip_address, nil) do
+              refute @server.sshable?
+            end
+          end
+        end
+      end
+
+
+      describe "when the ssh_ip_address exists" do
+        # Define these constants which would be imported by net-ssh once loaded
+        module Net
+          module SSH
+            class AuthenticationFailed  < RuntimeError
+            end
+            class Disconnect < RuntimeError
+            end
+          end
+        end
+
+        describe "and ssh times out" do
+          it "is false" do
+            @server.stub(:ready?, true) do
+              @server.stub(:ssh_ip_address, "10.0.0.1") do
+                raises_timeout = lambda { |_time| raise Timeout::Error.new }
+                Timeout.stub(:timeout, raises_timeout) do
+                  refute @server.sshable?
+                end
+              end
+            end
+          end
+        end
+
+        describe "and it raises Net::SSH::AuthenticationFailed" do
+          it "is false" do
+            @server.stub(:ready?, true) do
+              @server.stub(:ssh_ip_address, "10.0.0.1") do
+                raise_error = lambda { |_cmd, _options| raise Net::SSH::AuthenticationFailed.new }
+                @server.stub(:ssh, raise_error) do
+                  refute @server.sshable?
+                end
+              end
+            end
+          end
+
+          it "resets SSH timeout" do
+            @server.instance_variable_set(:@sshable_timeout, 8)
+            @server.stub(:ready?, true) do
+              @server.stub(:ssh_ip_address, "10.0.0.1") do
+                raise_error = lambda { |_cmd, _options| raise Net::SSH::AuthenticationFailed.new }
+                @server.stub(:ssh, raise_error) do
+                  @server.sshable?
+                  assert_nil @server.instance_variable_get(:@sshable_timeout), nil
+                end
+              end
+            end
+          end
+        end
+
+        describe "and it raises Net::SSH::Disconnect" do
+          it "is false" do
+            @server.stub(:ready?, true) do
+              @server.stub(:ssh_ip_address, "10.0.0.1") do
+                raise_error = lambda { |_cmd, _options| raise Net::SSH::Disconnect.new }
+                @server.stub(:ssh, raise_error) do
+                  refute @server.sshable?
+                end
+              end
+            end
+          end
+
+          it "resets SSH timeout" do
+            @server.instance_variable_set(:@sshable_timeout, 8)
+            @server.stub(:ready?, true) do
+              @server.stub(:ssh_ip_address, "10.0.0.1") do
+                raise_error = lambda { |_cmd, _options| raise Net::SSH::Disconnect.new }
+                @server.stub(:ssh, raise_error) do
+                  @server.sshable?
+                  assert_nil @server.instance_variable_get(:@sshable_timeout), nil
+                end
+              end
+            end
+          end
+        end
+
+        describe "and it raises SystemCallError" do
+          it "is false" do
+            @server.stub(:ready?, true) do
+              @server.stub(:ssh_ip_address, "10.0.0.1") do
+                raise_error = lambda { |_cmd, _options| raise SystemCallError.new("message, 0") }
+                @server.stub(:ssh, raise_error) do
+                  refute @server.sshable?
+                end
+              end
+            end
+          end
+
+          it "does not increase SSH timeout" do
+            @server.stub(:ready?, true) do
+              @server.stub(:ssh_ip_address, "10.0.0.1") do
+                raise_error = lambda { |_cmd, _options| raise SystemCallError.new("message, 0") }
+                @server.stub(:ssh, raise_error) do
+                  @server.sshable?
+                  assert_equal @server.instance_variable_get(:@sshable_timeout), 8
+                end
+              end
+            end
+          end
+        end
+
+        describe "and ssh completes within the designated timeout" do
+          it "is true" do
+            @server.stub(:ready?, true) do
+              @server.stub(:ssh_ip_address, "10.0.0.1") do
+                @server.stub(:ssh, "datum") do
+                  assert @server.sshable?
+                end
+              end
+            end
+          end
+        end
+
+        describe "when called successively" do
+          describe "and ssh times out" do
+            it "increases the timeout factor by 1.5" do
+              @server.stub(:ready?, true) do
+                @server.stub(:ssh_ip_address, "10.0.0.1") do
+                  raises_timeout = lambda do |time|
+                    assert(time == 8)
+                    raise Timeout::Error.new
+                  end
+                  Timeout.stub(:timeout, raises_timeout) do
+                    refute @server.sshable?
+                  end
+
+                  raises_timeout = lambda do |time|
+                    assert_equal(12, time)
+                    raise Timeout::Error.new
+                  end
+                  Timeout.stub(:timeout, raises_timeout) do
+                    refute @server.sshable?
+                  end
+                end
+              end
+            end
+
+            it "does not increase timeout beyond 60s" do
+              @server.stub(:ready?, true) do
+                @server.stub(:ssh_ip_address, "10.0.0.1") do
+                  raises_timeout = lambda { |_time| raise Timeout::Error.new }
+                  Timeout.stub(:timeout, raises_timeout) do
+                    5.times { refute @server.sshable? }
+                  end
+
+                  raises_timeout = lambda do |time|
+                    assert_equal(60, time)
+                    raise Timeout::Error.new
+                  end
+                  Timeout.stub(:timeout, raises_timeout) do
+                    refute @server.sshable?
+                  end
+
+                  raises_timeout = lambda do |time|
+                    assert_equal(60, time)
+                    raise Timeout::Error.new
+                  end
+                  Timeout.stub(:timeout, raises_timeout) do
+                    refute @server.sshable?
+                  end
+                end
+              end
+            end
+
+            describe "when ssh eventually succeeds" do
+              it "resets the timeout to the initial value" do
+                @server.stub(:ready?, true) do
+                  @server.stub(:ssh_ip_address, "10.0.0.1") do
+                    raises_timeout = lambda do |time|
+                      assert(time == 8)
+                      raise Timeout::Error.new
+                    end
+                    Timeout.stub(:timeout, raises_timeout) do
+                      refute @server.sshable?
+                    end
+
+                    @server.stub(:ssh, "datum") do
+                      assert @server.sshable?
+                    end
+
+                    raises_timeout = lambda do |time|
+                      assert_equal(8, time)
+                      raise Timeout::Error.new
+                    end
+                    Timeout.stub(:timeout, raises_timeout) do
+                      refute @server.sshable?
+                    end
+                  end
+                end
+              end
+            end
+
+          end
+        end
+
+      end
+    end
+
+  end
+end

spec/core/cache_spec.rb

@@ -49,7 +49,7 @@ def initialize(opts = {})
 
     # diff namespace, diff metadata
     Fog::Cache.namespace_prefix = "different-namespace"
-    assert_equal Fog::Cache.metadata[:last_dumped], nil
+    assert_nil Fog::Cache.metadata[:last_dumped]
     # still accessible per namespace
     Fog::Cache.namespace_prefix = "for-service-user-region-foo"
     assert_equal Fog::Cache.metadata[:last_dumped],  "Tuesday, November 8, 2016"

spec/fog_attribute_spec.rb

@@ -231,7 +231,7 @@ def string.to_time
 
   describe ":default => 'default_value'" do
     it "should return nil when default is not defined on a new object" do
-      assert_equal model.bool, nil
+      assert_nil model.bool
     end
 
     it "should return the value of the object when default is not defined" do
@@ -259,18 +259,18 @@ def string.to_time
 
     it "should return nil on a persisted object without a value" do
       model.merge_attributes(:id => "some-crazy-id")
-      assert_equal model.default, nil
+      assert_nil model.default
     end
 
     it "should return nil when an attribute with default value is setted to nil" do
       model.default = nil
-      assert_equal model.default, nil
+      assert_nil model.default
     end
   end
 
   describe ".has_one" do
     it "should create an instance_variable to save the association object" do
-      assert_equal model.one_object, nil
+      assert_nil model.one_object
     end
 
     it "should create a getter to save the association model" do
@@ -292,7 +292,7 @@ def string.to_time
 
   describe ".has_one_identity" do
     it "should create an instance_variable to save the association identity" do
-      assert_equal model.one_identity, nil
+      assert_nil model.one_identity
     end
 
     it "should create a getter to load the association model" do