Reject LXD connection options that conflict with the platform block - #106
Merged
Conversation
A suite with a top-level lxd: block builds a shared wrapper and creates its projects, networks, and profiles on that one server. Every lxd node was then constructed through the wrapper, and the node's own remote_addr, trust_token, certificates, skip_verify, and socket were dropped without a word — the instance was created locally while the suite read as though it targeted a remote server, so the tests passed against a machine the suite never named. Honouring the node's connection instead would create the instance on a server where the suite's projects, networks, and profiles do not exist, so the conflict is now reported rather than resolved: a node that sets any server-selecting option alongside an lxd: block is rejected, naming every conflicting option and both ways to fix it. --check catches it, and so does a real run through the wrapper. Nothing changes for a node that selects no server, or for a suite with no lxd: block, where the node manages its own connection as before.
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.
Closes #92.
The problem
A suite with a top-level
lxd:block builds a shared wrapper. Everylxdnode was then constructed through it, and the node's own connection options were dropped without a word:The instance was created locally while the suite read as though it targeted a remote server. Nothing warned,
--checkdidn't flag it, and the tests passed — against a machine the suite never named. The same node definition behaved differently depending on whether an unrelated platform block was present.The existing doc comment on
NewLxdNodeWithWrappereven said so: "Remote connection configuration in node options will be ignored."Why reject rather than honour
The issue offered both. Honouring the node's connection is the wrong repair: the wrapper creates the suite's projects, networks, and profiles on its server, so an instance created elsewhere would reference resources that do not exist there. That trades a silent wrong-machine failure for a confusing missing-resource one.
Multi-server support would mean making the platform manager itself multi-server — a much larger change, and not what the issue describes. So the conflict is reported instead:
Every conflicting option is named at once, and both ways out are stated.
Unchanged
lxd:block: the node manages its own connection exactly as before, soexamples/lxd/lxd-remote.yamlkeeps working.Verification
Against the live LXD daemon on this machine:
--check— the wrapper path is where the bug lived, and--checknever reaches it, so both paths are covered independently;lxd:block and an ordinary node still creates the instance, runs its test, and tears down cleanly (Pass: 00001).Unit tests cover the conflict, the no-platform-block case, a node selecting no server, and that all seven server-selecting options are reported together.
Build, vet, gofmt, and the full suite are green; the docs site builds.
Unrelated observation
While testing I hit
failed to create project default: Project "default" already exists— thelxd:block's project creation is not idempotent, so a suite naming an existing project fails setup. Separate from this change and not touched here; say the word and I'll file it.