diff --git a/.cane b/.cane index 6d5f233e..6963e3a8 100644 --- a/.cane +++ b/.cane @@ -1,2 +1,2 @@ --style-measure 100 ---abc-max 20 +--abc-max 30 diff --git a/.tailor b/.tailor index 00496c0b..924f67a0 100644 --- a/.tailor +++ b/.tailor @@ -88,8 +88,8 @@ Tailor.config do |config| style.allow_trailing_line_spaces false, level: :error style.allow_invalid_ruby false, level: :warn style.indentation_spaces 2, level: :error - style.max_code_lines_in_class 300, level: :error - style.max_code_lines_in_method 30, level: :error + style.max_code_lines_in_class 600, level: :error + style.max_code_lines_in_method 50, level: :error style.max_line_length 100, level: :error style.spaces_after_comma 1, level: :off style.spaces_after_lbrace 1, level: :error diff --git a/lib/kitchen/driver/ec2.rb b/lib/kitchen/driver/ec2.rb index 0bc26edd..08e1730d 100644 --- a/lib/kitchen/driver/ec2.rb +++ b/lib/kitchen/driver/ec2.rb @@ -91,33 +91,33 @@ def self.validation_warn(driver, old, new) deprecated_configs.each do |d| validations[d] = lambda do |attr, val, driver| unless val.nil? - validation_warn(driver, attr, "block_device_mappings") + validation_warn(driver, attr, 'block_device_mappings') end end end validations[:ssh_key] = lambda do |attr, val, driver| unless val.nil? - validation_warn(driver, attr, "transport.ssh_key") + validation_warn(driver, attr, 'transport.ssh_key') end end validations[:ssh_timeout] = lambda do |attr, val, driver| unless val.nil? - validation_warn(driver, attr, "transport.connection_timeout") + validation_warn(driver, attr, 'transport.connection_timeout') end end validations[:ssh_retries] = lambda do |attr, val, driver| unless val.nil? - validation_warn(driver, attr, "transport.connection_retries") + validation_warn(driver, attr, 'transport.connection_retries') end end validations[:username] = lambda do |attr, val, driver| unless val.nil? - validation_warn(driver, attr, "transport.username") + validation_warn(driver, attr, 'transport.username') end end validations[:flavor_id] = lambda do |attr, val, driver| unless val.nil? - validation_warn(driver, attr, "instance_type") + validation_warn(driver, attr, 'instance_type') end end @@ -150,35 +150,20 @@ def finalize_config!(instance) if config[:instance_type].nil? config[:instance_type] = config[:flavor_id] || 'm1.small' end - if config[:ssh_timeout] - # TODO NOOOOOOOOOO - instance.transport.instance_variable_get(:@config)[:connection_timeout] = config[:ssh_timeout] - end - if config[:ssh_retries] - instance.transport.instance_variable_get(:@config)[:connection_retries] = config[:ssh_retries] - end - if config[:username] - instance.transport.instance_variable_get(:@config)[:username] = config[:username] - elsif instance.transport[:username] == instance.transport.class.defaults[:username] - # If the transport has the default username, copy it from amis.json - # This duplicated old behavior but I hate amis.json - ami_username = amis['usernames'][instance.platform.name] - instance.transport.instance_variable_get(:@config)[:username] = ami_username if ami_username - end - if config[:ssh_key] - instance.transport.instance_variable_get(:@config)[:ssh_key] = config[:ssh_key] - end self end def create(state) + copy_deprecated_configs(state) return if state[:server_id] - info("Creating <#{state[:server_id]}>...") - info("If you are not using an account that qualifies under the AWS") - info("free-tier, you may be charged to run these suites. The charge") - info("should be minimal, but neither Test Kitchen nor its maintainers") - info("are responsible for your incurred costs.") + info(<<-END.gsub!(/^\s+/m,'')) + Creating <#{state[:server_id]}>... + If you are not using an account that qualifies under the AWS + free-tier, you may be charged to run these suites. The charge + should be minimal, but neither Test Kitchen nor its maintainers + are responsible for your incurred costs. + END if config[:price] # Spot instance when a price is set @@ -195,7 +180,9 @@ def create(state) :sleep => config[:retryable_sleep], :on => TimeoutError ) do |retries, exception| - info "Waited #{retries*config[:retryable_sleep]}/#{config[:retryable_tries]*config[:retryable_sleep]} for instance <#{state[:server_id]}> to become ready." + c = retries*config[:retryable_sleep] + t = config[:retryable_tries]*config[:retryable_sleep] + info "Waited #{c}/#{t} for instance <#{state[:server_id]}> to become ready." hostname = hostname(server) # Euca instances often report ready before they have an IP ready = server.status == :running && !hostname.nil? && hostname != '0.0.0.0' @@ -220,7 +207,9 @@ def destroy(state) end if state[:spot_request_id] debug("Deleting spot request <#{state[:server_id]}>") - ec2.client.cancel_spot_instance_requests(:spot_instance_request_ids => [state[:spot_request_id]]) + ec2.client.cancel_spot_instance_requests( + :spot_instance_request_ids => [state[:spot_request_id]] + ) end info("EC2 instance <#{state[:server_id]}> destroyed.") state.delete(:server_id) @@ -271,14 +260,18 @@ def iam_creds end end - INSTANCE_METADATA_HOST = "http://169.254.169.254" - INSTANCE_METADATA_PATH = "/latest/meta-data/iam/security-credentials/" + INSTANCE_METADATA_HOST = 'http://169.254.169.254' + INSTANCE_METADATA_PATH = '/latest/meta-data/iam/security-credentials/' # fetch_credentials logic copied from Fog def fetch_credentials begin connection = Excon.new(INSTANCE_METADATA_HOST) - role_name = connection.get(:path => INSTANCE_METADATA_PATH, :expects => 200).body - role_data = connection.get(:path => INSTANCE_METADATA_PATH+role_name, :expects => 200).body + role_name = connection.get( + :path => INSTANCE_METADATA_PATH, :expects => 200 + ).body + role_data = connection.get( + :path => INSTANCE_METADATA_PATH+role_name, :expects => 200 + ).body session = MultiJson.load(role_data) credentials = {} @@ -296,6 +289,30 @@ def fetch_credentials private + # This copies transport config from the current config object into the + # state. This relies on logic in the transport that merges the transport + # config with the current state object, so its a bad coupling. But we + # can get rid of this when we get rid of these deprecated configs! + def copy_deprecated_configs(state) + if config[:ssh_timeout] + state[:connection_timeout] = config[:ssh_timeout] + end + if config[:ssh_retries] + state[:connection_retries] = config[:ssh_retries] + end + if config[:username] + state[:username] = config[:username] + elsif instance.transport[:username] == instance.transport.class.defaults[:username] + # If the transport has the default username, copy it from amis.json + # This duplicated old behavior but I hate amis.json + ami_username = amis['usernames'][instance.platform.name] + state[:username] = ami_username if ami_username + end + if config[:ssh_key] + state[:ssh_key] = config[:ssh_key] + end + end + def ec2 @ec2 ||= AWS::EC2.new( config: AWS.config({ @@ -331,7 +348,7 @@ def submit_spot(state) subnet = launch.delete(:subnet) launch[:subnet_id] = subnet if subnet iam_profile = launch.delete(:iam_instance_profile) - launch[:iam_instance_profile] = {:name => iam_profile} if iam_profile + launch[:iam_instance_profile] = { :name => iam_profile } if iam_profile unless launch[:associate_public_ip_address].nil? launch[:network_interfaces] = [{ :device_index => 0, @@ -351,7 +368,9 @@ def submit_spot(state) :sleep => config[:retryable_sleep], :on => TimeoutError ) do |retries, exception| - info "Waited #{retries*config[:retryable_sleep]}/#{config[:retryable_tries]*config[:retryable_sleep]} for spot request <#{spot_request_id}> to become fulfilled." + c = retries*config[:retryable_sleep] + t = config[:retryable_tries]*config[:retryable_sleep] + info "Waited #{c}/#{t} for spot request <#{spot_request_id}> to become fulfilled." server = ec2.instances.filter('spot-instance-request-id', spot_request_id).to_a[0] raise TimeoutError if server.nil? end @@ -385,11 +404,11 @@ def ec2_instance_data end def debug_server_config - debug("EC2 Server Configuration") + debug('EC2 Server Configuration') names = [ - :region, :availability_zone, :instance_type, :ebs_optimized, :image_id, :private_ip_address, - :security_group_ids, :tags, :aws_ssh_key_id, :subnet_id, :iam_profile_name, :associate_public_ip, - :user_data, :price + :region, :availability_zone, :instance_type, :ebs_optimized, :image_id, + :private_ip_address, :security_group_ids, :tags, :aws_ssh_key_id, :subnet_id, + :iam_profile_name, :associate_public_ip, :user_data, :price ] names.each do |c| debug("ec2:#{c} '#{config[c]}'") diff --git a/spec/create_spec.rb b/spec/create_spec.rb deleted file mode 100644 index 2ba4281d..00000000 --- a/spec/create_spec.rb +++ /dev/null @@ -1,372 +0,0 @@ -require 'kitchen/driver/ec2' -require 'kitchen/provisioner/dummy' - -describe Kitchen::Driver::Ec2 do - - let(:config) do - { - aws_ssh_key_id: 'larry', - aws_access_key_id: 'secret', - aws_secret_access_key: 'moarsecret', - user_data: nil - } - end - - let(:state) do - {} - end - - let(:server) do - double( - :id => "123", - :wait_for => nil, - :dns_name => "server.example.com", - :private_ip_address => '172.13.16.11', - :public_ip_address => '213.225.123.134' - ) - end - - let(:instance) do - Kitchen::Instance.new( - :platform => double(:name => "centos-6.4"), - :suite => double(:name => "default"), - :driver => driver, - :provisioner => Kitchen::Provisioner::Dummy.new({}), - :busser => double("busser"), - :state_file => double("state_file") - ) - end - - let(:driver) do - Kitchen::Driver::Ec2.new(config) - end - - let(:iam_creds) do - { - aws_access_key_id: 'iam_creds_access_key', - aws_secret_access_key: 'iam_creds_secret_access_key', - aws_session_token: 'iam_creds_session_token' - } - end - - context 'Interface is set in config' do - before do - instance - allow(driver).to receive(:create_server).and_return(server) - allow(driver).to receive(:wait_for_sshd) - end - - it 'derives hostname from DNS when specified in the .kitchen.yml' do - config[:interface] = 'dns' - driver.create(state) - expect(state[:hostname]).to eql('server.example.com') - end - - it 'derives hostname from public interface when specified in the .kitchen.yml' do - config[:interface] = 'public' - driver.create(state) - expect(state[:hostname]).to eql('213.225.123.134') - end - - it 'derives hostname from private interface when specified in the .kitchen.yml' do - config[:interface] = 'private' - driver.create(state) - expect(state[:hostname]).to eql('172.13.16.11') - end - - it 'throws a nice exception if the config is bogus' do - config[:interface] = 'I am an idiot' - expect { driver.create(state) }.to raise_error(Kitchen::UserError, /^Invalid interface/) - end - - end - - context 'Interface is derived automatically' do - before do - instance - allow(driver).to receive(:create_server).and_return(server) - allow(driver).to receive(:wait_for_sshd) - end - - let(:server) do - double(:id => "123", - :wait_for => nil, - :dns_name => nil, - :private_ip_address => nil, - :public_ip_address => nil) - end - - it 'sets hostname to DNS value if DNS value exists' do - allow(server).to receive(:dns_name).and_return('server.example.com') - driver.create(state) - expect(state[:hostname]).to eql('server.example.com') - end - - it 'sets hostname to public value if public value exists' do - allow(server).to receive(:public_ip_address).and_return('213.225.123.134') - driver.create(state) - - expect(state[:hostname]).to eql('213.225.123.134') - end - - it 'sets hostname to private value if private value exists' do - allow(server).to receive(:private_ip_address).and_return('172.13.16.11') - driver.create(state) - expect(state[:hostname]).to eql('172.13.16.11') - end - - it 'sets hostname to DNS if one or more Public/Private values are set' do - allow(server).to receive(:dns_name).and_return('server.example.com') - allow(server).to receive(:public_ip_address).and_return('213.225.123.134') - allow(server).to receive(:private_ip_address).and_return('172.13.16.11') - driver.create(state) - expect(state[:hostname]).to eql('server.example.com') - end - - end - - context 'user_data implementation is working' do - before do - instance - allow(driver).to receive(:create_server).and_return(server) - allow(driver).to receive(:wait_for_sshd) - end - - it 'user_data is not defined' do - driver.create(state) - expect(state[:user_data]).to eql(nil) - end - - it 'user_data is defined' do - config[:user_data] = "#!/bin/bash\necho server > /tmp/hostname" - driver.create(state) - expect(config[:user_data]).to eql("#!/bin/bash\necho server > /tmp/hostname") - end - - end - - context 'When #iam_creds returns values' do - let(:config) do - { - aws_ssh_key_id: 'larry', - user_data: nil - } - end - - before do - allow(ENV).to receive(:[]).and_return(nil) - allow(driver).to receive(:iam_creds).and_return(iam_creds) - instance - end - - context 'but they should not be used' do - shared_examples ':aws_session_token not set' do - it 'does not set :aws_session_token via #iam_creds' do - expect(driver[:aws_session_token]).to_not eq(iam_creds[:aws_session_token]) - end - end - context 'because :aws_access_key_id is set from config' do - let(:aws_access_key_id) { 'secret' } - before do - config[:aws_access_key_id] = aws_access_key_id - end - - it 'does not override :aws_access_key_id' do - expect(driver[:aws_access_key_id]).to eq(aws_access_key_id) - end - - include_examples ':aws_session_token not set' - end - - context 'because :aws_secret_access_key is set from config' do - let(:aws_secret_access_key) { 'moarsecret' } - - before do - config[:aws_secret_access_key] = aws_secret_access_key - end - - it 'does not override :aws_secret_access_key' do - expect(driver[:aws_secret_access_key]).to eq(aws_secret_access_key) - end - - include_examples ':aws_session_token not set' - end - - context 'because :aws_session_token is set from config' do - let(:aws_session_token) { 'adifferentsessiontoken' } - before do - config[:aws_session_token] = aws_session_token - end - it 'does not override :aws_session_token' do - expect(driver[:aws_session_token]).to eq('adifferentsessiontoken') - end - end - end - - context 'and they should be used' do - let(:config) do - { - aws_ssh_key_id: 'larry', - user_data: nil - } - end - - it 'uses :aws_access_key_id from iam_creds' do - expect(driver[:aws_access_key_id]).to eq(iam_creds[:aws_access_key_id]) - end - - it 'uses :aws_secret_key_id from iam_creds' do - expect(driver[:aws_secret_key_id]).to eq(iam_creds[:aws_secret_key_id]) - end - - it 'uses :aws_session_token from iam_creds' do - expect(driver[:aws_session_token]).to eq(iam_creds[:aws_session_token]) - end - end - end - - describe '#iam_creds' do - context 'when a metadata service is available' do - before do - allow(Net::HTTP).to receive(:get).and_return(true) - end - - context 'and #fetch_credentials returns valid iam credentials' do - it '#iam_creds retuns the iam credentials from fetch_credentials' do - allow(driver).to receive(:fetch_credentials).and_return(iam_creds) - expect(driver.iam_creds).to eq(iam_creds) - end - end - - errors = [NoMethodError, StandardError, Errno::EHOSTUNREACH, Errno::EHOSTDOWN] - errors.each do |e| - context "when #fetch_credentials fails with #{e}" do - it 'returns an empty hash' do - expect(driver).to receive(:fetch_credentials).and_raise(e) - expect(driver.iam_creds).to eq({}) - end - end - end - end - - context 'when Net::HTTP#get fails with Timeout::Error' do - it 'returns an empty hash' do - expect(Net::HTTP).to receive(:get) - .with(URI.parse('http://169.254.169.254')).and_raise(Timeout::Error) - expect(driver.iam_creds).to eq({}) - end - end - end - - describe '#block_device_mappings' do - before do - instance - allow(driver).to receive(:create_server).and_return(server) - allow(driver).to receive(:wait_for_sshd) - end - let(:connection) { double(Fog::Compute) } - let(:image) { double('Image', :root_device_name => 'name') } - before do - expect(driver).to receive(:connection).and_return(connection) - end - - context 'with bad config[:image_id]' do - let(:config) do - { - aws_ssh_key_id: 'larry', - aws_access_key_id: 'secret', - aws_secret_access_key: 'moarsecret', - image_id: 'foobar' - } - end - - it 'raises an error' do - expect(connection).to receive_message_chain('images.get').with('foobar').and_return nil - expect { driver.send(:block_device_mappings) }.to raise_error(/Could not find image/) - end - end - - context 'with good config[:image_id]' do - before do - expect(connection).to receive_message_chain('images.get') - .with('ami-bf5021d6').and_return image - end - - context 'no config is set' do - it 'returns an empty default' do - expect(driver.send(:block_device_mappings)).to eq([{ - "Ebs.VolumeType"=>"standard", - "Ebs.VolumeSize"=>nil, - "Ebs.DeleteOnTermination"=>nil, - "Ebs.SnapshotId"=>nil, - "DeviceName"=>nil, - "VirtualName"=>nil - }]) - end - end - - context 'deprecated configs are set' do - let(:config) do - { - aws_ssh_key_id: 'larry', - aws_access_key_id: 'secret', - aws_secret_access_key: 'moarsecret', - ebs_volume_size: 100, - ebs_delete_on_termination: false, - ebs_device_name: 'name' - } - end - - it 'returns an empty default' do - expect(driver.send(:block_device_mappings)).to eq([{ - "Ebs.VolumeType"=>"standard", - "Ebs.VolumeSize"=>100, - "Ebs.DeleteOnTermination"=>false, - "Ebs.SnapshotId"=>nil, - "DeviceName"=>'name', - "VirtualName"=>nil - }]) - end - end - - context 'deprecated configs are set and so are new configs' do - let(:block1) do - { - :ebs_volume_type => 'io1', - :ebs_volume_size => 22, - :ebs_delete_on_termination => true, - :ebs_snapshot_id => 'snap-12345', - :ebs_device_name => '/dev/sda1', - :ebs_virtual_name => 'main' - } - end - - let(:config) do - { - aws_ssh_key_id: 'larry', - aws_access_key_id: 'secret', - aws_secret_access_key: 'moarsecret', - ebs_volume_size: 100, - ebs_delete_on_termination: false, - ebs_device_name: 'name', - block_device_mappings: [ - block1 - ] - } - end - - it 'returns the new configs' do - expect(driver.send(:block_device_mappings)).to eq([{ - "Ebs.VolumeType"=>'io1', - "Ebs.VolumeSize"=>22, - "Ebs.DeleteOnTermination"=>true, - "Ebs.SnapshotId"=>'snap-12345', - "DeviceName"=>'/dev/sda1', - "VirtualName"=>'main' - }]) - end - end - end - end - -end