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

Feature : docker-run.sh script + docker container build #1844

Merged
merged 21 commits into from Mar 18, 2019

Conversation

Projects
None yet
4 participants
@PicoCreator
Copy link
Contributor

PicoCreator commented Feb 22, 2019

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Documentation Update

Description

A single bash script that helps quickly setup either a DEV or DEMO environment

bash-3.2$ ./docker-run.sh 
#---
#
# This script will perform the following steps ... 
#
# 1) Stop and remove any docker container with the name 'dev-to-postgres' and 'dev-to'
# 2) Reset any storage directories if RUN_MODE starts with 'RESET-'
# 3) Build the dev.to docker image, with the name of 'dev-to:dev' or 'dev-to:demo'
# 4) Deploy the postgres container, mounting '_docker-storage/postgres' with the name 'dev-to-postgres'
# 5) Deploy the dev-to container, with the name of 'dev-to-app', and sets up its port to 3000
#
# To run this script properly, execute with the following (inside the dev.to repository folder)...
# './docker-run.sh [RUN_MODE] [Additional docker envrionment arguments]'
#
# Alternatively to run this script in 'interactive mode' simply run
# './docker-run.sh INTERACTIVE-DEMO'
#
#---
#---
#
# RUN_MODE can either be the following
#
# - 'DEV'  : Start up the container into bash, with a quick start guide
# - 'DEMO' : Start up the container, and run dev.to (requries ALGOLIA environment variables)
# - 'RESET-DEV'   : Resets postgresql and upload data directory for a clean deployment, before running as DEV mode
# - 'RESET-DEMO'  : Resets postgresql and upload data directory for a clean deployment, before running as DEMO mode
# - 'INTERACTIVE-DEMO' : Runs this script in 'interactive' mode to setup the 'DEMO'
#
# So for example to run a development container in bash its simply
# './docker-run.sh DEV'
#
# To run a simple demo, with some dummy data (replace <?> with the actual keys)
# './docker-run.sh DEMO -e ALGOLIASEARCH_APPLICATION_ID=<?> -e ALGOLIASEARCH_SEARCH_ONLY_KEY=<?> -e ALGOLIASEARCH_API_KEY=<?>'
#
# Finally to run a working demo, you will need to provide either...
# './docker-run.sh .... -e GITHUB_KEY=<?> -e GITHUB_SECRET=<?> -e GITHUB_TOKEN=<?>
#
# And / Or ...
# './docker-run.sh .... -e TWITTER_ACCESS_TOKEN=<?> -e TWITTER_ACCESS_TOKEN_SECRET=<?> -e TWITTER_KEY=<?> -e TWITTER_SECRET=<?>
#
# Note that all of this can also be configured via ENVIRONMENT variables prior to running the script
#
#---

And does the deployment using docker. Includes option to do a reset prior to deployment.

Optional contextual information provided here : https://dev.to/uilicious/adopt-your-own-devto----with-a-single-command-almost-1c04

Need advice on ...

if someone can guide me on how to run dev.to in "Production" mode, it would be great in improving the overall docker container performance

Added to documentation?

What gif best describes this PR

quick demo

What gif best describes how it makes you feel?

how i feel

@CLAassistant

This comment has been minimized.

Copy link

CLAassistant commented Feb 22, 2019

CLA assistant check
All committers have signed the CLA.

@benhalpern

This comment has been minimized.

Copy link
Collaborator

benhalpern commented Feb 25, 2019

if someone can guide me on how to run dev.to in "Production" mode, it would be great in improving the overall docker container performance

If I understand the question correctly, it would be by setting the RAILS_ENV variable to production.

Let me know if we can be helpful in pushing this through. @maestromac let's be on top of this.

@PicoCreator

This comment has been minimized.

Copy link
Contributor Author

PicoCreator commented Feb 26, 2019

Hmm, did that for the container, and got the following error dump...

rails aborted!
ArgumentError: The api_key parameter cannot be blank
/usr/local/bundle/gems/timber-2.6.2/lib/timber/log_devices/http.rb:71:in `initialize'
/usr/src/app/config/environments/production.rb:95:in `new'
/usr/src/app/config/environments/production.rb:95:in `block in <top (required)>'

Full error log : https://gist.github.com/PicoCreator/8c3c054f406cfd24d57ef461fc3cb5a4

I suspect there might be additional "minimum requirements" to be passed forward for production deployment (beyond algolia) - not too sure what does api_key in the following error dump means (my wild guess says S3)

If additional keys beyond algolia is needed for production deployment, this is probably more of a documentation issue.


#
# DB migration
# (@TODO - someone please confirm if i should use bin/rake or bin/rails for this step, and also if I can safely call htis on every startup)

This comment has been minimized.

Copy link
@maestromac

maestromac Feb 28, 2019

Collaborator
Suggested change
# (@TODO - someone please confirm if i should use bin/rake or bin/rails for this step, and also if I can safely call htis on every startup)

Lets us bin/rails for this step. It's not necessary to run this on every startup but it would mitigate unexpected pending migration error. Optionally, can this be baked in as an option for the user?

This comment has been minimized.

Copy link
@PicoCreator

PicoCreator Mar 1, 2019

Author Contributor

Added in DB_SETUP=true and DB_MIGRATE=true environment variables for the container.

uilicious@a64b0f3

I would prefer it to be on by default in "demo" mode or even "production" mode - to help mitigate pending migration error - gitlab does the same approach for example, and it helps making upgrades pain free.

@maestromac

This comment has been minimized.

Copy link
Collaborator

maestromac commented Feb 28, 2019

@PicoCreator that ArgumentError: The api_key parameter cannot be blank is from Timber, our logging tool.

@PicoCreator

This comment has been minimized.

Copy link
Contributor Author

PicoCreator commented Mar 1, 2019

@maestromac - added the env variable to disable timber and make it optional in seperate pull request : #1933

@PicoCreator PicoCreator force-pushed the uilicious:feature/docker-run-script branch from a64b0f3 to 2c2b503 Mar 8, 2019

@PicoCreator

This comment has been minimized.

Copy link
Contributor Author

PicoCreator commented Mar 11, 2019

Doing an update : after disabling timber in #1933

It seems like production requires the additional following overwrites, to skip certain production checks, which i manage to skip by setting up the following environment variables.

APP_PROTOCOL : ""
APP_DOMAIN: localhost:3000
SECRET_KEY_BASE: a60edc976c913b19fd9fc8118936fbe1df2b07f4eecc5ad32f975e33cd4ea36b150c1ce933b681b90874a46568041629003dcbfc07238f7dca91741bcd1ec870

Up next would be to disable Airbrake which now throws an error for missing project id

/usr/local/bundle/gems/airbrake-ruby-3.2.5/lib/airbrake-ruby.rb:242:in `configure': :project_id is required (Airbrake::Error)

Because it seems like "production mode" would need to slowly peel off the onion on the various configuration to support for it, with minor flags added to disable them. (like the timber flag)

@maestromac Does it make sense to move forward with merging DEV / DEMO mode first? Cleaning up anything that needs cleaning on it.

With production mode being done in a seperate merge request?

@PicoCreator PicoCreator changed the title [WIP] Feature : docker-run.sh script + docker container build Feature : docker-run.sh script + docker container build Mar 11, 2019

@pr-triage pr-triage bot added the PR: unreviewed label Mar 11, 2019

@maestromac

This comment has been minimized.

Copy link
Collaborator

maestromac commented Mar 11, 2019

@PicoCreator yeah that make sense. Let's do that.

@maestromac

This comment has been minimized.

Copy link
Collaborator

maestromac commented Mar 11, 2019

I am getting a lot of the errors below when running DEV mode. Any reason we can't install bare-bone node instead of using docker-node's Dockerfile?

image

@PicoCreator

This comment has been minimized.

Copy link
Contributor Author

PicoCreator commented Mar 12, 2019

Im guessing those messages refer to the docker build errors / warning for node.js? Which can be safely ignored.

The node.js setup steps are taken from the official nodejs alpine docker file : https://github.com/nodejs/docker-node/blob/master/8/alpine/Dockerfile without modification.

As both ruby & node.js are needed for this project, and i cant find an official alpine image containing both prebuilt

The options are to either

  • use the node.js base image, then install ruby
  • use the ruby base image, then install node.js (current dockerfile)

Either option is fine, could swap it around and see if there are less compilation "warning logs"

@PicoCreator

This comment has been minimized.

Copy link
Contributor Author

PicoCreator commented Mar 12, 2019

@maestromac : Flipped it around in latest commit - definitely much less error messages. And even if there are - most are compilation warnings, or what is very clearly just a warning.

@PicoCreator

This comment has been minimized.

Copy link
Contributor Author

PicoCreator commented Mar 18, 2019

@maestromac : poke

@maestromac

This comment has been minimized.

Copy link
Collaborator

maestromac commented Mar 18, 2019

Sorry about that and thanks for the ping. I got it working now but I'm thinking our prior implementation of

FROM ruby:$SOME_VERSION
RUN curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | apt-key add -
RUN curl -sL https://deb.nodesource.com/setup_8.x | bash
RUN echo "deb https://dl.yarnpkg.com/debian/ stable main" | tee /etc/apt/sources.list.d/yarn.list
RUN apt-get update -qq && apt-get install -y \
   nodejs \
   yarn \
   ...and so on 
 && apt-get -q clean \
 && rm -rf /var/lib/apt/lists

Is enough to get the app running but we can keep it this for now since it's working.

@maestromac
Copy link
Collaborator

maestromac left a comment

LGTM! Thank you for some amazing PR!

uilicious/dev.to
```

> PS : Someone from official dev.to team should create their own container namespace and update this segment after merger (if any)

This comment has been minimized.

Copy link
@maestromac

maestromac Mar 18, 2019

Collaborator

Yup I will do this

@maestromac maestromac merged commit 3104c48 into thepracticaldev:master Mar 18, 2019

5 checks passed

codeclimate All good!
Details
codeclimate/diff-coverage 100% (50% threshold)
Details
codeclimate/total-coverage 89% (0.0% change)
Details
deploy/netlify Deploy preview ready!
Details
license/cla Contributor License Agreement is signed.
Details

@pr-triage pr-triage bot added PR: merged and removed PR: unreviewed labels Mar 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.