The Puppet Labs Issue Tracker has Moved: https://tickets.puppetlabs.com

Bug #7316

puppet face applications (subcommands) delivered via modules should work

Added by Dan Bode over 3 years ago. Updated almost 2 years ago.

Status:ClosedStart date:05/02/2011
Priority:ImmediateDue date:
Assignee:-% Done:

0%

Category:Faces
Target version:3.1.0
Affected Puppet version:3.0.0 Branch:https://github.com/puppetlabs/puppet/pull/1319
Keywords:face faces subcommand application module plugin pluginsync backlog

We've Moved!

Ticket tracking is now hosted in JIRA: https://tickets.puppetlabs.com

This issue is currently not available for export. If you are experiencing the issue described below, please file a new ticket in JIRA. Once a new ticket has been created, please add a link to it that points back to this Redmine ticket.


Description

If you deliver a new face that consists of:

  • application
  • face
  • action for face

via pluginsync, then the application isn’t actually found, and worse, it taunts you by showing it to you in the list of available subcommands.


Related issues

Related to Puppet - Refactor #6753: Faces should be available as `puppet ${face}` without a s... Accepted 03/17/2011
Related to Puppet - Refactor #7749: Bootstrapping Puppet in Ruby code has nasty scope cycles... Closed 06/01/2011
Related to Puppet - Feature #4248: Load "library" plugins that are used by multiple puppet f... Accepted 07/15/2010
Related to Puppet - Bug #15212: get rid of support for run-mode sections in puppet config... Accepted 06/25/2012
Related to Puppet - Bug #15211: libdir et al should not be able to be set for client prog... Rejected 06/25/2012
Related to Puppet - Bug #15300: Pluginsync collisions Accepted 06/29/2012
Related to Puppet - Bug #15214: Get rid of coupling between run_mode and confdir/vardir Duplicate 06/25/2012
Related to Puppet - Bug #17156: Regression: Can't restrict the plugins loaded by puppet d... Rejected
Related to Puppet - Bug #12173: Masters cannot reliably distinguish between multiple vers... Accepted 01/25/2012
Related to Puppet - Bug #15529: Puppet settings should be initialized prior to running an... Closed 07/15/2012
Related to Puppet - Bug #16277: Initialise Puppet settings before running tests to enable... Duplicate 09/06/2012
Related to Puppet - Feature #12126: make autoloader able to reload changed plugins Closed 01/24/2012
Related to Puppet - Bug #6426: autoloader doesn't use Ruby requires correctly Closed 02/23/2011
Related to Puppet - Bug #3674: autoloader doesn't propagate failures Closed 04/26/2010
Related to Puppet Documentation - Bug #16646: Don't instruct people to use #loadall within custom funct... Rejected 10/01/2012
Related to Puppet - Bug #15215: refactor places where we're explicitly referencing LOAD_P... Duplicate 06/25/2012
Related to Puppet - Feature #7788: Puppet should allow rubygems to deliver new functionality Closed 06/04/2011
Related to Puppet - Bug #14589: Puppet performs excessive stats when loading types (and o... Investigating 05/18/2012
Related to Puppet - Bug #11858: Plugin load paths are not consistent between runs Closed 01/09/2012
Related to Puppet - Feature #12085: The application name 'apply' is hardcoded and no other ap... Closed 01/23/2012
Related to Puppet - Bug #13948: libdir does not always get added to LOAD_PATH Closed 04/13/2012
Related to Puppet - Bug #14152: Hooks for dependent settings not called Duplicate 04/24/2012
Related to Puppet - Bug #7991: Issue with using custom functions in templates. Closed 06/20/2011
Related to Puppet - Bug #4549: not all puppet functions are available in the template scope Closed 08/17/2010
Related to Puppet - Bug #17305: puppet module build fails with Error: undefined method `d... Duplicate
Related to Puppet - Bug #17554: runinterval ignored on Windows Closed
Related to Puppet - Bug #17371: Setting owner, group, mode for files specified in puppet.... Closed
Related to Puppet - Feature #6907: Ensure providers can be used in the same puppet run that ... Closed 03/30/2011
Related to Puppet - Bug #17747: Inability to use gems as dependencies in custom providers... Duplicate
Related to Puppet - Feature #17783: No public interface to reinitialize all settings to find ... Accepted
Related to Puppet - Bug #17136: puppet node doesn't respect ssldir from puppet.conf Closed
Related to Puppet - Bug #8750: puppetmaster needs restart to pick up on changes in types... Duplicate 08/03/2011
Related to Puppet - Bug #13898: faces silently fail against master Closed 04/11/2012
Related to Puppet - Bug #1027: autoloading fails to find providers with pluginsync Closed
Related to Puppet - Bug #18042: Deprecate face versions Accepted
Related to Puppet - Bug #18154: Master and agent should not share libdir Accepted
Related to Puppet - Bug #18459: Changing plugindest causes puppet to not be able to load ... Accepted
Related to Puppet - Bug #18461: master and agent on the same host load code from the wron... Accepted
Duplicated by Puppet - Bug #9018: Faces cannot add subcommands when distributed as a module Duplicate 08/15/2011
Duplicated by Puppet - Bug #14073: puppet apply cannot find types in modules on Windows Duplicate 04/18/2012
Duplicated by Puppet - Bug #16651: Installing the cloud provisioner module breaks the node s... Duplicate 10/01/2012
Duplicated by Puppet - Bug #3947: puppet executable does not load all ruby files from modul... Duplicate 06/06/2010

