wefork / wekan

The open-source Trello-like kanban (built with Meteor)
https://wekan.io
MIT License
61 stars 12 forks source link

Docker script does not default arguments correctly #90

Closed DMCShep closed 7 years ago

DMCShep commented 7 years ago

There seems to be a problem with the default configuration of the docker scripts in the repo. I am using the following files (appended with ".txt" manually just for attaching here):

docker-compose.yml Dockerfile

The only change I've made thus far is to change the port line from 80:80 to 8081:8081 for ease of Nginx configuration. When I run sudo docker-compose up -d with or without my change, it produces this output:

Wefork_Wekan.txt

The error starts here:

--2017-01-27 22:24:25--  https://nodejs.org/dist//node--.tar.gz
Resolving nodejs.org (nodejs.org)... 104.20.22.46, 104.20.23.46, 2400:cb00:2048:1::6814:172e, ...
Connecting to nodejs.org (nodejs.org)|104.20.22.46|:443... connected.
HTTP request sent, awaiting response... 404 Not Found
2017-01-27 22:24:26 ERROR 404: Not Found.

The commands from the Dockerfile that result in this seem to be:

    # Download nodejs
    wget https://nodejs.org/dist/${NODE_VERSION}/node-${NODE_VERSION}-${ARCHITECTURE}.tar.gz && \
    wget https://nodejs.org/dist/${NODE_VERSION}/SHASUMS256.txt.asc && \

Both ${NODE_VERSION} and ${ARCHITECTURE} fill in with an empty string in those commands.

To help diagnose the issue, I temporarily altered the top of the Dockerfile to look like this:

FROM debian:wheezy
MAINTAINER wefork

ENV BUILD_DEPS="wget curl bzip2 build-essential python git ca-certificates"
ENV GOSU_VERSION=1.10
ARG NODE_VERSION=v0.10.48
ARG METEOR_RELEASE=1.3.5.1
ARG NPM_VERSION=3.10.10
ARG ARCHITECTURE=linux-x64
ARG SRC_PATH=./

# Print args for debugging
RUN echo BUILD_DEPS is [${BUILD_DEPS}]
RUN echo GOSU_VERSION is [${GOSU_VERSION}]
RUN echo NODE_VERSION is [${NODE_VERSION}]
RUN echo METEOR_RELEASE is [${METEOR_RELEASE}]
RUN echo NPM_VERSION is [${NPM_VERSION}]
RUN echo ARCHITECTURE is [${ARCHITECTURE}]
RUN echo SRC_PATH is [${SRC_PATH}]

# Copy the app to the image
COPY ${SRC_PATH} /home/wekan/app

Then the result to the terminal is this: Wefork_Wekan_Debug.txt

Result of my debug lines:

BUILD_DEPS is [wget curl bzip2 build-essential python git ca-certificates]
GOSU_VERSION is [1.10]
NODE_VERSION is []
METEOR_RELEASE is []
NPM_VERSION is []
ARCHITECTURE is []
SRC_PATH is []

So it seems anything specified as ARG is getting defaulted to a blank string, rather than the specified default.

stephenmoloney commented 7 years ago

@DMCShep

What versions of docker-compose and docker are your running?

Aeny202 commented 7 years ago

Running into the same issue

root@KS-Docker-01:~/wekan# /usr/local/bin/docker-compose -v
docker-compose version 1.8.1, build 878cff1
root@KS-Docker-01:~/wekan# docker -v
Docker version 1.13.0, build 49bf474
root@KS-Docker-01:~/wekan# uname -a
Linux KS-Docker-01 4.8.0-34-generic #36-Ubuntu 

Changing:

ENV BUILD_DEPS="wget curl bzip2 build-essential python git ca-certificates"
ENV GOSU_VERSION=1.10
ARG NODE_VERSION=v0.10.48
ARG METEOR_RELEASE=1.3.5.1
ARG NPM_VERSION=3.10.10
ARG ARCHITECTURE=linux-x64
ARG SRC_PATH=./

To:

ENV BUILD_DEPS="wget curl bzip2 build-essential python git ca-certificates"
ENV GOSU_VERSION=1.10
ENV NODE_VERSION=v0.10.48
ENV METEOR_RELEASE=1.3.5.1
ENV NPM_VERSION=3.10.10
ENV ARCHITECTURE=linux-x64
ENV SRC_PATH=./

Got me a little further in the process only the fail when installing Meteor with a bunch these errors:

tar: .meteor/packages/babel-compiler: Directory renamed before its status could be extracted

Either my workaround is wrong or there's more fixing needed before this is going to build.

stephenmoloney commented 7 years ago

@Aeny202 ARG should work just as well as ENV with docker so I don't think this is an issue with the Dockerfile.

In terms of the other error, can you specifiy your system architecture?

echo -n $(uname -s | tr '[:upper:]' '[:lower:]'); echo -n "-"; echo $(uname -p | tr '[:upper:]' '[:lower:]');

or just

uname -a and include all the info.

ghost commented 7 years ago

I experience the same error

tar: .meteor/packages/babel-compiler: Directory renamed before its status could be extracted

I am using Windows 10

$ uname -a
MSYS_NT-10.0 mycomputer 2.6.1(0.306/5/3) 2017-01-14 09:41 x86_64 Msys
$ docker -v                                                              
Docker version 1.13.0, build 49bf474                                        
$ docker-compose -v                                                      
docker-compose version 1.10.0, build 4bd6f1a0                            

Possibly related: https://github.com/docker/hub-feedback/issues/727 https://github.com/docker/docker/issues/19647

stephenmoloney commented 7 years ago

@centigrade-thomas-becker That was some very useful information there. Thanks! The error seems to be described well at this link https://github.com/docker/docker/issues/19647#issuecomment-256794331. I'd be inclined to say that this is an issue relatively unrelated to wekan and there seems to be someone working on a fix.

ghost commented 7 years ago

Since the problem seems to be related to the overlay storage driver, I configured my docker daemon to use another storage.

Initially it was using overlay2. When I switched to overlay to aufs the tar-related error disappeared, and I got one step further. The different storage drivers used by Docker are documented here.

But now I have another error:

Downloading Meteor distribution

Meteor 1.3.5.1 has been installed in your home directory (~/.meteor).

Now you need to do one of the following:

  (1) Add "$HOME/.meteor" to your path, or
  (2) Run this command as root:
        cp "/home/wekan/.meteor/packages/meteor-tool/1.3.5_1/mt-os.linux.x86_64/scripts/admin/launch-meteor" /usr/bin/meteor

Then to get started, take a look at 'meteor --help' or see the docs at
docs.meteor.com.
npm ERR! Linux 4.9.4-moby
npm ERR! argv "node" "/home/wekan/.meteor/packages/meteor-tool/.1.3.5_1.1sksnpz++os.linux.x86_64+web.browser+web.cordova/mt-os.linux.x86_64/dev_bundle/bin/npm" "install" "--save" "xss"
npm ERR! node v0.10.46
npm ERR! npm  v3.10.5
npm ERR! path /home/wekan/app/node_modules/xss
npm ERR! code EXDEV
npm ERR! errno 52

npm ERR! EXDEV, rename '/home/wekan/app/node_modules/xss'
npm ERR!
npm ERR! If you need help, you may report this error at:
npm ERR!     <https://github.com/npm/npm/issues>

npm ERR! Please include the following file with any support request:
npm ERR!     /home/wekan/app/npm-debug.log

The cause of the "EXDEV rename" error seems to be that both aufs and overlay don't support rename operations:

stephenmoloney commented 7 years ago

@centigrade-thomas-becker

So my understanding is

Do you mind sending me a link to the steps needed to change docker to use aufs storage?

I'll see later about resolving this once I get the bug on my own system.

Thanks.

ghost commented 7 years ago

In order to select a different storage driver you have to edit the config file for the docker daemon, and then restart the daemon.

I am using Windows 10 so I just used the docker icon in the system tray (Settings => Daemon => Advanced) to set my driver to:

{
  "storage-driver": "aufs"
}

You can check your current storage driver with docker info

Also, take note that once you change your storage driver all your current containers and images won't be available anymore. At least not until you switch back to your previous storage driver.

DMCShep commented 7 years ago

Chiming in: I'm in the process of setting up a new server for testing the install process. The one I used before was shared with some other sites and ran into issues with insufficient resources as a result. It was the latest version of docker and docker-compose when I ran into the issue above.

That said, It sounds like you guys are well on your way to working out the kinks on your own, even if it's a docker bug and not a wekan bug. Keeping tabs on this issue.

If it helps, I was running Ubuntu 16.04 or 16.10 before (don't recall which), but will test with 16.10 when server setup is done. Taking a while because I'm generating a "baseline" image to revert back to after each test to make a sort of "clean slate".

ghost commented 7 years ago

I finally got it working.

First, I tried to switch from npm to yarn to avoid the EXDEV error during the step gosu wekan /home/wekan/.meteor/meteor npm install --save xss && \. But yarn as a meteor command is not supported in meteor 1.3, and I did not want to open that can of worms.

Once wekan supports meteor 1.4 we should be able to use yarn as described here.

So I ended up removing the line gosu wekan /home/wekan/.meteor/meteor npm install --save xss && \ altogether, and it seems that Wekan works fine even without it. Since a security feature is now missing (protection against cross site scripting) this is obviously not a viable solution. But at least it is running now.

stephenmoloney commented 7 years ago

Ok, great work @centigrade-thomas-becker. Useful information there.

In that case, my suggestion for the Dockerfile then is that we wait until wekan is updated to > meteor 1.4.1.3 and then a PR can be made to switch to yarn.

I would think upgrading wekan to meteor 1.4 is a high priorty, @xet7 ?

xet7 commented 7 years ago

@stephenmoloney

Is this default arguments issue still a problem? When I yesterday merged Wefork to Wekan, Docker Hub did build working docker image that has still gosu commands in it: https://hub.docker.com/r/mquandalle/wekan/tags/ https://hub.docker.com/r/mquandalle/wekan/~/dockerfile/

I did run Wekan with:

docker run -d --restart=always --name wekan-db mongo:3.2.11

docker run -d --restart=always --name wekan --link "wekan-db:db" -e "MONGO_URL=mongodb://db" -e "ROOT_URL=http://localhost:8080" -p 8080:80 mquandalle/wekan:latest

Sorry maybe I just didn't read all messages in this post more carefully what's the exact problem. Please write me a short TL;DR version.

Anyway I still have pull requests to integrate and more stuff to move from Wefork to Wekan (issues, wiki etc) so I don't know how high priority is upgrading yet, I'm trying to keep up with speed that Wekan community is moving.

stephenmoloney commented 7 years ago

RE: Is this default arguments issue still a problem?

In short, my conclusion is that once wekan is upgraded to version of meteor 1.4 +, that we move from npm to yarn to avoid some of these issues. I will create a new issue for this now.

Perhaps, we can close this issue now, the discussion has moved off the title anyways. @xet7

DMCShep commented 7 years ago

Closing issue for reasons described above

stephenmoloney commented 7 years ago

@xet7 I've made a few changes in my directory and I'm actually noticing this issue brought up by @DMCShep now which I couldn't replicate before this. So Yes, in fact, I think @DMCShep is right about the ARG being a problem.

The good news is I've come up with a fix, I'll make a pull request to wekan/wekan for it soon.