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

Prerelease of FTLDNS image #289

Closed
wants to merge 16 commits into from
Closed

Prerelease of FTLDNS image #289

wants to merge 16 commits into from

Conversation

diginc
Copy link
Collaborator

@diginc diginc commented Jul 5, 2018

Description

Pi-Hole FTLDNS pre release changes

Motivation and Context

Prep work to confirm the image is ready for FTLDNS

How Has This Been Tested?

pytests updated. Running resulting image for personal use.

Types of changes

New version!

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@diginc
Copy link
Collaborator Author

diginc commented Jul 12, 2018

Available test images:

  • For x86
    • diginc/pi-hole:prerelease
  • For ARM per FTLDNS #295
    • diginc/pi-hole-multiarch:debian_aarch64_prerelease
    • diginc/pi-hole-multiarch:debian_armhf_prerelease

@diginc
Copy link
Collaborator Author

diginc commented Jul 13, 2018

Reminder: contingent on the next release not using block pages, but NULL DNS responses by default, I may remove the validation of "ServerIP" being set and exiting the container startup when not set since it will not be required anymore.

Perhaps I'll add a 'BlockMode=X' env, when when set to 'Page' requires ServerIP also, but when set to 'Null' ServerIP would not be required.

@nightah
Copy link

nightah commented Jul 20, 2018

Looks like some of the default behaviour like grabbing and updating the default adlists doesn't work in this pre-release, not sure if that's by design or an oversight.

I also noticed that when updating the Top Domains and Top Clients list within the API/Web Interface location that dnsmasq errors would pop up, it looks like it was attempt to start up a seperate instance of dnsmasq when these were updated.

Restarting the container would resolve the error being thrown.

Happy to run it for longer and provide other feedback if necessary.

@diginc
Copy link
Collaborator Author

diginc commented Jul 20, 2018

There are no adlists by default in the new FTLDNS pi-hole installer, there is a prompt for users to select the current 'recommended' ones they want to opt into. I haven't made an equivalent way for users to opt-in to the lists of ads. I'll add something for this and do more checking on the updating.

Once the lists are selected they seemed to update OK during container startup and the web interface, I'm just re-using my old 3.3.1 container volume data to seed the lists in all real world tests so far though.

I saw the pihole-FTL restart errors too, I am pushing a fix for those now.

@nightah
Copy link

nightah commented Jul 20, 2018

I had previously specified the dns server for the pi-hole container as 127.0.0.1 and that seemed to have issues when I re-used my old container volume. When I removed that setting things seem to be working again.

Few other issues I seem to encounter with the new version:

  • Query Log is stuck on Processing... and does not show any recent queries.
  • Tools -> Tail pihole.log results in the following error: Failed to open log file. Check permissions!
    I suspect this is because it's running as UID/GID root / dip vs root / root

All the other features seem to more or less work appropriately with my limited testing.

EDIT: Seems the first issue with the Query Log might have been related to the my setup specifically with DNSSEC.
I actually have pi-hole pointing to my router and the router forwards all external DNS queries to Google's public DNS', however with this setup on the FTL branch all external queries fail.

@nightah
Copy link

nightah commented Jul 20, 2018

Also noticed that the both Query Types and Queries answered by pie charts on the Dashboard appear to have missing pieces.

You can see here what I mean. Again could be unrelated but that doesn't seem to be an issue on the main release. Happy to lodge this upstream if you're experiencing the same problem.

EDIT: The graphing problem was related to the browser cache, clearing this sorted this one out.

Have you noticed much of a difference in memory consumption with the FTLDNS branch?
I'm seeing 20-22x memory increase. Granted it's not a large amount despite the fact but it's significantly higher than the standard branch.

* Removed some old switch statements from alpine no longer required
* Limit parallel tests to 2 to help prevent test failure caused by race condition starting parallel tests/containers
* Began introducing a new ENV NO_SETUP to skip the majority of startup script 'setup' functions eventually

Signed-off-by: Adam Hill <adam@diginc.us>
Signed-off-by: Adam Hill <adam@diginc.us>
Signed-off-by: Adam Hill <adam@diginc.us>
@diginc diginc mentioned this pull request Aug 4, 2018
@diginc
Copy link
Collaborator Author

diginc commented Aug 4, 2018

Moving this branch to match pi-hole's release branch name scheme (release/v4.0) and made a new pull request: #300

@diginc diginc closed this Aug 4, 2018
@diginc diginc deleted the prerelease branch August 7, 2018 03:53
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

Successfully merging this pull request may close these issues.

2 participants