Moved package installs for Windows to Chocolatey

[kovukono]

Pull Request Checklist

Is this in reference to an existing issue? No

General

• [X] Update Changelog following the conventions laid out here

• [X] Update README with any necessary configuration snippets

• [x] Cookstyle (rubocop) passes

• [x] Rspec (unit tests) passes

• [x] Inspec (integration tests) passes

New Features

• [X] Added a Testing Artifact as either an automated test or a manual artifact on the PR.

• [X] Added documentation for it to the README.md

Purpose

Move Windows to a package manager instead of tarball extraction.

Known Compatibility Issues

This may cause unexpected package upgrades on Windows when moving to this. However, this can be avoided by setting 6.1.0 as the version for the sensu_agent and sensu_ctl resources.

[kovukono]

@majormoses Is there anything further that needs to be added or changed for the PR?

[majormoses]

@kovukono I will have to take a closer look but I do think there is a breaking change in the sense of moving the interface for the installed version of ctl from a chef attribute to something you need to pass to the resource. Not a big deal other than it needs to be documented/communicated in the changelog and done in the appropriate major release.

[derekgroh]

@kovukono what do you show the windows path is for the choco install?

Looks like it was updated to c:\Program Files\Sensu\sensu-cli\bin

[kovukono]

@derekgroh It looks as if that's where Chocolatey is placing it.

[derekgroh]

The following changes should be made: libraries/helpers_sensuctl.rb

-          'c:\Program Files\Sensu\sensu-cli\bin\sensuctl'
+          'c:\Program Files\Sensu\sensu-cli\bin'

test/integration/ctl/ctl_spec.rb

-  describe command('cmd.exe /c "echo %PATH%"') do
-    its('stdout') { should include 'c:\Program Files\Sensu\sensu-cli\bin\sensuctl' }
+  describe os_env('PATH', 'system') do
+    its('split') { should include 'C:\Program Files\Sensu\sensu-cli\\\\bin' }
  end
Note: there's a bug with the packaging from Sensu as an extra `\` is being added between sensu-cli and bin.

[kovukono]

I agree on the change to the test, but not the libraries. sensuctl_bin is intended to point to the executable, not the folder. We potentially could change the configure command to use sensuctl_bin if you want to standardize it. It's also possible that asset installation/updates never worked for Windows, but I couldn't tell you as I haven't found a single Bonsai asset that supports Windows.

[derekgroh]

Oh yes, you're right, it is calling the actual executable not the path.

[majormoses]

Note: there's a bug with the packaging from Sensu as an extra \ is being added between sensu-cli and bin.

I think the "squeeze" method might be of some value here until there is an upstream fix.

$ irb
irb(main):001:0> test_string='C:\Program Files\Sensu\sensu-cli\\\\bin'.freeze
=> "C:\\Program Files\\Sensu\\sensu-cli\\\\bin"
irb(main):002:0> test_string.squeeze
=> "C:\\Program Files\\Sensu\\sensu-cli\\bin"
irb(main):003:0> test_string.squeeze('\\')
=> "C:\\Program Files\\Sensu\\sensu-cli\\bin"

[kovukono]

I've rebased off of the changes made in #129, but I don't have an answer as to why the configure action failed in this last run. configure is pretty much the one action I didn't touch for ctl, and I've had no success replicating this failure on a clean Windows VM. I will note that we have had issues with configure failing as an action due to timeout on Linux platforms, and rerunning it often succeeds, but we can't do that without an additional commit. What can I do to get this PR finished?

[derekgroh]

You try flipping Chef Zero and GitHub Windows Path (Deprecated Add-Path) steps, if you want to rule out a Github actions environment issue, but sensu_ctl has always been pretty delicate on windows IMO.

[derekgroh]

Actually I'm really confused, my branch which the PR was based off of, has GitHub Windows Path (Deprecated Add-Path) first.

Maybe there was a change that required switching the order since then?

You shouldn't need to do any of your other changes.

[kovukono]

@derekgroh I swapped them, but unfortunately got a lint error that prevented the tests from even getting to the Windows tests. I went ahead and found that the problem looks that it's unable to find sensuctl.exe when it's running, so I used the full path like we have for Linux. Unfortunately, it's failing now on the entity list.

[kovukono]

FIgured it out--it was the same issue with that the PR Is fixing for the old bad bin path. Fixed it for Github and it's working fine.

[kovukono]

Is there anything further I need to do to get this merged after it's been approved?

[majormoses]

I'd like one final :+1: / approval from @derekgroh if possible. If we don't have something by Wed Dec 23rd then I will merge this without it.

I will release a new major version with this change. Thank you so much for your great work, especially since I don't run anywhere with windows + chef combination.

[derekgroh]

Yep, good here.

[kovukono]

I rebased against master since the git workflow got changed and caused a merge conflict. The new CI is failing on Amazon, however. I don't know why this didn't get merged with the approvals before--is there any chance it can get merged now?

[majormoses]

@kovukono I honestly don't recall. I will look at doing a release this week.