OHAI-573: Only claim to be an LXC Host if we're pretty sure that we are

4 files changed, 135 insertions, 13 deletions

diff --git a/lib/ohai/plugins/linux/virtualization.rb b/lib/ohai/plugins/linux/virtualization.rb
index f41c0db6..49d01565 100644
--- a/lib/ohai/plugins/linux/virtualization.rb
+++ b/lib/ohai/plugins/linux/virtualization.rb
@@ -16,12 +16,20 @@
 # limitations under the License.
 #
 
+require 'ohai/util/file_helper'
+
+include Ohai::Util::FileHelper
+
 Ohai.plugin(:Virtualization) do
   provides "virtualization"
 
+  def lxc_version_exists?
+    which('lxc-version')
+  end
+
   collect_data(:linux) do
-    virtualization Mash.new
-    virtualization[:systems] = Mash.new
+    virtualization Mash.new unless virtualization
+    virtualization[:systems] = Mash.new unless virtualization[:systems]
 
     # if it is possible to detect paravirt vs hardware virt, it should be put in
     # virtualization[:mechanism]
@@ -165,9 +173,18 @@ Ohai.plugin(:Virtualization) do
         virtualization[:system] = "lxc"
         virtualization[:role] = "guest"
         virtualization[:systems][:lxc] = "guest"
-      elsif File.read("/proc/self/cgroup") =~ %r{\d:[^:]+:/$}
-        virtualization[:system] = "lxc"
-        virtualization[:role] = "host"
+      elsif lxc_version_exists? && File.read("/proc/self/cgroup") =~ %r{\d:[^:]+:/$}
+        # lxc-version shouldn't be installed by default
+        # Even so, it is likely we are on an LXC capable host that is not being used as such
+        # So we're cautious here to not overwrite other existing values (OHAI-573)
+        unless virtualization[:system] && virtualization[:role]
+          virtualization[:system] = "lxc"
+          virtualization[:role] = "host"
+        end
+
+        # In general, the 'systems' framework from OHAI-182 is less susceptible to conflicts
+        # But, this could overwrite virtualization[:systems][:lxc] = "guest"
+        # If so, we may need to look further for a differentiator (OHAI-573)
         virtualization[:systems][:lxc] = "host"
       end
     end
diff --git a/lib/ohai/util/file_helper.rb b/lib/ohai/util/file_helper.rb
new file mode 100644
index 00000000..1864e5d4
--- /dev/null
+++ b/lib/ohai/util/file_helper.rb
@@ -0,0 +1,35 @@
+# Author:: Lamont Granquist (<lamont@opscode.com>)
+#
+# Copyright:: Copyright (c) 2013-14 Chef Software, Inc.
+#
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Copied from chef/lib/chef/util/selinux.rb
+
+module Ohai
+  module Util
+    module FileHelper
+      def which(cmd)
+        paths = ENV['PATH'].split(File::PATH_SEPARATOR) + [ '/bin', '/usr/bin', '/sbin', '/usr/sbin' ]
+        paths.each do |path|
+          filename = File.join(path, cmd)
+          return filename if File.executable?(filename)
+        end
+        false
+      end
+    end
+  end
+end
+
diff --git a/spec/unit/plugins/linux/virtualization_spec.rb b/spec/unit/plugins/linux/virtualization_spec.rb
index 18a95768..890c425d 100644
--- a/spec/unit/plugins/linux/virtualization_spec.rb
+++ b/spec/unit/plugins/linux/virtualization_spec.rb
@@ -358,8 +358,9 @@ CGROUP
       @plugin[:virtualization].should == {'systems' => {}}
     end
 
-    it "should set lxc host if /proc/self/cgroup only has / mounts" do
-      self_cgroup=<<-CGROUP
+    context "/proc/self/cgroup only has / mounts" do
+      before(:each) do
+        self_cgroup=<<-CGROUP
 8:blkio:/
 7:net_cls:/
 6:freezer:/
@@ -369,12 +370,36 @@ CGROUP
 2:cpu:/
 1:cpuset:/
 CGROUP
-      File.should_receive(:exists?).with("/proc/self/cgroup").and_return(true)
-      File.stub(:read).with("/proc/self/cgroup").and_return(self_cgroup)
-      @plugin.run
-      @plugin[:virtualization][:system].should == "lxc"
-      @plugin[:virtualization][:role].should == "host"
-      @plugin[:virtualization][:systems][:lxc].should == "host"
+        File.should_receive(:exists?).with("/proc/self/cgroup").and_return(true)
+        File.stub(:read).with("/proc/self/cgroup").and_return(self_cgroup)
+      end
+    
+      it "sets lxc host if lxc-version exists" do
+        @plugin.stub(:lxc_version_exists?).and_return("/usr/bin/lxc-version")
+        @plugin.run
+        @plugin[:virtualization][:system].should == "lxc"
+        @plugin[:virtualization][:role].should == "host"
+        @plugin[:virtualization][:systems][:lxc].should == "host"
+      end
+
+      it "does not set the old virtualization attributes if they are already set" do
+        @plugin.stub(:lxc_version_exists?).and_return("/usr/bin/lxc-version")
+        @plugin[:virtualization] = Mash.new
+        @plugin[:virtualization][:system] = "the cloud"
+        @plugin[:virtualization][:role] = "cumulonimbus"
+        @plugin.run
+        @plugin[:virtualization][:system].should_not == "lxc"
+        @plugin[:virtualization][:role].should_not == "host"
+      end
+
+      it "does not set lxc host if lxc-version does not exist" do
+        @plugin.stub(:lxc_version_exists?).and_return(false)
+        @plugin.run
+        @plugin[:virtualization][:system].should be_nil
+        @plugin[:virtualization][:role].should be_nil
+        @plugin[:virtualization].should == {'systems' => {}}
+      end
+
     end
 
     it "should not set virtualization if /proc/self/cgroup isn't there" do
diff --git a/spec/unit/util/file_helper_spec.rb b/spec/unit/util/file_helper_spec.rb
new file mode 100644
index 00000000..5c6cf7f6
--- /dev/null
+++ b/spec/unit/util/file_helper_spec.rb
@@ -0,0 +1,45 @@
+# Author:: Bryan McLellan <btm@loftninjas.org>
+#
+# Copyright:: Copyright (c) 2014 Chef Software, Inc.
+#
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+require 'spec_helper'
+require 'ohai/util/file_helper'
+
+class FileHelperMock
+  include Ohai::Util::FileHelper
+end
+
+
+describe "Ohai::Util::FileHelper" do
+  let(:file_helper) { FileHelperMock.new }
+
+  before(:each) do
+    File.stub(:executable?).and_return(false)
+  end
+
+  describe "which" do
+    it "returns the path to an executable that is in the path" do
+      File.stub(:executable?).with('/usr/bin/skyhawk').and_return(true)
+
+      expect(file_helper.which('skyhawk')).to eql "/usr/bin/skyhawk"
+    end
+
+    it "returns false if the executable is not in the path" do
+      expect(file_helper.which('the_cake')).to be_false
+    end
+  end
+end