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

Dockerized version of the application #66

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Dockerized version of the application #66

wants to merge 4 commits into from

Conversation

misterhtmlcss
Copy link

Purpose:

Reduce tech support for when students launch this application inside of Vagrant/host system.

Updates:

  • Removed body-parser
  • Updated everything to the latest version.

Gaps:

  • Would need to add Docker to Vagrant or to student host systems.
  • Student will need to use Heroku Container service to deploy

Suggestions/Next steps

  • Add ability to deploy via npm start. This can be done with an ENV, but I didn't do it.
  • Write documents for Heroku Container deploy
  • Possibly add a Cypress test suite for this project.

@misterhtmlcss
Copy link
Author

@kvirani can you take a look at this and weigh in on whether this should be closed or merged?

@kvirani
Copy link
Member

kvirani commented Oct 23, 2020

@FrancisBourgouin would love your input into this given that you're working on new vagrant and WSL2 images.

@misterhtmlcss is there a lot of startup issues with this app? Why would it not be launchable with the traditional non-containerized approach?

@misterhtmlcss
Copy link
Author

misterhtmlcss commented Oct 23, 2020

Hi @kvirani, I don't know what else is going on, so it's hard to say with absolute certainty that my approach is better, but I'll layout my thoughts. Also I'll pull @geeklady2 (Shannon) into this too as she has her own separate experiences from my own with Docker.

Straight up, I have had issues with every project and in some cases it's been a consistent and growing problem.

I hope you don't mind itemized form; just makes it easier for me to express my thoughts.

General Reasons for Docker and why I did this test project as a showcase:

  1. No setup issues. They are all gone. Students use docker-compose up and it'll work.
  2. Projects have different versions of just about every piece of software. Docker is perfect for varying project configurations. Whatever version is best for that project can stay that way until the time is available to upgrade it. Then that project can be upgraded independently without concern for broader versioning issues. IMO Vagrant is not the best platform anymore for our use case any more.
  3. Docker containers are small and fast compared to Vagrant.
  4. Docker containers are more common, easier to configure and work with than Vagrant AND less scary too.
  5. Docker is the industry standard for:
    • Project setup
    • CI/CD Pipelines
    • E2E Testing
    • More and better deployment options
    • Native deploys to Heroku, AWS, Azure, etc. They all have a container system. This is a very in-demand and valuable skill
  6. Platform incompatibility. WSL2 is awesome, but it would be even better if we used Docker inside WSL2, then we could support Mac, Linux and WSL users are all the same.
  7. This is a great skill set that will differentiate our students in an area that is rapidly growing both in jobs and as a portion of the value pie in IT/Software Dev.

@emilecantin
Copy link

emilecantin commented Oct 23, 2020

I'd just like to weigh in here with my 2 cents: From my own corner of the industry, it seems like Vagrant never really "caught on" that much. It seemed to have been mostly popular with the Ruby folks, but outside that I haven't seen it much. I have had exactly 0 clients using it.

By contrast, Docker is massively popular, and seems to hover at around 50% of my clients either using it or thinking about using it. I think Docker skills would be much more valuable to our students than Vagrant skills. I'd argue for dropping Vagrant in the first few weeks & going with natively-installed node via nvm, up until Tweeter, where I'd introduce Docker. Jungle could probably still be distributed as a Vagrant project (the concept would hopefully be easily understood by then).

Regarding actual consumption of a Docker-based project, it's indeed just cloning a repo, maybe setting up environment variables (for API keys & other secrets if applicable) and running docker-compose up -d. It rarely fails.

Of course, this is my own personal experience, from my own personal corner of the industry (React & node Web dev).



COPY package*.json ./
RUN npm install --silent

Choose a reason for hiding this comment

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

I'd run npm ci here instead; it'll install exactly the versions used in package-lock.json.

Copy link
Author

Choose a reason for hiding this comment

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

That's a great point Emile.

COPY package*.json ./
RUN npm install --silent

COPY . ./

Choose a reason for hiding this comment

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

The way I usually do this step is by individually copying the relevant top-level files or directories. In this case, it'll probably cause problems if the student did an "npm install" before building the image.

Copy link
Author

Choose a reason for hiding this comment

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

Can you give me a more specific suggestion?

Choose a reason for hiding this comment

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

I think we only need public and server in this case, so I'd do:

COPY ./public ./public
COPY ./server ./server

- "3000:3000"
volumes:
- .:/src/app
- /src/app/node_modules

Choose a reason for hiding this comment

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

Not sure why you make node_modules a volume?

Copy link
Author

Choose a reason for hiding this comment

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

Like all things weird I had a problem once and it was a while ago and it fixed it. Sometimes when I copy and paste across most of my existing processes I forget to delete something. Thanks for pointing this out.

"docker:init": "docker-compose up --build --force-recreate",
"docker:start": "docker-compose start",
"docker:halt": "docker-compose stop",
"docker:destroy": "docker-compose down -v"

Choose a reason for hiding this comment

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

I'd probably just add these commands to the readme, so the students get familiar with them. npm run docker:halt" is arguably harder to remember than docker-compose stop`.

Copy link
Author

Choose a reason for hiding this comment

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

I actually put halt because it's the word they are familiar with from Vagrant. I use stop, but I'm indifferent on this point.

Choose a reason for hiding this comment

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

Good point; I think it's a good idea for the transition, but if we go all-in on Docker, it'll be better to use the Docker terms from the start.

dockerfile: Dockerfile
ports:
- "5000:5000"
- "3000:3000"

Choose a reason for hiding this comment

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

Why port 3000?

Copy link
Author

Choose a reason for hiding this comment

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

Quoted from above: Like all things weird I had a problem once and it was a while ago and it fixed it. Sometimes when I copy and paste across most of my existing processes I forget to delete something. Thanks for pointing this out.

Also I just wrote this quickly. I never thought it would go to production as is, so no judging please ;)
Consider this just a quick and dirty draft that I put out there and I'm happy to update it and make it final after I hear there is appetite for this kind of change to our way of doing things.

Choose a reason for hiding this comment

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

No problem! That's why we do code reviews ;)

@geeklady2
Copy link

I'm on board with all of these comments.

I've never encountered vagrant in the workplace, but then I'm not a Ruby developer. However, I have encountered docker a fair amount and have had no issues with getting it going on Windows, linux, or Mac. It is quite likely students will encounter docker in their career.

I have literally spent hours diagnosing system issues, which from what I can see is due to incompatibilities in libraries between projects and expecting mentors to know all of this when working in silos and in some cases only 3 hours a week is in my opinion asking too much. Docker is one solution to this issue, another would be to provide to each mentor a list of common issues for each project and the solution (this will need to be updated regularly.

The docker solution would allow the project to be out-dated, or use the latest tools without causing issues with the other projects. LHL can keep a docker registry (https://docs.docker.com/registry/) which you can think of as version control for docker. If the latest updates don't work, students can pull the previous docker version.

There are issues with keeping software up-to-date in the docker containers and application secure and safe, but in this case that's a low priority.

@misterhtmlcss
Copy link
Author

Also based on the slack convo, I also think that @jensen would like to probably have a iron in this fire. Jensen I leave it with you to voice as wish. I won't prod you further :)

@jensen
Copy link
Contributor

jensen commented Oct 23, 2020

Thanks for the invite, although I think Docker is a great tool which I use for my own projects, I don't believe it will solve our problems here. I'll leave it up to you to discuss and change if you wish.

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.

5 participants