-
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
Conversation
@@ -57,7 +57,7 @@ def check_from_resource | |||
spec['runtime_assets'] = new_resource.runtime_assets if new_resource.runtime_assets | |||
spec['secrets'] = new_resource.secrets if new_resource.secrets | |||
spec['stdin'] = new_resource.stdin | |||
spec['subdue'] = new_resource.subdue if new_resource.subdue | |||
spec['subdues'] = new_resource.subdues if new_resource.subdues |
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:
type: Check
api_version: core/v2
metadata:
created_by: s_sensu
labels:
sensu.io/managed_by: sensuctl
name: check_process
namespace: default
spec:
check_hooks: null
command: check_process.rb
env_vars: null
executed: 0
handlers:
high_flap_threshold: 0
history: null
interval: 15
is_silenced: false
issued: 0
last_ok: 0
low_flap_threshold: 0
occurrences: 0
occurrences_watermark: 0
output: ""
output_metric_format: ""
output_metric_handlers: null
pipelines: []
proxy_entity_name: ""
publish: true
round_robin: false
runtime_assets:
- runtime_ruby
scheduler: ""
secrets: null
status: 0
stdin: false
subdue: null
subdues:
- begin: "2022-04-18T16:00:00-05:00"
end: "2022-04-18T17:00:00-05:00"
repeat:
- weekdays
- begin: "2022-04-22T16:00:00-05:00"
end: "2022-04-22T17:00:00-05:00"
repeat:
- fridays
- begin: "2022-04-23T00:00:00-05:00"
end: "2022-04-23T23:59:59-05:00"
repeat:
- saturdays
- begin: "2022-04-24T00:00:00-05:00"
end: "2022-04-24T17:00:00-05:00"
repeat:
- sundays
subscriptions:
- check_process
timeout: 0
total_state_change: 0
ttl: 0
@@ -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 comment
The 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 comment
The 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.
@@ -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 comment
The 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 comment
The 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.
Pull Request Checklist
Is this in reference to an existing issue?
General
Update Changelog following the conventions laid out here
Update README with any necessary configuration snippets
Cookstyle (rubocop) passes
Rspec (unit tests) passes
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
With the release of Sensu Go 6.7 check subdue has been added again. This PR adds that functionality back to the cookbook.
Known Compatibility Issues
Can be added to previous versions of Sensu Go but the spec will not be recognized when the json file is uploaded to the backends.