Please review pull request #515: Extra debug logging during plugin loading opened by (cprice-puppet)

Description:

As a new-ish puppet user, it is incredibly easy to screw up your
module path (e.g., by adding a directory that directly contains
module/plugin contents to the module path, rather than adding the
parent directory. This basically means that there would be a
'lib' directory directly in your module path.)

This commit just adds a few debug messages that may give a user
hints about what could be wrong with their module path when
we are able to detect these sorts of situations.

  • Opened: Mon Feb 20 19:08:01 UTC 2012
  • Based on: puppetlabs:master (69db777c3ad5dd4c91db5046d5d4ab928350242f)
  • Requested merge: cprice-puppet:cleanup/master/improved-logging-during-plugin-loading (eb70e49cf73253773314f1a657fa56cf63fc325e)

Diff follows:

diff --git a/acceptance/tests/pluginsync/apply_should_sync_plugins.rb b/acceptance/tests/pluginsync/apply_should_sync_plugins.rb
index bbbc69a..943238f 100644
--- a/acceptance/tests/pluginsync/apply_should_sync_plugins.rb
+++ b/acceptance/tests/pluginsync/apply_should_sync_plugins.rb
@@ -6,6 +6,8 @@
 
 on agents, "mkdir -p #{basedir}/1/a/lib/ #{basedir}/2/a/lib"
 
+# create two modules called "a", in different paths... this is intended to validate precedence in the module path;
+#  if two modules are found with the same name, the one that is found earlier in the path should be used.
 create_remote_file(agents, "#{basedir}/1/a/lib/foo.rb", "#1a")
 create_remote_file(agents, "#{basedir}/2/a/lib/foo.rb", "#2a")
 on agents, puppet_apply("--modulepath=#{basedir}/1:#{basedir}/2 --pluginsync -e 'notify { \"hello\": }'") do
diff --git a/lib/puppet/file_serving/mount/plugins.rb b/lib/puppet/file_serving/mount/plugins.rb
index a9048ab..1e5dcef 100644
--- a/lib/puppet/file_serving/mount/plugins.rb
+++ b/lib/puppet/file_serving/mount/plugins.rb
@@ -16,6 +16,7 @@ def find(relative_path, request)
   def search(relative_path, request)
     # We currently only support one kind of search on plugins - return
     # them all.
+    Puppet.debug("Warning: calling Plugins.search with empty module path.") if request.environment.modules.empty?
     paths = request.environment.modules.find_all { |mod| mod.plugins? }.collect { |mod| mod.plugin_directory }
     if paths.empty?
       # If the modulepath is valid then we still need to return a valid root
diff --git a/lib/puppet/module.rb b/lib/puppet/module.rb
index c3eff6b..e849333 100644
--- a/lib/puppet/module.rb
+++ b/lib/puppet/module.rb
@@ -64,8 +64,17 @@ def initialize(name, options = {})
     # A boolean method to let external callers determine if
     # we have files of a given type.
     define_method(type +'?') do
+      Puppet.debug("No #{type} found; path not specified") unless path
       return false unless path
-      return false unless FileTest.exist?(subpath(type))
+
+      type_subpath = subpath(type)
+      type_subpath_exists = FileTest.exist?(type_subpath)
+
+      Puppet.debug("No #{type} found in subpath '#{type_subpath}' " +
+          "(file / directory does not exist)") unless type_subpath_exists
+      return false unless type_subpath_exists
+
+      Puppet.debug("#{type} subpath found: '#{type_subpath}'")
       return true
     end
 
diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb
index 06be16d..5f88848 100644
--- a/lib/puppet/node/environment.rb
+++ b/lib/puppet/node/environment.rb
@@ -104,7 +104,15 @@ def module(name)
   # Return all modules from this environment.
   # Cache the list, because it can be expensive to create.
   cached_attr(:modules, Puppet[:filetimeout]) do
-    module_names = modulepath.collect { |path| Dir.entries(path) }.flatten.uniq
+    module_names =
+        modulepath.collect do |path|
+          module_names = Dir.entries(path)
+          Puppet.debug("Warning: Found directory named 'lib' in module path ('#{path}/lib'); unless " +
+              "you are expecting to load a module named 'lib', your module path may be set " +
+               "incorrectly.") if module_names.include?("lib")
+          module_names
+        end .flatten.uniq
+
     module_names.collect do |path|
       begin
         Puppet::Module.new(path, :environment => self)
diff --git a/spec/unit/file_serving/mount/plugins_spec.rb b/spec/unit/file_serving/mount/plugins_spec.rb
index 09da124..a5bc050 100755
--- a/spec/unit/file_serving/mount/plugins_spec.rb
+++ b/spec/unit/file_serving/mount/plugins_spec.rb
@@ -37,7 +37,7 @@
 
   describe "when searching for files" do
     it "should use the node's environment to find the modules" do
-      @environment.expects(:modules).returns []
+      @environment.expects(:modules).at_least_once.returns []
       @environment.stubs(:modulepath).returns ["/tmp/modules"]
 
       @mount.search("foo", @request)

    

--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.

Reply via email to