History

#1 Updated by Anonymous over 3 years ago

  • Status changed from Unreviewed to Needs Decision
  • Assignee set to Nigel Kersten
  • Affected Puppet version set to 2.7.0rc1

Nigel, I don’t know if we want to support this or not, but it is a genuine issue. The Faces code doesn’t make substantial effort to massage the module path into the Ruby load path, and uses only the later to try and locate code.

Given we extensively use require for loading, not having those library directories on the Ruby load path will require substantial extra work, as well as reimplementing the require mechanism ourselves.

On the other hand, obviously we don’t already put the lib directories on the module path into the Ruby load path, so I don’t know what side-effects that change might have on the system; that makes me nervous about putting this into the RC series for 2.7, and about putting it in post-2.7.0, since both of those might be the worst choice depending on what assumptions this tripped up.

#2 Updated by Nigel Kersten over 3 years ago

  • Tracker changed from Feature to Bug
  • Subject changed from faces should support loading faces from modulepath to puppet applications delivered via pluginsync don't work.
  • Status changed from Needs Decision to Accepted
  • Assignee deleted (Nigel Kersten)
  • Target version set to 2.7.1

#3 Updated by James Turnbull over 3 years ago

  • Priority changed from Normal to High

I actually think this is a higher priority – not only does pluginsync partially work giving you the disappointing illusion that things are okay :) But without it Faces are very manual to deploy and can’t really be developed and shared – which seems to defeat some of their purpose. YMMV. :)

#4 Updated by Luke Kanies over 3 years ago

Can someone provide an idea of what’s actually wrong here? There are a lot of “doesn’t work” reports in this, but very little detail.

In talking with James, he says it looks like the Faces are found but not the applications. In looking at the application code, AFAICT it’s loading applications from the $LOAD_PATH, Puppet’s lib dir is in the $LOAD_PATH, and the applications should be copied to Puppet’s lib dir.

What actually goes wrong in this sequence?

#5 Updated by James Turnbull over 3 years ago

So tested so far.

I pluginsync the face.

  1. When I run puppet as a normal (non-root) user I don’t see anything.
  2. When I run puppet as the root user – my face shows up in the output of puppet help.
  3. When I try to run the face I get:
$ puppet github
Error: Unknown Puppet subcommand 'github'

#6 Updated by Anonymous over 3 years ago

So tested so far. I pluginsync the face.

Did you sync the puppet/application/github.rb file as well?

When I run puppet as a normal (non-root) user I don’t see anything. When I run puppet as the root user – my face shows up in the output of puppet help. When I try to run the face I get:

$ puppet github Error: Unknown Puppet subcommand ‘github’

My best guess, given the (entirely uninvestigated, since we have no time scheduled to faces at the moment) reports, is that the application is getting synced, but not seen by the loader, so it fails with “unknown subcommand”.

#7 Updated by Nigel Kersten over 3 years ago

Daniel has it. The synced application isn’t found by the autoloader.

#8 Updated by Nigel Kersten over 3 years ago

  • Priority changed from High to Urgent

#9 Updated by Nick Lewis over 3 years ago

  • Assignee set to Nick Lewis

#10 Updated by Anonymous over 3 years ago

  • Target version changed from 2.7.1 to 2.7.x

#11 Updated by Nick Lewis over 3 years ago

What’s happening is that command_line simply looks in $LOAD_PATH, rather than using the autoloader.

I’ve experimented with connecting it to the autoloader, which works great except for some interesting load order issues. Primarily around defaults.rb getting executed, which tries to set Puppet[:name] (which is read-only after that) to Puppet.application_name, which obviously hasn’t been set yet. It may work to make Puppet[:name] not read-only, and set it in command_line after loading the application (or in Application during the ‘run’ sequence), possibly with a meaningless default. Or without a default at all, since it’s not actually configurable.

#12 Updated by Anonymous over 3 years ago

What’s happening is that command_line simply looks in $LOAD_PATH, rather than using the autoloader.

I’ve experimented with connecting it to the autoloader, which works great except for some interesting load order issues. Primarily around defaults.rb getting executed, which tries to set Puppet[:name] (which is read-only after that) to Puppet.application_name, which obviously hasn’t been set yet.

Ah! Another part of the fragile load ordering issue identified in

7749. Awesome.

I will connect the two tickets together, and see where that gets us to and all.

#13 Updated by Carl Caum over 3 years ago

It works if you set your RUBYLIB to puppet’s $vardir/lib

#14 Updated by Jeff McCune over 3 years ago

  • Keywords set to face faces subcommand application module plugin pluginsync

#15 Updated by Jeff McCune over 3 years ago

  • Subject changed from puppet applications delivered via pluginsync don't work. to puppet face applications (subcommands) delivered via pluginsync and as modules should work

Modules

Since #9018 was closed as a duplicate of this, I just want to make sure it’s clear that a solution to this ticket requires that puppet stand alone with a modulepath set correctly should also find the face application sub-commands. That is to say, the scope of this ticket is NOT simply limited to pluginsync.

#16 Updated by Jeff McCune about 3 years ago

  • Affected Puppet version deleted (2.7.0rc1)

Nick,

I’m trying to plan the remaining work for Cloud Provisioner in CK and this ticket impacts my work in that I may or may not be able to implement additional subcommands (application) beyond “puppet node” if this is resolved in 2.7.4.

Are you in a position to estimate if this ticket will be resolved in 2.7.4 or not?

Thanks, -Jeff

#17 Updated by K Hightower about 3 years ago

  • Assignee changed from Nick Lewis to K Hightower

#18 Updated by Nick Lewis about 3 years ago

  • Assignee changed from K Hightower to Nick Lewis

I’m working on this now. Making applications loadable is fairly straightforward, although there’s some weird issues around setting modulepath and libdir, because we can’t use the config file for those settings (because we don’t have a runmode, so where do we look?). Also, naturally, applications need to be run as a user that can read them. This means on the master they need to be run as root or the user the master runs as, and on the agent, they probably need to be run as root.

#19 Updated by Nick Lewis about 3 years ago

  • Status changed from Accepted to Investigating

While applications are loadable from modules, loading the faces on which they depend is proving much more difficult. The main problem is that we don’t allow actions to be redefined, so we can’t load a face or action more than once, so we use require. But require is out of our control as far as module path goes. We could require via the autoloader, but that would necessarily use the absolute path, which doesn’t do anything to prevent multiple loading in the case where the file IS in the regular load path, and gets required again with its relative path. Unfortunately, because faces are a public API, this is a real concern. Requiring an action would autoload the face, which would autoload the action and we get a conflict.

The closest thing to a solution I’ve got as yet is to add a hook to the modulepath setting to populate the load path with the lib dir of every module in each directory of the modulepath. Unfortunately, that doesn’t play well with environments, and would also mean that if a module is added, we wouldn’t find it until at least the filetimeout (when we refresh the modulepath).

Alternatively, we could have a mechanism to “pluginsync” the contents of module libs into the libdir, which would greatly simplify the situation.

#20 Updated by Nigel Kersten about 3 years ago

Great update Nick. Just pointing out that not playing well with environments is a huge problem, and I’d like to have a chat in person if that’s the only feasible solution we have.

#21 Updated by Nick Lewis about 3 years ago

So to clarify before I get myself further entangled in this mess: is it actually a requirement that we be able to run applications from the modulepath on the master without first having pluginsunc them? I guess the case where this matters is a master which is not also an agent. Do we need to support that case, or is sufficient to say in that case you must add the modules you need to your RUBYLIB?

#22 Updated by Anonymous about 3 years ago

  • Status changed from Investigating to Needs Decision
  • Assignee changed from Nick Lewis to Nigel Kersten

Nigel Kersten wrote:

Great update Nick. Just pointing out that not playing well with environments is a huge problem, and I’d like to have a chat in person if that’s the only feasible solution we have.

After some discussion, it sounds like this is more complex: for a master/agent model it shouldn’t be using content from modules before they are pushed to the system with pluginsync. (That also helps address the case of environments, where you might have different versions of the same application in multiple environments.)

There might be some complexity around the puppet apply case, where we don’t do pluginsync, but we do want to see some code from modules. That, fairly obviously, doesn’t apply to other top level commands in Puppet though. (…as, if we are running apply we can’t also be running something else. :)

I have assigned this back to Nigel for now, since he can’t work out the details today…

#23 Updated by Nigel Kersten almost 3 years ago

I’m not sure what actual decision is left here for me to make.

  • We don’t need to support loading apps from modules on a master that haven’t been pluginsynced to the local agent
  • We do need to play well with environments
  • We do need to work with puppet apply and a local module. If this means we need to implement “pluginsync” on puppet apply (from the local modulepath source) for consistency, I’m ok with that.

#24 Updated by Anonymous almost 3 years ago

  • Status changed from Needs Decision to Accepted
  • Assignee deleted (Nigel Kersten)

Thanks. That seems to cover any questions – perhaps just a restatement was all that was needed. :)

#25 Updated by eric sorenson almost 3 years ago

  • Assignee set to Nick Lewis

Triage-a-thon: This is Accepted, marked as Urgent, and target for 2.7.x even — but has no Assignee. Sending it to Nick as he seems to be the one doing the actual work here. :)

#26 Updated by Anonymous almost 3 years ago

  • Assignee deleted (Nick Lewis)

#27 Updated by Chris Price almost 3 years ago

  • Assignee set to Chris Price

#28 Updated by Nick Lewis almost 3 years ago

I have a partial branch for this, that adds support for loading applications from $libdir, as well as from the modulepath, by using the autoloader. The latter behavior is undesired, so should be removed, but there is some work around refactoring the bootstrap process a bit which may be useful.

https://github.com/nicklewis/puppet/tree/ticket/2.7.x/7316

#29 Updated by Chris Price almost 3 years ago

Initial acceptance test for this here:

https://github.com/puppetlabs/puppet/pull/518

