Give --check stand-in nodes the real node type's capabilities - #99
Merged
Conversation
--check substituted one mock for every node, and that mock's capabilities matched no real node type. It implemented reboot, so a suite rebooting a docker node validated clean and failed only on a real run once the containers existed. It did not implement snapshots, so every suite containing a snapshot step was rejected outright, even on lxd where the step is valid. Capabilities now live in one table in nodetypes, consulted by both the stand-in nodes and the "supported: ..." half of the construction-time errors. A test asserts the table matches what each node type actually implements, using zero-value pointers so no daemon is needed, and fails if a known node type is missing from it — the drift that caused this. The reboot messages gain lxd-vm, which the hardcoded lists had omitted.
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.
Stacked on #98.
The problem
--checksubstituted a single mock for every declared node, and that mock's capability set matched no real node type — it was wrong in both directions:Rebooter, so a suite rebooting a docker node validated clean and failed only on a real run, after the containers had been created.Snapshotter, so every suite containing asnapshotstep was rejected outright — including onlxd, where the step is perfectly valid. That made--checkunusable for any suite using snapshots.The fix
Capabilities now live in one table in
nodetypes, consulted by both the stand-in nodes and thesupported: ...half of the construction-time error messages.NewCheckNode(nodeType)returns a stand-in implementing exactly what that type really implements.Before / after, same two suites:
Note the message now includes
lxd-vm, which the hardcoded(supported: lxd, ssh)strings had omitted — a third, smaller inaccuracy the table fixes by construction.Drift guard
The table caused this bug by being implicit, so the replacement is tested rather than trusted.
TestCapabilityTableMatchesNodeTypescompares the table against what each node type actually implements — using zero-value pointers, so interfaces are checked without constructing anything or touching a daemon — and fails if any type the factory accepts is missing from the test.TestCheckNodeMirrorsRealCapabilitiesasserts each stand-in matches its type.A node type that gains or loses a capability without updating the table now fails the build rather than silently putting
--checkback out of step.Verification
--check.go build,go vet,gofmt, and the full test suite are green; the docs site builds.--checksections in the README,cli.md, andsteps.mdnow describe what it actually does.