Oh, code, where art thou? -- Fixing #7316

[Andy Parker]

In Puppet 3.0.0 we tried to get a fix in for #7316, but, after many valiant efforts, that did not come to pass. For Puppet 3.1.0 we are trying again.

The problem boils down to the issue that when the Puppet::Util::CommandLine code tries to find the ruby code for the subcommand to execute, it hasn't yet figured out the modulepath. Without knowing the modulepath, we can't find applications or faces that are in modules. The reason it doesn't know the modulepath is because the modulepath is tied to the section of the configuration file that it should use, which is only determined by the run mode. The run mode is only known *after* we have loaded the application. And therein lies the problem: 

    In order to figure out where to look for an application to load we need to load the 

    application so that it can tell us where we should look for the application to load.

Josh Cooper and I (and previously Jeff McCune) have been combing through the code and redmine and trying to piece together all of the ramifications of this problem (you can see the result of that from the list of related issues). The issue and its relations show up in a lot of places (environments for instance) and in several different guises (loading utility code in a module, being able to initialize puppet for use as a library). Each time we thought we understood what was going on we came up with a proposed solution, but as soon as we learned a bit more, that solution fell apart.

We've documented most of that at https://docs.google.com/document/d/1hNky74PkNgNxTkfxtoz2if6gAYrrPhudVXJSZCsxSMo/edit

At the moment Proposal 4 is looking the most promising. It is to pretty much undo the changes for #2935 (which added run_mode) and change to a configuration system such that each subcommand has a section in the configuration file. This means that we can statically know what part of the configuration file to use for any given subcommand without first having to load any code first.

I've hacked (and hacked is really the right word) together some changes that start doing this on a branch in my repo. https://github.com/zaphod42/puppet/tree/spike/master/subcommands-as-conf-sections

Any changes around this could have wide ranging impact and so we really need to make sure we get this right. Can anyone think of what doing this would horribly break? Other solutions that would achieve similar results, but might work better?

Andy

[Erik Dalén]

On Tuesday 13 November 2012 at 00:59, Andy Parker wrote:
> At the moment Proposal 4 is looking the most promising. It is to pretty much undo the changes for #2935 (which added run_mode) and change to a configuration system such that each subcommand has a section in the configuration file. This means that we can statically know what part of the configuration file to use for any given subcommand without first having to load any code first.
>
> I've hacked (and hacked is really the right word) together some changes that start doing this on a branch in my repo. https://github.com/zaphod42/puppet/tree/spike/master/subcommands-as-conf-sections
>
> Any changes around this could have wide ranging impact and so we really need to make sure we get this right. Can anyone think of what doing this would horribly break? Other solutions that would achieve similar results, but might work better?

Wouldn't doing this would require configuration changes in some use cases? For example I have puppet masters that are clients to other puppet masters with a different CA, so in the master section I have a different ssldir. With this change I would have to copy that to a "cert" and "ca" section as well.

I'm kind of okay with doing that, but it is definitely not a backwards compatible change, so for puppet 4.0? :)

--
Erik Dalén

[Daniel Pittman]

Yes, Erik is right about that. The CA, apply, and a few other
commands would no longer pick up the same configuration they do today.

--
Daniel Pittman
⎋ Puppet Labs Developer – http://puppetlabs.com
♲ Made with 100 percent post-consumer electrons

[Andy Parker]

I don't think so. The application can still change what run_mode it is going to use, but run_mode will mean "application setting section". I removed the caching from the Settings so once the run_mode is changed, it should just start operating with that different set of configuration settings. However, just like today, there can be all sorts of unintended consequences of doing so.

 

I'm kind of okay with doing that, but it is definitely not a backwards compatible change, so for puppet 4.0? :)

I'd like to find a way that can fix this in 3.x. 

--
Erik Dalén



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

[Jeff McCune]

On Thu, Nov 15, 2012 at 12:24 PM, Andy Parker <an...@puppetlabs.com> wrote:
> On Tue, Nov 13, 2012 at 4:03 PM, Erik Dalén <erik.gus...@gmail.com>
> wrote:

[snip]

>> Wouldn't doing this would require configuration changes in some use cases?
>> For example I have puppet masters that are clients to other puppet masters
>> with a different CA, so in the master section I have a different ssldir.
>> With this change I would have to copy that to a "cert" and "ca" section as
>> well.
>
>
> I don't think so. The application can still change what run_mode it is going
> to use, but run_mode will mean "application setting section". I removed the
> caching from the Settings so once the run_mode is changed, it should just
> start operating with that different set of configuration settings. However,
> just like today, there can be all sorts of unintended consequences of doing
> so.

Does this still leave us in the difficult situation where the Faces
application file itself lives along the modulepath, so we use one
configuration file section to determine the module path, then the mode
(section) changes out from underneath us, and we use another section
to resolve the rest of the settings? How would a new action
augmenting the `puppet module` application behave in this situation?

>> I'm kind of okay with doing that, but it is definitely not a backwards
>> compatible change, so for puppet 4.0? :)
>
>
> I'd like to find a way that can fix this in 3.x.

Yes, this effort will substantially improve faces, so it would be
great if we can get it into 3.x. Face actions often need to access
settings and faces are a big extension point for Puppet so I think
this fits well into the settings and code loading theme for Puppet
3.1.

-Jeff

[Andy Parker]

On Thu, Nov 15, 2012 at 9:46 AM, Jeff McCune <je...@puppetlabs.com> wrote:

On Thu, Nov 15, 2012 at 12:24 PM, Andy Parker <an...@puppetlabs.com> wrote:
> On Tue, Nov 13, 2012 at 4:03 PM, Erik Dalén <erik.gus...@gmail.com>
> wrote:

[snip]

>> Wouldn't doing this would require configuration changes in some use cases?
>> For example I have puppet masters that are clients to other puppet masters
>> with a different CA, so in the master section I have a different ssldir.
>> With this change I would have to copy that to a "cert" and "ca" section as
>> well.
>
>
> I don't think so. The application can still change what run_mode it is going
> to use, but run_mode will mean "application setting section". I removed the
> caching from the Settings so once the run_mode is changed, it should just
> start operating with that different set of configuration settings. However,
> just like today, there can be all sorts of unintended consequences of doing
> so.

Does this still leave us in the difficult situation where the Faces
application file itself lives along the modulepath, so we use one
configuration file section to determine the module path, then the mode
(section) changes out from underneath us, and we use another section
to resolve the rest of the settings?  How would a new action
augmenting the `puppet module` application behave in this situation?

Yeah, we are still in that situation. The settings that are seen during the run are not necessarily in line with what was used during application discover. There is actually even a slightly larger issue: the modulepath during discovery is being added to the ruby LOAD_PATH so that utility code can be loaded. This means that it may end up with things that it maybe "shouldn't" have access to because it started with one modulepath and then wanted to run in "master" mode to get a different modulepath. I don't think this is a problem for any existing applications, though, since I'm special casing master and agent to start in the current manner.

 

>> I'm kind of okay with doing that, but it is definitely not a backwards
>> compatible change, so for puppet 4.0? :)
>
>
> I'd like to find a way that can fix this in 3.x.

Yes, this effort will substantially improve faces, so it would be
great if we can get it into 3.x.  Face actions often need to access
settings and faces are a big extension point for Puppet so I think
this fits well into the settings and code loading theme for Puppet
3.1.

-Jeff

[Daniel Pittman]

On Thu, Nov 15, 2012 at 2:45 PM, Andy Parker <an...@puppetlabs.com> wrote:
> On Thu, Nov 15, 2012 at 9:46 AM, Jeff McCune <je...@puppetlabs.com> wrote:
>
>> Does this still leave us in the difficult situation where the Faces
>> application file itself lives along the modulepath, so we use one
>> configuration file section to determine the module path, then the mode
>> (section) changes out from underneath us, and we use another section
>> to resolve the rest of the settings? How would a new action
>> augmenting the `puppet module` application behave in this situation?
>
> Yeah, we are still in that situation. The settings that are seen during the
> run are not necessarily in line with what was used during application
> discover. There is actually even a slightly larger issue: the modulepath
> during discovery is being added to the ruby LOAD_PATH so that utility code
> can be loaded. This means that it may end up with things that it maybe
> "shouldn't" have access to because it started with one modulepath and then
> wanted to run in "master" mode to get a different modulepath. I don't think
> this is a problem for any existing applications, though, since I'm special
> casing master and agent to start in the current manner.

At least `puppet resource` and `apply` probably depend on the same
run_mode vs LOAD_PATH thing, because they interact directly with the
type and provider system. Ditto the resource face.

[Andy Parker]

I just looked into the code and neither one of those looks like they try to change run_mode, which means they would just run with the default "user" settings. This should continue to "just work" with the proposed change.

 

--
Daniel Pittman
⎋ Puppet Labs Developer – http://puppetlabs.com
♲ Made with 100 percent post-consumer electrons

[Andy Parker]

I just noticed that there is a puppet/application/resource.rb and a puppet/face/resource.rb. I don't think the face is actually accessible from the command line.

[Daniel Pittman]

Oh, good. I must have misunderstood something in the proposal - I
thought that defaults would be based on application, now, so they
would no longer see the same module path as the agent. Glad to be
mistaken, and thanks for correcting me.

[Andy Parker]

You are right that they are based on application in the proposal, but the inheritance for sections would be [<application>] inherits from [user] inherits from [main] except for agent and master, which would continue to inherit directly from [main]. With that inheritance and no changes on the part of a user of puppet all of the applications should continue to get the same settings they got before, except in the cases where they try setting the run_mode in the application, in which case there are a few differences related to the LOAD_PATH.

I'll point out now that until http://projects.puppetlabs.com/issues/15770 is resolved, there is a possible collision between environment and application.

 

--
Daniel Pittman
⎋ Puppet Labs Developer – http://puppetlabs.com
♲ Made with 100 percent post-consumer electrons