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

Make the image more consistent with The Docker Way #115

Open
ArwynFr opened this issue Feb 5, 2020 · 7 comments
Open

Make the image more consistent with The Docker Way #115

ArwynFr opened this issue Feb 5, 2020 · 7 comments
Assignees

Comments

@ArwynFr
Copy link

ArwynFr commented Feb 5, 2020

Do you want to request a feature or report a bug?
feature

What is the expected behavior?
This issue is a continuation to ONLYOFFICE/Docker-DocumentServer#136

We got docker-compose if you want separate containers

Using the docker-compose file will make services run in separate containers, but it is still not the docker way of running an application such as onlyoffice documentserver. The docker image will still include binaries and libraries needed for running the 3 external services : postegresql, rabbitmq, and redis. This makes the image 1,8 GB large when the actual /var/www directory is only 740 MB. Considering nginx:alpine being 22MB, and node:alpine being 113MB, that's 1GB lost when these services run in separate containers. Moreover, users can't upgrade the binaries in the container easily : you need to wait for an image upgrade (which usually happens when onlyoffice is upgraded, not when redis has published a security hotfix), or recompile your own. Images eventually contain outdated binaries, creating potential vulnerabilities within the system. Also, your Dockerfile includes a VOLUME /var/lib/postgresql instruction which will create a volume for each container even when you're not actually running the posgresql instance within the container.

As a first step, I'd suggest you remove the PostgreSQL, RabbitMQ and Redis services from the container, and add them as requirements for the service to run. Include a guide to configuring the existing environment variables and example docker-compose files that depends on official images for these products (I suggest you include major version tags), so that rookie users may use them as quickstart solutions. Advanced users can tweak their own if they want. This requires work but does not impact application architecture.

As a second step, I'd consider removing the logrotate and HTTPS support from the Dockerfile. There's a lot of log management in the image. Log files are written to a volume and rotated, which is a nice feature but actually beyond the responsibility of an application docker image. This prevents classic docker container log management tools to handle the logs efficiently in a production docker environment, by forwarding container output to syslogd, splunk, etc ... There's also a lot of code in the Dockerfile regarding HTTPS support. That's something that is very bound to the transport layer rather than the application one. For production, users shall consider using a separate container for reverse-proxying, routing, load balancing, and TLS encryption in front of any application container. There are excellent products doing this job, and it gives a better separation of concern, with a guarantee you don't break your HTTPS support when touching your application containers.

As a third step, I'd suggest removing the supervisor stack and working on the application architecture. The application is actually made of an nginx app, backed by a few nodeJs services run with supervisor. The supervisor stack means the services are run in the same container as the nginx app, which is not the Docker way of running each process in a dedicated container. The Docker way of handling this application would be to distribute the 5 nodeJs services that currently run with supervisor as 5 independent docker images, based on official node image. The rest of the application should be distributed a 6th image, based on official nginx. You'd use environment variables to bind the nginx app to the underlying nodeJs services. This would in turn allow administrators to scale services based on charge (for instance one would be able to load-balance 3 spellchecker instances) or distribute them on a cluster / swarm. Also, you should consider breaking the application main features in separate images, if that makes sense. A data-container image looks like a good candidate since you outsourced that into a second container in your docker-compose file.

The last part is probably the most challenging part and will require extensive architecture work, but could allow an excellent ability to scale/distribute the application load.

DocumentServer Docker tag:
latest

Host Operating System:
linux debian x64

@ShockwaveNN
Copy link

Thanks for your advice, we got some plans to redone our Dockerimage
@agolybev take a look

@igwyd
Copy link
Member

igwyd commented Jun 14, 2024

Hello @ArwynFr, sorry for the late reply. We have a repository with separate containers https://github.com/ONLYOFFICE/Docker-Docs/tree/master

@ArwynFr
Copy link
Author

ArwynFr commented Jun 14, 2024

Thanks for your feedback ; will look into it later, expect feedback no tater than end of month

@ArwynFr
Copy link
Author

ArwynFr commented Jun 26, 2024

Just tested the new version, which is much more in line with Docker concepts, thanks!
There is still a bit of redundancy between images but it's definitely going in the right direction.

A few remarks:

@igwyd
Copy link
Member

igwyd commented Jul 3, 2024

Thanks for rewiev!
We have discription about DS_URL in README.
We use node:lts-buster-slim, not node18.
The full image postgre is used here for easy test deployment docker-compose. In fact we use these images for the kubernetes cluster with installation in helm https://github.com/ONLYOFFICE/Kubernetes-Docs

@ArwynFr
Copy link
Author

ArwynFr commented Jul 3, 2024

We have discription about DS_URL in README.

I overlooked that line when reading :(

We use node:lts-buster-slim, not node18.

node:lts-buster-slim is actually an alias for node:18.2.0-buster:

image

metrics is also implicitly based on node 18:

image

I know that should rather be your base image editor responsibility, just wanted to give you a heads up.

@igwyd
Copy link
Member

igwyd commented Jul 4, 2024

You are right, thank you, I will report this to the developers.

@Rita-Bubnova Rita-Bubnova transferred this issue from ONLYOFFICE/Docker-DocumentServer Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants