search

canonical / pebble

Pebble is a lightweight Linux service manager with layered configuration and an HTTP API.
https://canonical-pebble.readthedocs-hosted.com/
GNU General Public License v3.0
146 stars 54 forks source link

fix: validate whole plan, rather than topmost two layers #363

Closed tonyandrewmeyer closed 7 months ago

tonyandrewmeyer commented 7 months ago

This PR moves layer/plan validation into dedicated functions, with Layer.Validate covering validation that is specific to a layer (such as an invalid value) and Plan.Validate covering validation that covers the plan as a whole (such as cycles or missing values).

Plan validation includes layer validation, and is done in the service manager's updatePlanLayers and in plan.CombineLayers.

We also use the check period as the upper bound for the timeout in checks, which was happening before, but less explicitly.

No additional validation is done - all of the validation code is moved rather than changed.

Fixes #349

flotter commented 7 months ago

@tonyandrewmeyer b.t.w this is a real coincidence that you are cleaning this up a bit, because I am on a kind of similar mission for slightly different reasons, and I've done in my PoC exactly what you conceptionally doing here. I've also split it up between Layer and Plan validation, but you went a bit further, actually cleaning up what is inside each category, which is super nice and brave :) Looking forward getting this merged.