(SERVER-571) Add puppet-agent libdir to ruby-load-path #513
Conversation
Without this patch Hiera backend plugins distributed as modules are not functioning as expected in Puppet Server 2.0. The expected behavior is that the plugin is usable in puppetserver after a successful agent run that has synchronized plugins. This patch addresses the problem by adding the `/opt/puppetlabs/puppet/cache/lib` path to the ruby-load-path setting which lines up with the [Puppet Installation Layout][layout]. [layout]: http://goo.gl/BWyXpo
Without this patch there is not acceptance test that covers the situation where an end user installs a module with a custom hiera backend and then successfully uses the hiera back end in the puppetserver. This patch addresses the problem by adding an acceptance test based on the steps to reproduce described in [SERVER-571](https://tickets.puppetlabs.com/browse/SERVER-571).
|
+1 but @camlow325 and I had been discussing the possibility of putting this in for 2.1, in which case it will need to be re-targeted. |
| @@ -2,7 +2,7 @@ | |||
| jruby-puppet: { | |||
| # Where the puppet-agent dependency places puppet, facter, etc... | |||
| # Puppet server expects to load Puppet from this location | |||
| ruby-load-path: [/opt/puppetlabs/puppet/lib/ruby/vendor_ruby] | |||
| ruby-load-path: [/opt/puppetlabs/puppet/lib/ruby/vendor_ruby, /opt/puppetlabs/puppet/cache/lib] | |||
There was a problem hiding this comment.
This concerns me, because it means the environment that the agent (on the master) pluginsyncs & applies the catalog from, will affect how the master compiles catalogs for all other agents potentially in different environments.
There was a problem hiding this comment.
@joshcooper is the libdir not included in the load path for the master under Passenger?
There was a problem hiding this comment.
@joshcooper I understand that this is distasteful. Is it a new issue with Puppet 4, though, or did we have this problem in the past as well? Do you have an alternative suggestion for how hiera backends in modules could be loaded by the master if we don't use the pluginsync / agent path as has apparently been done in the past?
There was a problem hiding this comment.
@joshcooper is the libdir not included in the load path for the master under Passenger?
And, specifically, the agent libdir?
There was a problem hiding this comment.
@camlow325 Prior to 4.0, the master (webrick, passenger, puppetserver) and agent shared the same libdir (/var/lib/puppet/lib). And yep, we did have this issue in the past (which is why I filed https://projects.puppetlabs.com/issues/18154). So I agree with you, the current patch is no worse than it used to be.
There was a problem hiding this comment.
@cprice404 @camlow325 given we're at puppet 4.0 and server 2.0, wouldn't now be the time to document the breaking change that we already implemented in SERVER-357? The master and agent no longer share libdir, so you can't use the agent to distribute hiera backends like this.
I think the technical issue preventing us from moving forward, is as @hlindberg mentioned, hiera loads its backends using require (since there is no puppet autoloader in that context). I think the way to fix that is for hiera to always use an absolute path to load the backend (like we do when loading helper code from a type, provider, report processor, etc, e.g.
require Pathname.new(__FILE__).dirname + "relative/path/to/backend"
Then we could just use r10k to distribute the module to the server.
There was a problem hiding this comment.
(The information in this comment is incorrect, see the follow-on comment.)
@joshcooper Thanks for following up on this. @cprice404 raised an interesting question in the comments SERVER-571, "why isn't 1.0.8 affected?"
I assumed that puppetserver 1.0.8 would not suffer this issue because 1.0.x does not have the AIO pathing changes. However, upon verification of that assumption I just found out 1.0.8 is suffering from this issue.
Even more perplexing to me is that puppetserver 1.0.2 is not experiencing this issue.
However, both 1.0.2 and 1.0.8 have the same ruby-load-path setting of ruby-load-path: [/usr/share/ruby/vendor_ruby/].
I'll post more details in that ticket, but at this point I'm sufficiently confused about the nature of this issue that I'm also concerned this pull request isn't the correct solution.
There was a problem hiding this comment.
@joshcooper Nevermind, the discrepancy I saw where 1.0.8 seemed to be affected was a result of me failing to restart the puppetserver service immediately after reconfiguring the hiera backend.
There was a problem hiding this comment.
@joshcooper Back to the discussion, I'm a bit confused at how switching to an absolute path in require statements would solve this issue. What is the value of __FILE__ in your example?
Are you thinking we'd return to a world where we load the code directly from the module directory structure, or are you thinking that the absolute path wouldn't be relative to __FILE__ but rather relative to the agent's plugin sync directory?
Maybe something like hiera doing a require "#{Puppet[:libdir]}/hiera/backends/#{backend_name}.rb"?
There was a problem hiding this comment.
Not to create too many threads along the same conversation, and assuming Josh is thinking along the lines of constructing an absolute path to the agent's libdir...
Wouldn't this mean the only way hiera could load backends would be if they're located in the agent plugin sync directory? Do we have a sense of how end users distribute backend extensions, e.g. are backends located in Ruby gems supported? If so, an absolute path solution might break support for that behavior, or similar behaviors.
|
👍 pending the outcome of the discussion with @joshcooper about the I like the new test - might be good to try to get this into the core Ruby Puppet acceptance test suite at some point since I don't think there's anything Puppet Server-specific about it. Also 👍 to @cprice404's suggestion that we target this at 2.1.x instead of master. |
|
My +1 was premature. :) I hadn't understood the change in behavior between Puppet 3.x and Puppet 4.x, and now, having read @joshcooper 's comments, I'd prefer his proposal (consider this behavior to be a valid, breaking change) to the changes in this PR. So I think I'm in favor of just closing this PR at the moment. This thread is currently going on on both the Jira ticket and the PR, perhaps we could consolidate it on the Jira ticket since that will be easier to search for history in the future? |
|
@camlow325 Are we closing this? Just going through PRs and this hasn't seen activity in 15 days, and Chris is proposing to close it and perhaps continue the discussion on the ticket. |
|
I feel like we're at an impasse on this based on the discussion continuing on in the JIRA ticket. I'm going to reassign SERVER-571 to @ahpook and move it to "needs information" for now. Assuming that takes some time to sort out, the best course may be to just close this PR for now. @jeffmccune - you okay with that? |
|
@jeffmccune Closing this PR. Feel free to re-open if necessary. |
Without this patch Hiera backend plugins distributed as modules are not
functioning as expected in Puppet Server 2.0. The expected behavior is that
the plugin is usable in puppetserver after a successful agent run that has
synchronized plugins.
This patch addresses the problem by adding the
/opt/puppetlabs/puppet/cache/libpath to the ruby-load-path settingwhich lines up with the Puppet Installation Layout.