Skip to content

(#12357) Make facter_dot_d look in Puppet[:confdir]/facts.d #40

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

jeffmccune
Copy link
Contributor

On Windows, we have no folders that match up to the default set of
directories the facter_dot_d fact looks in by default. This is a
problem because the Puppet Enterprise installer writes out the following
facts by default, and our modules require them to be present:

% cat /etc/puppetlabs/facter/facts.d/puppet_enterprise_installer.txt
fact_stomp_port=61613
fact_stomp_server=puppetmaster
fact_is_puppetagent=true
fact_is_puppetmaster=true
fact_is_puppetconsole=true

On windows, the Puppet confdir is quite variable. On 2003 systems we
default to the All Users application data directory. On 2008 systems we
default to the ProgramData directory. The actual configuration
directory varies depending on the Puppet or Puppet Enterprise branding.

In order to simplify all of this variable behavior, this patch fixes the
problem by automatically looking for facts in Puppet[:confdir]/facts.d
There is no change in behavior if Puppet is not actually loaded into
memory.

This patch paves the way for the MSI installer to use an IniFile element
to write custom facts during installation. Since we're already writing
custom data into the puppet.conf file, and the 'facts.d' directory will
be a sub-folder of the folder containing puppet.conf, we'll be
reasonably guaranteed this implementation will be robust for all Windows
platforms that Puppet itself runs on.

(#12357) Add node_vardir custom fact

Without this patch the PE modules don't have a way to identify a
filesystem path where it's OK to place variable data related to managing
the target node. This is a problem when a module like pe_compliance
needs to write a wrapper script to the node's filesystem.

This patch addresses the problem by exposing the node's Puppet[:vardir]
setting as a Facter fact.

This fact value will be set to nil if Puppet is not loaded into
memory. If Puppet is loaded, e.g. using facter --puppet or using
puppet agent or puppet apply then the fact will automatically set
the value to Puppet[:vardir]

The value of this setting is subject to Puppet's run_mode.

This patch implements a new utility method in the standard library
module named Facter::Util::PuppetSettings.with_puppet. The method
accepts a block and will only invoke the block if the Puppet library is
loaded into the Ruby process. If Puppet is not loaded, the method
always returns nil. This makes it easy to define Facter facts that only
give values if Puppet is loaded in memory.

@jeffmccune
Copy link
Contributor Author

These shouldn't be targeting master... Need to close these.

@jeffmccune jeffmccune closed this Mar 2, 2012
@jeffmccune
Copy link
Contributor Author

This is actually targeted at master.

@jeffmccune jeffmccune reopened this Mar 2, 2012
@@ -182,3 +182,8 @@ def create

Facter::Util::DotD.new("/etc/facter/facts.d").create
Facter::Util::DotD.new("/etc/puppetlabs/facter/facts.d").create

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems as though those values might not be applicable on Windows, so should be conditionally out or something?

In the same vein, perhaps it is worth considering using a platform independent path, so that we can figure out which one is applicable and not, eg, load your FOSS facts in PE, and vice-versa? Coexistence, on Unix, is considered a required feature, and separating the paths feels like a good decision in supporting those.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Fri, Mar 2, 2012 at 6:44 PM, Daniel Pittman <
[email protected]

wrote:

@@ -182,3 +182,8 @@ def create

Facter::Util::DotD.new("/etc/facter/facts.d").create
Facter::Util::DotD.new("/etc/puppetlabs/facter/facts.d").create
+

Seems as though those values might not be applicable on Windows, so should
be conditionally out or something?

The intent is currently to add facts located at the path if the path
exists. The initialization methods do not add anything if the path doesn't
exist, and the path doesn't exist on Windows so it works itself out.

In the same vein, perhaps it is worth considering using a platform
independent path, so that we can figure out which one is applicable and
not, eg, load your FOSS facts in PE, and vice-versa? Coexistence, on Unix,
is considered a required feature, and separating the paths feels like a
good decision in supporting those.

I'd like to move to platform independent paths eventually but the existing
behavior is intended to work with Puppet FOSS and Puppet Enterprise. The
current paths aren't ideal since they're hard-coded but they work for the
default configuration scenarios.

Luckily, the new path works with any platform and any Puppet scenario
because the path is relative to Puppet[:confdir]. Rather than change the
existing behavior I recommend moving facts that are meant for a specific
Puppet configuration to go under /facts.d/

-Jeff

@slippycheeze
Copy link
Contributor

Other than the one comment, this looks good to me for now. +1

@kbarber
Copy link
Contributor

kbarber commented Mar 4, 2012

So lets continue the pervious conversation here:

What I'm doing is slightly more complicated but it's better.

Cool - yeah I kind of get what your doing, I guess I'm just pondering if its needed/necessary that's all :-).

As it stands, puppetversion.rb will always bring Puppet into memory if
Puppet happens to be in the load path. This behavior totally disregards if
the user specified --puppet or not.

Yeah - but --puppet and the puppetversion.rb fact are really solving 2 different issues and aren't related per se. --puppet fixed the load path, and puppetversion finds the version of puppet. Going forward there may be more software versions we add to facter for example. So - I can't imagine because a user has dropped --puppet they still don't want puppetversion to work for example :-).

What I'm doing is looking to see if Puppet is loaded in memory before
leaping into it. If it's not loaded, I don't do anything.

Yeah - I got that much.

I don't see the need to load the entire Puppet code base just to get a
version string. I'm pretty sure puppetversion.rb should be fixed in
Puppet. In fact, I'm pretty sure we've moved the version constant out into
it's own modules so we can require it nice and quickly without also
bringing in all of Puppet.

I'm not sure how probable this is without the full require anyway - but I tell you what - lets chat about this tomorrow as I'll be in the office. I'm interested in your opinion on pulling in Puppet for puppetversion.rb as its obviously something 'we just do' already like I mentioned. If there is a better way lets get it into the product.

@jeffmccune
Copy link
Contributor Author

@kbarber OK, this one is ready as well. I added the require line that was missing.

Jeff McCune added 2 commits March 5, 2012 16:26
Without this patch the PE modules don't have a way to identify a
filesystem path where it's OK to place variable data related to managing
the target node.  This is a problem when a module like pe_compliance
needs to write a wrapper script to the node's filesystem.

This patch addresses the problem by exposing the node's Puppet[:vardir]
setting as a Facter fact.

This fact value will be set to `nil` if Puppet is not loaded into
memory.  If Puppet is loaded, e.g. using `facter --puppet` or using
`puppet agent` or `puppet apply` then the fact will automatically set
the value to Puppet[:vardir]

The value of this setting is subject to Puppet's run_mode.

This patch implements a new utility method in the standard library
module named `Facter::Util::PuppetSettings.with_puppet`.  The method
accepts a block and will only invoke the block if the Puppet library is
loaded into the Ruby process.  If Puppet is not loaded, the method
always returns nil.  This makes it easy to define Facter facts that only
give values if Puppet is loaded in memory.
On Windows, we have no folders that match up to the default set of
directories the facter_dot_d fact looks in by default.  This is a
problem because the Puppet Enterprise installer writes out the following
facts by default, and our modules require them to be present:

    % cat /etc/puppetlabs/facter/facts.d/puppet_enterprise_installer.txt
    fact_stomp_port=61613
    fact_stomp_server=puppetmaster
    fact_is_puppetagent=true
    fact_is_puppetmaster=true
    fact_is_puppetconsole=true

On windows, the Puppet confdir is quite variable.  On 2003 systems we
default to the All Users application data directory.  On 2008 systems we
default to the ProgramData directory.  The actual configuration
directory varies depending on the Puppet or Puppet Enterprise branding.

In order to simplify all of this variable behavior, this patch fixes the
problem by automatically looking for facts in Puppet[:confdir]/facts.d
There is no change in behavior if Puppet is not actually loaded into
memory.

This patch paves the way for the MSI installer to use an IniFile element
to write custom facts during installation.  Since we're already writing
custom data into the puppet.conf file, and the 'facts.d' directory will
be a sub-folder of the folder containing puppet.conf, we'll be
reasonably guaranteed this implementation will be robust for all Windows
platforms that Puppet itself runs on.
@jeffmccune jeffmccune closed this Mar 6, 2012
@jeffmccune jeffmccune reopened this Mar 6, 2012
@jeffmccune
Copy link
Contributor Author

Correct merge target is in: #44

@jeffmccune jeffmccune closed this Mar 6, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants