Repository navigation
fix: an overlay an upgrade adds to the chain takes its default - #88
Merged
Merged
Conversation
keel-core 0.1.3 added vip to Core's overlays, and after apt full-upgrade every keel command on two Keel Web nodes refused /etc/keel/instance.yaml: "overlays.vip: not declared; every overlay of the chain of web is written out". A spec written before the chain had vip cannot name it, and an upgrade must never break a working machine. An overlay the spec leaves out, and did not declare when it was last applied, takes the default its manifest gives for installation.mode: every command that reads the spec warns "overlays.vip: not declared (default: disabled)", apply --system converges it to that default, and diff lists it as "not declared (default: disabled, observed disabled)", or as drift when the machine is off the default. The spec on disk is never rewritten; inspect already writes the overlay out. apply --system records the overlays the spec itself declared, never the defaults, in /var/lib/keel/spec/overlays.yaml after a run that did not fail. An overlay in that record that the spec leaves out stays the error of decisions 0027 and 0041. Without a record nothing left out is refused. No default is taken without installation.mode, or for an ask default.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What broke
keel-core 0.1.3 added
vipto Core's overlays. Afterapt full-upgrade(keel 0.20.0, keel-core 0.1.3, keel-overlay-vip 0.1.1), every keel command on two Keel Web nodes failed:Error: /etc/keel/instance.yaml: overlays.vip: not declared; every overlay of the chain of web is written out, enabled or disabled (decisions 0027, 0041)Rule 25 held the spec against the chain as it is now, so any overlay added to a chain made every spec written before it invalid.
The fix: option (a)
An overlay of the chain that the spec does not declare, and did not declare when it was last applied, takes the default its manifest gives for
installation.mode:overlays.vip: not declared (default: disabled): the chain of web gained it after this spec was last applied, so it takes the manifest's default for simple; write it outapply --systemconverges it to that defaultdiffreportsoverlays.vip: not declared (default: disabled, observed disabled), or drift when the machine is off the defaultinspectalready wrote it out from systemd, so a spec written by inspect is complete (now tested)To tell "the chain gained it" from "the operator dropped it",
apply --systemrecords the overlays the spec itself declared (never the filled defaults) in/var/lib/keel/spec/overlays.yamlafter a run that did not fail. An overlay in that record that the spec leaves out is still the rule 25 error. With no record (noapply --systemsince this version), nothing left out is refused. No default is taken withoutinstallation.mode, or when the default isask.Why (a) and not (b)
0027 makes the spec the operator's document and the source of truth; 0041 rule 25 wants it explicit. Option (b), a maintainer script editing
/etc/keel/instance.yaml, would change a file the operator owns, replays elsewhere and may keep under configuration management, without anyone choosing it. Option (a) keeps the spec untouched and keeps the explicit-spec intent for everything the operator wrote, while the emitted spec (inspect, 0039's re-emission) still writes the new overlay out. The record uses the spec at its last apply rather than a manifest version: the manifest carries no per-overlay version, and adding one would raisemanifest_version.Rule 25 needs an amendment for this: Keel-Linux/handbook#54 (proposed, for approval).
Tests
tests/test_spec_new_overlay.py, written first (red on main):vipkeeps validating, applying and diffing after the chain gainsvip, with the warning;vipbehaves as before (no warning,same (disabled)), and once applied, dropping it is the error;keel inspectwritesoverlays.vip: disabled;ask, no mode, requires held against defaults, the record's reading, writing and failure to write.Locally the suite passes except the socket tests in
test_mesh_channel.py,test_mesh_vipbridge.pyandtest_mesh_bridge.py, which fail the same way onmainin this sandbox.Version 0.20.1, with a changelog entry.