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

feat: add dockerfile 🐳 #76

Merged
merged 5 commits into from
Aug 18, 2020
Merged

Conversation

aslafy-z
Copy link
Collaborator

@aslafy-z aslafy-z commented Oct 3, 2018

Add a Dockerfile in order to ease website development with fixed dependencies and tooling.

Two variants:

  • devswag:dev: development image, does not contain any code but all the tooling to work with it
  • devswag: production image, uses nginx to serve the built artifacts

More infos: https://github.com/aslafy-z/swag-for-dev/blob/add-dockerfile/CONTRIBUTING.md#lets-start-hacking

I just pushed the two variants to my docker-hub if anyone wants to try them without downloading the code (can't use dev images as it needs to be plugged to the code using volumes): https://hub.docker.com/r/zadki3l/devswag/tags/

TODO

  • You tell me

Copy link

@darshkpatel darshkpatel left a comment

Choose a reason for hiding this comment

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

Use ADD / VOLUME to get the website files from the folder into the container, also add a CMD line to start the container easily.

@aslafy-z
Copy link
Collaborator Author

aslafy-z commented Oct 3, 2018

No worries I'm working on it

@swapagarwal swapagarwal added the enhancement New feature or request label Oct 3, 2018
@darshkpatel
Copy link

Cool ! , I'd also suggest to have a dev command with nodemom so people can use the docker container while developing too.

@aslafy-z aslafy-z closed this Oct 8, 2018
@aslafy-z aslafy-z reopened this Oct 8, 2018
Copy link

@darshkpatel darshkpatel left a comment

Choose a reason for hiding this comment

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

LGTM, merge if the image builds.

@aslafy-z
Copy link
Collaborator Author

aslafy-z commented Oct 8, 2018

Notice I had to put the Dockerfile at the root of the repo, making it able to grab data.json for production release. As this project looks more and more like a piece of software than a list, we could maybe move all the content from site/ folder back to the root.

@darshkpatel
Copy link

@aslafy-z +1 on that

@plibither8
Copy link
Collaborator

@aslafy-z I think it's best if we create a new issue regarding moving all content from site/ to the root folder (as a suggestion). I'm in support of this, since the main content is the site now.

@aslafy-z
Copy link
Collaborator Author

aslafy-z commented Oct 8, 2018

Do you guys feel good if the dev image does not include any other files that package.json and package-lock.jon and requires a volume to work? Not the production one for sure.

@aslafy-z
Copy link
Collaborator Author

aslafy-z commented Oct 8, 2018

Would be great to merge this before fixing #98, I did some changes in the doc that would conflict with doc changes for #98.

@plibither8
Copy link
Collaborator

Noted!

site/README.md Outdated Show resolved Hide resolved
site/README.md Outdated Show resolved Hide resolved
@aslafy-z aslafy-z force-pushed the add-dockerfile branch 2 times, most recently from 93cacab to d5d1c44 Compare October 10, 2018 14:29
@aslafy-z
Copy link
Collaborator Author

aslafy-z commented Oct 10, 2018

I just rebased all the commits. It needs a review, especially on the doc side as I'm still learning english 😉.

@swapagarwal I'd be happy to add a step in the CI that builds the image, do you mind giving me (or the collaborator team, would love to be part of it though) some access to it? Thank you

@aslafy-z
Copy link
Collaborator Author

I still need to delete site/dist/.gitkeep file from releases, so we need to merge #93 before this one.

@aslafy-z
Copy link
Collaborator Author

I added a nginx.conf file configured the same as netlify.toml, it only misses SSL. Not sure if it has to stay here or maybe to a contrib directory. WDYT?

@aslafy-z aslafy-z changed the title WIP: Add Dockerfile Add Dockerfile Oct 11, 2018
This was referenced Oct 11, 2018
@aslafy-z
Copy link
Collaborator Author

@darshkpatel Could you please redo your review ?

@aslafy-z aslafy-z force-pushed the add-dockerfile branch 3 times, most recently from ae7427c to 51af4a4 Compare October 12, 2018 18:24
@swapagarwal
Copy link
Owner

@aslafy-z I missed your comment somehow. 😇
Added you as a collaborator. Welcome to the team! 🎉
Let me know which CI you need access to?

@vikaspotluri123 You have helped a lot in shaping the website. Sent you an invite too! 😉

package.json Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@vikaspotluri123
Copy link
Contributor

@aslafy-z You mixed port 8000 with port 8080, you need to normalize across the file. I was going to suggest changes, but there are lots of occurrances.

@aslafy-z
Copy link
Collaborator Author

@vikaspotluri123 I thought I changed it everywhere. Done now.

If you have permissions issues, try to drop the -u $(id -u):$(id -g).

@vikaspotluri123
Copy link
Contributor

If you have permissions issues, try to drop the -u $(id -u):$(id -g).

I'm going to trust you on this. Docker on Windows makes mapping volumes to an existing file very difficult, and my linux vm is on a mechanical drive.

@aslafy-z
Copy link
Collaborator Author

I think I found it! dist folder has to exist
see https://stackoverflow.com/questions/44574078/mount-non-existing-host-directory-into-non-root-container

I was able to reproduce using https://labs.play-with-docker.com/

# Change user to non-root

adduser -D devswag
chmod o+rw /var/run/docker.sock # ugly
su - devswag
# That has to be another copy/paste

git clone https://github.com/aslafy-z/swag-for-dev -b add-dockerfile
cd swag-for-dev/
docker build -t devswag:dev --target base .
mkdir dist # CREATE DIST FOLDER
docker run -it --rm \
  -v $(pwd)/data.json:/devswag/data.json \
  -v $(pwd)/src:/devswag/src \
  -v $(pwd)/gulpfile.js:/devswag/gulpfile.js \
  -v $(pwd)/get-data.js:/devswag/get-data.js \
  -v $(pwd)/dist:/devswag/dist \
  -u $(id -u):$(id -g) \
  -p 8080:8080 -p 35729:35729 devswag:dev

@swapagarwal
Copy link
Owner

I pulled the latest commit and docker run... serves the website at http://127.0.0.1:8080/ correctly (without running mkdir dist).

@aslafy-z aslafy-z mentioned this pull request Oct 23, 2019
2 tasks
@aslafy-z aslafy-z changed the title Add Dockerfile 🐳 feat: add dockerfile 🐳 May 9, 2020
@aslafy-z aslafy-z merged commit 7921715 into swapagarwal:master Aug 18, 2020
@aslafy-z aslafy-z deleted the add-dockerfile branch August 18, 2020 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants