deprecate looking for EC2 by macbtm/deprecate_ec2_mac

we should have enough other methods now, hints have been around for a while.
this detects xen, not ec2.

also cleans up some confusing whitespacing in the test.

2 files changed, 55 insertions, 42 deletions

diff --git a/lib/ohai/plugins/ec2.rb b/lib/ohai/plugins/ec2.rb
index 7ad80e5e..1aeb5316 100644
--- a/lib/ohai/plugins/ec2.rb
+++ b/lib/ohai/plugins/ec2.rb
@@ -33,6 +33,12 @@ Ohai.plugin(:EC2) do
     network[:interfaces].values.each do |iface|
       unless iface[:arp].nil?
         if iface[:arp].value?("fe:ff:ff:ff:ff:ff")
+          # using MAC addresses from ARP is unreliable because they could time-out from the table
+          # fe:ff:ff:ff:ff:ff is actually a sign of Xen, not specifically EC2
+          deprecation_message <<-EOM
+Detected EC2 by the presence of fe:ff:ff:ff:ff:ff in the ARP table. This method is unreliable and will be removed in a future version of ohai. Use knife-ec2 or create "/etc/chef/ohai/hints/ec2.json" instead.
+EOM
+          Ohai::Log.warn(deprecation_message)
           Ohai::Log.debug("has_ec2_mac? == true")
           return true
         end
diff --git a/spec/unit/plugins/ec2_spec.rb b/spec/unit/plugins/ec2_spec.rb
index 0b02df1c..41102c7e 100644
--- a/spec/unit/plugins/ec2_spec.rb
+++ b/spec/unit/plugins/ec2_spec.rb
@@ -50,51 +50,52 @@ describe Ohai::System, "plugin ec2" do
       allow(File).to receive(:exist?).and_return(false)
     end
 
-context "with common metadata paths" do
-   let(:paths) do
-     { "meta-data/" => "instance_type\nami_id\nsecurity-groups",
-       "meta-data/instance_type" => "c1.medium",
-       "meta-data/ami_id" => "ami-5d2dc934",
-       "meta-data/security-groups" => "group1\ngroup2"
-     }
-   end
-    it "recursively fetches all the ec2 metadata" do
-      paths.each do |name,body|
-        expect(@http_client).to receive(:get).
-          with("/2012-01-12/#{name}").
-          and_return(double("Net::HTTP Response", :body => body, :code => "200"))
+    context "with common metadata paths" do
+      let(:paths) do
+        { "meta-data/" => "instance_type\nami_id\nsecurity-groups",
+          "meta-data/instance_type" => "c1.medium",
+          "meta-data/ami_id" => "ami-5d2dc934",
+         "meta-data/security-groups" => "group1\ngroup2"
+        }
       end
-      expect(@http_client).to receive(:get).
-        with("/2012-01-12/user-data/").
-        and_return(double("Net::HTTP Response", :body => "By the pricking of my thumb...", :code => "200"))
 
-      @plugin.run
+      it "recursively fetches all the ec2 metadata" do
+        paths.each do |name,body|
+          expect(@http_client).to receive(:get).
+            with("/2012-01-12/#{name}").
+            and_return(double("Net::HTTP Response", :body => body, :code => "200"))
+        end
+        expect(@http_client).to receive(:get).
+          with("/2012-01-12/user-data/").
+          and_return(double("Net::HTTP Response", :body => "By the pricking of my thumb...", :code => "200"))
 
-      expect(@plugin[:ec2]).not_to be_nil
-      expect(@plugin[:ec2]['instance_type']).to eq("c1.medium")
-      expect(@plugin[:ec2]['ami_id']).to eq("ami-5d2dc934")
-      expect(@plugin[:ec2]['security_groups']).to eql ['group1', 'group2']
-    end
+        @plugin.run
 
-    it "fetches binary userdata opaquely" do
-      paths.each do |name,body|
-        expect(@http_client).to receive(:get).
-          with("/2012-01-12/#{name}").
-          and_return(double("Net::HTTP Response", :body => body, :code => "200"))
+        expect(@plugin[:ec2]).not_to be_nil
+        expect(@plugin[:ec2]['instance_type']).to eq("c1.medium")
+        expect(@plugin[:ec2]['ami_id']).to eq("ami-5d2dc934")
+        expect(@plugin[:ec2]['security_groups']).to eql ['group1', 'group2']
       end
-      expect(@http_client).to receive(:get).
-        with("/2012-01-12/user-data/").
-        and_return(double("Net::HTTP Response", :body => "^_<8B>^H^H<C7>U^@^Csomething^@KT<C8><C9>,)<C9>IU(I-.I<CB><CC>I<E5>^B^@^Qz<BF><B0>^R^@^@^@", :code => "200"))
 
-      @plugin.run
+      it "fetches binary userdata opaquely" do
+        paths.each do |name,body|
+          expect(@http_client).to receive(:get).
+            with("/2012-01-12/#{name}").
+            and_return(double("Net::HTTP Response", :body => body, :code => "200"))
+        end
+        expect(@http_client).to receive(:get).
+          with("/2012-01-12/user-data/").
+          and_return(double("Net::HTTP Response", :body => "^_<8B>^H^H<C7>U^@^Csomething^@KT<C8><C9>,)<C9>IU(I-.I<CB><CC>I<E5>^B^@^Qz<BF><B0>^R^@^@^@", :code => "200"))
 
-      expect(@plugin[:ec2]).not_to be_nil
-      expect(@plugin[:ec2]['instance_type']).to eq("c1.medium")
-      expect(@plugin[:ec2]['ami_id']).to eq("ami-5d2dc934")
-      expect(@plugin[:ec2]['security_groups']).to eql ['group1', 'group2']
-      expect(@plugin[:ec2]['userdata']).to eq(Base64.decode64("Xl88OEI+XkheSDxDNz5VXkBeQ3NvbWV0aGluZ15AS1Q8Qzg+PEM5PiwpPEM5\nPklVKEktLkk8Q0I+PENDPkk8RTU+XkJeQF5RejxCRj48QjA+XlJeQF5AXkA="))
+        @plugin.run
+
+        expect(@plugin[:ec2]).not_to be_nil
+        expect(@plugin[:ec2]['instance_type']).to eq("c1.medium")
+        expect(@plugin[:ec2]['ami_id']).to eq("ami-5d2dc934")
+        expect(@plugin[:ec2]['security_groups']).to eql ['group1', 'group2']
+        expect(@plugin[:ec2]['userdata']).to eq(Base64.decode64("Xl88OEI+XkheSDxDNz5VXkBeQ3NvbWV0aGluZ15AS1Q8Qzg+PEM5PiwpPEM5\nPklVKEktLkk8Q0I+PENDPkk8RTU+XkJeQF5RejxCRj48QjA+XlJeQF5AXkA="))
+      end
     end
-end
 
     it "should parse ec2 network/ directory as a multi-level hash" do
       expect(@http_client).to receive(:get).
@@ -123,7 +124,7 @@ end
 
       expect(@plugin[:ec2]).not_to be_nil
       expect(@plugin[:ec2]['network_interfaces_macs']['12:34:56:78:9a:bc']['public_hostname']).to eql('server17.opscode.com')
-    end
+    end # context with common metadata paths
 
     context "with ec2_iam cloud file" do
       before do
@@ -251,17 +252,23 @@ end
       expect(@plugin[:ec2]['metrics']).to be_nil
       expect(@plugin[:ec2]['metrics_vhostmd']).to be_nil
     end
-  end
-
-  describe "with ec2 mac and metadata address connected" do
-    it_should_behave_like "ec2"
+  end # shared examples for ec2
 
+  describe "without dmi, with xen mac, and metadata address connected" do
     before(:each) do
       allow(IO).to receive(:select).and_return([[],[1],[]])
       @plugin[:network][:interfaces][:eth0][:arp] = {"169.254.1.0"=>"fe:ff:ff:ff:ff:ff"}
     end
+
+    it_should_behave_like "ec2"
+
+    it "warns that the arp table method is deprecated" do
+      expect(Ohai::Log).to receive(:warn).with(/will be removed/)
+      @plugin.has_ec2_mac?
+    end
   end
 
+
   describe "without ec2 mac and metadata address connected" do
     it_should_behave_like "!ec2"