diff --git a/.rubocop.yml b/.rubocop.yml index 33837d299..f63e2e79b 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -13,3 +13,6 @@ Metrics/CyclomaticComplexity: Metrics/PerceivedComplexity: Max: 30 + +Metrics/AbcSize: + Max: 60 diff --git a/lib/kitchen/driver/aws/instance_generator.rb b/lib/kitchen/driver/aws/instance_generator.rb index 3fe22da60..0c6fb1aa9 100644 --- a/lib/kitchen/driver/aws/instance_generator.rb +++ b/lib/kitchen/driver/aws/instance_generator.rb @@ -30,13 +30,12 @@ class Aws # @author Tyler Ball 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 diff --git a/lib/kitchen/driver/ec2.rb b/lib/kitchen/driver/ec2.rb index ec5e302ba..ca3f9923e 100644 --- a/lib/kitchen/driver/ec2.rb +++ b/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__), diff --git a/spec/kitchen/driver/ec2/instance_generator_spec.rb b/spec/kitchen/driver/ec2/instance_generator_spec.rb index bf3da4dd0..df73b92e1 100644 --- a/spec/kitchen/driver/ec2/instance_generator_spec.rb +++ b/spec/kitchen/driver/ec2/instance_generator_spec.rb @@ -24,12 +24,10 @@ 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" } @@ -37,20 +35,28 @@ 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 diff --git a/spec/kitchen/driver/ec2_spec.rb b/spec/kitchen/driver/ec2_spec.rb index d978e83d4..2c087d6f9 100644 --- a/spec/kitchen/driver/ec2_spec.rb +++ b/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(