-
Notifications
You must be signed in to change notification settings - Fork 22
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
Feature/check subdue #141
base: master
Are you sure you want to change the base?
Feature/check subdue #141
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,6 +28,7 @@ | |
|
||
resource_name :sensu_active_directory | ||
provides :sensu_active_directory | ||
unified_mode true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what is the min version of chef is required to use this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unified Mode is unavailable prior to 14.14 according to the docs. I've been adding these to appease cookstyle in my dev environment but isn't strictly necessary for this change. |
||
|
||
action_class do | ||
include SensuCookbook::Helpers | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,6 +28,7 @@ | |
|
||
resource_name :sensu_check | ||
provides :sensu_check | ||
unified_mode true | ||
provides :sensu_go_check | ||
|
||
property :check_hooks, Array | ||
|
@@ -44,7 +45,7 @@ | |
property :runtime_assets, Array | ||
property :secrets, Array | ||
property :stdin, [true, false], default: false | ||
property :subdue, Hash | ||
property :subdues, Array | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if they are both supported then we should add rather than change an interface There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my previous comment. I wanted to avoid confusion by aligning with the actual Check Resource name. |
||
property :subscriptions, Array, required: true | ||
property :timeout, Integer | ||
property :ttl, Integer | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are both valid?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion no, but it's unclear how the folks at Sensu intended to resolve this as the original subdue implementation still appears as null in the CheckConfig reference. See the YML output from their cron example check vs the actual subdues documentation section.
In practice one of my example checks using this new functionality lists both but I'm unable to set the original subdue as anything other than null: