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 for Triton discovery #18

Closed
wants to merge 15 commits into from
Closed

Support for Triton discovery #18

wants to merge 15 commits into from

Conversation

misterbisson
Copy link
Contributor

@misterbisson misterbisson commented May 7, 2017

Carries from #17:

  • Update Prometheus to 1.5.2
  • Update ContainerPilot to 2.7.2
  • Update to Consul 0.7.2
  • Add support for Triton metrics
  • Re-organize repo to bring it in line with other Autopilot blueprints
  • Add CI tests

Fixes #16, #8, and #5

This addresses code review questions and issues in addition to the above.

Next steps:

  • Autodetect the Triton DC, per Data center confusion #19
  • Update /README.md with more details, including:
  • Add a note about needing to init/update the submodule when pulling it down
  • Update /examples/triton/README.md with more detail, including a note about copying the user's TLS key
  • Update /test/tests.sh so that it sets all the correct env vars
  • Resolve makefile bugs with setting vars for local docker and docker-compose commands

@misterbisson misterbisson mentioned this pull request May 7, 2017
image := $(namespace)/prometheus
testImage := $(namespace)/prometheus-testrunner

#dockerLocal := DOCKER_HOST= DOCKER_TLS_VERIFY= DOCKER_CERT_PATH= docker
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This does not work on my Mac with Docker Machine. Replacing it with dockerLocal := docker does.

Error is:

$ make build
DOCKER_HOST= DOCKER_TLS_VERIFY= DOCKER_CERT_PATH= docker build -f test/Dockerfile -t=autopilotpattern/prometheus-testrunner:branch-triton-support .
Cannot connect to the Docker daemon. Is the docker daemon running on this host?
make: *** [test-runner] Error 1

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, that's unfortunate and is because Docker Machine uses DOCKER_HOST, etc. and Docker for Mac does not. The reason we're doing this is because we want the person (or Jenkins job) running the tests to be able to set their credentials to point to the environment-under-test but then run the tests from the local machine so that the keys never leave the machine running the tests.

This will effect every blueprint where we're doing testing this way. I'm certainly open to new suggestions on how to tackle it though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is now fixed by using https://github.com/joyent/triton-docker-cli, though the code here hasn't been updated.

COPY examples/triton/docker-compose.yml /src/triton/docker-compose.yml

# install tests
COPY test/testing/testcases.py /src/testcases.py
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Step 7 : COPY test/testing/testcases.py /src/testcases.py
lstat test/testing/testcases.py: no such file or directory
make: *** [test-runner] Error 1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'm an idiot. That was a submodule and I need to fetch its contents.

@@ -1,31 +0,0 @@
# Prometheus demonstration of the autopilot pattern
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 new pattern is to put this in /examples/triton

@@ -1,19 +0,0 @@
prometheus:
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 new pattern is to put this in /examples/docker/docker-compose.yml

RUN set -ex \
&& export CONSUL_VERSION=0.7.5 \
&& export CONSUL_CHECKSUM=40ce7175535551882ecdff21fdd276cef6eaab96be8a8260e0599fadb6f1f5b8 \
&& curl --retry 7 --fail -vo /tmp/consul.zip "https://releases.hashicorp.com/consul/${CONSUL_VERSION}/consul_${CONSUL_VERSION}_linux_amd64.zip" \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding Consul as a coprocess, per #8

"services": [
{
"name": "prometheus",
"port": 9090,
"health": ["curl", "-so", "/dev/null", "http://localhost:9090/metrics"],
"health": ["curl", "-fso", "/dev/null", "http://localhost:9090/metrics"],
Copy link
Contributor Author

@misterbisson misterbisson May 7, 2017

Choose a reason for hiding this comment

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

We expect if the request fails that curl will return a non-zero exit, yes?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes 👍

"poll": 10,
"ttl": 25
}
],
"coprocesses": [{{ if .CONSUL_AGENT }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Consul co-process, per #8

dns_suffix: 'cmon.{{env "TRITON_DC"}}.triton.zone'
endpoint: 'cmon.{{env "TRITON_DC"}}.triton.zone'
tls_config:
- ca_file: '{{env "TLSCA_PATH"}}'
Copy link
Contributor Author

@misterbisson misterbisson May 7, 2017

Choose a reason for hiding this comment

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

These var names are incorrect and need to be fixed.

These must match the new vars added to https://github.com/autopilotpattern/prometheus/pull/18/files#diff-86aea33913dc22bc9ba97b5f7b8c956e



# Consul acts as our service catalog and is used to coordinate global state among
# our Redis containers
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Redis?

@@ -0,0 +1,4 @@
echo CONSUL=prometheus-consul.svc.${TRITON_ACCOUNT}.${TRITON_DC}.cns.joyent.com > /src/triton/_env
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will need updating to include the changes in /examples/triton/setup.sh

- Make the config file (appear to) work
@@ -4,55 +4,83 @@ FROM alpine:3.4
# artisanally hand-rolling curl and the rest of our stack we'll just use
# Alpine so we can use `docker build`.

RUN apk add --update curl
RUN apk add --update \
bash \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It turns out the -n test in the prestart.sh is bash-specific. Of all the choices I had to fix it, I decided to add bash.

"name": "consul-agent (host:{{ .CONSUL }})",
"command": ["/usr/local/bin/consul", "agent",
"-data-dir=/var/lib/consul",
"-config-dir=/etc/consul",
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 alignment of these two dirs with the dirs created in the Dockerfile is frustratingly important.

@@ -1,8 +1,8 @@
# my global config
global:
scrape_interval: 15s # By default, scrape targets every 15 seconds.
evaluation_interval: 15s # By default, scrape targets every 15 seconds.
# scrape_timeout is set to the global default (10s).
evaluation_interval: 13s #
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 examples show the evaluation_interval slightly shorter than the scrape_interval, so...

@misterbisson misterbisson mentioned this pull request May 8, 2017

# Are we on Triton? Do we _not_ have a user-defined DC?
# Set the DC automatically from mdata
if [ -n ${TRITON_DC} ] \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@geek did you spot this one?

@misterbisson
Copy link
Contributor Author

Closing in favor of #21

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.

3 participants