These don’t deal with anything related to environments yet.

#30 Updated by Matthew Black almost 3 years ago

Nick Lewis wrote:

I have a partial branch for this, that adds support for loading applications from $libdir, as well as from the modulepath, by using the autoloader. The latter behavior is undesired, so should be removed, but there is some work around refactoring the bootstrap process a bit which may be useful.

https://github.com/nicklewis/puppet/tree/ticket/2.7.x/7316

Not sure why it would need to load from the modulepath, wouldnt it load it from libdir for the environment that the server requested from? Even the server running the puppet master would download it to its libdir on a plugin sync for its environment.

#31 Updated by Chris Price almost 3 years ago

correct. we won’t be loading from the module path.

#32 Updated by Anonymous almost 3 years ago

  • Status changed from Accepted to Merged - Pending Release
  • Target version changed from 2.7.x to 2.7.11

#33 Updated by Chris Price over 2 years ago

  • Status changed from Merged - Pending Release to Code Insufficient
  • Target version changed from 2.7.11 to 3.x

This isn’t actually fixed yet. Pull request 518 ( https://github.com/puppetlabs/puppet/pull/518 ) was merged, but that just contains a (failing) acceptance test that I wanted to get in in case anyone else besides myself was looking at this ticket.

#34 Updated by Chris Price over 2 years ago

  • Status changed from Code Insufficient to In Topic Branch Pending Review
  • Branch set to https://github.com/puppetlabs/puppet/pull/571

#35 Updated by Chris Price over 2 years ago

  • Status changed from In Topic Branch Pending Review to Merged - Pending Release

#36 Updated by Anonymous over 2 years ago

  • Target version changed from 3.x to 3.0.0

#37 Updated by Jeff McCune over 2 years ago

  • Status changed from Merged - Pending Release to Code Insufficient

Re-opening

I’m smoke testing this with puppet 3.0rc (Head db604e35).

Here’s my smoke testing setup:

  • puppet master running via webrick on my mac using puppet master --confdir /vagrant/etc/puppet.30 -v --no-daemonize
  • puppet agent running on my mac using puppet agent --confdir /vagrant/etc/puppet.30 --test
  • Only two modules installed, cloud_provisioner 1.0.4-2-gf68dacd and stdlib 2.3.3

I trigger this issue by doing the following commands:

  1. puppet agent --confdir /vagrant/etc/puppet.30 --test
  2. puppet help --confdir /vagrant/etc/puppet.30

Here’s what I get:

$ puppet help --confdir /vagrant/etc/puppet.30                    
Error: Could not autoload puppet/face/node/classify: no such file to load -- puppet/cloudpack
Error: Could not autoload puppet/face/node/classify: no such file to load -- puppet/cloudpack
Error: Try 'puppet help help help' for usage

Here’s the puppet.conf I’m using. (Note, I have explicitly split the agent vardir from the master vardir since I’m running on the same node.)

# /vagrant/etc/puppet.30/puppet.conf
[main]
certname = maynard
node_name_value = maynard
server = maynard
confdir = /vagrant/etc/puppet.30
vardir = $confdir/var.shared
ssldir = $vardir/ssl

[agent]
certname = maynard.agent
node_name_value = maynard.agent
vardir = $confdir/var.agent

[master]
vardir = $confdir/var.master

Here’s the trace:

Error: Could not autoload puppet/face/node/classify: no such file to load -- puppet/cloudpack
/Users/jeff/.rvm/rubies/ruby-1.8.7-p334/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:36
/Users/jeff/.rvm/rubies/ruby-1.8.7-p334/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:36
/Users/jeff/vms/puppet/modules/cloud_provisioner/lib/puppet/face/node/classify.rb:1
/Users/jeff/vms/puppet/src/puppet/lib/puppet/util/autoload.rb:59
/Users/jeff/vms/puppet/src/puppet/lib/puppet/util/autoload.rb:59
/Users/jeff/vms/puppet/src/puppet/lib/puppet/util/autoload.rb:74
/Users/jeff/vms/puppet/src/puppet/lib/puppet/util/autoload.rb:72
/Users/jeff/vms/puppet/src/puppet/lib/puppet/util/autoload.rb:72
/Users/jeff/vms/puppet/src/puppet/lib/puppet/util/autoload.rb:200
/Users/jeff/vms/puppet/src/puppet/lib/puppet/interface.rb:113
/Users/jeff/vms/puppet/src/puppet/lib/puppet/interface.rb:45
/Users/jeff/vms/puppet/src/puppet/lib/puppet/face/node.rb:2
/Users/jeff/.rvm/rubies/ruby-1.8.7-p334/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:36
/Users/jeff/.rvm/rubies/ruby-1.8.7-p334/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:36
/Users/jeff/vms/puppet/src/puppet/lib/puppet/interface/face_collection.rb:103
/Users/jeff/vms/puppet/src/puppet/lib/puppet/interface/face_collection.rb:59
/Users/jeff/vms/puppet/src/puppet/lib/puppet/interface/face_collection.rb:20
/Users/jeff/vms/puppet/src/puppet/lib/puppet/interface.rb:58
/Users/jeff/vms/puppet/src/puppet/lib/puppet/face/help.rb:122
/Users/jeff/vms/puppet/src/puppet/lib/puppet/face/help/global.erb:12
/Users/jeff/vms/puppet/src/puppet/lib/puppet/face/help.rb:118
/Users/jeff/vms/puppet/src/puppet/lib/puppet/face/help.rb:118
/Users/jeff/vms/puppet/src/puppet/lib/puppet/face/help.rb:118
/Users/jeff/vms/puppet/src/puppet/lib/puppet/face/help/global.erb:5
/Users/jeff/vms/puppet/src/puppet/lib/puppet/face/help.rb:94
/Users/jeff/vms/puppet/src/puppet/lib/puppet/interface/action.rb+eval[wrapper]:208
/Users/jeff/vms/puppet/src/puppet/lib/puppet/interface/action.rb+eval[wrapper]:208
/Users/jeff/vms/puppet/src/puppet/lib/puppet/application/face_base.rb:239
/Users/jeff/vms/puppet/src/puppet/lib/puppet/application/face_base.rb:239
/Users/jeff/vms/puppet/src/puppet/lib/puppet/application.rb:350
/Users/jeff/vms/puppet/src/puppet/lib/puppet/application.rb:342
/Users/jeff/vms/puppet/src/puppet/lib/puppet/application.rb:436
/Users/jeff/vms/puppet/src/puppet/lib/puppet/application.rb:342
/Users/jeff/vms/puppet/src/puppet/lib/puppet/util.rb:529
/Users/jeff/vms/puppet/src/puppet/lib/puppet/application.rb:342
/Users/jeff/vms/puppet/src/puppet/lib/puppet/util/command_line.rb:74
/Users/jeff/vms/puppet/src/puppet/bin/puppet:10
Error: Could not autoload puppet/face/node/classify: no such file to load -- puppet/cloudpack
/Users/jeff/vms/puppet/src/puppet/lib/puppet/util/autoload.rb:66
/Users/jeff/vms/puppet/src/puppet/lib/puppet/util/autoload.rb:74
/Users/jeff/vms/puppet/src/puppet/lib/puppet/util/autoload.rb:72
/Users/jeff/vms/puppet/src/puppet/lib/puppet/util/autoload.rb:72
/Users/jeff/vms/puppet/src/puppet/lib/puppet/util/autoload.rb:200
/Users/jeff/vms/puppet/src/puppet/lib/puppet/interface.rb:113
/Users/jeff/vms/puppet/src/puppet/lib/puppet/interface.rb:45
/Users/jeff/vms/puppet/src/puppet/lib/puppet/face/node.rb:2
/Users/jeff/.rvm/rubies/ruby-1.8.7-p334/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:36
/Users/jeff/.rvm/rubies/ruby-1.8.7-p334/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:36
/Users/jeff/vms/puppet/src/puppet/lib/puppet/interface/face_collection.rb:103
/Users/jeff/vms/puppet/src/puppet/lib/puppet/interface/face_collection.rb:59
/Users/jeff/vms/puppet/src/puppet/lib/puppet/interface/face_collection.rb:20
/Users/jeff/vms/puppet/src/puppet/lib/puppet/interface.rb:58
/Users/jeff/vms/puppet/src/puppet/lib/puppet/face/help.rb:122
/Users/jeff/vms/puppet/src/puppet/lib/puppet/face/help/global.erb:12
/Users/jeff/vms/puppet/src/puppet/lib/puppet/face/help.rb:118
/Users/jeff/vms/puppet/src/puppet/lib/puppet/face/help.rb:118
/Users/jeff/vms/puppet/src/puppet/lib/puppet/face/help.rb:118
/Users/jeff/vms/puppet/src/puppet/lib/puppet/face/help/global.erb:5
/Users/jeff/vms/puppet/src/puppet/lib/puppet/face/help.rb:94
/Users/jeff/vms/puppet/src/puppet/lib/puppet/interface/action.rb+eval[wrapper]:208
/Users/jeff/vms/puppet/src/puppet/lib/puppet/interface/action.rb+eval[wrapper]:208
/Users/jeff/vms/puppet/src/puppet/lib/puppet/application/face_base.rb:239
/Users/jeff/vms/puppet/src/puppet/lib/puppet/application/face_base.rb:239
/Users/jeff/vms/puppet/src/puppet/lib/puppet/application.rb:350
/Users/jeff/vms/puppet/src/puppet/lib/puppet/application.rb:342
/Users/jeff/vms/puppet/src/puppet/lib/puppet/application.rb:436
/Users/jeff/vms/puppet/src/puppet/lib/puppet/application.rb:342
/Users/jeff/vms/puppet/src/puppet/lib/puppet/util.rb:529
/Users/jeff/vms/puppet/src/puppet/lib/puppet/application.rb:342
/Users/jeff/vms/puppet/src/puppet/lib/puppet/util/command_line.rb:74
/Users/jeff/vms/puppet/src/puppet/bin/puppet:10
Error: Try 'puppet help help help' for usage

#38 Updated by Jeff McCune over 2 years ago

  • Assignee deleted (Chris Price)

I’d like to make sure this issue has properties set such that the 3.0 release is blocked until this is resolved. Are all of the fields set correct for this to happen? (I think it’s just the target version field and the fact that it’s now open, right?)

#39 Updated by Chris Price over 2 years ago

Jeff, how did you install the modules? from the forge or are they just git clones? I don’t see why that should make a difference but just trying to make sure I have all the data points that I need in order to try to repro.

#40 Updated by Chris Price over 2 years ago

The first time I ran this, the error I got was:

Error: Could not autoload puppet/face/node/install: no such file to load -- guid
/home/cprice/.rvm/rubies/ruby-1.8.7-p334/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:36      
/home/cprice/.rvm/rubies/ruby-1.8.7-p334/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:36      
/home/cprice/work/puppet/test/client/var/lib/puppet/cloudpack.rb:3           

I installed the guid gem and then got the same error about fog. So, while there is probably a valid puppet issue here, there is a separate issue here that might trump everything; that is that we don’t have a good story for how to deal with gem dependencies via the forge.

#41 Updated by Chris Price over 2 years ago

Now that I’m past the gem stuff, I’m getting this:

Error: Could not autoload puppet/face/node_aws/create: "--tags=": already defined in puppet

This is a known issue that was discussed at length in an e-mail thread; it was decided that faces should not be allowed to define options that collided with the names of puppet settings. This was implemented in #13898 and a few related tickets.

If this is what is causing the problem for you, Jeff, then my understanding of the current situation is that this is a problem with the cloud provisioner module and not with puppet. However, since I’m not getting exactly the same error message that you are, there could be something else going on as well; the only two differences that I am aware of between your environment and mine are:

  1. I haven’t yet tried the config file with a separate vardir for master and agent (I am explicitly specifying both on the command line), and
  2. I could not find the git hash that you referenced for cloud provisioner, so I’m just using the current master version of it.

#42 Updated by Chris Price over 2 years ago

I will spend a bit more time trying to repro Jeff’s exact error as soon as I get a chance.

#43 Updated by Chris Price over 2 years ago

Putting aside the issue with the config file specifying alternate vardirs for a moment (granting that it is a valid issue that we need to address):

When running both master and agent/client with explicit “—confdir” and “—vardir” options (pointing to separate directories for master vs agent), the only errors that I get are about the face overriding puppet’s “tags” and “group” settings. I made a very small change to my cloudpack.rb file:

def add_create_options(action)
  add_platform_option(action)
  add_region_option(action)
  add_availability_zone_option(action)
  #add_tags_option(action)
  add_image_option(action)
  add_type_option(action)
  add_keyname_option(action)
  add_subnet_option(action)
  #add_group_option(action)
end

To prevent those option/settings collisions, and now things work fine for me as far as I can tell; puppet help shows the “node_aws” face as expected, and I appear to be able to run actions inside of it (up to the point where my lack of AWS credentials stops me).

Of significant note here: this means that the face is successfully loading the cloudpack.rb file and executing the ‘require’ lines:

require 'rubygems'
require 'guid'
require 'fog'
require 'net/ssh'
require 'puppet/network/http_pool'
require 'puppet/cloudpack/progressbar'
require 'puppet/cloudpack/utils'

I believe that this contradicts the earlier statement about not being able to use normal ruby “require” statements. Please correct me if I am wrong.

This still leaves open the issue about the puppet.conf file needing to be able to specify a different vardir for both the master and the agent, which I’ll continue to investigate; but I believe that to be a much smaller problem and I believe that the findings above may significantly narrow the scope of the reported issue. Again, please correct me if I am mistaken.

#44 Updated by Chris Price over 2 years ago

OK, I’ve played around with the config file and the multiple vardir settings, and have been able to reproduce and diagnose the error that Jeff described.

There are a couple of things going on here. First of all, when you run a master and an agent on the same node, and they share the same confdir, then they will also share the same modulepath (unless you’ve explicitly overridden it). When you do an agent run the facts are synced to the agent libdir. Unfortunately, the autoloader looks in both the module path and the libdir for facts, so this would explain why Jeff saw the facts being loaded twice. This is annoying and stupid behavior, and Patrick and I have discussed on several occasions the fact that the autoloader should never load from the modulepath for any application other than ‘master’; but it should be harmless in this case. (I will explain why in more detail below.)

The second issue is that Jeff’s config file uses three vardirs, not two. It uses a separate vardir for master, agent, and ‘main’. If you do a “—configprint vardir” for either master or agent using his config file, you will see that these settings are being respected. The problem is that when you run “puppet help”, your libdir is now the one specified in main, and NOT the one specified in agent. Since the faces / modules that you just synced down to the agent libdir are NOT in the libdir for the “help” application, bad things start to happen; because the autoloader can’t find what it is looking for in the libdir, it starts looking in the modulepath. This is when you get the “no such file to load — puppet/cloudpack”. So, while it sucks that the autoloader is falling back to the module path (and that should eventually be fixed, as I mentioned above), the real issue here is that libdir is not set correctly.

So what should be done about this? I can see a few options; they would all be relatively easy to implement, but each a bit clumsy in its own way. The first option is that we could force all applications (including help and other faces) to explicitly look for the “agent” vardir/libdir settings and prefer those over ones found in “main”. This feels like a hack to me, but it would make Jeff’s config file work as he expected.

The second option would be to say that it is a configuration error to define a vardir or libdir in the “agent” section of the config file. You could still define them in both “main” and “master” if you really wanted the master to use separate paths from the other tools. Basically it makes sense for master to have its own libdir if desired, but all of the client-side tools need to share a libdir so it seems wrong to allow that to be configured in the “agent” section of the config file (since it applies more broadly than to just the agent).

I believe that Jeff mentioned that this multiple-vardirs-in-the-same-puppet-conf file setup was how we ship PE out of the box. I had a look at the puppet.conf.erb file in the PE “dist” repo, and it does not look to me like that is actually the case. We only set ‘vardir’ once in that file; in the “main” section. This means that all puppet applications (master, agent, and all faces including ‘help’) will share the same vardir, and thus the same libdir. Therefore I do not believe that the issue that Jeff reported will occur; I expect synced faces to work properly (but have not tested that yet).

#45 Updated by Chris Price over 2 years ago

The long and short of the above:

  1. we eventually need to improve the autoloader’s decision-making about including the modulepath, but this is more of an annoyance than a show-stopper for now.
  2. we need to decide on the semantics of how to enforce that all client-side tools must use the same libdir, and then implement that (this should be a 1 or 2-day project, at most)
  3. the cloud provisioner module needs to be updated to conform to the design decisions that were made as part of #13898. This basically entails renaming some options, so this should be a very minimal fix as well.

I am reasonably certain that after those things are done we will have met our stated litmus test about being able to run cloud provisioner against telly.

#46 Updated by Chris Price over 2 years ago

One other minor update: during discussions around this ticket it was mentioned that we may have introduced a possible regression with respect to loading facts multiple times. I think that the relevant tickets on that topic are #12464, #3741, #12310 (if we are thinking of the same issue). I am pretty sure that that is a separate issue and that we haven’t regressed on it; there is the unfortunate situation that I mentioned above where, in the case that the master and agent are on the same machine and sharing the same modulepath, facts might be loaded twice… however, this should not affect any other nodes.

I have less personal experience with the fact-loading bits of the code so Patrick would be a more credible authority on that particular topic.

#47 Updated by eric sorenson over 2 years ago

  • Assignee set to eric sorenson

#48 Updated by eric sorenson about 2 years ago

  • Status changed from Code Insufficient to Closed

This bug as it’s titled is only half correct; pluginsync turns out to be a bad way to deliver faces and other complex plugins. Modules are indeed supported for loading faces, and loading faces from the modulepath works correctly now.

#49 Updated by Anonymous about 2 years ago

eric sorenson wrote:

This bug as it’s titled is only half correct; pluginsync turns out to be a bad way to deliver faces and other complex plugins. Modules are indeed supported for loading faces, and loading faces from the modulepath works correctly now.

So, to make sure I understand: we can no longer send faces or any other complex plugin to the Puppet agent with PluginSync – instead they need to be shipped as full modules, even though the agent doesn’t use them?

Is there some documented way to package them so that you don’t have to ship a Puppet module, but rather an OS package that you can manage? If not, that is probably going to make our users less unhappy than just disabling the entire feature has.

#50 Updated by eric sorenson about 2 years ago

Daniel Pittman wrote:

So, to make sure I understand: we can no longer send faces or any other complex plugin to the Puppet agent with PluginSync – instead they need to be shipped as full modules, even though the agent doesn’t use them?

more because the the agent doesn’t use them. if the agent does use a complex agent-specific plugin (by which i mean “a type, provider, or fact that has require /some/other/pluginsynced/module in its code”) this will work better than 2.7 because plugins are lazy-loaded, not loaded as soon as they are downloaded (that require could fail before if the required file had not yet been downloaded)

Is there some documented way to package them so that you don’t have to ship a Puppet module, but rather an OS package that you can manage? If not, that is probably going to make our users less unhappy than just disabling the entire feature has.

Sure: as a gem, with #7788. Any OS package that installs them alongside the regular ruby code (though to be fair this is not well-documented).

#51 Updated by eric sorenson about 2 years ago

I should comment too that I do not think the situation I outlined above is ideal, nor is it the absolute final word on this issue — on one level it seems absurd that something as fundamental as loading code is this difficult, but tons of smart people have crashed ships upon its rocky shores.

#52 Updated by Jeff McCune about 2 years ago

@eric, @andy

The first (dirty but “working”) pass at making faces work directly from the modulepath is published in topic branch fix/3.x/7316_load_faces_from_modulepath.

https://github.com/jeffmccune/puppet/tree/fix/3.x/7316_load_faces_from_modulepath

#53 Updated by Jeff McCune about 2 years ago

  • Status changed from Closed to Re-opened

I’m re-opening this because I’ve been unable to make faces load from the modulepath. I don’t want this to somehow slip through the cracks as being closed.

Eric, we might want to create a specific, targeted issue for the modulepath work.

Please see the history of the 3.x branch for the change sets and why they’ve been reverted.

-Jeff

#54 Updated by Chris Price about 2 years ago

Jeff,

If you directly check out the final relevant commit of my original pull request:

https://github.com/cprice-puppet/puppet/commit/860d2ef79f7ee0629d76273a710169b8a5b2d31a

and then try loading faces from the modulepath, does it work properly? Because at the time, I tried a ton of permutations and did not find any problems with it.

I know that recent attempts to re-base those old commits would have conflicts with this:

https://github.com/puppetlabs/puppet/commit/b53e600

and potentially this:

https://github.com/puppetlabs/puppet/commit/d41ac798fb27d562745160c35525f7de59869a5b

but, I mean, at the time I thought that both you and Nigel spent a fair amount of time testing that pull request from the CLI and I thought it had been working.

#55 Updated by Jeff McCune about 2 years ago

I think the current problem is just my overlooking a method that needs to be changed when resolving the merge conflicts.

As soon as I get of BART and situate in the hotel I’m going to find out for sure.

I don’t think there’s a “big” issue here, just an oversight on a detail on my part.

#56 Updated by Jeff McCune about 2 years ago

  • Status changed from Re-opened to In Topic Branch Pending Review
  • Assignee deleted (eric sorenson)
  • Priority changed from Urgent to Normal
  • Target version changed from 3.0.0 to 3.0.x
  • Affected Puppet version set to 3.0.0

We missed the boat for this working for 3.0.0.

Re-targeting against 3.0.x and putting back into a status that reflects reality.

Current pull request: https://github.com/puppetlabs/puppet/pull/1190

There is also a root cause analysis scheduled today for 2PM to 4PM to figure out why some of this code has been reverted at least 3 times leading up to the 3.0.0 release.

#57 Updated by Jeff McCune about 2 years ago

I’ve also filed #16651 which specifically calls out the bug we’ve introduced (or rather failed to fix?) in 3.0.0.

I think a new bug is warranted because we’re explicitly breaking existing functionality on the node subcommand, which is a subtlety not captured in #7316, #4248, #14073 or #3947

#58 Updated by Anonymous about 2 years ago

  • Target version changed from 3.0.x to 3.x

Merged into master so that we get into 3.1.0. This is a larger change than just a simple bugfix.

Merge in commit https://github.com/puppetlabs/puppet/commit/bdda511d06ed6f89b58343e815585957a33db94e

#59 Updated by Anonymous about 2 years ago

  • Status changed from In Topic Branch Pending Review to Merged - Pending Release

#60 Updated by Anonymous about 2 years ago

  • Status changed from Merged - Pending Release to Code Insufficient

This has gone back and forth several times now. The last attempt didn’t work, so back to the drawing board.

#61 Updated by eric sorenson about 2 years ago

  • Assignee set to eric sorenson
  • Keywords changed from face faces subcommand application module plugin pluginsync to face faces subcommand application module plugin pluginsync backlog

#62 Updated by eric sorenson about 2 years ago

  • Priority changed from Normal to High

#63 Updated by eric sorenson about 2 years ago

  • Subject changed from puppet face applications (subcommands) delivered via pluginsync and as modules should work to puppet face applications (subcommands) delivered via modules should work

#64 Updated by Jeff McCune about 2 years ago

See 17156 please

If we tackle this before we tackle #17156, it’d be nice to kill them both at once as they’re closely related.

I just want to update this with the note of addressing the other ticket as it might get buried and we use puppet doc to automatically generate our documentation.

-Jeff

#65 Updated by Anonymous about 2 years ago

  • Target version changed from 3.x to 3.1.0

This is part of our plan for 3.1.0

#66 Updated by Anonymous about 2 years ago

I’ve linked this to a bunch more issues around loading code using the ruby autoloader since fixing this issue will have wide-ranging implications since we have found ourselves getting stuck in how settings are loaded, when they are loaded, and how code gets loaded in. This is why I’ve marked this as related to issues around loading functions in templates.

#67 Updated by Anonymous about 2 years ago

  • Assignee changed from eric sorenson to Anonymous
  • Branch deleted (https://github.com/puppetlabs/puppet/pull/571)

A document that contains a lot of things that we found while investigating this can be found at https://docs.google.com/a/puppetlabs.com/document/d/1hNky74PkNgNxTkfxtoz2if6gAYrrPhudVXJSZCsxSMo/edit

The first step that we were going to take was to get CommandLine to use puppet as a library in order to drive out any assumptions around how things where being done. Right now it is too intertwined with private methods that we had a very hard time unraveling what it was doing and how things related to eachother, which were complaints that we often heard from others who tried to use puppet as a library to do things.

https://github.com/puppetlabs/puppet/pull/1288 contains those changes.

#68 Updated by Josh Cooper almost 2 years ago

  • Status changed from Code Insufficient to In Topic Branch Pending Review
  • Branch set to https://github.com/puppetlabs/puppet/pull/1319

#69 Updated by Anonymous almost 2 years ago

  • Status changed from In Topic Branch Pending Review to Merged - Pending Release

#70 Updated by eric sorenson almost 2 years ago

  • Priority changed from High to Immediate

#71 Updated by Matthaus Owens almost 2 years ago

  • Status changed from Merged - Pending Release to Closed

Released in Puppet 3.1.0-rc1

Also available in: Atom PDF