-
Notifications
You must be signed in to change notification settings - Fork 53
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Summary: Add docker support. #47
base: master
Are you sure you want to change the base?
Conversation
Create a docker image. It checks out vircadia-web-sdk, compile and install for vircadia-web. Port 8080 is open for the service. This image can be used as a port of docker-compose at vircadia-domain-server-docker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good. This built and, after some tweaking, worked.
@@ -0,0 +1,62 @@ | |||
FROM ubuntu:20.04 as web-base | |||
|
|||
RUN apt update && apt install -y git curl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should use apt-get
instead of plain apt
to get rid of the warning "WARNING: apt does not have a stable CLI interface. Use with caution in scripts."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure!
@@ -0,0 +1,62 @@ | |||
FROM ubuntu:20.04 as web-base | |||
|
|||
RUN apt update && apt install -y git curl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RUN apt-get update && apt-get upgrade && apt-get install -y git curl
That is, shouldn't there be an "upgrade" after the "update"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah... it doesn't hurt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah... it doesn't hurt.
rm ./nodesource_setup.sh | ||
|
||
FROM web-base as web-sdk-builder | ||
RUN apt update |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this 'update' necessary since it was done in the base image?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah... drop it.
FROM web-base as web-sdk-builder | ||
RUN apt update | ||
# checkout repo | ||
RUN mkdir -p /usr/local/src && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since Docker layer caching might use a cached layer even if the v-web sources have changed, adding a ADD https://api.github.com/repos/vircadia/vircadia-web/git/refs/heads/master vircadia-web-git-version.json
before this RUN will break the cache if the sources have changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea!!! Add this line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding this line causes an error like following:
21 40.93 npm WARN deprecated uuid@3.4.0: Please upgrade to version 7 or higher. Older versions may use Math.random() in certain circumstances, which is known to be problematic. See https://v8.dev/blog/math-random for details.
#21 41.21 npm WARN deprecated coffee-script@1.12.7: CoffeeScript on NPM has moved to "coffeescript" (no hyphen)
#21 50.11 npm WARN deprecated querystring@0.2.0: The querystring API is considered Legacy. new code should use the URLSearchParams API instead.
#21 71.51 npm ERR! code ERR_SOCKET_TIMEOUT
#21 71.51 npm ERR! network Socket timeout
#21 71.51 npm ERR! network This is a problem related to network connectivity.
Any idea why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw those errors also but figured they were related to v-web-sdk configuration/versioning. Maybe need a revisit of the v-web-sdk build independent of the Docker image. Wonder what v-web-sdk has for package-lock and other requirements.
|
||
RUN apt update && apt install -y sudo vim | ||
RUN usermod -aG sudo ${USER} && \ | ||
echo '%sudo ALL=(ALL) NOPASSWD: ALL' >> /etc/sudoers && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure adding to sudo
. If this is a deployed version, the whole point of creating a user to run the app is so there run environment is somewhat safe. Maybe have some "debug build" option?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No it is only dev and debug. Will comment it out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would be the proper command to run the app?
I split build
web-base
| - ubuntu20.04
| - node 16, quasar, and vue
|- web-sdk-build => produces vircadia-web-sdk-2022.1.2.tgz
|- web-build => build app
|- web-run => run app
- create cadia user,
- copy /usr/local/src/vircadia-web/package*.json /home/cadia/vircadia-web
- copy /usr/local/src/vircadia-web/dist /home/cadia/vircadia-web/dist
- Run npm install --production
This will remove "sudo".
Can you recommend me a command line that runs the app?
RUN apt update && apt install -y git curl | ||
RUN curl -sL https://deb.nodesource.com/setup_14.x -o nodesource_setup.sh && \ | ||
bash ./nodesource_setup.sh && \ | ||
apt install nodejs && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This installs NodeJS v14. The SDK's package-lock.json
is version 2 which implies NodeJS v16. There are some warning because of this. Might need to use NVM or similar to get a different NodeJS version.
Also, v-web-sdk should probably have an "engine:" section added to its package.json
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering what would be the right. if so, let's update to 16.
|
||
DVERSION=latest | ||
|
||
docker run -it --rm \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I first ran the image, I got the error:
vircadia@nyxx:~/vircadia-web-mb/docker> ./run-docker-vircadia-web.sh
Log file: /home/cadia/vircadia-web/log/20220213.1741.log
/home/cadia/run-vircadia-web.sh: line 16: /home/cadia/vircadia-web/log/20220213.1741.log: Permission denied
vircadia@nyxx:~/vircadia-web-mb/docker>
which says that the mounted log directory didn't have the permissions to create files in the mounted directory.
This run file should check and/or change the permissions of the target log directory to allow the Docker account "cadia" to write into the mounted dir.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok. need to change how it runs the app. For now it requires sudo. I am going to drop 'sudo' and move app to /home/cadia. Let me do it.
-p 8080:8080 \ | ||
-p 8081:8081 \ | ||
-p 8082:8082 \ | ||
--volume ${BASE}/log:/home/cadia/vircadia-web/log \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be a "--detach" to run the container as a service in the background.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep.
-p 8080:8080 \ | ||
-p 8081:8081 \ | ||
-p 8082:8082 \ | ||
--volume ${BASE}/log:/home/cadia/vircadia-web/log \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is to be a service, add a "--restart=unless-stopped"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
@@ -33,6 +33,9 @@ export const FalseValue = "false"; | |||
|
|||
export const DefaultConfig: { [key: string]: string } = { | |||
"Default_Metaverse_Url": "https://metaverse.vircadia.com/live", | |||
// docker compose metaverse |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enhance this comment to say this is a debugging option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep. will update the comment.
This PR has a merge conflict. :) |
d0c6125
to
507ff53
Compare
c0e1bda
to
b120d11
Compare
Create a docker image. It checks out vircadia-web-sdk,
compile and install for vircadia-web.
Port 8080 is open for the service.
This image can be used as a port of docker-compose at
vircadia-domain-server-docker.