Update tests and compose files #7
Conversation
| - CONSUL=consul | ||
| links: | ||
| - consul:consul | ||
| ports: |
There was a problem hiding this comment.
Should we add a comment here about how this and most of the other ports declarations should not be open for production, at least on Triton?
See also autopilotpattern/wordpress#44
There was a problem hiding this comment.
Should we just do one better and remove these exposed ports? They're not needed for any example or blog post.
There was a problem hiding this comment.
It is easier to experiment with them when open, and for somebody not familiar with Triton, it's kind of important to explain.
There was a problem hiding this comment.
Also: expose and ports are redundant if they are the same values.
| image: autopilotpattern/redis:3.2.8-r1.0.0 | ||
| mem_limit: 512m | ||
| restart: always | ||
| expose: |
There was a problem hiding this comment.
Is the expose stanza repeated?
There was a problem hiding this comment.
Indeed it is sir.
| network_mode: bridge | ||
|
|
||
| webserver: | ||
| image: jasonpincin/ap-redis-example-webserver:1.0.0 |
There was a problem hiding this comment.
Where's the code for this?
There was a problem hiding this comment.
My bad. It's in examples/webserver
There was a problem hiding this comment.
I can publish this as autopilotpattern/ap-redis-example-webserver:1.0.0, was just publishing under my scope while iterating on the blog post and testing.
There was a problem hiding this comment.
I'd rather not use an ap prefix on any images in our namespace, but autopilotpattern/redis-example-webserver:1.0.0 looks good to me.
| local keyId=/$MANTA_USER/$MANTA_SUBUSER/keys/$MANTA_KEY_ID | ||
| local keyId=/$MANTA_USER/keys/$MANTA_KEY_ID | ||
| if [[ "${MANTA_SUBUSER}" != "" ]]; then | ||
| keyId=/$MANTA_USER/$MANTA_SUBUSER/keys/$MANTA_KEY_ID |
There was a problem hiding this comment.
Is this the first implementation to actually support subusers? Right on.
|
Mostly LGTM, but what's the reasoning for decoupling the tests from the examples? Seems like the sort of thing that will potentially get out of sync. |
| restart: always | ||
| mem_limit: 128m | ||
| labels: | ||
| - triton.cns.services=redis-consul |
There was a problem hiding this comment.
Thank you for namespacing this.
|
This looks pretty good. I think the example Compose files could use some inline comments, but I've got no other concerns. |
| services: | ||
| redis: | ||
| image: autopilotpattern/redis:3.2.8-r1.0.0 | ||
| mem_limit: 4g |
There was a problem hiding this comment.
Would this do better with either a g4-general-4G or even a g4-highram-16G package?
There was a problem hiding this comment.
| redis: | ||
| image: autopilotpattern/redis:3.2.8-r1.0.0 | ||
| mem_limit: 4g | ||
| restart: always |
There was a problem hiding this comment.
# Joyent recommends setting instances to always restart on Triton
| mem_limit: 4g | ||
| restart: always | ||
| labels: | ||
| - triton.cns.services=redis |
There was a problem hiding this comment.
# This label sets the CNS name, Triton's automatic DNS
# Learn more at https://docs.joyent.com/public-cloud/network/cns
| - triton.cns.services=redis | ||
| environment: | ||
| - CONTAINERPILOT=file:///etc/containerpilot.json | ||
| - affinity:com.docker.compose.service!=~redis |
There was a problem hiding this comment.
# This helps distribute Redis instances throughout the data center
# Learn more at https://www.joyent.com/blog/optimizing-docker-on-triton#controlling-container-placement
| env_file: _env | ||
| network_mode: bridge | ||
|
|
||
| consul: |
There was a problem hiding this comment.
This may deserve a comment about what Consul does, why it's here, etc.
| command: > | ||
| /usr/local/bin/containerpilot | ||
| /bin/consul agent -server | ||
| -bootstrap-expect 1 |
There was a problem hiding this comment.
Maybe add a comment about how to switch this so it runs as an HA, multi-node cluster, as in https://github.com/autopilotpattern/wordpress/blob/master/docker-compose.yml#L20-L21 ?
| restart: always | ||
| mem_limit: 128m | ||
| ports: | ||
| - 8000 |
There was a problem hiding this comment.
Run this on port 8000 on Triton?
There was a problem hiding this comment.
The example app was hard-coded to run on 8000, so that it worked locally. I could change the app to accept a PORT env var and listen on that, then pass that in... it complicates the example a bit, so I didn't bother. You have another preference?
https://github.com/joyent/blog/pull/308#discussion_r105173417 |
|
Per conversation with @tgross, I'm going to ditch the idea of having a separate set of compose files for CI, use those in examples, and just update test harness to generate |
|
:pickles: |
|
@misterbisson I know you already gave pickles, but one last push which has the harness create the _env now so that the examples look to users like they should. If still pickles, I'll merge this. |
|
:pickles: |
Updated examples, added many comments. Added small example web server, using the AutoPilot pattern, to read values from Redis. Updated tests.