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

V2.11.1 update [WIP] #84

Merged
merged 30 commits into from Jan 18, 2017
Merged

V2.11.1 update [WIP] #84

merged 30 commits into from Jan 18, 2017

Conversation

diginc
Copy link
Collaborator

@diginc diginc commented Jan 8, 2017

Alpine's build is in progress still but debian and debian arm are building using a major Dockerfile refactor. The following debian tags are available for testing (I don't quite trust/have not vetted them enough to promote to defaults yet):

diginc/pi-hole:debian_v2.11.1
diginc/pi-hole:arm_v2.11.1

The official scripts used to install pi-hole have come a long way from when I wrote my own version of their installers in a dockerized way. Since they've improved so much I'm no longer running my own version of the package requirements or pi-hole script installations. This helps because when changes are made to the core installer I simply import them to the dockerfile rather than reproduce them.

A side effect of using the official installer is the .git data is left in the docker, enable rolling updates (untested). Destroying and pulling new images is still a superior update pattern for docker IMO, more moving parts = more problems. This is a feature I don't plan on spending a lot of time testing and will always go to my answer of 'docker rm/docker up` when problems occur as a result of trying an upgrade in this style.

There is still some cleanup that needs to be done on this branch like deleting the git submodules since git clones are used. Ideally I want the git repos be cloned to specific version tags for reproduce-ability too (old builds can be re-done and patched if necessary)

diginc and others added 10 commits November 12, 2016 18:08
* Noticed:
 * tini arm is out!  Go ARM TINI - thanks @krallin
 * tini alpine is behind on 0.9 - grabed latest the same way as debians
 * Redundent run steps between requirements and tini, merged
 * inconsistency in dockerfiles, made consistent
 * I kept writing noticed in front of all these items, used nesting instead
Tini update and arm version
@diginc
Copy link
Collaborator Author

diginc commented Jan 8, 2017

Ping #79 #81

Please checkout the dev tags I made for this branch.

@diginc diginc mentioned this pull request Jan 8, 2017
@diginc
Copy link
Collaborator Author

diginc commented Jan 9, 2017

Tests still need to be fixed. Hope to get that done tomorrow.

@wtreur
Copy link
Contributor

wtreur commented Jan 12, 2017

Not sure if this is the right place to put this, so please forgive me. I'm willing to create a separate ticket if needed.

Pi-hole recently introduced standalone password protection for the admin page [1] Version 2.11.1 is the first one for docker-pi-hole, where you can use this. I noticed the following bug/unsupported feature.

  • When running the 2.11.1-debian container I'm setting the password using docker exec docker exec my-pihole-container sudo pihole -a -p my-ultra-secret-password
  • This will put the password hash in /etc/pihole/setupVars.conf and the admin ui is now password protrected
  • Note that I've set /etc/pihole as docker-volume so these files are persistent between container instances (which I confirmed by adding my own empty test file in there)
  • However, when destroying and restarting a new container, the password hash is removed from /etc/pihole/setupVars.conf

I am new to pi-hole so I couldn't immediately figure out how to fix, but I'll try to dig into it a bit further when I have some spare time later this week.

[1] - pi-hole/web#197

@diginc
Copy link
Collaborator Author

diginc commented Jan 12, 2017

@wtreur sounds like you might have lost the volume of the original container and created another volume, instead of re-using/attaching the original. Maybe check docker volume ls to see if it's showing new ones spawned when you go through your cycle of creating a new container.

Personally I never trust docker's volumes without a host directory mapping. I always have a src directory on my docker host like: -v /dockerdata/etc/pihole:/etc/pihole

@wtreur
Copy link
Contributor

wtreur commented Jan 13, 2017

@diginc This doesn't seem to be the case. This is wat I tried:

  1. Run container with mounted volume:
    docker run --name pwd-test -v $(pwd)/etc/:/etc/pihole/ -d -e ServerIP=127.0.0.1 diginc/pi-hole:debian_v2.11.1

  2. Set the password:
    docker exec pwd-test sudo pihole -a -p my-ultra-secret-password

  3. Confirm password is stored on host dir
    more ./etc/setupVars.conf
    Notice the WEBPASSWORD in the output:

    IPV4_ADDRESS=127.0.0.1
    IPV6_ADDRESS=
    PIHOLE_DNS_1=8.8.8.8
    PIHOLE_DNS_2=8.8.4.4
    DNS_FQDN_REQUIRED=false
    DNS_BOGUS_PRIV=false
    WEBPASSWORD=45b6827f77170a90f2a683572685ffbe934e72e16fbafa9f3dd0c468bddd45f5

  4. Stop and remove container
    docker stop pwd-test && docker rm pwd-test

  5. Start new container with mounted volume
    docker run --name pwd-test -v $(pwd)/etc/:/etc/pihole/ -d -e ServerIP=127.0.0.1 diginc/pi-hole:debian_v2.11.1

  6. Check password
    more ./etc/setupVars.conf
    Notice no WEBPASSWORD in the output:

    IPV4_ADDRESS=127.0.0.1
    IPV6_ADDRESS=
    PIHOLE_DNS_1=8.8.8.8
    PIHOLE_DNS_2=8.8.4.4
    DNS_FQDN_REQUIRED=false
    DNS_BOGUS_PRIV=false

I highly suspect this is because the pi-hole installation overwrites this file. Changes in whitelist.txt are persistent between container instances.

@matthewdavis
Copy link

Just deployed the new 2.11.1 container and things look ok so far. Nicely done.

@diginc
Copy link
Collaborator Author

diginc commented Jan 17, 2017

Thanks, good to hear. The debian tag is working in my environment pretty well too. Since I haven't given an update recently here is what I've got done / to do:

  • I have some new changes that I haven't pushed which I believe addresses @wtreur's issue.
  • I've yet to figure out what broke alpine's PHP though
    • All ad URLs end up as direct downloads instead of rendered pages. Admin works fine though.
  • There are a couple tests that I think maybe invalid or I haven't figure out entirely how to reproduce the broken test manually yet.
  • Figured out the weird travis-ci build bug is because I wan't pointing the py.test cmd directly at the tests folder.

I'll push what I have tonight and hopefully get the remaining issues ironed out soon.

@mat1th
Copy link

mat1th commented Jan 18, 2017

Nice work! Thank you for your time you spend in this project!

@wtreur
Copy link
Contributor

wtreur commented Jan 18, 2017

Thanks for doing this! I will try to take a look at the password fix later today.

@diginc
Copy link
Collaborator Author

diginc commented Jan 18, 2017

I think there are some more tests to run against the new pi-hole functionality but the base ad blocking functionality is working so I'll roll this out to master.

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.

None yet

7 participants