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

FYI - group management sync will not fully work due to current schema #26

Open
dispo2 opened this issue Jan 13, 2021 · 7 comments
Open

Comments

@dispo2
Copy link

dispo2 commented Jan 13, 2021

I have raised a 'feature request' on the pihole discourse for this but just FYI the future group sync feature (which I have been looking at coding) will not fully work due to the use of the group keyword for the table name (only this table is affected).

The use of a keyword results in a syntax error when trying to export the table to CSV.

A workaround is to manually create the groups to match but I'm not sure if this will cause issues as internally the DB's will be different.

Coding wise the change is very easy with additional commands for the 3 group tables and 2 client tables but the client side would raise issues if the piholes are in different locations and therefore see different clients. This might require the client sync aspect to be optional via command line flag ??

@dispo2
Copy link
Author

dispo2 commented Jan 13, 2021

quick update - manually creating the groups on all 'slave' piholes does appear to work as a quick fix however they need to be created / displayed in the same order as on the master.

@dispo2
Copy link
Author

dispo2 commented Jan 13, 2021

so I have a workaround fix for exporting group in pushes, just enclose the table name so its 'group' (thanks discourse)
however this does not appear to work with the .import command as it still raises syntax errors so pulls do not work.

@dispo2
Copy link
Author

dispo2 commented Jan 24, 2021

So the feature request didnt get anywhere.
The only way I have been able to get this to work is make the group import a 3 stage process not 2, so now its a drop group, import to temp, rename temp to group.

$SUDO sqlite3 $gravity_db "DROP TABLE 'group';"
$SUDO sqlite3 $gravity_db -header -csv ".import group.csv group_tmp"
$SUDO sqlite3 $gravity_db "ALTER TABLE group_tmp RENAME TO 'group';"

This is required because .import produces syntax errors when working with group or group enclosed in '' "" `` etc. I couldnt get any to work. Its messy but at least the import and rename works.

@dispo2
Copy link
Author

dispo2 commented Jan 25, 2021

going around in circles with this - I let this run through a push/pull cycle and it failed on one machine but worked on another. The fail was during the pull process with sqlite3 being unable to rename the group_tmp table due to conflicts (apparently) within the views.

sqlite> ALTER TABLE group_tmp RENAME TO 'group';
Error: error in view vw_whitelist: no such table: main.group
{this is after dropping the original group table}

I'll play some more but I've seen this previously and have no idea what's going on. This only ever appears with the 'group' table so I'm pretty sure its the name of the table forcing extra steps that is the issue but there's not a lot I can do about that. From reading some documentation it appears that renaming tables in sqlite can be problematic due to the way sqlite stores the schema compared to say full sql.

The only main difference I can find between the systems is sqlite version. On the push and failing pull machine its
SQLite 3.27.2 2019-02-25 16:06:06

On the working pull machine its
SQLite 3.16.2 2017-01-06 16:32:41

So at the moment I am back to manually syncing groups on the various pihole before attempting to pull and have disabled the above drop/import/rename code within my testing.

At least (for me) the groups rarely change

@dispo2
Copy link
Author

dispo2 commented Jan 27, 2021

Firstly thank you very much for this code. I love how it works and how easy it makes syncing my piholes.

I now have working code for group and client sync thanks to various info's and help from the pihole discourse.
The fix required various changes to the cloudsync code (which I was only playing with to see if I could get this working).

I am happy to share what I have, it appears to fully work with group and client sync for all the push/pull functions. Client sync is optional via changing a variable within the script. I didnt want to just paste it here as its not my underlying code.

Let me know and I will post it here or do a pull request, though I did end up making quite a few changes.

Key things I had to change

  • added variables for the group and client tables
  • changed all sqlite3 calls to be pihole-FTL sqlite3 - this uses the sqlite3 version built into FTL and avoids issues of different sqlite versions on different platforms and repos
  • modified the drop table command to be PRAGMA foreign_keys=0;DROP TABLE xx required by the above sqlite change. This might not be the best way of doing this, I'm still looking/testing. Without the pragma command the drop fails with foreign key errors.
  • included quotes around the group table name as required by sqlite "PRAGMA foreign_keys=0;DROP TABLE "group";"
  • added a variable to enable or disable the dropping and importing of client tables (enabled by default)

Other things

  • i decided to make the client aspect a variable edit rather than command line flag because it just got messy with too many additional options. This requires an edit of the script but only if client sync is not required (the rarer option I would think) and people are already doing all sorts of git work/edits to get this working.
  • made the export and drop/import sections into their own functions to save on duplication. mainly because I was editing this a lot with testing and I was doing every edit twice.
  • the first time this is run (if changing from the non-group version of cloudsync) requires an initpush/initpull to make sure the requires file are in git.
  • the table name group causes issues, as mentioned above. Its the pihole schema so it cant be changed. The workarounds I have create as many issues as they fix - for example to reliably work with the group table you need a very current version of sqlite which is why I switched to using the FTL built in version. This introduces version issues as the access to FTL sqlite was only added in I think v5.4. Using the FTL sqlite also then generates foreign key errors which have to be disabled for the drop table command to work. This doesnt fix the problem it hides it. I think the foreign keys issue has always been there but not reported.
  • this raises the whole question of whether the modified code is fit for release as it's now dependent on a whole bunch of factors that it previously wasn't.

@bast69
Copy link

bast69 commented Aug 26, 2021

Hi @dispo2 , do you think it's possible to share your awesome work ?

@dispo2
Copy link
Author

dispo2 commented Aug 30, 2021

This is my modified script which I haven't updated it since Feb 2021, it's pretty solid but use at your own risk. I use this across three pihole, two on the same LAN which sync clients, the third is in the cloud which does not.

All credit to @stevejenkins for the original code, I have made various changes to enable group and client sync, which are enabled by default but can be individually disabled as required by editing variables at the top of the scipt.

This post will of course be deleted if requested by steve as this is not my code.

The use / syntax of the code is unchanged so use --initpush / push, --initpull / pull as normal.
All push / push functions are via the github software and are unchanged from the original script so creation of repositories and userid / key access still has to be configured before the script will work.

If for some reason a pull messes up or clients / groups get out of sync on a 'slave' then a delete of groups / clients and an initpull / pull usually fixes it.

There is no risk to the 'master' as the only change is that additional tables are pushed to github.
This is version='5.0gc4' to reflect that its pihole-cloudsync V5 with group/client sync additions revision 4

pihole-cloudsync.sh.txt

Delete the .txt extension to use.

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

2 participants