Merge pull request #995 from chef/reuse_metadata_connections

Improvements to EC2 metadata handling

4 files changed, 39 insertions, 40 deletions

diff --git a/lib/ohai/mixin/ec2_metadata.rb b/lib/ohai/mixin/ec2_metadata.rb
index be268434..886191fe 100644
--- a/lib/ohai/mixin/ec2_metadata.rb
+++ b/lib/ohai/mixin/ec2_metadata.rb
@@ -42,37 +42,41 @@ module Ohai
       EC2_METADATA_ADDR = "169.254.169.254" unless defined?(EC2_METADATA_ADDR)
       EC2_SUPPORTED_VERSIONS = %w{ 1.0 2007-01-19 2007-03-01 2007-08-29 2007-10-10 2007-12-15
                                    2008-02-01 2008-09-01 2009-04-04 2011-01-01 2011-05-01 2012-01-12
-                                   2014-11-05 2014-02-25 2016-04-19 2016-06-30 2016-09-02}
-
+                                   2014-02-25 2014-11-05 2015-10-20 2016-04-19 2016-06-30 2016-09-02 }
       EC2_ARRAY_VALUES = %w{security-groups}
       EC2_ARRAY_DIR    = %w{network/interfaces/macs}
       EC2_JSON_DIR     = %w{iam}
 
       def best_api_version
-        response = http_client.get("/")
-        if response.code == "404"
-          Ohai::Log.debug("ec2 metadata mixin: Received HTTP 404 from metadata server while determining API version, assuming 'latest'")
-          return "latest"
-        elsif response.code != "200"
-          raise "Unable to determine EC2 metadata version (returned #{response.code} response)"
-        end
-        # Note: Sorting the list of versions may have unintended consequences in
-        # non-EC2 environments. It appears to be safe in EC2 as of 2013-04-12.
-        versions = response.body.split("\n")
-        versions = response.body.split("\n").sort
-        until versions.empty? || EC2_SUPPORTED_VERSIONS.include?(versions.last)
-          pv = versions.pop
-          Ohai::Log.debug("ec2 metadata mixin: EC2 shows unsupported metadata version: #{pv}") unless pv == "latest"
-        end
-        Ohai::Log.debug("ec2 metadata mixin: EC2 metadata version: #{versions.last}")
-        if versions.empty?
-          raise "Unable to determine EC2 metadata version (no supported entries found)"
+        @api_version ||= begin
+          Ohai::Log.debug("ec2 metadata mixin: Fetching http://#{EC2_METADATA_ADDR}/ to determine the latest supported metadata release")
+          response = http_client.get("/")
+          if response.code == "404"
+            Ohai::Log.debug("ec2 metadata mixin: Received HTTP 404 from metadata server while determining API version, assuming 'latest'")
+            return "latest"
+          elsif response.code != "200"
+            raise "Unable to determine EC2 metadata version (returned #{response.code} response)"
+          end
+          # Note: Sorting the list of versions may have unintended consequences in
+          # non-EC2 environments. It appears to be safe in EC2 as of 2013-04-12.
+          versions = response.body.split("\n").sort
+          until versions.empty? || EC2_SUPPORTED_VERSIONS.include?(versions.last)
+            pv = versions.pop
+            Ohai::Log.debug("ec2 metadata mixin: EC2 lists metadata version: #{pv} not yet supported by Ohai") unless pv == "latest"
+          end
+          Ohai::Log.debug("ec2 metadata mixin: Latest supported EC2 metadata version: #{versions.last}")
+          if versions.empty?
+            raise "Unable to determine EC2 metadata version (no supported entries found)"
+          end
+          versions.last
         end
-        versions.last
       end
 
       def http_client
-        Net::HTTP.start(EC2_METADATA_ADDR).tap { |h| h.read_timeout = 30 }
+        @conn ||= Net::HTTP.start(EC2_METADATA_ADDR).tap do |h|
+          h.read_timeout = 10
+          h.keep_alive_timeout = 10
+        end
       end
 
       # Get metadata for a given path and API version
@@ -84,6 +88,7 @@ module Ohai
       #   `nil` and continue the run instead of failing it.
       def metadata_get(id, api_version)
         path = "/#{api_version}/meta-data/#{id}"
+        Ohai::Log.debug("ec2 metadata mixin: Fetching http://#{EC2_METADATA_ADDR}#{path}")
         response = http_client.get(path)
         case response.code
         when "200"
@@ -97,31 +102,28 @@ module Ohai
       end
 
       def fetch_metadata(id = "", api_version = nil)
