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

Addition of podman support and sorts of improvements #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

leleobhz
Copy link

Hello @alireaza !

Thank you a lot by creating this Dockerfile. I made some improvements and I hope you like it and merge the changes following:

** Support for podman
** Change from ubuntu to debian:stable-slim for a smaller image (<200mb) ** Improvements on apt handling
** Tini addition allowing anydesk to properly receive signals from shell/terminal/docker|podman stop|kill|rm ** README.md improvements

This commit was tested on podman rootless.

** Support for podman
** Change from ubuntu to debian:stable-slim for a smaller image (<200mb)
** Improvements on apt handling
** [Tini](https://github.com/krallin/tini) addition allowing anydesk to
properly receive signals from shell/terminal/docker|podman stop|kill|rm
** README.md improvements

This commit was tested on podman rootless.
@alireaza
Copy link
Owner

alireaza commented Mar 8, 2023

Hello @leleobhz

I looked at the changes and improvements.

I agree about switching Ubuntu to Debian, but I remember trying this before and having trouble running AnyDesk.
I will check again.

About "xauth", unfortunately, in some Linux distributions, this library is not installed, and users may encounter errors.
For example, server distributions that may install a simple graphic environment on them but do not install xauth.
I tried to implement the structure in such a way that they can be used in most distributions.

About "Tini", If you are using Docker 1.13 or greater, Tini is included in Docker itself. This includes all versions of Docker CE.
To enable Tini, just pass the --init flag to docker run.

Note: The reason for separating the layers is to make the build process faster and take up less space in the rest of the images.
But when all of them are combined together, they only make the space too large.
https://dzone.com/articles/docker-layers-explained

About "podman", it is the same as docker and they are not different.
But thanks for updating README.md

But as for using ENTRYPOINT, its best use is in sending parameters to AnyDesk.
https://devtron.ai/blog/cmd-and-entrypoint-differences

Thank you for your contribution

@alireaza alireaza requested review from alireaza and removed request for alireaza March 8, 2023 08:26
@leleobhz
Copy link
Author

leleobhz commented Mar 8, 2023

Hello @leleobhz

Hello @alireaza !

I looked at the changes and improvements.

I agree about switching Ubuntu to Debian, but I remember trying this before and having trouble running AnyDesk. I will check again.

Anydesk tells in download page the .deb version also runs on Debian. This commit works with podman + Fedora host and I'm using it as production environment since I dont like the way Anydesk package installs on system.

About "xauth", unfortunately, in some Linux distributions, this library is not installed, and users may encounter errors. For example, server distributions that may install a simple graphic environment on them but do not install xauth. I tried to implement the structure in such a way that they can be used in most distributions.

Some clarification here is needed: XAuth issues does not relate to container except for xauth binary presence. Let's take xorg-x11-xauth from Fedora as example:

$ rpm -ql xorg-x11-xauth
/usr/bin/xauth
/usr/lib/.build-id
/usr/lib/.build-id/0e
/usr/lib/.build-id/0e/a78161105f8d5a7102e8806733f7bb1612f08e
/usr/share/doc/xorg-x11-xauth
/usr/share/doc/xorg-x11-xauth/COPYING
/usr/share/doc/xorg-x11-xauth/README.md
/usr/share/man/man1/xauth.1.gz

Main issue about Xauth is on host. Host MUST authorize connections and it will check client in the internal ACL. In fact, if you properlly redirect Xorg socket and authorize connection on host (not in container), container will work without modification.

About "Tini", If you are using Docker 1.13 or greater, Tini is included in Docker itself. This includes all versions of Docker CE. To enable Tini, just pass the --init flag to docker run.

I prefer explicitly use Tini because it properlly handle signals - task that is assumed only to PID1. Some projects also create their wrappers for this (Like Node-RED).

It's in some matter problematic allow user to choose handle PID1 issues with --init because a misbehaviour in this matter by Anydesk may lead to zombie process and fix this kind of issue requires machine reboot. All issues with zombie subprocess may be avoid using a explicit init system. Also, tini only increase size by 23k and avoid any kind of trouble with zombies w/o user intervention.

Note: The reason for separating the layers is to make the build process faster and take up less space in the rest of the images. But when all of them are combined together, they only make the space too large. https://dzone.com/articles/docker-layers-explained

I understand multi-stage builds and this kind of build isn't for everything. Image size on previous Dockerfile is increased by apt update/upgrade and this is not needed because debian releases docker images with security updates already on base image. This kind of approach just increases layer size. Main reason for changing Ubuntu to Debian is related to this and related to slim variant usage, with less packages installed in root layer (So this also reduce surfice to need copy files from a layer in other stage to avoid size issues - since root layer is already compact).

About "podman", it is the same as docker and they are not different. But thanks for updating README.md

podman and docker does have diferences too. I decided to copy build step to keep documentation complete, but run instructions differ because rootless matter of podman. Security label and userns are not required nor recommended on docker because docker handles this as root, but podman does not, and require user to disable SELinux to use anydesk docker image does not make much sense.

But as for using ENTRYPOINT, its best use is in sending parameters to AnyDesk. https://devtron.ai/blog/cmd-and-entrypoint-differences

ENTRYPOINT is by definition immutable and points to first process. Thinking UX, to change CMD, you just need to run docker run -it --rm --name someshell debian:stable-slim /bin/bash. For this command, ENTRYPOINT runs the CMD (This case, /bin/bash I pointed. By other side, change on ENTRYPOINT requires argument AND if some CMD follows the image-name, it will become argument for ENTRYPOINT.

Since /tini -- is an immutable argument, but anydesk aren't (I can run shell inside anydesk instead anydesk directly - an this way, shell also got protected about PID1 issues), I think this implementation protects the image from unpredicted behaviora

Thank you for your contribution

I want to thank you back! You code saved-me a ton of time in a desperate momment, so i think its fair try to contribute back.

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

Successfully merging this pull request may close these issues.

None yet

2 participants