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

Add CONSUL_DATACENTER_NAME env #46

Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ services:

In our experience, including a Consul cluster within a project's `docker-compose.yml` can help developers understand and test how a service should be discovered and registered within a wider infrastructure context.

#### Environment Variables

- `CONSUL_DATACENTER_NAME`: sets the name of the data center in which the Consul cluster is running.

### Clients

ContainerPilot utilizes Consul's [HTTP Agent API](https://www.consul.io/api/agent.html) for a handful of endpoints, such as `UpdateTTL`, `CheckRegister`, `ServiceRegister` and `ServiceDeregister`. Connecting ContainerPilot to Consul can be achieved by running Consul as a client to a cluster (mentioned above). It's easy to run this Consul client agent from ContainerPilot itself.
Expand Down
3 changes: 3 additions & 0 deletions bin/consul-manage
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ set -eo pipefail
preStart() {
_log "Updating consul advertise address"
sed -i "s/CONTAINERPILOT_CONSUL_IP/${CONTAINERPILOT_CONSUL_IP}/" /etc/consul/consul.hcl

_log "Updating consul datacenter name"
sed -i "s/CONSUL_DATACENTER_NAME/${CONSUL_DATACENTER_NAME:-dc1}/" /etc/consul/consul.hcl
Copy link
Contributor

Choose a reason for hiding this comment

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

I like configuring via environment variable, I think it makes sense here. Do we even need to do anything with metadata mentioned in #23?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The environment variable make it straightforward to manually set the datacenter and makes testing and iteration easier, but it doesn't incorporate the automatic datacenter detection that seems to be described in #23.

I can address that part of #23 if we can decide on how it would be triggered compared to the method implemented in this PR for manual DC specification.

Options seem to include:

  • CONSUL_DATACENTER_NAME=*AGREED UPON STATIC VALUE* is checked specifically in preStart and triggers a metadata check if it matches. Anything but the static value would be assumed to be a manually-specified DC
  • CONSUL_DATACENTER_AUTODETECT=triton would invoke triton-specific attempts to discover the current datacenter and allow for future implementations for other cloud providers
  • CONSUL_DATACENTER_AUTODETECT=1 would attempt to check different methods of discovering the current datacenter, though involve more "magic" (i.e. brittle or potentially-unexpected behavior)
  • CONSUL_DATACENTER_AUTODETECT_TRITON=1 would invoke triton-specific attempts to discover the current datacenter, establishing the pattern for other providers to user their own envs
  • something else?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, and the env var satisfies non-Triton use cases. I wasn't entirely set on using metadata but we could marry the two ideas for convenience.

  • Use CONSUL_DATACENTER_NAME to manually set/force the datacenter name.
  • If env var isn't present AND we have /native/usr/sbin/mdata-get then poll metadata for sdc:datacenter_name.
  • Finally, use the default dc1 if none of the above conditions gave us a custom value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Falling back to automatic detection seems like the best option for ease-of-use, great suggestion!

I'll include the steps taken in the README so users can see how it'll work without having to dig into the source.

}

#
Expand Down
1 change: 1 addition & 0 deletions etc/consul.hcl
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
bind_addr = "CONTAINERPILOT_CONSUL_IP"
datacenter = "CONSUL_DATACENTER_NAME"
data_dir = "/data"
client_addr = "0.0.0.0"
ports {
Expand Down
1 change: 1 addition & 0 deletions local-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@ services:
- 8500
environment:
- CONSUL=consul
- CONSUL_DATACENTER_NAME=dc1
command: >
/usr/local/bin/containerpilot