Closed Bug 891991 Opened 7 years ago Closed 7 years ago

Add support for Windows to PuppetAgain manifests


(Infrastructure & Operations :: RelOps: Puppet, task)

Windows 7
Not set


(Not tracked)



(Reporter: markco, Assigned: dustin)




(2 files)

Initial work for Puppet on Windows is being done on an ix machine locally,  so it is separated from the live production Puppet servers. Using this bug to track manifest and modules during this part of the process.
Blocks: 798657
Component: RelOps → Server Operations: RelEng
Product: Infrastructure & Operations →
QA Contact: arich
We should talk about this - better to use an environment on one of the masters rather than use puppet apply.
Component: Server Operations: RelEng → RelOps: Puppet
Product: → Infrastructure & Operations
QA Contact: arich → dustin
Mark, what's the status here?

I think this is one of the three prongs of Puppet-on Windows, and may be the prong I work on.
Summary: The initial PuppetAgain Window Tester manifests/modules prior to version control. → Add support for Windows to PuppetAgain manifests
There is not much change or ground covered on this yet. Right now, with what time i can, I am focusing on getting the certs onto a Windows box and a Windows solution for .
t-w732-ix-003 has been setup for this. It uses the standard tester accounts and password. 

The puppet server is set to via registry key. 

The keys and certs have been set up in c:\program data\puppetlabs\puppet\etc\ssl. 
However, at this time when running puppet agent it is still requesting a cert from the server.
I'll get this so that the manifests actually parse without errors.  Then we can all work together on making them do things.
Assignee: mcornmesser → dustin
There is a mismatch with the private key.

C:\Program Files\Puppet Labs\Puppet\bin>puppet ca list
Error: The certificate retrieved from the master does not match the agent's priv
ate key.
Certificate fingerprint: 

To fix this, remove the certificate from both the master and the agent and then
start a puppet run, which will automatically regenerate a certficate.
On the master:
  puppet cert clean
On the agent:
  rm -f C:/ProgramData/PuppetLabs/puppet/etc/ssl/certs/
  puppet agent -t
