Suggested systemd service file to insure pihole.db is populated with data if DBINTERVAL= is non-default

graysky2 · July 25, 2024, 7:02am

You can find the differences here: https://raw.githubusercontent.com/max72bra/pi-hole-ftl-archlinux-customization/master/arch-ftl-5.25.2-1.patch

After those are applied, pi-hole-FTL is built like this: https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=pi-hole-ftl#n42

Finally, the package is created starting here: https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=pi-hole-ftl#n49

DL6ER · July 25, 2024, 5:28pm

Ah, yeah. I'm constantly getting confused with v5/v6 of Pi-hole being discussed, sorry for that. The pihole.log file path is not controlled by pihole-FTL in v5.x and this seems to be missing on the Arch Linux deployment (at least from the fragments I have seen).

However, you will be able to change all default paths in v6 from this file.

Replying to myself here: Yes, if the pihole.log log file cannot be created, the embedded dnsmasq core inside pihole-FTL goes into some kind of meta-failed-mode where it shouldn't be doing anything actually (i.e. not responding to DNS requests, either). I think we should improve on this and just created a PR (targeting the v6.0 beta) for this:

github.com/pi-hole/FTL

Fix termination in dnsmasq failure mode

development-v6 ← new/sigtest

opened 05:28PM - 25 Jul 24 UTC

DL6ER

+67 -31

# What does this implement/fix? When the embedded `dnsmasq` fails to start be…cause of some incorrect configuration and reversing to the previous configuration isn't possible, FTL remains being active so users can saee the web interface and the critical `dnsmasq` error message shown in the diagnosis system. However, certain misconfigurations (e.g., `files.log.dnsmasq` invalid) can cause the process to get into some strange meta-state where `SIGTERM` is incorrectly translated into a meta-signal on which nobody is listening. As a result, the entire process starts to ignore `SIGTERM` altogether and can only forcefully be killed though `SIGKILL`. Finding this defect was quite a challenge which is carefully documented on the linked Discourse discussion. This PR contains a rather simple fix: Only translate `SIGTERM` into a meta-state if we know `dnsmasq` is up and running and handling signals as it should. If `dnsmasq` fails to initialize at any point, FTL remains active at reacting itself to `SIGTERM` events. #### How to test Set `files.log.dnsmasq = "/var/log/pih/pihole.log"` (or any other invalid path) and observe that `kill $(cat /var/run/pihole-FTL.pid)` does not cause FTL to actually terminate. Repeat on this branch and see that it does work. --- **Related issue or feature (if applicable):** https://discourse.pi-hole.net/t/suggested-systemd-service-file-to-insure-pihole-db-is-populated-with-data-if-dbinterval-is-non-default/71213 **Pull request in [docs](https://github.com/pi-hole/docs) with documentation (if applicable):** N/A --- **By submitting this pull request, I confirm the following:** 1. I have read and understood the [contributors guide](https://docs.pi-hole.net/guides/github/contributing/), as well as this entire template. I understand which branch to base my commits and Pull Requests against. 2. I have commented my proposed changes within the code. 3. I am willing to help maintain this change if there are issues with it later. 4. It is compatible with the [EUPL 1.2 license](https://opensource.org/licenses/EUPL-1.1) 5. I have squashed any insignificant commits. ([`git rebase`](http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html)) ## Checklist: - [x] The code change is tested and works locally. - [x] I based my code and PRs against the repositories `developmental` branch. - [x] I [signed off](https://docs.pi-hole.net/guides/github/how-to-signoff/) all commits. Pi-hole enforces the [DCO](https://docs.pi-hole.net/guides/github/dco/) for all contributions - [x] I [signed](https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits) all my commits. Pi-hole requires signatures to verify authorship - [x] I have read the above and my PR is ready for review.

For v5.0, you should be able to apply the same fix by ensuring the log file exists (presumably configured in /etc/dnsmasq.d/01-pihole.conf). If this file is missing (almost looks like this from the AUR Package site), not much will work anyway.

Bottom line: Glad we found the difference and at least for Pi-hole v6.0, the PR avoids the process becoming un-terminate-able.

graysky2 · July 25, 2024, 8:44pm

Arch does have both log files but they have been moved. See the patch I referenced several posts above. Here is the relevant section:

diff -uprN FTL-5.25.2/src/config.c FTL-5.25.2.cust/src/config.c
--- FTL-5.25.2/src/config.c     2024-05-08 21:59:50.000000000 +0200
+++ FTL-5.25.2.cust/src/config.c        2024-05-09 13:51:35.638439266 +0200
@@ -76,7 +76,7 @@ void getLogFilePath(void)
        }
 
        // Read LOGFILE value if available
-       // defaults to: "/var/log/pihole/FTL.log"
+       // defaults to: "/run/log/pihole-ftl/pihole-FTL.log"
        buffer = parse_FTLconf(fp, "LOGFILE");
 
        errno = 0;
@@ -84,7 +84,7 @@ void getLogFilePath(void)
        if(buffer == NULL || sscanf(buffer, "%127ms", &FTLfiles.log) != 1)
        {
                // Use standard path if no custom path was obtained from the config file
-               FTLfiles.log = strdup("/var/log/pihole/FTL.log");
+               FTLfiles.log = strdup("/run/log/pihole-ftl/pihole-FTL.log");
        }
 
        // Test if memory allocation was successful

I can confirm having both log files on the machine exhibiting the bug.

DL6ER · July 26, 2024, 4:35am

I don't see how this patch moves the file we're taking about: /var/log/pihole/pihole.log

graysky2 · July 26, 2024, 5:07pm

My bad, the Arch package for pi-hole as well as for pi-hole-FTL have been modified. It seems that the mods to pi-hole shown below are what is doing it. I am 100% sure that I have /run/log/pihole/pihole.log as a result.

See these two patches:
arch-server-core-5.18.3-1.patch
arch-server-admin-5.21-1.patch

Anf for the record, the PKGBUILD itself.

DL6ER · July 26, 2024, 7:34pm

Looking at the patches you linked here, it seems Arch missed the most important spot exactly causing what we've seen throughout this journey:

github.com/pi-hole/pi-hole

advanced/01-pihole.conf

be5a8dc51


      
          
          domain-needed
          
          localise-queries
          
          bogus-priv
          
          no-resolv
          
          log-queries
          log-facility=/var/log/pihole/pihole.log
          
          log-async

graysky2 · July 26, 2024, 8:01pm

I have that in /etc/dnsmasq.d/01-pihole.conf:

% grep log-fac /etc/dnsmasq.d/01-pihole.conf
log-facility=/run/log/pihole/pihole.log

It is included in dnsmasq.include which is installed to the package as /etc/dnsmasq.d/01-pihole.conf, see line 100 in the PKGBUILD:

install -Dm644 dnsmasq.include "$pkgdir"/etc/dnsmasq.d/01-pihole.conf

DL6ER · July 27, 2024, 6:27am

Then I really don't know (again) what the difference under Arch Linux is that is causing FTL to behave like this. TBH, it is quite inconvenient and confusing to try to debug based on reading diffs alone and the project scattered into so many individual packages in an environment I'm not familiar with. It seems there's always something I'm missing.

My suggestion at this point would maybe rather be focusing on bringing the AUR package up to speed with the development-v6 branches which are soon expected to become development and then (hopefully?) be released as Pi-hole v6.0 later this year. We already know from our tests above that SIGTERM does work here as expected.

graysky2 · July 27, 2024, 1:10pm

Thanks for the engagement on this @DL6ER