Fix docker setup by roschaefer · Pull Request #233 · Human-Connection/WebApp

roschaefer · 2018-08-14T23:47:58Z

x

roschaefer · 2018-08-16T15:55:40Z

Ok, so there is a slight hack here. The docker image must have the right .env at build time. So, I added all environment variables which are public to .env.staging and .env.example respectively. We still need .env.production. During build, depending on build argument ENV_FILE, the desired config file is moved into place. All secret environment variables can be passed as environment variables at runtime and are not be exposed to the client. As long as we have no secrets that need to be available at build time, this approach should work.

appinteractive · 2018-08-16T20:57:46Z

So you say that we are not able anymore to deploy that container outside of the fixed environments we specify on build? And we are not able to specify vars (which would be „variable“) at runtime anymore?

That point is critical as we have to be able to deploy fast with changes settings without waiting for a whole build every time.

The standard behavior for docker images should be always, build once, deploy everywhere. We really need to keep that workflow intact.

appinteractive

error Found incompatible module```

trying to fix it...

appinteractive · 2018-08-15T10:13:48Z

Dockerfile

+WORKDIR /WebApp/
+# --no-cache: download package index on-the-fly, no need to cleanup afterwards
+# --virtual: bundle packages, remove whole bundle at once, when done
+RUN apk --no-cache --virtual build-dependencies add git python make g++


@roschaefer What about security updates? Dont we need them?

Also why do you need phython, make and g++?

I was having troubles to build other dependencies. Surprisingly. I haven't researched the issue but as far as I remember that was related to the newer node version of this docker container. 🤔

with node:8-alpine it does work, with node:alpine not

appinteractive · 2018-08-15T10:14:57Z

Dockerfile

@@ -1,46 +1,32 @@
-FROM node:8.9-alpine
+FROM node:alpine


Are you sure that we should go with the cutting edge versions? Not at least use LTS or similar?

appinteractive · 2018-08-15T10:17:20Z

Dockerfile

-ENV WEBAPP_HOST=0.0.0.0
+# optional git commit hash
+ARG BUILD_COMMIT
+ENV BUILD_COMMIT=$BUILD_COMMIT


Changing parts should always go lower to increase cache efficienty at build time.

appinteractive · 2018-08-15T10:24:53Z

Dockerfile

+# provide defaults
+COPY .env.template /WebApp/.env
+RUN yarn run build
+CMD ["pm2", "start", "node", "build/main.js", "-n", "frontend", "-i", "2", "--attach"]


maybe use yarn start:pm2?

appinteractive · 2018-08-15T10:35:38Z

plugins/api.js

    clear: () => {
      const res = app.$cookies.removeAll()
-      if (process.env.NODE_ENV === 'development') {
+      if (env.NODE_ENV === 'development') {


are you sure you will geht that from the nuxt env?

Yes, that's a nuxtjs feature.

Via process.env.baseUrl. Via context.env.baseUrl, see context API.

See https://nuxtjs.org/api/configuration-env/

Ah, you mean NODE_ENV in particular? Let me see..

appinteractive · 2018-08-15T10:37:59Z

plugins/raven-server.js

-  const secrets = dotenv.parse(await readFileSync(resolve('./server/.env-secrets')))
-
-  if (!process.client && secrets.SENTRY_DNS_PRIVATE) {
+  if (!process.client && process.env.SENTRY_DNS_PRIVATE) {


this approach would get you the variable at build time but not at runtime as nuxt edges them in there. Or do I miss something?

This file is executed on the server. I double checked at the time, that console.log(process.env.WHATEVER) spits out the WHATEVER variable server-side but not client-side.

If we had env variables that we need on the client, we would have to explicitly whitelist them.

appinteractive · 2018-08-15T10:39:13Z

nuxt.config.js

  modules: [
-    'cookie-universal-nuxt',
-    '@nuxtjs/dotenv'
+    ['@nuxtjs/dotenv', {


don't you not need to also overwrite the nuxt config like discribed here: https://github.com/nuxt-community/dotenv-module#using-env-file-in-nuxtconfigjs

? The config file name? I was playing with if statements in here based on the current process.env.NODE_ENV when nuxt reads in this file but that never fully worked as it should. So I went back to env-sub.

appinteractive · 2018-08-31T15:26:43Z

@roschaefer how was your procedure for testing if the env vars work correctly?

Done

appinteractive · 2018-08-31T20:00:53Z

Just as a reference after we had a talk together: the goal is to not build the application every time the docker image is started but only run it while at the same time providing current environemnt variables that might be passed to the docker command to the application (without rebuild)

roschaefer · 2018-08-31T21:43:01Z

@appinteractive changing the app via env variables should be possible now.

appinteractive · 2018-09-17T11:49:18Z

docker build -t humanconnection/frontend-nuxt .

Still getting the same issue:

Step 13/17 : RUN yarn install --production=false --frozen-lockfile --non-interactive
 ---> Running in 05485f872598
yarn install v0.23.2
[1/4] Resolving packages...
[2/4] Fetching packages...
error nuxt@1.4.0: The engine "node" is incompatible with this module. Expected version ">=8.0.0".
error Found incompatible module

Changing the minimum node version in the package.json does not help

"engines": {
    "node": ">= 10.0.0",
    "yarn": ">= 1.3.0"
  },

appinteractive · 2018-09-17T12:53:04Z

I fixed the issue by pinning the node version to the future Node 10.* LTS.

@roschaefer how about the env issues? Any updates there? Sounded like there are still not resolved!?

roschaefer · 2018-09-17T14:05:02Z

@appinteractive I didn't get any build issues at the time, my last commit also greened the build server. Glad if it is fixed on more than one machine now 😆 . This branch here is OK 👌 the env issues are a problem in #228 but not here.

appinteractive · 2018-09-17T14:30:52Z

@roschaefer I still could not test it as I can’t access the docker image via browser. Any suggestions how to test if the bars are really there?

- no bash scripts * error prone * platform dependent)

* .nuxt seems to be a build folder and should not be synced * load config into process.env for local development

... and remove redundancy in docker-compose files

With server side rendering it is simply not possible to run the webapp without a bridged network

The point of this docker-compose file is to give a running *local* environment. It is not complete up to this point. Eventually we could share this configuration with our staging system online. Also this commit passes secret environment variables from host system to container.

* Install nuxt-env * Replace env with app.$env when possible * Remove custom plugins/env.js

* add yarn run build to Dockerfile * remove envsub (now unnecessary) - all configurations are controlled via env variables passed from host to container * remove .env.template

@ionphractal

thanks @ionphractal

We will need that for the client in the future, anyways.

Lulalaby · 2018-09-22T14:27:36Z

@appinteractive & @roschaefer Ready for #233 ?

appinteractive · 2018-09-22T15:32:05Z

Can test it as soon as Monday. Staging is down until Monday too. Would like to first get it working to see if this merge brakes something.

Lulalaby · 2018-09-22T16:44:38Z

Okay

* less redundancy * different environments (local staging + local development)

See: Human-Connection/API#169 (comment)

roschaefer · 2018-10-01T11:09:32Z

.travis.yml

+# install current version of yarn
+before_install:
+  - curl -o- -L https://yarnpkg.com/install.sh | bash
+  - export PATH="$HOME/.yarn/bin:$PATH"


@appinteractive is this redundant?

Yes it can be removed. Was caused by the unversionated bode image before.

roschaefer requested review from appinteractive and visualjerk and removed request for appinteractive August 16, 2018 15:55

roschaefer force-pushed the fix-docker-setup branch 2 times, most recently from 7d14a67 to 09ed9c6 Compare August 16, 2018 16:17

roschaefer force-pushed the fix-docker-setup branch 3 times, most recently from 175170b to 8f790f3 Compare August 19, 2018 11:19

roschaefer mentioned this pull request Aug 19, 2018

[WIP] Replace config scripts #232

Closed

appinteractive previously requested changes Aug 31, 2018

View reviewed changes

appinteractive mentioned this pull request Sep 2, 2018

Multithreading via PM2 #214

Open

2 tasks

appinteractive removed the request for review from visualjerk September 17, 2018 09:53

roschaefer added 8 commits September 18, 2018 22:16

Complete rewrite of configuration

343a7a3

- no bash scripts * error prone * platform dependent)

Simplify Dockerfile and docker-compose.yml

159054f

Provide the BUILD_COMMIT on the build server

0fcf885

Update base image of Dockerfile

1da0906

Give image a name as on build server

8a5b3a1

Provide a staging docker-compose.yml for WebApp

ada1f5b

Various fixes:

3867418

* .nuxt seems to be a build folder and should not be synced * load config into process.env for local development

Prefix yarn scripts based on purpose in docker

7d4c13b

... and remove redundancy in docker-compose files

roschaefer added 4 commits September 18, 2018 22:16

Remove docker-compose.override.yml

2433ff3

With server side rendering it is simply not possible to run the webapp without a bridged network

Remove custom code to read process.env at runtime

964f232

* Install nuxt-env * Replace env with app.$env when possible * Remove custom plugins/env.js

Don't rebuild the app at runtime

b5058ed

* add yarn run build to Dockerfile * remove envsub (now unnecessary) - all configurations are controlled via env variables passed from host to container * remove .env.template

roschaefer force-pushed the fix-docker-setup branch from 12d35b3 to b5058ed Compare September 18, 2018 21:46

roschaefer added 4 commits September 19, 2018 19:01

Pin versions of dependencies for stability

03a77bb

Bugfix: Duplicate thumbor in docker-compose.yml

5570729

Remove obsolete step in Dockerfile

47c7e35

thanks @ionphractal

Whitelist BUILD_COMMIT, fix logout error

601a6f8

We will need that for the client in the future, anyways.

roschaefer force-pushed the fix-docker-setup branch 2 times, most recently from cbbe3b7 to 656a35e Compare September 19, 2018 17:47

roschaefer and others added 3 commits September 19, 2018 19:48

Bugfix: process.env in non-plugin raven-server

656a35e

fixing docker build

e69f6a3

Merge branch 'develop' into fix-docker-setup

26346e5

Lulalaby changed the title ~~Fix docker setup~~ [WIP] Fix docker setup Sep 22, 2018

Michael Both and others added 3 commits September 30, 2018 13:58

Use docker bridge for dev setup

b638dfa

Use global network of API docker-compose

703fcde

* less redundancy * different environments (local staging + local development)

Incorporate @ionphractal 's feedback thanks 👍

01054f4

See: Human-Connection/API#169 (comment)

roschaefer changed the title ~~[WIP] Fix docker setup~~ Fix docker setup Oct 1, 2018

roschaefer commented Oct 1, 2018

View reviewed changes

roschaefer force-pushed the fix-docker-setup branch from aeefc0a to 01054f4 Compare October 1, 2018 11:51

roschaefer merged commit 8053a24 into develop Oct 1, 2018

delete-merged-branch bot deleted the fix-docker-setup branch October 1, 2018 12:04

Conversation

roschaefer commented Aug 14, 2018 • edited by Lulalaby Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

roschaefer commented Aug 16, 2018

Uh oh!

appinteractive commented Aug 16, 2018

Uh oh!

appinteractive left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

appinteractive commented Aug 31, 2018

Uh oh!

appinteractive commented Aug 31, 2018

Uh oh!

roschaefer commented Aug 31, 2018

Uh oh!

appinteractive commented Sep 17, 2018

Uh oh!

appinteractive commented Sep 17, 2018

Uh oh!

roschaefer commented Sep 17, 2018

Uh oh!

appinteractive commented Sep 17, 2018

Uh oh!

Lulalaby commented Sep 22, 2018

Uh oh!

appinteractive commented Sep 22, 2018

Uh oh!

Lulalaby commented Sep 22, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

roschaefer commented Aug 14, 2018 •

edited by Lulalaby

Loading