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

Add gladys assistant #1044

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Empty file.
20 changes: 20 additions & 0 deletions gladys-assistant/docker-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
version: "3.7"

services:
web:
image: gladysassistant/gladys:v4@sha256:f694d49bf57426cfc37590eb71a19b0e183cd3e8cbae966ef093bb3d199d318b
restart: on-failure
stop_grace_period: 1m
privileged: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need it to be privileged?

Copy link
Contributor

Choose a reason for hiding this comment

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

@cicoub13 do you happen to know if all of the following are required for Gladys functionality:
priveleged: true
cgroup: host
- /var/run/docker.sock:/var/run/docker.sock

Copy link
Contributor

Choose a reason for hiding this comment

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

In particular, I'm wondering how important binding the Docker socket is:
/var/run/docker.sock:/var/run/docker.sock

This gives the Gladys container complete control of the Docker daemon, which we can't allow because Gladys can then issue Docker commands to the host's Docker daemon, where other app containers are running.

Copy link
Author

@cicoub13 cicoub13 May 20, 2024

Choose a reason for hiding this comment

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

For the Docker daemon binding, it's needed from our side to create, restart other containers when a user adds an integration like Zigbee2Mqtt / MQTT / Node-Red.
Gladys will automatically pull, configure and start a new container
We try to be as smooth for the users but still managed integrations cleanly by using docker each time it's needed.

For the cgroup and privileged, let me confirm it with the community/developers.

Choose a reason for hiding this comment

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

We use the cgroup in Gladys to get Gladys current containerId in Docker ( https://github.com/GladysAssistant/Gladys/blob/master/server/lib/system/system.getGladysContainerId.js#L32 )

We use this to check if the container is in the correct state (network_mode = host for example)

privileged is used for some integration like Bluetooth to be able to scan for Bluetooth devices

Home Assistant Umbrel integration uses it too: https://github.com/getumbrel/umbrel-apps/blob/master/home-assistant/docker-compose.yml#L8

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks very much for the explanations @cicoub13 and @Pierre-Gilles. Let me run this by Luke to figure out the best path forward.

Copy link
Contributor

Choose a reason for hiding this comment

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

@cicoub13 @Pierre-Gilles - understood regarding cgroup and priveleged: true. These are fine to keep to maintain app functionality. Thanks for confirming their necessity.

I have added some thoughts on the docker socket mount here: #1044 (comment)

network_mode: host
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider specifying only the ports you need to be accessible or using app-auth

Copy link
Contributor

@nmfretz nmfretz May 20, 2024

Choose a reason for hiding this comment

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

Ah, I think for now the Gladys container will need to run in host network mode. This is because Gladys needs to have network device discovery capabilities and access to multicast UDP traffic (e.g., for UPnP) is that correct @cicoub13?

We have some ideas on how to get this to work with bridge networks in the future, but there isn't really a way to do this on umbrelOS right now.

Copy link
Author

@cicoub13 cicoub13 May 20, 2024

Choose a reason for hiding this comment

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

Yes, exactly. We need host for because we are doing network discovery in local network https://demo.gladysassistant.com/dashboard/integration/device/lan-manager/config
Home Assistant is working the same way. If you find a way to work differently, we could adapt yes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good 👌

cgroup: host
volumes:
- ${APP_DATA_DIR}/data/gladysassistant:/var/lib/gladysassistant
- /var/run/docker.sock:/var/run/docker.sock
- /dev:/dev
Copy link
Contributor

Choose a reason for hiding this comment

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

This is really weird.

Copy link
Contributor

Choose a reason for hiding this comment

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

Binding the entire /dev directory is something we probably have to do here so that Gladys Assistant can communicate with any USB that may need to be plugged into the user's device (e.g., a USB dongle for zigbee support).

We can't know in advance what devices a user does or does not have plugged in, so we need to mount the entire /dev directory. We're definitely open to suggestions on how to get around this though, as the goal should always be to restrict a container's access and permissions as much as possible.

This device issue is pretty tricky though. For example, let's just assume that the only device a user ever needs to plug in to work with Gladys gets connected as /dev/ttyACM0. The logical solution in that case would seem to be to use this bind mount:
/dev/ttyACM0:/dev/ttyACM0

But there is a huge problem with this in Docker, which is that if the user does not have a device plugged in at /dev/ttyACM0 then Docker will error out and exit when trying to bring up the container because the /dev/ttyACM0 file doesn't exist. This means that users who install Gladys without a usb installed at that file will get a broken install (Gladys won't even start), and any user that starts up Gladys after removing that USB device will also have a broken installation.

Copy link
Author

Choose a reason for hiding this comment

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

Exactly. We need to connect to existing or new plugged usb devices and there is no other way to do it.

Home Assistant is doing the same here https://github.com/getumbrel/umbrel-apps/blob/master/home-assistant/docker-compose.yml#L11

- /run/udev:/run/udev:ro
environment:
NODE_ENV: production
SQLITE_FILE_PATH: /var/lib/gladysassistant/gladys-production.db
SERVER_PORT: 5081
TZ: Europe/Paris
25 changes: 25 additions & 0 deletions gladys-assistant/umbrel-app.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
manifestVersion: 1
id: gladys-assistant
category: automation
name: Gladys Assistant
version: "v4"
tagline: A privacy-first, open-source home assistant
description: >-
Gladys Assistant is a privacy-first, open-source home assistant that
runs on any Linux machine: a Raspberry Pi, a NAS, a VPS, or a server at home.
developer: Pierre-Gilles Leymarie
website: https://gladysassistant.com/
repo: https://github.com/GladysAssistant/Gladys
support: https://en-community.gladysassistant.com
dependencies: []
port: 5081
gallery:
- 1.jpg
- 2.jpg
- 3.jpg
path: ""
defaultUsername: ""
defaultPassword: ""
releaseNotes: ""
submitter: Cyril Beslay
submission: https://github.com/getumbrel/umbrel/pull/1044