This is because the host's fqdn from facter ( doesn't match the fqdn from reverse DNS (

We'll need to change one of those.  It might be easy to just set FACTER_fqdn globally to the DC name (wintest in this case) at install time - perhaps just based on reverse DNS.
OK, we actually need to set FACTER_hostname and FACTER_domain, which puppet glues together to make the certname.  Weird, but OK.  I'll add that to the
Depends on: 899691
The Puppet_tester_86x gpo has been setup to update the following environment variables: 
%FACTER_domain% =

After applying gpupdate doubled checked and ran "facter hostname" and "facter domain" out of the command prompt with Puppet, and received the proper results.
Attached patch bug891991.patchSplinter Review
Bug 891991: Add support for Windows

    This lets toplevel::base complete without errors or repeated changes on
    This touches a lot of files!  The big deals were:
     * mode bits, for which I wrote a puppet function
     * can't purge all users (TODO)
     * no sudoers
     * concat had to be rewritten to use ruby and not shell


C:\Users\root>"c:\program files\puppet labs\puppet\bin\puppet" agent --test --environment=dmitchell
m --trace
Info: Retrieving plugin
Info: Loading facts in C:/ProgramData/PuppetLabs/puppet/var/lib/facter/concat_basedir.rb
Info: Loading facts in C:/ProgramData/PuppetLabs/puppet/var/lib/facter/concat_ruby_interpreter.rb
Info: Loading facts in C:/ProgramData/PuppetLabs/puppet/var/lib/facter/facter_dot_d.rb
Info: Loading facts in C:/ProgramData/PuppetLabs/puppet/var/lib/facter/ip6tables_version.rb
Info: Loading facts in C:/ProgramData/PuppetLabs/puppet/var/lib/facter/iptables_persistent_version.rb
Info: Loading facts in C:/ProgramData/PuppetLabs/puppet/var/lib/facter/iptables_version.rb
Info: Loading facts in C:/ProgramData/PuppetLabs/puppet/var/lib/facter/num_masters.rb
Info: Loading facts in C:/ProgramData/PuppetLabs/puppet/var/lib/facter/pe_version.rb
Info: Loading facts in C:/ProgramData/PuppetLabs/puppet/var/lib/facter/puppet_vardir.rb
Info: Loading facts in C:/ProgramData/PuppetLabs/puppet/var/lib/facter/root_home.rb
Info: Loading facts in C:/ProgramData/PuppetLabs/puppet/var/lib/facter/vmwaretools_version.rb
Info: Caching catalog for
Info: Applying configuration version '9f44f69f17ea'
Notice: Finished catalog run in 5.59 seconds
Attachment #785083 - Flags: review?(rail)
Attachment #785083 - Flags: feedback?
Attachment #785083 - Flags: feedback? → feedback?(mcornmesser)
Comment on attachment 785083 [details] [diff] [review]

Review of attachment 785083 [details] [diff] [review]:

        ,@;@;@;@;@;@/ )@;@;
      ,;@;@;@;@;@;@|_/@' e\
     (|@;@:@\@;@;@;@:@(    \
       '@;@;@|@;@;@;@;'`"--'    it!
         ) //   | ||
         \ \\   | ||
          \ \\  ) \\  
           `"`  `"``

::: modules/concat/manifests/fragment.pp
@@ +37,5 @@
>      $backup = 'puppet') {
> +  include concat::setup
> +
> +  $safe_name   = regsubst($name, $concat::setup::pathchars, '_', 'G')
> +  $safe_target_name = regsubst($target, $concat::setup::pathchars, '_', 'GM')

$concat::setup::pathchars doesn't contain \n and \r (currently we have \n there). Is this expected?

::: modules/puppet/manifests/settings.pp
@@ +4,5 @@
>  class puppet::settings {
>      include ::config
> +    $conf = $::operatingsystem ? {
> +        Windows => 'c:/ProgramData/PuppetLabs/puppet/etc/puppet.conf',

Will this work on XP?
Attachment #785083 - Flags: review?(rail) → review+
XP support is still a bit of an unknown, since Puppet doesn't officially support it.

The \n was only in one of the safechars regexps (and both should match).  I'll add it to $pathchars.
$vardir != /var.  This broke puppet.conf everywhere.  I'm working on recovery.
I initially revered only the puppet.conf parts of this patch, but I just reverted the entire thing.
Assignee: mcornmesser → dustin
After reverting, I ran

  sed -i '' -e '/[rls][ous][gnl]dir = /s!.vardir!/var!' /etc/puppet/puppet.conf; head /etc/puppet/puppet.conf

on all 1499 hosts in the puppet dashboard's node table.  A sample of those are able to run puppet without any trouble afterward, so I think this is fixed.  I also think we'll just use explicit paths on Windows!
Attached patch bug891991.patchSplinter Review


diff --git a/modules/puppet/templates/puppet.conf.erb b/modules/puppet/templates/puppet.conf.erb
index f7adb95..b3246d6 100644
--- a/modules/puppet/templates/puppet.conf.erb
+++ b/modules/puppet/templates/puppet.conf.erb
@@ -5,14 +5,20 @@
 # Managed by puppet
+<% if scope.lookupvar('::operatingsystem') == 'windows' -%>
+    # vardir is C:/ProgramData/PuppetLabs/puppet/var on Windows
     logdir = $vardir/log/puppet
     rundir = $vardir/run/puppet
     ssldir = $vardir/lib/puppet/ssl
-<% if scope.lookupvar('::operatingsystem') == 'windows' -%>
     diff = c:\windows\system32\cmd.exe
     diff_args = /c echo "DIFFS DISABLED -"
 <%- else -%>
+    # vardir is /var/lib/puppet on POSIX systems; logdir and rundir aren't subdirectories
+    logdir = /var/log/puppet
+    rundir = /var/run/puppet
+    ssldir = /var/lib/puppet/ssl
     diff = echo
     diff_args = DIFFS DISABLED -
 <% end -%>
@@ -36,15 +42,17 @@
     # things
     pluginsync = true
+<% if scope.lookupvar('::operatingsystem') == 'windows' -%>
     # default for this on Windows is completely bogus ($vardir/lib/puppet)
     plugindest = $vardir/lib/puppet/lib
+<% end -%>
     # default to looking in the production/ subdirectory of /etc/puppet
     manifestdir = $confdir/production/manifests
     modulepath = $confdir/production/modules
     templatedir = $confdir/production/templates
-    ssldir = $vardir/lib/puppetmaster/ssl
+    ssldir = /var/lib/puppetmaster/ssl
     manifest = $manifestdir/site.pp
     config_version = /etc/puppet/ $confdir/production
     ca = false


So, this now should not affect puppet.conf measurably on POSIX.

I'll do some additional testing before landing this.
Attachment #785876 - Flags: review?(rail)
OK, I tested this with my environment on CentOS and OS X.  I see no change to puppet.conf.
Attachment #785876 - Flags: review?(rail) → review+
Attachment #785083 - Flags: feedback?(mcornmesser)
Comment on attachment 785876 [details] [diff] [review]

Callek, can you take a look, too, and see if you see anything else that might cause significant problems?  I'm gunshy :)
Attachment #785876 - Flags: feedback?(bugspam.Callek)
Comment on attachment 785876 [details] [diff] [review]

Mark, I'd like to hear what you think, too.
Attachment #785876 - Flags: feedback?(mcornmesser)
This looks good to me. Taking in account this is all a new to me. 

I will start diving deeper into the TODO_WIN bits today.
Attachment #785876 - Flags: feedback?(mcornmesser) → feedback+
Depends on: 902577
Blocks: 902577
No longer depends on: 902577
No longer blocks: 902577
Depends on: 902577
Comment on attachment 785876 [details] [diff] [review]

Review of attachment 785876 [details] [diff] [review]:

Nothing here looks overly scary. I just hope you cleaned the gun chamber and did a few test fires before handing it out to the whole slave army. [/bad analogy]

::: modules/concat/files/concatfragments.rb
@@ +1,1 @@
> +# Script to concat files to a config file.

My only thought here is does this sync-up break the concat-ing of puppet.conf as well (which is what broke the last run iirc).

::: modules/ssh/manifests/userconfig.pp
@@ +24,5 @@
>      }
> +    File {
> +        mode => filemode(0600)
> +    }

I'm not a fan of this, based on how confusing old-puppet became with Defaults sprinkled in individual pp files. I recall us discussing in the past we'd avoid this at all costs.

::: modules/sudoers/manifests/init.pp
@@ +6,3 @@
> +    if $::operatingsystem != 'Windows' {
> +        # WIN-TODO: this should be replaced with an equivalent on windows

ultra-nitpick, s/WIN-TODO/TODO-WIN/ (or vice versa) throughout patch. be consistent!

::: modules/users/manifests/global.pp
@@ +9,5 @@
>      }
>      # set the bash prompt
> +    $profile_d = $::operatingsystem ? {
> +        Windows => 'c:/windows/profile.d',

mozbuild ships with a profile.d actually! I doubt we want to force the issue for that, and I'm not blocking on this choice, just wanted to call it out while I saw it.
Attachment #785876 - Flags: feedback?(bugspam.Callek) → feedback+
The change to concatfragments.rb (I assume that's what you mean by "sync-up") shouldn't break anything.  Do you see something it breaks (leading you to say "as well")?

I fixed the use of File { .. }, and the comments.

I've tested this on Linux and Darwin hosts, and only see comment/whitespace changes in puppet.conf.

Mozbuild's profile.d is probably a shell thing, right?  This will be for COM.EXE, hooked into KTS's allusers.bat.
Attachment #785876 - Flags: checked-in+
I don't see anything burning.
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.