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

Add support for Critical Plugins #1064

Merged
merged 1 commit into from
Oct 4, 2017
Merged

Conversation

jaymzh
Copy link
Collaborator

@jaymzh jaymzh commented Oct 3, 2017

Description

This adds a new Ohai configuration critical_plugins which will cause
Ohai to exit (or fail if inside of Chef) if the plugins in the list do
not run successfully.

This feature only supports Ohai 7-style named plugins, not the ancient
Ohai 6-style stuff.

Tests to follow, but:

$ cat /home/phild/ohai.rb
ohai.critical_plugins << :Filesystem
$ ./bin/ohai -c /home/phild/ohai.rb
[2017-10-03T13:43:14-07:00] INFO: The plugin path /etc/chef/ohai/plugins does not exist. Skipping...
[2017-10-03T13:43:14-07:00] ERROR: The following plugins marked as critical failed: [:Filesystem]

And when I run Chef:

[2017-10-03T13:44:11-07:00] FATAL: RuntimeError: The following plugins marked as critical failed: [:Filesystem2]. Failing Chef run.

Issues Resolved

This fixes #879.

Check List

@jaymzh jaymzh requested a review from a team October 3, 2017 20:46
Copy link
Contributor

@coderanger coderanger left a comment

Choose a reason for hiding this comment

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

Overall 👍 but tweaks.

@@ -92,6 +92,7 @@ def generate_mountpoint_view(fs)
end

collect_data(:linux) do
fail "i iz failing"
Copy link
Contributor

Choose a reason for hiding this comment

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

Oops :)

@@ -107,6 +108,21 @@ def run_plugins(safe = false, attribute_filter = nil)
Ohai::Log.error("Encountered error while running plugins: #{e.inspect}")
raise
end
critical_failed = []
@runner.failed_plugins.each do |failed|
if Ohai::Config.ohai[:critical_plugins].include?(failed)
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use a set intersection to do this, critical_failed = Ohai::Config.ohai[:critical_plugins] & @runner.failed_plugins.

@jaymzh jaymzh force-pushed the critical_plugins branch 2 times, most recently from 1449e61 to 4eb6f21 Compare October 3, 2017 21:02
Copy link
Contributor

@danielsdeleo danielsdeleo left a comment

Choose a reason for hiding this comment

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

tests are good 😄

Ohai::Log.error(msg)
exit
else
fail "#{msg}. Failing Chef run."
Copy link
Contributor

Choose a reason for hiding this comment

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

should have a specific exception class

@@ -92,6 +92,7 @@ def generate_mountpoint_view(fs)
end

collect_data(:linux) do
fail "i iz failing"
Copy link
Contributor

Choose a reason for hiding this comment

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

let's remember to remove this before merging 😄

@danielsdeleo
Copy link
Contributor

I like this feature. Would be worth thinking about adding some plugins to the list by default for Chef 14

@jaymzh jaymzh force-pushed the critical_plugins branch 2 times, most recently from 440168c to 34e6a65 Compare October 3, 2017 21:41
@jaymzh
Copy link
Collaborator Author

jaymzh commented Oct 3, 2017

OK, review comments, now failing with a unique exception, and a test.

Copy link
Contributor

@danielsdeleo danielsdeleo left a comment

Choose a reason for hiding this comment

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

🚀

Copy link
Contributor

@thommay thommay left a comment

Choose a reason for hiding this comment

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

jaymzh added a commit to jaymzh/ohai that referenced this pull request Oct 3, 2017
This is a backport of chef#1064 to Ohai 8

This fixes chef#879.

Signed-off-by: Phil Dibowitz <phil@ipom.com>
@jaymzh jaymzh mentioned this pull request Oct 3, 2017
4 tasks
jaymzh added a commit to jaymzh/ohai that referenced this pull request Oct 3, 2017
This is a backport of chef#1064 to Ohai 8

This fixes chef#879.

Signed-off-by: Phil Dibowitz <phil@ipom.com>
@jaymzh
Copy link
Collaborator Author

jaymzh commented Oct 3, 2017

Updated message for @tas50

@jaymzh
Copy link
Collaborator Author

jaymzh commented Oct 3, 2017

come on Travis, you can do it!

@jaymzh
Copy link
Collaborator Author

jaymzh commented Oct 4, 2017

Travis seems better, so updating this PR with a RELEASE_NOTES.md update to kick travis.

This adds a new Ohai configuration `critical_plugins` which will cause
Ohai to exit (or `fail` if inside of Chef) if the plugins in the list do
not run successfully.

This feature only supports Ohai 7-style named plugins, not the ancient
Ohai 6-style stuff.

Tests to follow, but:

```
$ cat /home/phild/ohai.rb
ohai.critical_plugins << :Filesystem
$ ./bin/ohai -c /home/phild/ohai.rb
[2017-10-03T13:43:14-07:00] INFO: The plugin path /etc/chef/ohai/plugins does not exist. Skipping...
[2017-10-03T13:43:14-07:00] ERROR: The following plugins marked as critical failed: [:Filesystem]
```

And when I run Chef:
```
[2017-10-03T13:44:11-07:00] FATAL: RuntimeError: The following plugins marked as critical failed: [:Filesystem2]. Failing Chef run.
```

This fixes chef#879.

Signed-off-by: Phil Dibowitz <phil@ipom.com>
@jaymzh
Copy link
Collaborator Author

jaymzh commented Oct 4, 2017

Fixing lint

@jaymzh jaymzh merged commit 978bcf9 into chef:master Oct 4, 2017
jaymzh added a commit that referenced this pull request Oct 4, 2017
This is a backport of #1064 to Ohai 8

This fixes #879.

Signed-off-by: Phil Dibowitz <phil@ipom.com>
@tas50 tas50 changed the title Critical Plugins Add support for Critical Plugins Oct 24, 2017
@chef chef locked and limited conversation to collaborators Feb 14, 2018
@jaymzh jaymzh deleted the critical_plugins branch May 4, 2018 21:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

make a required/critical plugins list
4 participants