Skip to content
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

(PUP-1691) Install Chocolatey #76

Conversation

ferventcoder
Copy link
Contributor

  • Dependency on powershell / stdlib
  • Need installer template
  • Manifest details
  • Add templated values
  • refresh path changes - dependency on windows_env
  • Delay suitability check - https://docs.puppetlabs.com/guides/provider_development.html#suitability
  • add ability to specify custom install location
  • specs
  • Look at Add custom fact for choco installpath. #62
  • validation and specs
  • ReadMe updates
  • choco_version fact
  • use versioncmp() to ensure we don't try to configure older choco clients
  • use $::serverversion, then $::clientversion
  • document new facts
  • [] Fix facter check for version 3 to public api

This is related to #61. This supersedes #24.

To install Chocolatey, we need to take a dependency on the powershell
module. To be able to validate user input, it's best to take a
dependency on stdlib as it has quite a bit of built-in validation.
@ferventcoder ferventcoder force-pushed the ticket/master/PUP-1691-install-chocolatey branch 2 times, most recently from cd184cc to e0f09c5 Compare July 30, 2015 22:56
Allow installing chocolatey with a custom package location. Also allow
using the built-in windows shell to unzip the package. The original request for
this had the ability to use the built-in Windows shell to perform the unzipping
and it keeps folks from having to download an external executable by
default. Allow 25 minutes for it to finish.
@ferventcoder ferventcoder force-pushed the ticket/master/PUP-1691-install-chocolatey branch from e0f09c5 to 114b131 Compare July 30, 2015 22:59
Depend on the windows_env module for path update changes. This is to
ensure they are propagated on the same run you install Chocolatey in.
For spec testing, ensure that the `.fixtures.yml` has all modules that
are dependencies listed.
Allow install timeout to be configurable. Move all variables to
params.pp.
Ensure that the windows environment variables are set and are present.
This should also allow Chocolatey to be installed and used on the first
run of Puppet as it will refresh the PATH environment variable so that
suitability is redetermined[1]. This stems from an original conversation[2] with
a few community members and @joshcooper about suitability determination not
being cached in newer versions of Puppet.

[1] https://docs.puppetlabs.com/guides/provider_development.html#suitability
[2] https://groups.google.com/d/topic/chocolatey/0hPNtAMmhNM/discussion
This includes a custom fact for chocolatey's installpath. It will attempt
to read the environment variable for ChocolateyInstall from the registry
and default to 'C:\ProgramData\Chocolatey' if it is not found. This allows
Chocolatey to look at an existing installation by defaut.

The `$chocolatey::params::install_location` variable will default to
this custom fact, but can be overridden when declaring the class.
@reidmv
Copy link

reidmv commented Aug 17, 2015

Super excited to see this being worked on.

Any reason there isn't an init.pp file? It's very Puppet-paradigm aligned to want to include chocolatey.

@ferventcoder
Copy link
Contributor Author

Any reason there isn't an init.pp file?

@reidmv What would you like to see in an init.pp? :)

@rismoney
Copy link
Contributor

I will mod this too.

@ferventcoder
Copy link
Contributor Author

What does that mean @rismoney ?

@reidmv
Copy link

reidmv commented Aug 18, 2015

@ferventcoder what the current chocolatey::install class does is effectively what I would expect a chocolatey class to do. Whatever the implementation, an init.pp defining a chocolatey class gives me a declarative user experience where I describe a thing I want, without getting into a mindset where I'm thinking about procedure or action necessary to create or implement the described state.

@rismoney
Copy link
Contributor

Not change functionality, just refactoring a little. Add rspec, not use erb, and some tweaks.

@ferventcoder
Copy link
Contributor Author

@reidmv the install of a thing I've seen in install.pp files as a puppet convention, whereas the init.pp I've seen typically manages execution of the thing. Since choco is a custom type/provider, execution is relegated to the provider. Am I off on this?

@rismoney
Copy link
Contributor

this is what i was thinking for the erb-
rismoney@f95f5e3

@reidmv
Copy link

reidmv commented Aug 18, 2015

@ferventcoder using an install.pp is part of a module internals pattern most formally described by R.I. Pienaar in 2009 and redux'd in 2012. It's typically used to break apart an overall configuration into sub-classes for ease of implementation.

When the pattern is used, the *::install, *::configure and *::service classes are typically considered private internals, and not classes users should apply directly. See this example from the puppet/mcollective module of that private status being enforced. In newer code classes can use the assert_private() function from puppetlabs/stdlib instead to more elegantly enforce the same thing.

If you wanted to use that pattern, an init.pp might be created that looked like the following, and the existing chocolatey::install class modified to reference the parameters set here rather than take parameters itself (that and the assert_private() function added):

class chocolatey (
  $chocolatey_download_url = $chocolatey::params::download_url,
  $use_7zip                = $chocolatey::params::use_7zip,
  $choco_install_location  = $chocolatey::params::install_location,
  $choco_install_timeout   = $chocolatey::params::install_timeout,
) {
  contain chocolatey::install
}

