Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

142 docker improvements #147

Merged
merged 9 commits into from
Jul 18, 2017
Merged

142 docker improvements #147

merged 9 commits into from
Jul 18, 2017

Conversation

BretFisher
Copy link
Member

WIP

Will eventually fix #142

@BretFisher BretFisher self-assigned this Jul 1, 2017
@BretFisher BretFisher requested review from wbprice and rydente July 1, 2017 07:47
@BretFisher BretFisher mentioned this pull request Jul 1, 2017
20 tasks
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
# THE SOFTWARE.
FROM node:alpine
FROM node:6.11
Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't want to use the Alpine image here?

Copy link
Member

Choose a reason for hiding this comment

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

We could also use the alpine version of the node Docker image.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry guys, just realized I had custom notifications for CfA repos to goto my CfA email which I really only check weekly. 😨

Short answer is we don't care as much about tiny single-instance images as we do about ease of use, and with alpine not using glibc or apt-get it'll make it trickier and a higher learning curve for new people. For example: I was getting build failures on npm install in alpine due to bugs unique to alpines c complier. Using default node skips all those issues.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Really? What kinds of issues?

Copy link
Member Author

Choose a reason for hiding this comment

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

Most common is npm module build failures that require c compiler with some issue related to their lack of glibc. Things are getting better all the time, but doesn't mean we need to be bleeding edge in this area for little benefit

@@ -0,0 +1,26 @@
version: '3.3'
Copy link
Collaborator

@rydente rydente Jul 3, 2017

Choose a reason for hiding this comment

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

I really don't like the idea of multiple Docker Compose files, it makes the original intent of just using docker-compose up a bit more difficult when differentiating between environments.

Although I heard from a previous co-worker that at your talk, you mentioned Docker Compose eventually going away and/or only used as a development tool? Is that because of the improvements in Swarm?

Copy link
Member

Choose a reason for hiding this comment

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

You're correct that Docker Compose is only intended for local development. It's preferred to use Docker Swarm instead in production.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well, what about this then?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure what we're talking about here.

the goal of a single docker-compose up is a local-dev-only goal. CI/CD will do all the extra things we'd need for testing and production deploy.

docker-compose cli is for dev and testing only. It doesn't understand multi-server clusters like Swarm

docker-compose.yml file is for dev, test, and prod docker Swarm. (using docker stack deploy in swarm)

We'll likely use override files with docker-compose config for production deploy, but not sure yet until we get everything needed into compose file and see what the dev-vs-prod differences are. We can't use extends, but overrides work great.

Does that help?

Copy link
Member Author

Choose a reason for hiding this comment

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

put another way:

The process I typically go through is making local dev as easy as possible because it's hand-typed commands by humans.

Ideally the dev should be able to install Docker, then:

git clone xxxxx
docker-compose up

Then get to work ;)

CI/CD will be automated so it can have more complex commands that combine compose files with docker-compose config and dealing with secrets, env vars, etc.

Copy link
Collaborator

@rydente rydente Jul 18, 2017

Choose a reason for hiding this comment

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

So, how is that the case (Compose is for local dev only) when they have explicit production environment support on the roadmap?

Not to mention, how would you do the same thing with Compose does (orchestrating lots of containers with a single configuration file) using Swarm only? Would that mean you'd have to translate the Compose config into a bash script or some sort of configuration management platform like Chef or Ansible to get this set up in production?

If that's the case, then why use Compose at all?

EDIT: I just saw docker stack deploy, so does that mean we'd use this Compose config to create a DAB, then deploy the DAB to a Swarm cluster? That would answer pretty much all my questions.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think you're getting it. docker-compose cli and docker-compose.yml file have different scopes and purposes. The yaml file is used in dev+test+prod (everywhere). It is used by both docker-compose cli for dev/test and docker cli for prod swarm. Different parts of that file work in different scenarios.

No DAB needed in swarm, DAB is legacy and you can use compose file directly.

Local dev:

docker-compose up
That uses docker-compose.yml to start all the containers with proper settings and build stuff. build: objects under each service in compose file are used by docker-compose cli only and ignored by docker stack commands. See docs on build: in yaml

Docker Swarm Prod:

docker stack deploy -c docker-compose.yml mystackname
Will deploy all the containers with proper deploy settings like how many replicas of each container to run, healthchecks, how to update the service, etc. See deploy: docs

Copy link
Member Author

Choose a reason for hiding this comment

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

Also @rydente that readme for compose you referenced predates any of the modern features like Docker Swarm, and needs to be clarified. I recommend ignoring it. Eventually, the docker-compose cli is expected to go away, as its features are implemented in the docker cli, not become more important as that readme suggests.

Copy link
Collaborator

@rydente rydente Jul 18, 2017

Choose a reason for hiding this comment

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

Oh sweet, I didn't know they did away with DABs, awesome! Thanks for the info, Bret!

Re: that readme, thank you. I think someone should make a PR/issue... 🤔

HMMMMM

But how would the CLI go away? Do they plan on using a single Docker engine to make that happen, but still use the file format for the orchestration?

@@ -74,7 +74,8 @@
"eslint-plugin-react": "^6.6.0",
"mocha": "^2.4",
"supertest": "^1.2",
"zombie": "^5.0.5"
"zombie": "^5.0.5",
"nodemon": "^1.11.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@wbprice
Copy link
Member

wbprice commented Jul 5, 2017

@BretFisher is this intended to be working as is? I pulled down this branch, ran docker-compose up and got this after all the setup had completed:
screen shot 2017-07-05 at 7 48 48 pm

@wbprice wbprice mentioned this pull request Jul 7, 2017
@BretFisher
Copy link
Member Author

@wbprice RE: your "clean exit" I didn't understand why node was exiting after starting up. Hence my question here: #142 (comment) because I assumed node was exiting when no seeded data. Happy to co-work this issue.

@wbprice
Copy link
Member

wbprice commented Jul 17, 2017

Resolve conflicts, and track GOOGLE_API_KEY=google_api_key in docker-compose and we're gtg.

@wbprice
Copy link
Member

wbprice commented Jul 18, 2017

Also, this affects setup directions in the readme.

@BretFisher
Copy link
Member Author

@wbprice does GOOGLE_API_KEY need real value for local dev?

@wbprice
Copy link
Member

wbprice commented Jul 18, 2017 via email

Copy link
Member

@wbprice wbprice left a comment

Choose a reason for hiding this comment

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

LGTM

@wbprice
Copy link
Member

wbprice commented Jul 18, 2017

Let's merge after the call today.

@BretFisher
Copy link
Member Author

--inspect still not working. It works when I'm not launching the app, wondering if it's possible that trails is affecting that on node startup.

@wbprice wbprice merged commit 97031e8 into develop Jul 18, 2017
@wbprice
Copy link
Member

wbprice commented Jul 18, 2017

I don't really know right now. Let's leave it as an outstanding issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docker Configuration Changes
3 participants