Providing logger to instance_generator, fixes test-kitchen/kitchen-ec…

…2#142

.rubocop.yml

@@ -13,3 +13,6 @@ Metrics/CyclomaticComplexity:
 
 Metrics/PerceivedComplexity:
   Max: 30
+
+Metrics/AbcSize:
+  Max: 60

lib/kitchen/driver/aws/instance_generator.rb

@@ -30,13 +30,12 @@ class Aws
       # @author Tyler Ball <tball@chef.io>
       class InstanceGenerator
 
-        include Logging
+        attr_reader :config, :ec2, :logger
 
-        attr_reader :config, :ec2
-
-        def initialize(config, ec2)
+        def initialize(config, ec2, logger)
           @config = config
           @ec2 = ec2
+          @logger = logger
         end
 
         # Transform the provided config into the hash to send to AWS.  Some fields
@@ -125,6 +124,7 @@ def block_device_mappings # rubocop:disable all
         # If the provided bdms match the root device in the AMI, emit log that
         # states this
         def debug_if_root_device(bdms)
+          return if bdms.nil? || bdms.empty?
           image_id = config[:image_id]
           image = ec2.resource.image(image_id)
           begin
@@ -136,7 +136,7 @@ def debug_if_root_device(bdms)
           end
           bdms.find { |bdm|
             if bdm[:device_name] == root_device_name
-              info("Overriding root device [#{root_device_name}] from image [#{image_id}]")
+              logger.info("Overriding root device [#{root_device_name}] from image [#{image_id}]")
             end
           }
         end

lib/kitchen/driver/ec2.rb

@@ -159,6 +159,8 @@ def finalize_config!(instance)
 
         if config[:availability_zone].nil?
           config[:availability_zone] = config[:region] + "b"
+        elsif config[:availability_zone] =~ /^[a-z]$/
+          config[:availability_zone] = config[:region] + config[:availability_zone]
         end
         # TODO: when we get rid of flavor_id, move this to a default
         if config[:instance_type].nil?
@@ -196,27 +198,7 @@ def create(state) # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
 
         state[:server_id] = server.id
         info("EC2 instance <#{state[:server_id]}> created.")
-        wait_log = proc do |attempts|
-          c = attempts * config[:retryable_sleep]
-          t = config[:retryable_tries] * config[:retryable_sleep]
-          info "Waited #{c}/#{t}s for instance <#{state[:server_id]}> to become ready."
-        end
-        begin
-          server = server.wait_until(
-            :max_attempts => config[:retryable_tries],
-            :delay => config[:retryable_sleep],
-            :before_attempt => wait_log
-          ) do |s|
-            hostname = hostname(s, config[:interface])
-            # Euca instances often report ready before they have an IP
-            s.exists? && s.state.name == "running" && !hostname.nil? && hostname != "0.0.0.0"
-          end
-        rescue ::Aws::Waiters::Errors::WaiterFailed
-          error("Ran out of time waiting for the server with id [#{state[:server_id]}]" \
-            " to become ready, attempting to destroy it")
-          destroy(state)
-          raise
-        end
+        wait_until_ready(server, state)
 
         info("EC2 instance <#{state[:server_id]}> ready.")
         state[:hostname] = hostname(server)
@@ -262,7 +244,7 @@ def ec2
       end
 
       def instance_generator
-        @instance_generator ||= Aws::InstanceGenerator.new(config, ec2)
+        @instance_generator ||= Aws::InstanceGenerator.new(config, ec2, instance.logger)
       end
 
       # This copies transport config from the current config object into the
@@ -334,6 +316,30 @@ def tag_server(server)
         server.create_tags(:tags => tags)
       end
 
