-
Notifications
You must be signed in to change notification settings - Fork 213
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 the capability for controller level checks #285
Conversation
examples/config.yaml
Outdated
@@ -1,4 +1,6 @@ | |||
checks: | |||
# reliability | |||
multipleReplicasForDeployment: warning |
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.
Can we move this to examples/config-full.yaml?
I think there are legitimate reasons to only have 1 replica (e.g. the polaris dashboard itself runs with 1 replica), but we could add this as an "optional check".
I'd love to build up a big community-driven library of checks that people could opt in or out of.
Codecov Report
@@ Coverage Diff @@
## master #285 +/- ##
==========================================
- Coverage 52.73% 52.02% -0.71%
==========================================
Files 12 12
Lines 639 690 +51
==========================================
+ Hits 337 359 +22
- Misses 269 284 +15
- Partials 33 47 +14
Continue to review full report at Codecov.
|
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.
Aside from one bit of refactoring, LGTM!
Thanks Bader! Excited to get this in for v1
pkg/validator/schema.go
Outdated
@@ -124,6 +126,17 @@ func applyPodSchemaChecks(conf *config.Configuration, controller kube.GenericWor | |||
if err != nil { | |||
return nil, err | |||
} else if check == nil { | |||
check, err = resolveCheck(conf, checkID, controller, config.TargetController, false) |
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.
I think this should go in its own function, applyControllerSchemaChecks
, which could get called in pkg/validator/controller.go
. That's similar to how we do both pod and container targets.
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.
Ohh yeah that makes a lot of sense, I was thinking of it still as controller checks, but it's still explicitly not a podSchemaCheck.
Brings along the JSON of the top level object. I also added a single check mostly for documentation purposes (but it could be a pretty useful check too)