search

sensu / sensu-chef

Sensu Chef cookbook.
https://supermarket.chef.io/cookbooks/sensu
Apache License 2.0
221 stars 280 forks source link

amazon_linux_2 support #614

Closed smcavallo closed 6 years ago

smcavallo commented 6 years ago

Signed-off-by: S.Cavallo smcavallo@hotmail.com

Use the correct repo and version for amazon linux 2 Currently the cookbook attempts to use the wrong yum_repo_releasever and package name for this platform.

Description

better support for amazon linux 2

Motivation and Context

better support for amazon linux 2 should also help address https://github.com/sensu/sensu-chef/issues/596

How Has This Been Tested?

Added chefspec tests.
Broke amazon platform into unique section of the case statement to avoid breaking other platforms.

Types of changes

Checklist:

smcavallo commented 6 years ago

@tas50 - thanks for catching that. I've checked in a fix and added a test case for the older platform. It feels a bit gross detecting the version this way. I used the versions @ https://github.com/chefspec/fauxhai/blob/master/PLATFORMS.md as a model and the release notes at https://aws.amazon.com/amazon-linux-ami/ which lists all the versions of the candidate OS.

would have preferred to create a library method for Sensu::Helpers.amazon_linux_2_version_string and/or full override of the version string as an attribute but tried to keep the refactoring to a minimum

smcavallo commented 6 years ago

@majormoses - made the suggested refactoring - hoping you can review

joshgarnett commented 6 years ago

Hey @smcavallo I just pulled this down to test and ran into this error:

================================================================================
Recipe Compile Error in /var/chef/cache/cookbooks/foo/recipes/base.rb
================================================================================

RuntimeError
------------
Unsupported Linux platform version 4.14.55-68.37.amzn2.x86_64 - rhel version unknown

Cookbook Trace:
---------------
  /var/chef/cache/cookbooks/sensu/libraries/sensu_helpers.rb:156:in `amazon_linux_2_rhel_version'
  /var/chef/cache/cookbooks/sensu/recipes/_linux.rb:98:in `block in from_file'
  /var/chef/cache/cookbooks/sensu/recipes/_linux.rb:93:in `from_file'
  /var/chef/cache/cookbooks/sensu/recipes/default.rb:34:in `from_file'
majormoses commented 6 years ago

@smcavallo thanks for making those changes, I am currently sick so it may be a few days for me to give a proper look but it looks like its failing. I will take a look when I am feeling a bit better and have the time to dig in.

smcavallo commented 6 years ago

@joshgarnett - I am wondering if this is ohai related. What version of chef-client and ohai are you using? From your output message it is showing the node['platform_version'] as 4.14.55-68.37.amzn2.x86_64 which is not what i'd expect to be listed in that field - from what I gather it looks like that's the node['os_version'] instead.

I just added chefspec tests for all available amazon linux 2 platform versions and all the tests pass.

joshgarnett commented 6 years ago

@smcavallo https://packages.chef.io/files/stable/chefdk/2.3.4/el/7/chefdk-2.3.4-1.el7.x86_64.rpm is what I'm currently installing

I pulled down your fixes and added some more changes at https://github.com/joshgarnett/sensu-chef/commit/cce3780ca0d7e2ebfe5d63ae97ef9cade078dcb7

majormoses commented 6 years ago

@joshgarnett is it working with your patch now?

joshgarnett commented 6 years ago

Yeah its working for me, just deployed a new sensu cluster using Amazon Linux 2

smcavallo commented 6 years ago

@majormoses and @joshgarnett - what are your thoughts on moving forward with this patch? I can integrate cce3780 into this branch/PR - the only change i'd make is that i'd note that code is a temporary workaround for older ohai which exists for backwards compatibility reasons. I don't know if it's possible to do that with chef code. once this cookbook no longer supports chef clients older than 14 that would make it easier to find/remove that code. basically....is there anything I can or should do to move this forward?

joshgarnett commented 6 years ago

@smcavallo the project README mentions support for Chef 12.14+, so I guess we are stuck supporting the older versions for awhile. The patch works for me, so I'm just going to stick with my branch until something similar gets merged upstream.

It's up to the Sensu team on how they want to handle it and their roadmap. It's not the prettiest of solutions, but does get the job done.

majormoses commented 6 years ago

I am good with moving forward on this patch. I agree since the issue is with ohai versions below a certain version. I think what we can do is just add a comment to indicate such. Something simple like this works for me:

# TODO: once we no longer support chef versions < [insert version] we should remove the check for amzon2 and remove this comment
return "7" if platform_version == "2" || platform_version.include?("amzn2")

Regarding version support we are supporting >= 13.3 see these issues for more details:

It looks like I need to update the README to reflect what the metadata states.

smcavallo commented 6 years ago

@majormoses - updated code + tests + comments are now included in this PR

smcavallo commented 6 years ago

@tas50 updated per https://github.com/chef/ohai/blob/master/CHANGELOG.md#v1430-2018-07-09

tas50 commented 6 years ago

Cool. 13.10 supports it too, but you'd never support 13.10 and not 14.0 so no need to mention that.

majormoses commented 6 years ago

I will do a final review this evening and if its all good I will merge and cut a release.

majormoses commented 6 years ago

released: https://supermarket.chef.io/cookbooks/sensu/versions/5.2.0

majormoses commented 6 years ago

@smcavallo thanks for the contribution!