Merge pull request #124 from test-kitchen/tball/errors

Fixing error where aws returns DNS name as empty string

Author: tyler-ball

.rubocop.yml

@@ -7,3 +7,9 @@ Style/Next:
 
 Style/DoubleNegation:
   Enabled: false
+
+Metrics/CyclomaticComplexity:
+  Max: 30
+
+Metrics/PerceivedComplexity:
+  Max: 30

lib/kitchen/driver/ec2.rb

@@ -20,9 +20,10 @@
 require "json"
 require "aws"
 require "kitchen"
-require "kitchen/driver/ec2_version"
+require_relative "ec2_version"
 require_relative "aws/client"
 require_relative "aws/instance_generator"
+require "aws-sdk-core/waiters/errors"
 
 module Kitchen
 
@@ -60,6 +61,7 @@ class Ec2 < Kitchen::Driver::Base # rubocop:disable Metrics/ClassLength
       end
       default_config :username,            nil
       default_config :associate_public_ip, nil
+      default_config :interface,           nil
 
       required_config :aws_ssh_key_id
       required_config :image_id
@@ -194,14 +196,21 @@ def create(state) # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
           t = config[:retryable_tries] * config[:retryable_sleep]
           info "Waited #{c}/#{t}s for instance <#{state[:server_id]}> to become ready."
         end
-        server = server.wait_until(
-          :max_attempts => config[:retryable_tries],
-          :delay => config[:retryable_sleep],
-          :before_attempt => wait_log
-        ) do |s|
-          hostname = hostname(s)
-          # Euca instances often report ready before they have an IP
-          s.state.name == "running" && !hostname.nil? && hostname != "0.0.0.0"
+        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
 
         info("EC2 instance <#{state[:server_id]}> ready.")
@@ -217,7 +226,7 @@ def destroy(state)
         server = ec2.get_instance(state[:server_id])
         unless server.nil?
           instance.transport.connection(state).close
-          server.terminate unless server.nil?
+          server.terminate
         end
         if state[:spot_request_id]
           debug("Deleting spot request <#{state[:server_id]}>")
@@ -235,8 +244,6 @@ def default_ami
         region && region[instance.platform.name]
       end
 
-      private
-
       def ec2
         @ec2 ||= Aws::Client.new(
           config[:region],
@@ -354,6 +361,8 @@ def hostname(server, interface_type = nil)
           potential_hostname = nil
           INTERFACE_TYPES.values.each do |type|
             potential_hostname ||= server.send(type)
+            # AWS returns an empty string if the dns name isn't populated yet
+            potential_hostname = nil if potential_hostname == ""
           end
           potential_hostname
         end

spec/kitchen/driver/ec2_spec.rb

@@ -71,12 +71,92 @@
     end
   end
 
-  describe "finalize_config!" do
+  describe "#finalize_config!" do
     it "defaults the availability zone if not provided" do
       expect(config[:availability_zone]).to eq(nil)
       driver.finalize_config!(instance)
       expect(config[:availability_zone]).to eq("us-east-1b")
     end
   end
 
+  describe "#hostname" do
+    let(:public_dns_name) { nil }
+    let(:public_ip_address) { nil }
+    let(:private_ip_address) { nil }
+    let(:server) {
+      double("server",
+        :public_dns_name => public_dns_name,
+        :public_ip_address => public_ip_address,
+        :private_ip_address => private_ip_address
+      )
+    }
+
+    it "returns nil if all sources are nil" do
+      expect(driver.hostname(server)).to eq(nil)
+    end
+
+    it "raises an error if provided an unknown interface" do
+      expect { driver.hostname(server, "foobar") }.to raise_error(Kitchen::UserError)
+    end
+
+    shared_examples "an interface type provided" do
+      it "returns public_dns_name when requested" do
+        expect(driver.hostname(server, "dns")).to eq(public_dns_name)
+      end
+      it "returns public_ip_address when requested" do
+        expect(driver.hostname(server, "public")).to eq(public_ip_address)
+      end
+      it "returns private_ip_address when requested" do
+        expect(driver.hostname(server, "private")).to eq(private_ip_address)
+      end
+    end
+
+    context "private_ip_address is populated" do
+      let(:private_ip_address) { "10.0.0.1" }
+
+      it "returns the private_ip_address" do
+        expect(driver.hostname(server)).to eq(private_ip_address)
+      end
+
+      include_examples "an interface type provided"
+    end
+
+    context "public_ip_address is populated" do
+      let(:private_ip_address) { "10.0.0.1" }
+      let(:public_ip_address) { "127.0.0.1" }
+
+      it "returns the public_ip_address" do
+        expect(driver.hostname(server)).to eq(public_ip_address)
+      end
+
+      include_examples "an interface type provided"
+    end
+
+    context "public_dns_name is populated" do
+      let(:private_ip_address) { "10.0.0.1" }
+      let(:public_ip_address) { "127.0.0.1" }
+      let(:public_dns_name) { "public_dns_name" }
+
+      it "returns the public_dns_name" do
+        expect(driver.hostname(server)).to eq(public_dns_name)
+      end
+
+      include_examples "an interface type provided"
+    end
+
+    context "public_dns_name returns as empty string" do
+      let(:public_dns_name) { "" }
+      it "returns nil" do
+        expect(driver.hostname(server)).to eq(nil)
+      end
+
+      context "and private_ip_address is populated" do
+        let(:private_ip_address) { "10.0.0.1" }
+        it "returns the private_ip_address" do
+          expect(driver.hostname(server)).to eq(private_ip_address)
+        end
+      end
+    end
+  end
+
 end