Given that this is not an overly complex module yet, that pattern make not make sense yet. It might be clearer and more straightforward just to have a single chocolatey class right now. At this point it's all internals. The user still only interacts with a class named after the thing they want to configure, whether it's a single class or broken into contained subclasses is implementation detail.

The issue description in PUP-4638 talks a little bit about class names too and is loosely analogous.

@ferventcoder
Copy link
Contributor Author

@reidmv thanks! I read those articles, and TIL. I will add in an init.pp - it all makes perfect sense in the way that it was written. :)

@ferventcoder
Copy link
Contributor Author

@rismoney I left comments, but overall, I like that approach.

ferventcoder added a commit to ferventcoder/puppet-chocolatey that referenced this pull request Aug 21, 2015
Introduce init.pp and set install.pp as a private class.
This allows the benefit of `include chocolatey` without needing
to think about how to install Chocolatey and do any additional
setup steps. This was suggested by @reidmv at
chocolatey-archive#76 (comment).

As an artifact of this change, move stdlib requirement to at
least 4.6.0. At some point in the future, it may be wise to
work around using both `private()` and `assert_private()`
with a custom function that determines what version of
Puppet and what version of stdlib.
@ferventcoder ferventcoder force-pushed the ticket/master/PUP-1691-install-chocolatey branch from 00ba7d4 to 3999916 Compare August 21, 2015 13:32
ferventcoder added a commit to ferventcoder/puppet-chocolatey that referenced this pull request Aug 21, 2015
Introduce init.pp and set install.pp as a private class.
This allows the benefit of `include chocolatey` without needing
to think about how to install Chocolatey and do any additional
setup steps. This was suggested by @reidmv at
chocolatey-archive#76 (comment).

As an artifact of adding `assert_private()`, move stdlib
requirement to at least 4.6.0.
@ferventcoder ferventcoder force-pushed the ticket/master/PUP-1691-install-chocolatey branch from 3999916 to 21b2e8b Compare August 21, 2015 13:38
@ferventcoder ferventcoder force-pushed the ticket/master/PUP-1691-install-chocolatey branch 7 times, most recently from 95cc6be to 125491f Compare September 8, 2015 21:33
Set autouninstaller to true by default.
Validate that the parameters being passed in meet
the expectations.
Similar to Facter for Puppet versions less than 3.5.0,
if Hiera version hasn't been set, use a version that
was released with older Puppet versions.

Additionally ensure that both facter and hiera are
treated this way no matter if the Puppet is on Windows
or on *nix.
@ferventcoder ferventcoder force-pushed the ticket/master/PUP-1691-install-chocolatey branch from 125491f to b59807a Compare September 9, 2015 13:43
begin
old_choco_message = 'Please run chocolatey /? or chocolatey help - chocolatey v'
#Facter::Core::Execution.exec is 2.0.1 forward
value = Facter::Util::Resolution.exec("#{choco_path} -v").gsub(old_choco_message,'').strip
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like this will need to support checking Facter.version after all based on the api that is available in v3.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually this was left in v3 so we don't need to do this.

Contain is not a supported keyword prior to Puppet 3.4.0.
If less than 3.4.0, use the anchor pattern instead.
Add examples of usage surrounding the installation of Chocolatey to the
readme and document the classes/parameters.
Confine boundaries on upper versions of dependencies. This allows
better compatibility as the major version barrier is a known semantic
versioning boundary for incompatible changes.
Expose a custom fact about Chocolatey's version. Do not
attempt to do configuration in versions of choco less
than 0.9.9. This does mean a second convergence will be
required to apply the configuration settings.
@ferventcoder ferventcoder force-pushed the ticket/master/PUP-1691-install-chocolatey branch from b59807a to d5e4ce8 Compare September 9, 2015 23:32
Add documentation for the new facts this module produces.
@ferventcoder ferventcoder force-pushed the ticket/master/PUP-1691-install-chocolatey branch from d5e4ce8 to 6396bb6 Compare September 9, 2015 23:44
@ferventcoder ferventcoder changed the title [WIP - DO NOT MERGE](PUP-1691) Install Chocolatey (PUP-1691) Install Chocolatey Sep 9, 2015
ferventcoder added a commit that referenced this pull request Sep 9, 2015
…ll-chocolatey

(PUP-1691) Install Chocolatey
@ferventcoder ferventcoder merged commit d5c0f37 into chocolatey-archive:master Sep 9, 2015
@ferventcoder ferventcoder deleted the ticket/master/PUP-1691-install-chocolatey branch September 9, 2015 23:53
@rismoney
Copy link
Contributor

rismoney commented Sep 9, 2015

well done-woot!!!

@ferventcoder
Copy link
Contributor Author

Release train is rolling right now :)

@ferventcoder
Copy link
Contributor Author

@ferventcoder ferventcoder added this to the 1.1.0 milestone Sep 30, 2015
ferventcoder pushed a commit to ferventcoder/puppet-chocolatey that referenced this pull request May 5, 2017
…ster/MODULES-4678_fix_config

(MODULES-4678) Explicitly close config on read
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