-        api_version ||= best_api_version
-        return {} if api_version.nil?
-
         metadata = {}
-        retrieved_metadata = metadata_get(id, api_version)
+        retrieved_metadata = metadata_get(id, best_api_version)
         if retrieved_metadata
           retrieved_metadata.split("\n").each do |o|
             key = expand_path("#{id}#{o}")
             if key[-1..-1] != "/"
               metadata[metadata_key(key)] =
                 if EC2_ARRAY_VALUES.include? key
-                  retr_meta = metadata_get(key, api_version)
+                  retr_meta = metadata_get(key, best_api_version)
                   retr_meta ? retr_meta.split("\n") : retr_meta
                 else
-                  metadata_get(key, api_version)
+                  metadata_get(key, best_api_version)
                 end
             elsif (not key.eql?(id)) && (not key.eql?("/"))
               name = key[0..-2]
               sym = metadata_key(name)
               if EC2_ARRAY_DIR.include?(name)
-                metadata[sym] = fetch_dir_metadata(key, api_version)
+                metadata[sym] = fetch_dir_metadata(key, best_api_version)
               elsif EC2_JSON_DIR.include?(name)
-                metadata[sym] = fetch_json_dir_metadata(key, api_version)
+                metadata[sym] = fetch_json_dir_metadata(key, best_api_version)
               else
-                fetch_metadata(key, api_version).each { |k, v| metadata[k] = v }
+                fetch_metadata(key, best_api_version).each { |k, v| metadata[k] = v }
               end
             end
           end
@@ -167,17 +169,14 @@ module Ohai
       end
 
       def fetch_userdata
-        api_version = best_api_version
-        return nil if api_version.nil?
-        response = http_client.get("/#{api_version}/user-data/")
+        Ohai::Log.debug("ec2 metadata mixin: Fetching http://#{EC2_METADATA_ADDR}/#{best_api_version}/user-data/")
+        response = http_client.get("/#{best_api_version}/user-data/")
         response.code == "200" ? response.body : nil
       end
 
       def fetch_dynamic_data
         @fetch_dynamic_data ||= begin
-          api_version = best_api_version
-          return {} if api_version.nil?
-          response = http_client.get("/#{api_version}/dynamic/instance-identity/document/")
+          response = http_client.get("/#{best_api_version}/dynamic/instance-identity/document/")
 
           if json?(response.body) && response.code == "200"
             FFI_Yajl::Parser.parse(response.body)
diff --git a/spec/unit/plugins/ec2_spec.rb b/spec/unit/plugins/ec2_spec.rb
index 46888c6a..bf59c205 100644
--- a/spec/unit/plugins/ec2_spec.rb
+++ b/spec/unit/plugins/ec2_spec.rb
@@ -49,7 +49,7 @@ describe Ohai::System, "plugin ec2" do
       allow(t).to receive(:connect_nonblock).and_raise(Errno::EINPROGRESS)
       allow(Socket).to receive(:new).and_return(t)
       expect(@http_client).to receive(:get).
-        with("/").exactly(3).times.
+        with("/").
         and_return(double("Net::HTTP Response", :body => "2012-01-12", :code => "200"))
     end
 
diff --git a/spec/unit/plugins/eucalyptus_spec.rb b/spec/unit/plugins/eucalyptus_spec.rb
index 8e113a09..426301af 100644
--- a/spec/unit/plugins/eucalyptus_spec.rb
+++ b/spec/unit/plugins/eucalyptus_spec.rb
@@ -36,7 +36,7 @@ describe Ohai::System, "plugin eucalyptus" do
       allow(plugin).to receive(:http_client).and_return(@http_client)
 
       expect(@http_client).to receive(:get).
-        with("/").twice.
+        with("/").
         and_return(double("Net::HTTP Response", :body => "2012-01-12", :code => "200"))
       expect(@http_client).to receive(:get).
         with("/2012-01-12/meta-data/").
diff --git a/spec/unit/plugins/openstack_spec.rb b/spec/unit/plugins/openstack_spec.rb
index 9dd52aef..843f864c 100644
--- a/spec/unit/plugins/openstack_spec.rb
+++ b/spec/unit/plugins/openstack_spec.rb
@@ -160,7 +160,7 @@ EOM
         }'
       end
 
-      let(:http_client) { double("Net::HTTP", :read_timeout= => nil) }
+      let(:http_client) { double("Net::HTTP", { :read_timeout= => nil, :keep_alive_timeout= => nil } ) }
 
       def allow_get(url, response_body)
         allow(http_client).to receive(:get).