modernized the dockerfile, wrote a compose file to run mongodb inside…

[boutell]

… docker too, wrote a quick dockerized apostrophe tutorial in the README

[boutell]

The wait-for-it utility is recommended practice from the docker docs. I put it in because there's no other good way to know if mongo is actually ready to accept connections or not.

[boutell]

docker-compose exec

Huge win! Thank you. I updated the PR.

expose will expose db without authentication

Actually it won't as it turns out:

"Expose ports without publishing them to the host machine - they’ll only be accessible to linked services. Only the internal port can be specified."

This makes sense, we just want it to be visible to the service we linked to it. If someone wants to mongodump/mongorestore they can perhaps do that with docker-compose exec.

If this were not true, I would have received a bind error, as port 27017 is occupied by my mac's mongod.

Good to merge?

On Tue, Jan 7, 2020 at 9:42 AM Anthony Tarlao notifications@github.com wrote:

@falkodev commented on this pull request.

In README.md https://github.com/apostrophecms/apostrophe-boilerplate/pull/18#discussion_r363777338 :

+ + +You can connect normally in your browser by going to that address. + +4. You will note there is no `admin` account in the database yet. Let's fix that: + +bash +docker ps + + +Note the id of the `apostrophe` container. The id is a string like: `319baf76fe74` + +Then execute a command inside your container: + + +docker exec -it YOUR_ID_HERE node app apostrophe-users:add admin admin

It might be easier to type docker-compose exec apostrophe node app apostrophe-users:add admin admin because users don't have to find the container id (worth checking if the command works - I did not check)

In docker-compose.yml https://github.com/apostrophecms/apostrophe-boilerplate/pull/18#discussion_r363778201 :

• links:
• • mongo
• volumes:
• • ./data/uploads:/app/public/uploads
• environment:
• • APOS_MONGODB_URI=mongodb://mongo:27017/db
• depends_on:
• • mongo
• mongo:
• container_name: mongo
• image: mongo
• restart: always
• volumes:
• • ./data/db:/data/db
• expose:
• • "27017"

If people use this configuration in production, they will expose their db without authentication. Might be useful to explain in the documentation not to expose the port in production or protect the db with credentials

In Dockerfile https://github.com/apostrophecms/apostrophe-boilerplate/pull/18#discussion_r363778900 :

EXPOSE 3000 -CMD [ "npm", "start" ] +CMD [ "./scripts/wait-for-it.sh", "mongo:27017", "--", "npm", "start" ]

Good idea. I used the same mecanism (with a npm package dedicated for this task) for my own dockerized express apps

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe-boilerplate/pull/18?email_source=notifications&email_token=AAAH27NTVD2A4U5VBAIJ5H3Q4SIFNA5CNFSM4KDHIBXKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCQ4OZJI#pullrequestreview-339274917, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27MCHTNYGCYMD5UNLY3Q4SIFNANCNFSM4KDHIBXA .

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

[boutell]

Added Greg to the reviewers, would like his take before merging as he has more docker experience.

[gregvanbrug]

General question, is the purpose of this development or production?

[boutell]

Development, and small scale production ("I like docker and my budget calls for a digital ocean droplet, not kubernetes")

On Tue, Jan 7, 2020 at 10:45 AM Greg van Brug notifications@github.com wrote:

General question, is the purpose of this development or production?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe-boilerplate/pull/18?email_source=notifications&email_token=AAAH27MPVYTPQEDYSYG7KBTQ4SPRRA5CNFSM4KDHIBXKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIJJIJY#issuecomment-571642919, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27MUXOWFFWYVGHFYYG3Q4SPRRANCNFSM4KDHIBXA .

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

[boutell]

Greg, really good points here. Hmm.

The most important, to me, is figuring out what we're doing in dev and what we're doing in prod. Certainly we want development and testing within Docker to be easy. It could be the best answer to our Windows problems. (Maybe. I'm not sure what happens when a Docker shared volume wants to symlink something.)

You suggest that maybe in dev "lib/" should be a shared volume (right?) but I don't see that in the Dockerfile of a major client whose developers all use Docker for development. They are just copying the whole app in with COPY. I wonder how that works for testing of code changes. I'll ask.

Whereas in production, I'm wondering what the simplest practical update procedure is to wrap around the Dockerfile. We've addressed persistence here, but it's not clear how exactly you would deploy an update.

On Tue, Jan 7, 2020 at 2:34 PM Greg van Brug notifications@github.com wrote:

@gregvanbrug commented on this pull request.

In docker-compose.yml https://github.com/apostrophecms/apostrophe-boilerplate/pull/18#discussion_r363907320 :

@@ -0,0 +1,24 @@ +version: "2"

Should we be using version 3? That's the most current compose file version.

In docker-compose.yml https://github.com/apostrophecms/apostrophe-boilerplate/pull/18#discussion_r363907761 :

@@ -0,0 +1,24 @@ +version: "2" +services:

• apostrophe:
• container_name: apostrophe
• restart: always
• build: .
• ports:
• • "3000:3000"
• links:
• • mongo
• volumes:
• • ./data/uploads:/app/public/uploads

Wonder if we should add a note in the readme to also add the lib folder if you're using this for development. Otherwise, you'd have to restart the container on every change.

In docker-compose.yml https://github.com/apostrophecms/apostrophe-boilerplate/pull/18#discussion_r363908319 :

• container_name: apostrophe
• restart: always
• build: .
• ports:
• • "3000:3000"
• links:
• • mongo
• volumes:
• • ./data/uploads:/app/public/uploads
• environment:
• • APOS_MONGODB_URI=mongodb://mongo:27017/db
• depends_on:
• • mongo
• mongo:
• container_name: mongo
• image: mongo

Should we lock down the version number?

In Dockerfile https://github.com/apostrophecms/apostrophe-boilerplate/pull/18#discussion_r363908624 :

@@ -1,16 +1,7 @@ -FROM node:carbon

-# Create app directory -RUN mkdir -p /app +FROM node:current

Should we lock down the version number? I think :current defaults to the most recent node image.

In Dockerfile https://github.com/apostrophecms/apostrophe-boilerplate/pull/18#discussion_r363911224 :

RUN npm install

-# Mount persistent storage -VOLUME /app/data -VOLUME /app/public/uploads

+COPY . /app

Wondering if this would potentially overwrite what gets installed in the above RUN if the host machine has already npm installed.

In Dockerfile https://github.com/apostrophecms/apostrophe-boilerplate/pull/18#discussion_r363913672 :

EXPOSE 3000 -CMD [ "npm", "start" ] +CMD [ "./scripts/wait-for-it.sh", "mongo:27017", "--", "npm", "start" ]

If you wanted to get super fancy, you probably could even curl wait-for-it so we don't need to include a copy of it in our repo.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/apostrophecms/apostrophe-boilerplate/pull/18?email_source=notifications&email_token=AAAH27PDIEMJFYX2UMAFXY3Q4TKLRA5CNFSM4KDHIBXKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCQ5ZJWA#pullrequestreview-339449048, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAH27KJDB7SBW3IWUKF6G3Q4TKLRANCNFSM4KDHIBXA .

--

THOMAS BOUTELL | CHIEF TECHNOLOGY OFFICER APOSTROPHECMS | apostrophecms.com | he/him/his

[boutell]

This is a lot better than the Dockerfile we had before, yeah? Maybe we should merge this, describe it as an example of what is needed in the README rather than describing it as the one true way, and open tickets about improving clarity around best practices for dev and production dockerfiles?

[falkodev]

I agree

[boutell]

Cool, I am going to merge it but update the README and add a few comments clarifying this isn't the perfect dockerfile for all seasons.