+      def wait_until_ready(server, state)
+        wait_log = proc do |attempts|
+          c = attempts * config[:retryable_sleep]
+          t = config[:retryable_tries] * config[:retryable_sleep]
+          info "Waited #{c}/#{t}s for instance <#{state[:server_id]}> to become ready."
+        end
+        begin
+          server.wait_until(
+            :max_attempts => config[:retryable_tries],
+            :delay => config[:retryable_sleep],
+            :before_attempt => wait_log
+          ) do |s|
+            hostname = hostname(s, config[:interface])
+            # Euca instances often report ready before they have an IP
+            s.exists? && s.state.name == "running" && !hostname.nil? && hostname != "0.0.0.0"
+          end
+        rescue ::Aws::Waiters::Errors::WaiterFailed
+          error("Ran out of time waiting for the server with id [#{state[:server_id]}]" \
+            " to become ready, attempting to destroy it")
+          destroy(state)
+          raise
+        end
+      end
+
       def amis
         @amis ||= begin
           json_file = File.join(File.dirname(__FILE__),

spec/kitchen/driver/ec2/instance_generator_spec.rb

@@ -24,33 +24,39 @@
 describe Kitchen::Driver::Aws::InstanceGenerator do
 
   let(:config) { Hash.new }
-
   let(:resource) { instance_double(Aws::EC2::Resource) }
-
   let(:ec2) { instance_double(Kitchen::Driver::Aws::Client, :resource => resource) }
-
-  let(:generator) { Kitchen::Driver::Aws::InstanceGenerator.new(config, ec2) }
+  let(:logger)        { instance_double(Logger) }
+  let(:generator) { Kitchen::Driver::Aws::InstanceGenerator.new(config, ec2, logger) }
 
   describe "#debug_if_root_device" do
     let(:image_id) { "ami-123456" }
     let(:config) { { :image_id => image_id } }
     let(:image) { double("image") }
 
     before do
-      expect(resource).to receive(:image).with(image_id).and_return(image)
+      allow(resource).to receive(:image).with(image_id).and_return(image)
+    end
+
+    it "returns nil when provided nil" do
+      expect(generator.debug_if_root_device(nil)).to eq(nil)
+    end
+
+    it "returns nil when provided empty" do
+      expect(generator.debug_if_root_device([])).to eq(nil)
     end
 
     it "returns nil when the image cannot be found" do
       expect(image).to receive(:root_device_name).and_raise(
         ::Aws::EC2::Errors::InvalidAMIIDNotFound.new({}, "")
       )
-      expect(generator).to_not receive(:info)
-      expect(generator.debug_if_root_device({})).to eq(nil)
+      expect(logger).to_not receive(:info)
+      expect(generator.debug_if_root_device([{ :device_name => "name" }])).to eq(nil)
     end
 
     it "logs an info message when the device mappings are overriding the root device" do
       expect(image).to receive(:root_device_name).and_return("name")
-      expect(generator).to receive(:info)
+      expect(logger).to receive(:info)
       expect(generator.debug_if_root_device([{ :device_name => "name" }])).to eq(nil)
     end
   end

spec/kitchen/driver/ec2_spec.rb

@@ -88,6 +88,12 @@
       driver.finalize_config!(instance)
       expect(config[:availability_zone]).to eq("us-east-1b")
     end
+
+    it "expands the availability zone if provided as a letter" do
+      config[:availability_zone] = "d"
+      driver.finalize_config!(instance)
+      expect(config[:availability_zone]).to eq("us-east-1d")
+    end
   end
 
   describe "#hostname" do
@@ -171,6 +177,10 @@
   end
 
   describe "#submit_server" do
+    before do
+      expect(driver).to receive(:instance).at_least(:once).and_return(instance)
+    end
+
     it "submits the server request" do
       expect(generator).to receive(:ec2_instance_data).and_return({})
       expect(client).to receive(:create_instance).with(:min_count => 1, :max_count => 1)
@@ -184,6 +194,10 @@
       { :spot_instance_requests => [{ :spot_instance_request_id => "id" }] }
     }
 
+    before do
+      expect(driver).to receive(:instance).at_least(:once).and_return(instance)
+    end
+
     it "submits the server request" do
       expect(generator).to receive(:ec2_instance_data).and_return({})
       expect(actual_client).to receive(:request_spot_instances).with(