Skip to content
This repository has been archived by the owner on Oct 9, 2019. It is now read-only.

improve docker images, fix jsep and initial build #344

Merged
merged 5 commits into from Jul 3, 2019
Merged

improve docker images, fix jsep and initial build #344

merged 5 commits into from Jul 3, 2019

Conversation

wodka
Copy link
Contributor

@wodka wodka commented Jul 1, 2019

Description

improve docker handling to be able to just build the image and use it

Motivation and Context

it was not possible to use the docker file right away -> move to current node image with alpine to reduce size

lock the jsep dependency to 0.3.1 because later versions have a missing file for the entry point. (causing the jsep missing error)

also do not stop process if mailing information is not configured properly (enables easier testing)

if PORT and SOCKET_PORT is equal attach the socket.io server to the same domain

fix admin user creation

How Has This Been Tested?

using it as an image for our tellform instance in aws

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@Leopere
Copy link
Collaborator

Leopere commented Jul 2, 2019

First of all TYVM @wodka for the PR submission it was really breaking my heart that we couldn't make any progress here. All of the documentation is severely out of sync with the project itself.

Secondly, will this PR make a runnable copy of Tellform in the current state?

Thirdly also do not stop the process if mailing information is not configured properly (enables easier testing) makes me very happy because it was nearly impossible getting a reasonable system using Mailslurper or alternatives since this thing does not support plain text auth with a pseudo server.

We could potentially have a docker-compose file option which would use a Mailslurper or some variant. https://github.com/Leopere/docker-mailslurper

My end goal is to not do any TLS stuff ourselves I just want Traefik on the front end as our supported option if we're going docker we should embrace things that also embrace docker and not dated concepts that don't work by default. Works by default is how userspace apps ship there are no acceptable reasons why server stuff needs to ship any differently.

Finally, this app has many vulnerable dependencies unfortunately so I wouldn't entirely recommend using it in proper production until we can get them updated so please reach out to me on the Gitter via DM's if you're willing or interested in contributing further I'll accept your PR's.

@Leopere
Copy link
Collaborator

Leopere commented Jul 3, 2019

@Leopere
Copy link
Collaborator

Leopere commented Jul 3, 2019

Oh or join https://discord.gg/DEsPSVU

Copy link
Collaborator

@Leopere Leopere left a comment

Choose a reason for hiding this comment

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

Overall excellent PR I was very excited to see this kind of specific progress being made because it's important to get going with this first otherwise nobody has any idea where to go to get even the faintest baseline.

Minor changes requested and noted inline. I think that this PR should likely get the project functional again which is something that it's desperately needed for some time now.

The only other thing that I would LOVE to do is set default envvars in such a way that 99% of what can be configured is configured out of the box. However the order of precedence should be such that the Env-vars can be overridden by docker-compose.yml settings or in the docker run

docker-compose.yml Show resolved Hide resolved
docker-compose.yml Show resolved Hide resolved
docker-compose.yml Show resolved Hide resolved
docker-compose.yml Show resolved Hide resolved
docker-compose.yml Show resolved Hide resolved
server.js Show resolved Hide resolved
@wodka
Copy link
Contributor Author

wodka commented Jul 3, 2019

I could reduce the docker-compose.yml to the following and then remove the nginx config completly from the repo. Or this could be moved to a "how to deploy" section

version: "3"
services:
  redis:
    image: redis
  mongo:
    image: mongo
    volumes: 
      - "./data/mongo:/data"
  tellform:
    build:
      context: .
    environment:
      CREATE_ADMIN: "TRUE"
      SOCKET_PORT: "5000"
      SOCKET_PORT_EXTERN_VISIBLE: "TRUE"
      MONGODB_URI: mongodb://mongo/tellform
      REDIS_URL: redis://redis
      MAILER_SMTP_HOST: mail
      MAILER_SMTP_PORT: 1025
    volumes:
      - .:/opt/tellform
    links:
      - mongo
      - redis
    ports:
      - "5000:5000"
    depends_on:
      - mongo
      - redis
  mail:
    image: mailhog/mailhog
    ports:
      - "5050:8025"

this would then only spin up 3 containers (redis / mongo / tellform) and serve the websocket from the same host as well)

If the volume is is mounted it is still possible to use the container to install everything - just open a shell with docker-compose run tellform ash and then execute npm install. If playing around with the code always do a docker-compose restart tellform

then accessing localhost:5000 would be a working instance with admin root:root

accessing localhost:5050 would show the mailhog interface with all received mails

@wodka
Copy link
Contributor Author

wodka commented Jul 3, 2019

One thing that does not seem to work but I don't know if this was working before: the response count in the admin is not updating and always staying at 0 - this is something to work on separately I guess since the responses are logged.

@Leopere
Copy link
Collaborator

Leopere commented Jul 3, 2019

Fixes #342

@wodka
Copy link
Contributor Author

wodka commented Jul 3, 2019

@Leopere what do you think about the compose file in #344 (comment) ?

@Leopere
Copy link
Collaborator

Leopere commented Jul 3, 2019

If anything since web stuff frequently can't seem to exist without needing weird configurator scripts we could add a "bootstrap" container that would come up with the container with a volume mount for docker.sock just to run the commands and then remove/stop itself but that sounds a bit insane. The other option is a sane start.sh that would add any configuration changes by means of EnvVars that are either in an env file, docker-compose.yml or docker run -e finally if they pass in an application specific configuration file that should override everything else.

@Leopere
Copy link
Collaborator

Leopere commented Jul 3, 2019

@Leopere what do you think about the compose file in #344 (comment) ?

I'm still concerned about

    volumes:
      - .:/opt/tellform

I wonder if it should be.

    volumes:
      - './:/opt/tellform'

@wodka
Copy link
Contributor Author

wodka commented Jul 3, 2019

I think we are now talking about different things^^

Case A)

Someone checks out the repo and wants to build everything from source

then we use the provided docker-compose file -> this will also require them to run npm install manually (part of the build process)

Case B)

Someone just wants to run tellform

then they should not check out the repository -> the Dockerfile in this pull request builds everything it needs so that the image can be configured just by setting env variables.

in regard to the volume option -> both work fine

@Leopere
Copy link
Collaborator

Leopere commented Jul 3, 2019

Case A)

Someone checks out the repo and wants to build everything from source

then we use the provided docker-compose file -> this will also require them to run npm install manually (part of the build process)

As for Case A, we should probably aim for a multi-stage Dockerfile so that the npm install is always done first no?

For Case B someone just wanting to run Tellform should be pulling from the Docker Hub but I don't have access to /u/Tellform

@wodka
Copy link
Contributor Author

wodka commented Jul 3, 2019

the npm install is done in the docker file. Only not when the source itself is mounted (which in my perspective should only be done if a developer is working on it)

It is never a good idea to load the dependencies and build at boot time of the container -> everything should be packed and then distributed through dockerhub (so only the env vars need to change)

@Leopere
Copy link
Collaborator

Leopere commented Jul 3, 2019

Until I get access to the Tellform user or organization I can't set up an official one there unless I do something like Tellform2, Tellform-Official. Until then I guess Case 2 might be less than ideal if pulled from a central repo.

@wodka
Copy link
Contributor Author

wodka commented Jul 3, 2019

Then even if one uses it to push it to their own registry:

git clone tellform
docker build -t my-own-tellform .
docker push my-own-tellform

will provide users with an image based on the current packages they can use either in kubernets or any other docker env :)

@Leopere
Copy link
Collaborator

Leopere commented Jul 3, 2019

A multistage Dockerfile wouldn't build every time it would prebuild once and then only run the second/final stage. So you'd build the one and then COPY from=build /path /to/path then run from the final stage container. Like this container https://github.com/lbryio/lbry-docker/blob/master/chainquery/Dockerfile-linux-x86_64-production

https://docs.docker.com/develop/develop-images/multistage-build/

@Leopere
Copy link
Collaborator

Leopere commented Jul 3, 2019

At this point, it looks like it builds but all I'm getting is
screnshot

@wodka
Copy link
Contributor Author

wodka commented Jul 3, 2019

the error looks like if you did use the "volume".

Does this happen when you use the docker-compose.yml:

version: "3"
services:
  redis:
    image: redis
  mongo:
    image: mongo
    volumes: 
      - "./data/mongo:/data"
  tellform:
    build:
      context: .
    environment:
      CREATE_ADMIN: "TRUE"
      SOCKET_PORT: "5000"
      SOCKET_PORT_EXTERN_VISIBLE: "TRUE"
      MONGODB_URI: mongodb://mongo/tellform
      REDIS_URL: redis://redis
      MAILER_SMTP_HOST: mail
      MAILER_SMTP_PORT: 1025
    links:
      - mongo
      - redis
    ports:
      - "5000:5000"
    depends_on:
      - mongo
      - redis
  mail:
    image: mailhog/mailhog
    ports:
      - "5050:8025"

I personally avoid multistage docker files - especially if they are not required

@Leopere
Copy link
Collaborator

Leopere commented Jul 3, 2019

image
image
This is with your latest compose.

@wodka
Copy link
Contributor Author

wodka commented Jul 3, 2019

:( my mistake - I missed the bower install in the Docker build and it worked because I already had it locally installed

@Leopere
Copy link
Collaborator

Leopere commented Jul 3, 2019

Builds and runs.

@Leopere Leopere merged commit 210c754 into tellform:master Jul 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants