voxpupuli / puppet-staging

⛔️ Deprecated in favor of puppet-archive
https://forge.puppet.com/puppet/archive
Apache License 2.0
51 stars 109 forks source link

Add default Windows support #14

Closed reidmv closed 10 years ago

reidmv commented 11 years ago

Previously, the hard-coded default staging path of "/opt/staging" precluded touch-free use of this module on the Windows platform. This commit adds the params pattern in order to allow the module to work on Windows platforms without modification.

Further development would be a nice-to-have in terms of adding support for downloading various kinds of URLs; this is just the critical fix.

In the future, the params pattern may be removed in favor of Data-in-Modules, but this functionality is not yet well developed in Puppet. Puppet v3.4 should introduce good support for such things, but this is the best way I know how to do it until then.

reidmv commented 10 years ago

@nanliu ping :-)

Let me know if there's anything else I can do to help get staging to minimally functional on Windows. I could really use it in a Forge version of the module.

nanliu commented 10 years ago

Hmm, was trying to avoid additional module dependency and since I do not use windows wasn't sure how well I would be able to support this. Let me take a closer look and get back to you. Also did you intend to add /opt/csw/bin to the path since that appears to be breaking spec tests?

reidmv commented 10 years ago

Re: /opt/bin/csw I did add that but it should be in a different commit. Didn't mean to lump it in with this. I can split that out. On the side I also need this to work for Solaris, which keeps a version of wget in /opt/csw/bin and doesn't seem to have curl in a standard location by default.

reidmv commented 10 years ago

This can be refactored not to use joshcooper/powershell and to instead just call out to powershell the old fashioned way using the default Exec provider in Windows. Is that a preferred change?

cyberious commented 10 years ago

Reid look at the pull request I have you might be interested in it. Added password capabilities for it as well, didn't know you had this pull request already. Pretty much did what you did.

cyberious commented 10 years ago

Look up pget on forge will get download stage capabilities.

nanliu commented 10 years ago

Sorry took a while to get back, I would prefer #16 for adding support.

reidmv commented 10 years ago

16 adds powershell support, which enables users without curl or wget to use staging to fetch content. However, it does not address the issue of a platform-appropriate default staging::path parameter.

On Windows, given the following code in a master's site.pp:

staging::file { 'sample':
  source => 'puppet://modules/staging/sample',
}

puppet agent -t will return something like:

Info: Retrieving plugin
Info: Loading facts in gramData/PuppetLabs/puppet/var/lib/facter/concat_basedir.rb
Info: Loading facts in gramData/PuppetLabs/puppet/var/lib/facter/custom_auth_conf.rb
Info: Loading facts in gramData/PuppetLabs/puppet/var/lib/facter/facter_dot_d.rb
Info: Loading facts in gramData/PuppetLabs/puppet/var/lib/facter/ip6tables_version.rb
Info: Loading facts in gramData/PuppetLabs/puppet/var/lib/facter/iptables_persistent_version.rb
Info: Loading facts in gramData/PuppetLabs/puppet/var/lib/facter/iptables_version.rb
Info: Loading facts in gramData/PuppetLabs/puppet/var/lib/facter/os_maj_version.rb
Info: Loading facts in gramData/PuppetLabs/puppet/var/lib/facter/pe_version.rb
Info: Loading facts in gramData/PuppetLabs/puppet/var/lib/facter/postgres_default_version.rb
Info: Loading facts in gramData/PuppetLabs/puppet/var/lib/facter/puppetdb_server_status.rb
Info: Loading facts in gramData/PuppetLabs/puppet/var/lib/facter/puppet_vardir.rb
Info: Loading facts in gramData/PuppetLabs/puppet/var/lib/facter/root_home.rb
Info: Loading facts in gramData/PuppetLabs/puppet/var/lib/facter/staging_http_get.rb
Info: Loading facts in gramData/PuppetLabs/puppet/var/lib/facter/windows.rb
Info: Caching catalog fver2008r2a
Error: Failed to apply g: Parameter path failed on File[/opt/staging]: File paths must be fully qualified, not '/opt/staging' at /etc/puppetlabs/puppet/environments/production/modules/staging/manifests/init.pp:25
Wrapped exception:
File paths must be fully qualified, not '/opt/staging'

Staging does not work on Windows without platform-specific parameters being hand-passed (or configured in a hiera.yaml which includes osfamily or similar in the hierarchy). That was the original issue which this pull request sought to address.

@nanliu, would you be amenable to reviewing a new pull request which incorporates the #16 powershell implementation, and provides a fix for the staging::path issue by using a platform-appropriate default parameter, either by params pattern or by custom fact?

Is there an alternate pattern for fixing this which you would prefer? I'm happy to implement, just need to know which way to go.

nanliu commented 10 years ago

I'll make a hybrid PR for review.

reidmv commented 10 years ago

Thanks Nan! The first commit here (0173203433969c9cbf83be8afdc13d668a4ae697) is a decently clean params pattern fix which doesn't touch anything powershell, if that helps.

cyberious commented 10 years ago

I am completely fine with that default of C:\staging\ I would prefer it over c:\temp

Sent from Windows Mail

From: Reid Vandewiele Sent: ‎Monday‎, ‎January‎ ‎06‎, ‎2014 ‎8‎:‎06‎ ‎PM To: nanliu/puppet-staging Cc: Travis Fields

Thanks Nan! The first commit here (0173203) is a decently clean params pattern fix which doesn't touch anything powershell, if that helps.

— Reply to this email directly or view it on GitHub.