Add -check-config option to validate config.

[ppalucki]

Exists with proper exit code to (0-ok) and prints error message.

Can be used as a check before applying new configuration.

Just an idea - probably in our extension we will programatically API of config module - more about it here: https://github.com/intel/gardener-extension-cri-resmgr/issues/56

BTW: I found undocumented config-help/help arguments to are usefull - are they undocumented on purpose or just missed?

Output looks like:

# go run ./cmd/cri-resmgr/ -check-config sample-configs/balloons-policy.cfg
...
config ok
# echo $?
0

or

# go run ./cmd/cri-resmgr/ -check-config broken.cfg
validation error:  config error: module logger: given unknown configuration data Debuug
# echo $?
1

with broken.cfg:

logger:
    Debuug: "foo"

Marking it as draft because I don't how usefull it would be - probably it just checks syntax (proper keys and types) but not sematics e.g. ? (cpuset is parsed for example, but I can set policy.Active to non-existents and still returns ok!!!)

e.g. non-existing-policy.cfg

#cat non-existing-policy.cfg
policy:
    Active: "ThereIsNoSuchPolicy"

# go run ./cmd/cri-resmgr/ -check-config non-existing-policy.cfg
config ok

[codecov-commenter]

Codecov Report

Merging #899 (10d4065) into master (ae31e13) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #899   +/-   ##
=======================================
  Coverage   34.89%   34.89%           
=======================================
  Files          60       60           
  Lines        8869     8869           
=======================================
  Hits         3095     3095           
  Misses       5472     5472           
  Partials      302      302           

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[klihub]

LGTM

[klihub]

Exists with proper exit code to (0-ok) and prints error message.

Can be used as a check before applying new configuration.

Just an idea - probably in our extension we will programatically API of config module - more about it here: intel/gardener-extension-cri-resmgr#56

BTW: I found undocumented config-help/help arguments to are usefull - are they undocumented on purpose or just missed?

We have not properly added documentation for all conifguration bits, so it's not advertised.

...

Marking it as draft because I don't how usefull it would be - probably it just checks syntax (proper keys and types) but not sematics e.g. ? (cpuset is parsed for example, but I can set policy.Active to non-existents and still returns ok!!!)

Basically this effectively checks that the configuration file can be loaded and unmarshalled into the internal go data structure representations. That said, there is also a varying level of other validation done in the components that register themselves for configuration, since 1) with go one can override/wrap the default JSON/YAML marshaller and do an extra round of limited semantic validation in the custom one, and 2) our internal config infra allows one to associate post-update notifier callbacks to the data they registered for configuration.

e.g. non-existing-policy.cfg

#cat non-existing-policy.cfg
policy:
    Active: "ThereIsNoSuchPolicy"

# go run ./cmd/cri-resmgr/ -check-config non-existing-policy.cfg
config ok

Yeah... the generic policy layer does not do checking of the active policy after (re)configuration... mostly because ATM you can't change the active policy.

This PR and the feedback from you is actually pretty good timing. We're reworking the configuration bits in preparation for adding cleaner support for runtime-switchable policies. While, most of the basic plumbing is done, we're playing around with what extra functionality we want/need to add and how (exactly things like, support for semantic configuration checking, self-documenting configuration, etc.). For instance, if we want, I can easily add enforced checking that any data (type) which is registered for (receiving) configuration (data) needs to at least look like self-documenting..., for instance by requiring that they implement the interface

type [config.]Fragment interface {
    // Describe provides a self-documenting description for the configuration fragment.
    Describe() string
    // SampleYAMLConfig provides self-documenting sample configuration fragment as YAML,
    // optionally with inline comments. Unmarshallability of this data is verified during registration.
    SampleYAMLConfig() string
}

etc... I can share the devel branch with you for a sneak peak, pre-review.

[klihub]

etc... I can share the devel branch with you for a sneak peak, pre-review.

Here is my working branch and a draft PR for it: #903

[ppalucki]

Lets close it, and focuse on #903 - which looks as much more comprehesive solution - - I'm not longer sure I want to follow a path of "static" config validation - for ordinary user it makes no sense - it will just run cri-rm and check output - for our case I want to focus on policy passed by agent - if switching policy will be possible with agent (and maybe other options e.g. metric-interval) - then I don't need this low-level mechanic of validation.