Add filesync

[thatonedude3470]

Prerequisites checklist

• I have read the contributing guidelines.

What is the purpose of this pull request? (put an "x" next to an item)

• New feature

What changes did you make? (Give an overview)

• Added rsync to common packages in install-raspbian.sh
• Added cronjob via install-raspbian.sh (needs documentation in Wiki for manual install)
• Added new sync-to-drive.js which does all the logic for selecting and starting rsync
• Added new drivename.conf in config. It stores the drives/labels of drives that are copied to

Is there anything you'd like reviewers to focus on?

Currently there is no UI for selecting which drives to copy to. I don't code in PHP, someone else would have to do this
This pull request resolves #113

[andi34]

Thanks a lot!
Could you please add a y/n request to set the cron job?
And you also like to add a few lines to the FAQ (faq/faq.md, after that once run yarn build:faq) in this PR?

[thatonedude3470]

Yes, sounds like a good idea!
I'll try my best, haven't worked much with markdown tho.
I could also use some more testing by other people, four eyes see more than two.

[thatonedude3470]

Should I add rsync to update-booth.sh script?

[andi34]

Should I add rsync to update-booth.sh script?

Yes, please.

One thing to think about, at least needs documentation:
const DATA_DIR_NAME = 'data';
If we change the data folder name, the script won't work until we adjust it.

[thatonedude3470]

Okay done :)

If we change the data folder name, the script won't work until we adjust it.

This information could be read from the config file if I'm correct, so if someone got free time they could write a function that parses it out of there.

[andi34]

Thanks, yeah it needs a note in the FAQ too until it's read out. But would be good to have it in this PR too. Should have some time to take a look in about 2 weeks if no one is faster :)

[thatonedude3470]

Sounds good to me, sadly I don't have the nerve to write some parser from PHP to JSON.
Would it be an option to have PHP write the config to a file (e.g. as config.json to the config folder)?
Kinda in the same fashion it merges and exposes it to the JS code in browser, just as a file?
Then it'd be much easier just reading the JSON and getting the data path from there.

[jacques42]

I did not take a very close look but on a first glance, that sounds like a problem maybe solved already. Have a look here and think through if this could be reapplied in yours

photobooth/src/js/remotebuzzer_server.js

Lines 27 to 31 in 71c6fd0

	/* SOURCE PHOTOBOOTH CONFIG */
	const {execSync} = require('child_process');
	const stdout = execSync('cd api && php ./config.php').toString();
	const config = JSON.parse(stdout.slice(stdout.indexOf('{'), -1));

And maybe this allows to move away from drivename.conf to include that config parameter in the admin panel ( if this is something you want to consider at all).

[thatonedude3470]

@jacques42 Sounds good, I'll give it a try and come back once it's all nice and working!

[thatonedude3470]

Worked out fine as far as I can tell. Thanks a lot @jacques42 :)
Now the script gets the paths from PHP and the config is easily changeable in the future, if so desired.

Only thing that caught my attention is PHP needing sudo privileges because the script isn't called from www-data user.

[andi34]

Cron job is setup as root via install scrip, so shouldn't be a problem. Problems might appear if the cron job is setup as user pi or www-data user? Or they're able to detect the device? The config could also be written to an own array config with different rights on that file.

[thatonedude3470]

Problems might appear if the cron job is setup as user pi or www-data user?

Correct. I've tried setting up the cronjob with www-data but it doesn't work. It currently doesn't have the rights to mount drives with udisksctl and can't access the auto mounted drives from pi.
Same is true with pi user since it doesn't have rights to execute PHP.

Or they're able to detect the device?

As far as I can tell the script should work for every user that can execute ps, lsblk and udisksctl. If we ignore PHP for now. Since that is how it gets its information and mounts the drives (if unmounted).

The config could also be written to an own array config with different rights on that file.

If you mean creating a (JSON) file from the config-API that would mean the script could be run from any user that has access to the config file and the above mentioned rights. (Which could in theory be the pi user or a newly created user that has just the needed rights)

[andi34]

Whoops, just noticed I've got permissions to commit to your fork...
What I've added is untested... Ill remove the 2 commits tomorrow. Sorry!

[thatonedude3470]

No problem, that just happens :P

I've done some testing and been trying some stuff and finally came to a result that wouldn't do much more effort (writing extra files, etc.).
Since (most) systems include polkit we could write a config file to /etc/polkit-1/localauthority/50-local.d/rulename.pkla that would allow our www-data user to specifically mount drives with udisks2 (which is used by udisksctl in the script) and only that.

This is the rule I've come up with:

[Allow www-data to mount drives with udisks2]
Identity=unix-user:www-data
Action=org.freedesktop.udisks2.filesystem-mount*
ResultAny=yes
ResultInactive=yes
ResultActive=yes

Sadly the polkit version of Debian/Raspberry Pi OS doesn't support rules in etc/polkit-1/rules.d/ yet so we'd have to rely on the older plka rules.

Additionally I've experimented with adding the cronjob to the www-data user but I still need to test that a little more.
If this would work the only thing would be to disable the auto mount, so that the drives get mounted under the www-data user, else it doesn't have the permissions to start rsync to the mount folder.

[thatonedude3470]

Just did, your commits are now gone, lost in the history of git :p

[andi34]

Thanks :)
Lost, yes and no. Still can access them here and also have downloaded the changes as patch file in case we need it again ;)

Sadly I won't have time for about 1-2 weeks to test this, but will do once I am able to.

If there's enough positive feedback we can merge earlier.

Again thanks a lot for your contribution!

[thatonedude3470]

No problem, one can't take all the time without giving back :P

Sadly I can't do the UI for the admin panel myself so for testing the steps above need to to :s
If there's someone who could do it I'd be really glad about it ^^

[andi34]

Sure, I'll take a look at it once I am able to boot a PC.

[jacques42]

Hey @thatonedude3470 - thanks for pulling all this together, that's really great and I like it a lot.

When I was looking at the conversation about the need for cron, I wondered why not to do a "forever loop" in the node script itself. That avoids having to schedule via cron. The loop could be timebased using setTimeout() or event triggered using something like fs.watch().

And to ensure the script is active, similar to remote_buzzer.js the (re-)start of the script can be anchored in the main ìndex.php` file.

Maybe you considered all that and for good reasons it was not the right thing to do. And I don't want to get in the way at all. But I'd be happy to help and give it a shot if that's something you would consider to be an improvement. Just let me know. I'll anyways do some testing and report back what I find.

Edit: One early on finding, I run a pretty recent headless Raspberry Pi OS 10 (Buster), where the udisksctl utility is not pre-installed. I had to manually install package udisks2.

[thatonedude3470]

Thanks to all the help from @jacques42 !!
This should finally be it, if no other bugs are found.
I'll do a fresh install on two different PI's tomorrow and report back afterwards.
@andi34 and @jacques42 would be really nice if you can take some time and do one install to :)

[thatonedude3470]

After two installations I can say:

• The admin interface has a small bug, it shows following warning in the Sync tab:

Warning Illegal string offset 'type' in
/var/www/html/admin/index.php on line 77

• Everytime the index page gets opened (e.g. first time and after every picture) a new node instance get launched.
  This quickly gets problematic as every script has some thread and it spams the system badly. In addition it seems that all the scripts work at the same time (they all seem to start rsync) which besides spamming the log could cause problems.
  To check how many are running type: ps -A | grep node | wc -l in the bash

• The way we do the loop could be improved by using setInterval instead of calling itself every time

[jacques42]

Hey @thatonedude3470 - thanks for that, I very much appreciate it.

The error on the admin is due to two commits I submitted to Andi only after your fork was created. In anticipation of those commits, I already now added a setting in your fork in a way that the admin panel options will only show on linux, but not windows. Once merged to dev, this error will be gone.

I am somewhat speechless - I totally forgot to test the most common and basic use-case we have oml. So thanks for highlighting. I did enhance the services scripts even more, to yet be more robust. I am trying to make sure there is no stale process / no stale PID files hanging out there at any point.

And last I added a small bug fix in the writeFileSync() for the PID file as I believe the parameter was meant to be flags. I do suggest 'w' for flags as this will overwrite any existing file and a stale PID will would not block the service from starting up.

thatonedude3470/photobooth#2

[jacques42]

@andi34 I did not want to leave this feature incomplete, as I really like it. So I took the time and effort to finalize into a base version which is fully functional, robust and rebased to latest dev branch changes. You can find the code here https://github.com/andi34/photobooth-wip/tree/add-filesync. I'd appreciate if you would have a look and let me know what else would be needed from your perspective for to move it into the dev branch.