Add windows support

[ripclawffb]

This will add windows support for telegraf installation and configuration. In order to support installation of telegraf on windows, the chocolatey-chocolatey or puppet-chocolatey forge module is required.

[yankcrime]

I think a slightly more distinctive name for this variable would be useful, as at first glance you could be forgiven for thinking that this applies to non-Windows operating systems. $windows_package_url perhaps? The install class probably wants refactoring to better handle multiple platforms, but for now this at least keeps things clear.

[ripclawffb]

Sounds good. I have made the requested change.

[yankcrime]

Formatting - needs a space after the {.

[ripclawffb]

Fixed.

[ripclawffb]

Requested changes have been completed.

[ripclawffb]

Are there any other changes that need to be made before this can be merged?

[yankcrime]

Sorry @ripclawffb - work commitments and holiday has meant I've not had chance to revisit this properly. I've made a couple of suggestions - I'd mostly like to avoid some of the conditionals or at least keep them in one place, i.e in the params class.

[cosmopetrich]

Windows support would be a fantastic addition, and it looks like a good start has been made here.

Unless I'm mistaken, it looks like there'll also need to be changes to telegraf::input -- https://github.com/datacentred/puppet-telegraf/blob/master/manifests/input.pp#L30-L32

[ripclawffb]

Good catch! I can't believe I missed that. I'll address that as well.

I also started refactoring the module to move the conditional statements to the params class as suggested by yankcrime.

Thanks.

[lahma]

I would like to chime in with a suggestion/question. Would it be more beneficial to use the zip package as install source as it seems that now one would need to prepare the Telegraf chocolatey package? It seems that chocolatey's "official" one is not well updated.

If you look how for example how ElasticSearch's filebeat puppet manifests handle this, only raw file download and native installation is done. Telegraf even support the service installation via command line switch.

Naturally there wouldn't be a problem with chocolatey package if the package was properly kept up to date.

[cosmopetrich]

For what it's worth, I'm in favour of chocolatey.

Even discounting all the general benefits to using a package manager over manually downloading and extracting an archive, the puppet provider for chocolately provides a similar feature set to the providers for apt/yum which should keep the module a bit cleaner.

As far as the version chocolatey goes, there are procedures for dealing with outdated packages. It also looks like our very own @ripclawffb is the package maintainer. Hopefully he'll update the package if we ask him nicely.

[ripclawffb]

I periodically check for new versions of telegraf, but I guess since the holidays, I missed it. I will work on a chocolatey automatic package, so if that works out, it should self maintain. I went ahead and upgraded the package and submitted it for review.

I agree with @cosmopetrich as it's easier for chocolatey to handle the install, upgrade and uninstall logic.

[lahma]

@ripclawffb thank you for updating the package. Now that I know that the chocolatey package is in safe hands and will be updated, the current way of utilizing it sounds like the right way to go. Waiting eagerly to try out the first version of this puppet module on Windows!

[ripclawffb]

@cosmopetrich, regarding your comment about:
https://github.com/datacentred/puppet-telegraf/blob/master/manifests/input.pp#L30-L32

I couldn't really find documentation on the windows equivalent of this directory:
/etc/telegraf/telegraf.d/${name}.conf

I would assume it would be 'C:/Program Files/telegraf/telegraf.d/${name}.conf' or just ''C:/Program Files/telegraf/${name}.conf'.

Do you know what the correct path is?

[cosmopetrich]

Hmm yeah, the config directory doesn't seem to be very well documented in the main project.

There doesn't appear to be a default specified by telegraf (0) (1), it looks like the value of /etc/telegraf/conf.d is set by the initscript and that the service-install command doesn't specify it, influxdata/telegraf#1797 looks to be about that.

Probably easiest just to go with c:\program files\telegraf\telegraf.d for this module, though it won't work until upstream fixes that issue.

[yankcrime]

Sorry @ripclawffb - you'll need to rebase your proposed changes now that there's been a couple of other PRs merged.

[ripclawffb]

No worries. I was able rebase the changes into my branch. I believe I have made all the requested changes including moving one of the conditionals to the params.pp file. I'm open to suggestions on how to move the packages or service conditionals to params.pp.

I also renamed @cosmopetrich config_fragment_dir variable since I didn't think we needed two variables with the same path.

Let me know if I missed anything. Thanks!

[cosmopetrich]

config_folder is probably a better name anyway, since that's what Telegraf calls it in its command-line args.

[cosmopetrich]

I just gave this a test, there appear to be a couple of non-Windows-related issues.

• Invalid JSON in the metadata file due to a missing , on line 27.
• windows_package_url won't be set on linux, which will cause puppet to throw an "Unknown Variable" error

You can run the specs by doing the following from the root of the repo.

gem install puppetlabs_spec_helper rspec-puppet --no-ri --no-rdoc
rake spec

Even if the specs don't cover 100% of resources they can still provide a useful smoke test.

[ripclawffb]

Thanks @cosmopetrich, I went ahead and fixed those issues. Everything comes back clean now.

I was also able to move the conditional statement for the service to the params manifest.

[cosmopetrich]

You may need to make repo_type available on both Windows and Linux hosts as well. I tested against a number of Linux+Windows hosts earlier with these changes and had some success.

Re the config_directory problem, I put in a PR against telegraf earlier today that adds the missing -config-directory directive to the Windows service. Hopefully that makes it into master. In the meantime, I've added this to the Puppet manifest that invokes ::telegraf as a workaround, in case it's helpful for anyone else.

Package['telegraf'] ~> Exec['update-telegraf-service'] ~> Service['telegraf']
exec { 'update-telegraf-service':
  command     => join([
      'C:\Windows\System32\sc.exe', 'config', 'telegraf', 'binpath=',
      '"\"C:\Program Files\Telegraf\telegraf.exe\"',
      '-config', '\"C:\Program Files\Telegraf\telegraf.conf\"',
      '-config-directory', '\"C:\Program Files\Telegraf\telegraf.d\""',
    ], ' '),
    refreshonly => true,
}

With that in place, ::telegraf::input looks to work as expected on Windows.

[ripclawffb]

@cosmopetrich, I made repo_type available for both types of hosts. I tested on Windows and Linux as well and it configured both hosts successfully.

Also, looking forward to that PR request getting merged. Thanks for the tip!

[cosmopetrich]

Have you had any luck installing telegraf on Windows via the puppet agent service @ripclawffb? When I run puppet manually (puppet agent --test) everything works fine, however puppet runs triggered by the agent service will fail with errors relating to the telegraf service:

/Stage[main]/Telegraf::Service/Service[telegraf]: Could not evaluate: Cannot get status of telegraf, error was: The specified service does not exist as an installed service. - OpenService: The specified service does not exist as an installed service.

The telegraf service doesn't appear in services.msc, though the chocolatey log indicates that it should have been successfully installed.

2017-01-24 22:20:11,142 [INFO ] - Installing telegraf...
2017-01-24 22:20:11,142 [INFO ] -
2017-01-24 22:20:11,173 [DEBUG] - Running 'Start-ChocolateyProcessAsAdmin' with exeToRun:'C:\Program Files\telegraf\telegraf.exe', statements: '--service install '
2017-01-24 22:20:11,173 [DEBUG] - Elevating Permissions and running ["C:\Program Files\telegraf\telegraf.exe" --service install ]. This may take a while, depending on the statements.
2017-01-24 22:20:11,189 [DEBUG] - Setting RunAs for elevation
2017-01-24 22:20:19,345 [DEBUG] - Command ["C:\Program Files\telegraf\telegraf.exe" --service install ] exited with '0'.
2017-01-24 22:20:19,345 [DEBUG] - Finishing 'Start-ChocolateyProcessAsAdmin'
2017-01-24 22:20:19,345 [INFO ] - telegraf has been installed.

[cosmopetrich]

Ahah, looks like it's a problem with telegraf that's fixed in 1.1 -- influxdata/telegraf#1772. Let's hope the chocolatey package update gets approved soon.

[yankcrime]

Awesome, thanks for completing the requested changes @ripclawffb .

I've got a few changes of my own to merge in at some point over the next few days, after which I'll cut a new release which includes Windows support 🎉

[yankcrime]

We can remove this conditional block by putting the platform specific options into params.pp and then referencing those. i.e two new parameters $::telegraph::config_file_owner and $::telegraf::config_file_group.

[yankcrime]

As with the telegraf::config class, we could remove this conditional also by having the provider configured as a parameter and then passing the extra option to ensure_packages. I think the require chocolatey is redundant, just state its fundamental requirement in the readme.