Support for group_vars and host_vars #5
Conversation
This PR adds support for loading external variables from group_vars and host_vars directories. Also: + Refactor tests to use testify + Add few missing tests for public API functions + Fix warnings reported by the new Go
There was a problem hiding this comment.
Thanks! I've played with the tests a bit and the basic expectations seem to be met. The patch doesn't claim to support full-fledged variable precedence, which is fine, but maybe it's worth saying explicitly that it doesn't?
A couple of questions inline might or might not be worth answering in a comment before the walk function to briefly explain how it's supposed to work.
| if err != nil { | ||
| return nil | ||
| } | ||
| if filepath.Dir(path) == root { |
There was a problem hiding this comment.
So we ignore directories further down, right?
There was a problem hiding this comment.
Depends on what you mean by "ignore" :)
We ignore their names, yes, because they have no semantical meaning. Only names of the first-level entities have it: they determine the name of a group or a host to which we should assign variables.
However, we do not ignore their contents: we read all files and directories recursively.
The WalkDir function traverses the file tree in the "depth-first" order. So, whenever we visit any first-level entity, we should remember its name so that all variables read further will go to the entity with the corresponding name.
Data from parsed files is now stored in private fields that are not supposed to be modified after parsing. This allows us: + make Reconsile function idempotent + implement proper multi-stage variable merging Groups are now processed in lexical order, which matches how it is implemented in Ansible
|
@nemobis About variable precedence: we do our best to support it, and if there is a mismatch, it should be considered a bug. Let's go through that list. Role or playbook variables are completely out of the scope of this project. So, what's left is this part: Points 4 and 6 are a bit misleading: in reality, group variables apply with hierarchy precedence, that's why the This should match the variable processing logic introduced by the latest commit. |
* Group loops no longer cause parsing to hang * "all" group no longer contains self * Add String() methods to group and hosts * Fix wrong method call in populateInventoryVars * Reconsile inventory method is now much more robust
This PR adds support for loading external variables
from group_vars and host_vars directories. Also:
Fixes #3
Fixes #4