Switch sensu_ctl resouce to use archive_file

sensu / sensu-go-chef

Chef Library Cookbook for Sensu Go

https://sensu.io

MIT License

11 stars 22 forks source link

Switch sensu_ctl resouce to use archive_file #112

Closed webframp closed 3 years ago

webframp commented 3 years ago

This removes the need for a dependency on the seven_zip cookbook since archive_file is a builtin chef resources since Chef 15

fixes #110

Pull Request Checklist

Is this in reference to an existing issue?

Yes.

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

[ ] Added a Testing Artifact as either an automated test or a manual artifact on the PR.
[ ] Added documentation for it to the README.md

Purpose

Modernize archive handling and remove dependency on seven_zip cookbook

Known Compatibility Issues

Unknown. Windows testing is not complete in CI, will need manual testing.

webframp commented 3 years ago

@derekgroh Can you take a look at this and confirm that behavior is the same as before? I don't easily have a Windows setup to do enough testing on. Once we know this works the same I can update the readme docs and changelog.

webframp commented 3 years ago

@majormoses same for me, marked as Draft for now until we can confirm behavior better on Windows

derekgroh commented 3 years ago

enterprise needs to be removed from line 70 in the -Outfile

Had some networking issues that I had to work around related to the configuration action, but the install action is working successfully with the change.

We may want to introduce some guards to help speed up the run after it is setup as it runs through each resource every time.. Unfortunately, the exe doesn't publish with any version info to use so we'd have to track with another method, maybe a version.info file.

webframp commented 3 years ago

Good feedback @derekgroh thanks! I'll be working on the cookbook again over this weekend as I have time so any suggestions are appreciated

webframp commented 3 years ago

Rebased on latest master

derekgroh commented 3 years ago

This is now failing on windows with #116 merged in.

Resource needs to be updated to:

default_ca_cert = value_for_platform_family(
  %w(rhel fedora amazon) => '/etc/ssl/certs/ca-bundle.crt',
  'debian' => '/etc/ssl/certs/ca-certificates.crt',
  'windows' => ''
)

There seems to be an odd bug with the :configure resource as well as sensuctl isn't available to the shell until the second converge with execute, but powershell_script is successful. Not sure if this is an environment specific issue (found on 2012R2, 2016 and 2019) as I recall it working in the past without issue.

webframp commented 3 years ago

There seems to be an odd bug with the :configure resource as well as sensuctl isn't available to the shell until the second converge with execute, but powershell_script is successful. Not sure if this is an environment specific issue (found on 2012R2, 2016 and 2019) as I recall it working in the past without issue.

You might be referring to the behavior I saw a while back that led to my attempt at a fix in #74

I'm not sure of the state of change now since it's a bit old. It should be reviewed though to ensure correct behavior.

derekgroh commented 3 years ago

@webframp It is a Windows Path issue being immediately available to the execute resource.

My testing showed this could be solved by using powershell_script resource instead.

The challenge is you have to ensure that the test runs successfully the first time, because any secondary run will succeed, but only because the path value is already set.