Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update set_fact.sh to support multiple network #31

Merged
merged 1 commit into from
Mar 9, 2020

Conversation

quivalen
Copy link
Contributor

@quivalen quivalen commented Jul 8, 2019

Currently the role will fail to regather fact if there is more than 1 zerotier network configured

NETWORKS=$(zerotier-cli listnetworks | tail -n+2) will rightly contain the data with \n

But without the quotes echo will suppress the \n writing everything in one line.
Thus wc -l will return 1, confusing the file_content function that will lead to an incorrect json being produced

How to reproduce?
Without the fix, try to setup a second zerotier network - You will see that the regather fact step will fail, leading to no authorization of the node (as it cannot get the right fact)
If you inspect /etc/ansible/facts.d/zerotier.fact you will see that the json is malformed, the first network will fulfill

if [ "$counter" -eq "$network_count" ]; then
while all the others will match

Currently the role will fail to regather fact if $network_count > 1

That's because without the quotes echo will suppress the \n writing everything in one line. Thus wc -l will return `1`, confusing the file_content function that will lead to an incorrect json being produced
@m4rcu5nl m4rcu5nl changed the base branch from master to develop March 8, 2020 23:56
@m4rcu5nl m4rcu5nl self-requested a review March 8, 2020 23:57
@m4rcu5nl m4rcu5nl self-assigned this Mar 9, 2020
Copy link
Owner

@m4rcu5nl m4rcu5nl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch. Thanks!

@m4rcu5nl m4rcu5nl merged commit a84ea36 into m4rcu5nl:develop Mar 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants