SensorsIot / IOTstack

Docker stack for getting started on IOT on the Raspberry PI
GNU General Public License v3.0
1.45k stars 308 forks source link

add syncthing service to IOTstack #513

Closed pmoya-in-the-web closed 2 years ago

pmoya-in-the-web commented 2 years ago

Added syncthing service from linuxserver/syncthing:latest

Resources; .templates/syncthing/service.yml

Note. This is my first PR to the project so maybe I am missing several things to do it properly.

Paraphraser commented 2 years ago

I'm coming into this in the middle and I haven't tested anything so this may be completely off point but "permission errors" in this context usually indicate that you need an entrypoint.sh script to "self repair" the container before it gets going.

What generally happens is docker-compose creates any missing left-hand-side (external) paths in the volumes directives with root ownership. Then it launches the container as root and links up the external and internal volume mappings.

The entry-point script runs and execs the desired container process (eg syncthing) over the top.

Either the entry-point script or the container process itself will downgrade its privileges to run as the supplied PUID/PGID, which means it is then running as ID=1000 trying to write into a mapped folder owned by root.

What needs to happen is for the entry-point script to get into the act while it still has root privileges - before the deescalation of privileges occurs - to enforce a chown -R ${PUID}:${PGID} internal/path. That way, the mapped folders and their contents all appear to be owned by the desired container process.

Also note that PUID/PGID is only a convention. Something inside each container needs to expect those variables and react accordingly.

IOTstack has several examples of self-repair code - start with Mosquitto, telegraf, Python...

Old menu tried to handle this problem with directoryfix.sh scripts. That "worked" so long as nothing else came along to reset the structure, such as erasing a container's persistent storage area to get a clean slate. Self-repair is far more robust and reliable. It also affords the opportunity to initialise a container's persistent storage area where a container does not already do that for itself. Mosquitto is a good example of that too (used to crash and burn).

Paraphraser commented 2 years ago

I could also add that you can test this theory (that privileges are the problem) by starting with the container down, adding the suggested volume mappings to the service definition in docker-compose.yml, then pre-creating the external paths by hand:

$ cd  ~/IOTstack/volumes
$ sudo mkdir -p syncthing/data1 syncthing/data2
$ sudo chown -R pi:pi syncthing

and then bring up the container. Docker-compose won't interfere with a folder structure that is already there. Because the external structures are owned by "pi", the mapped folders inside the container will be owned by ID=1000, irrespective of whether the /etc/passwd file inside the container actually defines ID 1000 or associates it with a username.

Mosquitto is a good example of this too - in reverse. Inside the Mosquitto container, ID 1883 is the username "mosquitto". The Pi itself doesn't define ID=1883 so, when you look at Mosquitto's persistent storage area in IOTstack/volumes/mosquitto you see 1883 as the owner and group rather than a name.

Where things can get a bit confusing is if the ID being used is known to both the Pi and the container but under different names in the respective /etc/passwd files.

pmoya-in-the-web commented 2 years ago

So, if I understood it properly I have to add a docker-entrypoint.sh performing chown -Rc to every volume I define in service.yml (/data1 & /data2) in order to allow container user write access.

Do I need to create Dockerfile too or is it optional?

Paraphraser commented 2 years ago

The Dockerfile is how the entry point gets added. Assuming you have a copy of IOTstack cloned somewhere, go into IOTstack/.templates/mosquitto. The service.yml has a build directive. Once that file becomes part of docker-compose.yml it will point to IOTstack/.templates/mosquitto/Dockerfile. The Dockerfile has a FROM which points to the base image on DockerHub, and everything that follows makes changes to that base image to build a new local image. The local image is what gets instantiated to become the running container.

As well as an entry point script, the Dockerfile adds a couple of packages (tzdata so TZ actually works inside the container, and rsync which is needed by self-repair), plus copies sensible defaults ("sensible" in the IOTstack context) and sets up health check etc.

The local image becomes the container, the entry-point script runs, fixes any defaults that have disappeared, enforces ID=1883 ownership, and hands over to the Mosquitto broker.

Once you wrap your mind around how Mosquitto does it, search for "Dockerfile" in the other sub-directories of the .templates folder because there are a few variations on the theme.

I've been working on a BIND9 container. It has its own "sensible defaults" in the base image but they are a mixture of "immediately useful" and "not quite useful". Again, "useful" is in an IOTstack context where "self-repair" is the goal. The entry point script is a mix of copying and editing on-the-fly with sed. I think there's something along those lines in the telegraf template.

Node-RED is slightly different. Its Dockerfile is in IOTstack/services/nodered. The Dockerfile gets constructed by the menu when you select add-on nodes. But it's better to think of it like this. A Docker file in:

Of course, to see NodeRed's Dockerfile you actually have to run the menu and choose it as a container, plus choose at least one add-on node. Python is similar but you're better off reading the IOTstack doco on that.

If you study enough examples you'll get there. I hope that all makes sense.

ukkopahis commented 2 years ago

Paraphraser nicely describes how to customize the container (though linuxserver containers do offer additional customization hooks), but in this case I don't think that i necessary. We can work with what is already present in the container.

Checking the root folder of the container:

pi@raspberrypi:~/sync $ docker-compose exec syncthing ls -lan
total 96
drwxr-xr-x   1    0    0 4096 Mar 20 22:32 .
drwxr-xr-x   1    0    0 4096 Mar 20 22:32 ..
-rwxr-xr-x   1    0    0    0 Mar 20 22:32 .dockerenv
drwxr-xr-x   3 1000 1000 4096 Mar 20 22:32 app
drwxr-xr-x   1    0    0 4096 Jan 24 20:42 bin
drwx------   5 1000 1000 4096 Mar 20 22:34 config
drwxr-xr-x   1 1000 1000 4096 Jan 24 20:42 defaults
drwxr-xr-x   5    0    0  320 Mar 20 22:54 dev
-rwxrwxr-x   1    0    0 3579 Jan 24 20:42 docker-mods
drwxrwxr-x   1    0    0 4096 Mar 20 22:54 etc
drwxr-xr-x   2    0    0 4096 Jan 24 20:42 home
-rwxr-xr-x   1    0    0  389 Feb 15  2021 init
drwxr-xr-x   1    0    0 4096 Jan 24 20:42 lib
drwxr-xr-x   2    0    0 4096 Jan 24 20:42 libexec
drwxr-xr-x   5    0    0 4096 Jan 24 20:42 media
drwxr-xr-x   2    0    0 4096 Jan 24 20:42 mnt
drwxr-xr-x   2    0    0 4096 Jan 24 20:42 opt
dr-xr-xr-x 254    0    0    0 Mar 20 22:54 proc
drwx------   2    0    0 4096 Jan 24 20:42 root
drwxr-xr-x   1    0    0 4096 Mar 20 22:32 run
drwxr-xr-x   1    0    0 4096 Jan 24 20:42 sbin
drwxr-xr-x   2    0    0 4096 Jan 24 20:42 srv
dr-xr-xr-x  12    0    0    0 Mar 20 22:54 sys
drwxrwxrwt   1    0    0 4096 Jan 24 20:42 tmp
drwxrwxr-x   1    0    0 4096 Jan 24 20:42 usr
drwxr-xr-x   1    0    0 4096 Jan 24 20:42 var

we find the following folders owned by UID=1000:

As config is already in use for configuration, I propose[^1] to use the app folder for the actual synced folders. This is easily done by defining HOME.

Here's my docker-compose.yml testing it out:

version: "3.2"
services:
  syncthing:
    image: linuxserver/syncthing:latest
    container_name: syncthing
    hostname: raspberrypi #optional

    environment:
      - PUID=1000
      - PGID=1000
      - TZ=Europe/Madrid
      - HOME=/app
    volumes:
      - ./volumes/syncthing/config:/config
      - ./volumes/syncthing/data:/app
    ports:
      - 8384:8384 # Web UI
      - 22000:22000/tcp # TCP file transfers
      - 22000:22000/udp # QUIC file transfers
      - 21027:21027/udp # Receive local discovery broadcasts

Now the config and synced folder data are in separate trees, and are easily customizable to different folders.

[^1]: If you really want to do your due-diligence, research why the app and default folders are created, and that placing stuff in them won't break the container.

Paraphraser commented 2 years ago

Maybe we should go back one step. Starting position:

  syncthing:
    image: linuxserver/syncthing:latest
    container_name: syncthing
    environment:
      - PUID=1000
      - PGID=1000
      - TZ=Europe/Madrid
    volumes:
      - ./volumes/syncthing/config:/config
    ports:
      - "8384:8384" # Web UI
      - "22000:22000/tcp" # TCP file transfers
      - "22000:22000/udp" # QUIC file transfers
      - "21027:21027/udp" # Receive local discovery broadcasts
    restart: unless-stopped

On first launch docker-compose will do the equivalent of:

sudo mkdir -p "$HOME/IOTstack/syncthing/config"

Then the container will start and something inside it will respect the PUID and GUID environment variables and:

  1. Change the ownership on the "config" folder to ID=1000:

    $ ls -ld syncthing ; tree -puL 2 syncthing
    drwxr-xr-x 3 root root 4096 Mar 21 10:48 syncthing
    syncthing
    └── [drwx------ pi      ]  config
        ├── [-rw-r--r-- pi      ]  cert.pem
        ├── [-rw------- pi      ]  config.xml
        ├── [-rw------- pi      ]  config.xml.v0
        ├── [drwxr-xr-x root    ]  custom-cont-init.d
        ├── [drwxr-xr-x root    ]  custom-services.d
        ├── [-rw-r--r-- pi      ]  https-cert.pem
        ├── [-rw------- pi      ]  https-key.pem
        ├── [drwxr-xr-x pi      ]  index-v0.14.0.db
        ├── [-rw------- pi      ]  key.pem
        └── [drwxr-xr-x pi      ]  Sync
  2. Downgrade the syncthing process to run as ID=1000:

    $ ps -C syncthing -o pid,euser,ruser,suser,fuser,comm
        PID EUSER    RUSER    SUSER    FUSER    COMMAND
     891072 pi       pi       pi       pi       syncthing
     891094 pi       pi       pi       pi       syncthing

So far, so good.

However, I thought the question at hand was adding these volume mappings:

      - ./volumes/syncthing/data1:/data1
      - ./volumes/syncthing/data2:/data2

A test. Terminate the container, edit compose to add those volume mappings, erase persistent storage and start the container again. The result:

$ ls -ld syncthing ; tree -puL 2 syncthing
drwxr-xr-x 5 root root 4096 Mar 21 11:04 syncthing
syncthing
├── [drwx------ pi      ]  config
│   ├── [-rw-r--r-- pi      ]  cert.pem
│   ├── [-rw------- pi      ]  config.xml
│   ├── [-rw------- pi      ]  config.xml.v0
│   ├── [drwxr-xr-x root    ]  custom-cont-init.d
│   ├── [drwxr-xr-x root    ]  custom-services.d
│   ├── [-rw-r--r-- pi      ]  https-cert.pem
│   ├── [-rw------- pi      ]  https-key.pem
│   ├── [drwxr-xr-x pi      ]  index-v0.14.0.db
│   ├── [-rw------- pi      ]  key.pem
│   └── [drwxr-xr-x pi      ]  Sync
├── [drwxr-xr-x root    ]  data1
└── [drwxr-xr-x root    ]  data2

So, yes, the config folder is OK but the two data folders are still owned by root. That's because the "something" inside the container that knows to handle config doesn't know to also handle the other paths.

To solve that problem you either need the directoryfix.sh approach:

cd ~/IOTstack
for D in data1 data2 ; do
   sudo mkdir -p "./volumes/syncthing/$D"
   sudo chown -R "$USER":"$USER" "./volumes/syncthing/$D"
done
docker-compose up -d syncthing

or the entry-point script approach with a local Dockerfile to do the chown inside the container on each launch before the privileges are downgraded.

The directoryfix.sh approach is fragile because the user just has to know to run it when permission problems surface. The entry-point script approach is set-and-forget.

Paraphraser commented 2 years ago

Oh, I agree that the default for TZ should be Etc/UTC.

Also, we (IOTstack) try to follow the convention of quoting port numbers. You'll see how I've done it in my earlier post.

Quoting is recommended at compose file specs:

Screen Shot 2022-03-21 at 11 23 21

Rather than try to remember to always ask "is this port number less than 60?" (almost never true) and only quote ports in that situation, we accepted Docker's recommendation to fail safe.

It's a bit hit-and-miss. A while back I went on a pogrom and fixed them all but unquoted port numbers manage to creep in. I fix them when I see them.

Paraphraser commented 2 years ago

Another thing I think might be (slightly) wrong here is:

Screen Shot 2022-03-21 at 11 31 12

It looks to me like that is all indented by two spaces - just like it appears in docker-compose.yml. Having two leading spaces on each line is "correct" for old-menu branch but "incorrect" for master branch.

Please don't ask me to explain why this is the case. Slyke would have to give us the rationale. I just think of it as a rule I have to follow. If I'm adding a new container, I do it twice, once in master branch (two leading spaces removed), then in old-menu branch (two leading spaces in place).

I will admit to never having actually tested what happens if I have a service.yml with two leading spaces in place in master branch and then I run the (new) menu to select that container. For all I know it works perfectly well and I've been following the "rule" for no good reason.

Also, I can't think of another service.yml with blank lines in the middle. We do, as a convention, set the last line of a service.yml to be blank. I think that's probably more important in old-menu than new.

I hope that all makes some kind of sense.

ukkopahis commented 2 years ago

It looks to me like that is all indented by two spaces - just like it appears in docker-compose.yml. Having two leading spaces on each line is "correct" for old-menu branch but "incorrect" for master branch.

New menu (=master) actually parses the service.yml:s, as such indentation style doesn't matter at all, as long as it's a valid yaml-file. But for human comfort, it makes sense to have a consistent style.

ukkopahis commented 2 years ago

Pull-request for actual self-repair to the official Syncthing docker-image was rejected as overly broad, when it proposed to recursively fix permissions: https://github.com/syncthing/syncthing/pull/6648. I guess it's good to not break the whole system if a bind-mount is "wrongly" made to the filesystem root. No docker-compose.yml volume configuration should ever result in:

# chown -R pi:pi /

As Syncthing is a service meant for sharing large folder structures, sometimes as read-only, it's very likely a user may point a bind-mount to places where permissions must not be changed. In other services where such broad bind-mounts are very unlikely, can pretty safely continue to do self-repair of permissions.

ukkopahis commented 2 years ago

In summary this is what I propose:

  syncthing:
    image: linuxserver/syncthing:latest
    container_name: syncthing
    hostname: raspberrypi #optional
    environment:
      - PUID=1000
      - PGID=1000
      - HOME=/app
      - TZ=Etc/UTC
    volumes:
      - ./volumes/syncthing/config:/config
      - ./volumes/syncthing/data:/app
    #ports:
    #  - 8384:8384 # Web UI
    #  - 22000:22000/tcp # TCP file transfers
    #  - 22000:22000/udp # QUIC file transfers
    #  - 21027:21027/udp # Receive local discovery broadcasts
    network_mode: host

And of course documentation updated to reflect the separate data and config volumes.

Paraphraser commented 2 years ago

Pull-request for actual self-repair to the official Syncthing docker-image was rejected as overly broad

Oh I So Agree! That's why I've gone down the path of IOTstack-specific images before applying IOTstack-specific self-repair. It's reasonably safe to assume any self-repair code in an entry-point script in an IOTstack template folder matches the volume mappings defined in the same service definition that includes the build directive pointing to the Dockerfile that pulls in the entry-point script.

I grant it's not 100% bulletproof but I reckon anyone who takes an iotstack_ prefixed image and uses it as the basis for something outside its defined scope also incurs the obligation to do a bit of due diligence to understand how the image was built and whether violating the explicit assumptions revealed in its Dockerfile will result in "unintended consequences".

Paraphraser commented 2 years ago

I quite like that TZ=${TZ:-Etc/UTC}. Wasn't aware of that. Works for both export TZ=Australia/Sydney and putting TZ= in ~/IOTstack/.env. Which one did you have in mind? Should probably make that standard across-the-board.

ukkopahis commented 2 years ago

Not quite sure which would be best, but options are:

  1. Keep using TZ=Etc/UTC
  2. Use TZ=${TZ:-Etc/UTC} and add user documentation to manually echo TZ=$(cat /etc/timezone) >> .env
  3. Use TZ=${TZ:-Etc/UTC} with menu/build stack automatically doing echo TZ=$(cat /etc/timezone) >> .env (of course with proper checks)
  4. Don't define a TZ environment variable, but dynamically bind-mount directly to the host:
    volumes:
      - /etc/timezone:/etc/timezone:ro
      - /etc/localtime:/etc/localtime:ro
  5. Use TZ=%TZ% in templates, and implement build stack to replace it using /etc/timezone (optionally adding .env support)

Option 4. is pretty good. Maybe a bit obscure for beginners, but still pretty self-explanatory. I'm worried that it depends on /etc/timezone being correct set, which may not be if you copy docker-compose.yml to another machine.

Option 5. being the only one that: a) keeps all state in docker-compose.yml, and b) it-just-works. It will require some menu-coding, essentially #505 as neither of us wants to touch the current buildstack_menu.py.

Would option 2. be good in the interim before #505? We could add it to the docs, but I think we shouldn't document&teach things that will soon be deprecated. So not good if we agree on option 5. being the preferred final target.

So perhaps the currently right thing to do is option 1, just to keep using:

      - TZ=Etc/UTC

And when #505 enables it, go to option 5.

ps. @pmoya-in-the-web sorry we sidetracked your pull request a bit. The devil is in the details.

Paraphraser commented 2 years ago

Well, just sticking an export TZ= in .profile seems to work too. Screen shot shows with/without tests. The container definitely follows the env var known to the current shell at "up" time.

15F99515-1E38-4CAA-9936-A8CA6F1C77EC

We always seem to be reminding each other of "keep it simple" so maybe you're right about keeping the current form because it's simple and easy. That said, I find the idea of the default syntax quite compelling because TZ is used in so many containers so a "set once" (of whatever scheme) has its own "simplicity" too. I'm not planning to do anything more on this so it's up to you.

Ditto the "hijacking the PR" apology.

pmoya-in-the-web commented 2 years ago

e sidetracked your pull reque

I find this tip interesting ( for self knowledge too ). Thanks !

pmoya-in-the-web commented 2 years ago

https://github.com/SensorsIot/IOTstack/pull/513#issuecomment-1074525335

I like the idea. Maybe first MVP could be published with "/app" volume and then create a new issue to improve it.

Any objections ?

ukkopahis commented 2 years ago

Nope, good to go.

pmoya-in-the-web commented 2 years ago

Thanks to all. I just made a new commit with a mix of your proposals). What do you think know?

I would like to do a first completely useful MVP1 and after that start improving the capabilities of the container step by step.

I think know the syncthing is fully usable (in a basic mode)

ukkopahis commented 2 years ago
  • Network mode network_mode: host NOT set: Unsolved but runs ok. (i would like to be settable by user in menus but i still need to investigate how to do it)

I don't see any reason not to use host networking. And official Syncthing docs state that not doing so will result in slow local transfer rates. IOTstack is aimed for beginners and should be (or at least aim for) an it-just-works experience.

pmoya-in-the-web commented 2 years ago

As i understand, if you set network mode to host value, the container really control the host network allowing to modify it as it wants. You can not restrict the ports exposed. Thats why i would like to manage someway in options menu.

I have tested at home and I get about 8MB/s under a fiber with 100Mb/s. I think it is good enough for 90% users.

If none agree with me i can change to host mode but right now i think it is not really a very good practice at all.

pmoya-in-the-web commented 2 years ago

Hi, I have commited proposal; https://github.com/SensorsIot/IOTstack/pull/513#issuecomment-1074525335 from @ukkopahis