Create all group as per Ansible inventory script conventions #46
Conversation
This file contains 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
Jonnymcc
force-pushed
the
ansible-inv-script
branch
from
9c081a0 to
ee7178e
Compare
|
Not sure why the tests are failing. I'm a go beginner. |
Jonnymcc
pushed a commit
to Jonnymcc/terraform-inventory
that referenced
this pull request
Sep 16, 2016
Jonnymcc
force-pushed
the
ansible-inv-script
branch
from
1c036fe to
ab55899
Compare
lolchocotaco
suggested changes
Sep 21, 2016
| } | ||
| sort.Strings(strs) | ||
| i := sort.SearchStrings(strs, item) | ||
| if i < len(strs) && strs[i] != item { |
There was a problem hiding this comment.
According to the goDocs: https://golang.org/pkg/sort/#SearchStrings, this conditional excludes IP ranges that would go at the end of the list. Consider something like:
if i == len(strs) || (i < len(strs) && strs[i] != item) {
| @@ -139,7 +139,15 @@ const exampleStateFile = ` | |||
|
|
|||
| const expectedListOutput = ` | |||
| { | |||
| "all": {"datacenter": "mydc", "olddatacenter": "<0.7_format", "ids": [1, 2, 3, 4], "map": {"key": "value"}}, | |||
| "all": { | |||
| "hosts": ["10.0.0.1"], | |||
There was a problem hiding this comment.
It would be nice to update the test cases to test the unique host filtering functionality.
|
Thank you very much for the patch, and especially for the comprehensive explanation of the issue. Looks good to me. Merging. |
|
FYI, this PR is included in the v0.7-pre release. I'd greatly appreciate it if you could give it a try. |
|
Tested, looks good! |
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.
I first noticed that I was unable to run
terraform-inventory --listwithout first exportingTF_STATE=.terraform/terraform.tfstateand as per the README it appears as though having to export TF_STATE was optional. Second, I noticed that when runningansible-playbookwith the--list-hostsflagansible-playbookwould say there were no hosts found. When I removed--list-hostsit detected the host that the playbook should run against.I then ran
git bisectto track down the problem. And I was led to this commit 6ead28e. I noticed that in cli.go this change added an "all" group to the Ansible inventory, so I went to check the Ansible docs regarding the format expected from a script.http://docs.ansible.com/ansible/developing_inventory.html#script-conventions
The JSON generated from the master branch creates an "all" group containing a mapping of variable keys and values. According to the Ansible docs that mapping should be under the "vars" key of the "all" group.
After making these changes it has fixed both of the problems I was encountering.