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

Support space-separated list in addresses #406

Merged
merged 1 commit into from
Feb 5, 2018

Conversation

sw0x2A
Copy link
Contributor

@sw0x2A sw0x2A commented Jan 31, 2018

In Consul 1.0 and later [addresses from config_hash] can be set to a space-separated list of addresses to bind to [...]

https://www.consul.io/docs/agent/options.html#addresses

Using a space-separated list in addresses for 'http' is currently not supported because it would break config in init scripts and other places. Limiting this to the first one mentioned fixes this.

"In Consul 1.0 and later [addresses from config_hash] can be set to a space-separated list of addresses to bind to [...]" https://www.consul.io/docs/agent/options.html#addresses

Using a space-separated list in addresses for 'http' is currently not supported because it would break config in init scripts and other places. Limiting this to the first one mentioned fixes this.
@solarkennedy
Copy link
Contributor

Instead of splitting and using the first entry, I would rather we just send the space-separated thing down to all the places. All the things that use http_addr should be able to handle the space-separated notation (right?).

@sw0x2A
Copy link
Contributor Author

sw0x2A commented Jan 31, 2018

I would agree but all the cases where http_addr is used accept only one IP.

The config_hash is used to generate the config file where it needs the space-separated list.

The http_addr variable is only used to in cases to specify one address to query HTTP API: Init scripts, CLI parameter -http-addr=$http_addr:$port. I am not aware of a way to set multiple host:port in these cases.

@sw0x2A
Copy link
Contributor Author

sw0x2A commented Feb 5, 2018

Let me rephrase my answer. Instead of having all the things that use http_addr be able to handle the space-separated notation, I would introduce another variable to not have the same logic spread across the module and in templates. One with the first IP of the list, the other with the entire list. Since all places where the current http_addr is used are to give a value to the -http-addr CLI parameter, which is a single IP only, I changed the existing variable.

I could have introduced another, new variable with the entire list but it wouldn't be used by now, so I did not.

@solarkennedy solarkennedy merged commit 5a17f4e into voxpupuli:master Feb 5, 2018
@bastelfreak bastelfreak added the enhancement New feature or request label Jul 5, 2018
spuder pushed a commit to spuder/puppet-consul that referenced this pull request Feb 25, 2020
Support space-separated list in addresses
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants