remove ServerIP requirement, simpler example commands

Signed-off-by: Adam Hill <adam@diginc.us> Signed-off-by: Martin Buchleitner <mabunixda@gmail.com>
2019-02-07 19:51:15 -06:00 · 2019-02-07 19:51:15 -06:00 · 397ec03285
parent 138e061c17
commit 397ec03285
13 changed files with 135 additions and 97 deletions
--- a/Dockerfile.template
+++ b/Dockerfile.template
@ -30,6 +30,9 @@ EXPOSE 443
 ENV S6_LOGGING 0
 ENV S6_KEEP_ENV 1
 ENV S6_BEHAVIOUR_IF_STAGE2_FAILS 2
+
+ENV ServerIP 0.0.0.0
+ENV ServerIPv6 ::
 ENV FTL_CMD no-daemon
 ENV DNSMASQ_USER root

--- a/3
+++ b/3
@ -30,6 +30,9 @@ EXPOSE 443
 ENV S6_LOGGING 0
 ENV S6_KEEP_ENV 1
 ENV S6_BEHAVIOUR_IF_STAGE2_FAILS 2
+
+ENV ServerIP 0.0.0.0
+ENV ServerIPv6 ::
 ENV FTL_CMD no-daemon
 ENV DNSMASQ_USER root

--- a/3
+++ b/3
@ -30,6 +30,9 @@ EXPOSE 443
 ENV S6_LOGGING 0
 ENV S6_KEEP_ENV 1
 ENV S6_BEHAVIOUR_IF_STAGE2_FAILS 2
+
+ENV ServerIP 0.0.0.0
+ENV ServerIPv6 ::
 ENV FTL_CMD no-daemon
 ENV DNSMASQ_USER root

--- a/3
+++ b/3
@ -30,6 +30,9 @@ EXPOSE 443
 ENV S6_LOGGING 0
 ENV S6_KEEP_ENV 1
 ENV S6_BEHAVIOUR_IF_STAGE2_FAILS 2
+
+ENV ServerIP 0.0.0.0
+ENV ServerIPv6 ::
 ENV FTL_CMD no-daemon
 ENV DNSMASQ_USER root

--- a/3
+++ b/3
@ -30,6 +30,9 @@ EXPOSE 443
 ENV S6_LOGGING 0
 ENV S6_KEEP_ENV 1
 ENV S6_BEHAVIOUR_IF_STAGE2_FAILS 2
+
+ENV ServerIP 0.0.0.0
+ENV ServerIPv6 ::
 ENV FTL_CMD no-daemon
 ENV DNSMASQ_USER root

--- a/README.md
+++ b/README.md
@ -5,14 +5,53 @@
 </p>
 <!-- Delete above HTML and insert markdown for dockerhub : ![Pi-hole](https://pi-hole.github.io/graphics/Vortex/Vortex_with_text.png) -->

+## Quick Start
+
+[Docker-compose](https://docs.docker.com/compose/install/) example:
+
+```yaml
+version: "3"
+services:
+  pihole:
+    container_name: pihole
+    image: pihole/pihole:latest
+    # For DHCP it is recommended to remove these ports and instead add: network_mode: "host"
+    ports:
+      - "53:53/tcp"
+      - "53:53/udp"
+      - "80:80/tcp"
+      - "443:443/tcp"
+    environment:
+      TZ: 'America/Chicago'
+      # WEBPASSWORD: 'set a secure password here or it will be random'
+    # Volumes store your data between container upgrades
+    volumes:
+       - './etc-pihole/:/etc/pihole/'
+       - './etc-dnsmasq.d/:/etc/dnsmasq.d/'
+    dns:
+      - 127.0.0.1
+      - 1.1.1.1
+    # Recommended but not required (DHCP needs NET_ADMIN)
+    #   https://github.com/pi-hole/docker-pi-hole#note-on-capabilities
+    cap_add:
+      - NET_ADMIN
+    restart: unless-stopped
+```
+
+[Here is an equivilent docker run script](https://github.com/pi-hole/docker-pi-hole/blob/master/docker_run.sh).
+
+
+## Docker Pi-Hole v4.2.1
+
+ServerIP no longer a required enviroment variable!  Feel free to remove it unless you need it to customize lighttpd
+
 ## Docker Pi-Hole v4.1.1+ IMPORTANT upgrade notes

-Starting with the v4.1.1 release your Pi-hole container may encounter issues starting the DNS service unless ran with the following settings:
+Starting with the v4.1.1 release your Pi-hole container may encounter issues starting the DNS service unless ran with the following setting:

- `--cap-add=NET_ADMIN` This previously optional argument is now required or strongly encouraged
-    - Starting in a future version FTLDNS is going to check this setting automatically
 - `--dns=127.0.0.1 --dns=1.1.1.1` The second server can be any DNS IP of your choosing, but the **first dns must be 127.0.0.1**
    - A WARNING stating "Misconfigured DNS in /etc/resolv.conf" may show in docker logs without this.
+- 4.1 required --net-admin

 These are the raw [docker run cli](https://docs.docker.com/engine/reference/commandline/cli/) versions of the commands.  We provide no official support for docker GUIs but the community forums may be able to help if you do not see a place for these settings.  Remember, always consult your manual too!

--- a/bash_functions.sh
+++ b/bash_functions.sh
@ -86,12 +86,7 @@ prepare_configs() {
 }

 validate_env() {
-    if [ -z "$ServerIP" ] ; then
-      echo "ERROR: To function correctly you must pass an environment variables of 'ServerIP' into the docker container with the IP of your docker host from which you are passing web (80) and dns (53) ports from"
-      exit 1
-    fi;
-
-    # Required ServerIP is a valid IP
+    # Optional ServerIP is a valid IP
    # nc won't throw any text based errors when it times out connecting to a valid IP, otherwise it complains about the DNS name being garbage
    # if nc doesn't behave as we expect on a valid IP the routing table should be able to look it up and return a 0 retcode
    if [[ "$(nc -4 -w1 -z "$ServerIP" 53 2>&1)" != "" ]] || ! ip route get "$ServerIP" > /dev/null ; then
@ -182,12 +177,12 @@ setup_dnsmasq() {
    setup_dnsmasq_dns "$dns1" "$dns2" 
    setup_dnsmasq_interface "$interface"
    setup_dnsmasq_listening_behaviour "$dnsmasq_listening_behaviour"
-    setup_dnsmasq_user "${DNSMASQ_USER:-root}"
+    setup_dnsmasq_user "${DNSMASQ_USER}"
    ProcessDNSSettings
 }

 setup_dnsmasq_user() {
-    DNSMASQ_USER="${1}"
+    local DNSMASQ_USER="${1}"

    # Run DNSMASQ as root user to avoid SHM permission issues
    if grep -r -q '^\s*user=' /etc/dnsmasq.* ; then
--- a/docker_run.sh
+++ b/docker_run.sh
@ -1,35 +1,16 @@
-#!/bin/bash -e
-# Lookups may not work for VPN / tun0
-IP_LOOKUP="$(ip route get 8.8.8.8 | awk '{for(i=1;i<=NF;i++) if ($i=="src") print $(i+1)}')"
-IPv6_LOOKUP="$(ip -6 route get 2001:4860:4860::8888 | awk '{for(i=1;i<=NF;i++) if ($i=="src") print $(i+1)}')"
-
-# Just hard code these to your docker server's LAN IP if lookups aren't working
-IP="${IP:-$IP_LOOKUP}"  # use $IP, if set, otherwise IP_LOOKUP
-IPv6="${IPv6:-$IPv6_LOOKUP}"  # use $IPv6, if set, otherwise IP_LOOKUP
-
-# Default of directory you run this from, update to where ever.
-DOCKER_CONFIGS="$(pwd)"
-
-echo -e "### Make sure your IPs are correct, hard code ServerIP ENV VARs if necessary\nIP: ${IP}\nIPv6: ${IPv6}"
-
-# Default ports + daemonized docker container
-# Environment variables for docker can be defined in --env-file .env file
+#!/bin/bash
 docker run -d \
    --name pihole \
-    --env-file .env \
    -p 53:53/tcp -p 53:53/udp \
    -p 80:80 \
    -p 443:443 \
-    --cap-add=NET_ADMIN \
-    -v "${DOCKER_CONFIGS}/pihole/:/etc/pihole/" \
-    -v "${DOCKER_CONFIGS}/dnsmasq.d/:/etc/dnsmasq.d/" \
-    -e ServerIP="${IP}" \
-    -e ServerIPv6="${IPv6}" \
-    --restart=unless-stopped \
+    -e TZ="America/Chicago" \
+    -v "$(pwd)/etc-pihole/:/etc/pihole/" \
+    -v "$(pwd)/etc-dnsmasq.d/:/etc/dnsmasq.d/" \
    --dns=127.0.0.1 --dns=1.1.1.1 \
+    --restart=unless-stopped \
    pihole/pihole:latest

-
 printf 'Starting up pihole container '
 for i in $(seq 1 20); do
    if [ "$(docker inspect -f "{{.State.Health.Status}}" pihole)" == "healthy" ] ; then
--- a/doco-example.yml
+++ b/doco-example.yml
@ -1,36 +1,26 @@
 version: "3"
 services:
  pihole:
+    container_name: pihole
    image: pihole/pihole:latest
-    dns:
-      - 127.0.0.1
-      - 1.1.1.1
+    restart: unless-stopped
+    # If using DHCP it is recommended to comment ports and add: network_mode: "host"
    ports:
      - "53:53/tcp"
      - "53:53/udp"
      - "80:80/tcp"
      - "443:443/tcp"
+    environment:
+      TZ: 'America/Chicago'
+      # WEBPASSWORD: 'set a secure password here or it will be random'
+    # Volumes store your data between container upgrades
+    volumes:
+       - './etc-pihole/:/etc/pihole/'
+       - './etc-dnsmasq.d/:/etc/dnsmasq.d/'
+    dns:
+      - 127.0.0.1
+      - 1.1.1.1
+    # Recommended but not required (DHCP needs NET_ADMIN)
+    #   https://github.com/pi-hole/docker-pi-hole#note-on-capabilities
    cap_add:
      - NET_ADMIN
-    # required if Pi-hole is to provide DHCP
-    # network_mode: "host"
-    environment:
-      # enter your docker host IP here
-      ServerIP:
-      # IPv6 Address if your network supports it
-      #ServerIPv6: 192.168.1.55
-      # jwilder/proxy envs, see readme for more info
-      PROXY_LOCATION: pihole
-      VIRTUAL_HOST: pihole.yourdomain.local
-      VIRTUAL_PORT: 80
-    # Add your own custom hostnames you need for your domain
-    extra_hosts:
-      #   Point any of the jwilder virtual_host addresses
-      # to your docker host ip address
-      - 'pihole.yourdomain.local:192.168.1.55'
-    volumes:
-      # - '/etc/pihole/:/etc/pihole/'
-      # WARNING: if this log don't exist as a file on the host already
-      # docker will try to create a directory in it's place making for lots of errors
-      # - '/var/log/pihole.log:/var/log/pihole.log'
-    restart: always
--- a/s6/debian-root/etc/services.d/pihole-FTL/run
+++ b/s6/debian-root/etc/services.d/pihole-FTL/run
@ -1,7 +1,9 @@
 #!/usr/bin/with-contenv bash

-DNSMASQ_USER=${DNSMASQ_USER:-root}
-
 s6-echo "Starting pihole-FTL ($FTL_CMD) as ${DNSMASQ_USER}"
-# /var/log/pihole*.log has this data - this duplicates it in docker logs unless you send to dev null
 s6-setuidgid ${DNSMASQ_USER} pihole-FTL $FTL_CMD >/dev/null 2>&1
+
+# Notes on above:
+# - DNSMASQ_USER default of root is in Dockerfile & can be overwritten by runtime container env
+# - /var/log/pihole*.log has FTL's output that no-daemon would normally print in FG too
+#   prevent duplicating it in docker logs by sending to dev null
--- a/test/conftest.py
+++ b/test/conftest.py
@ -15,10 +15,6 @@ with open('{}/VERSION'.format(dotdot), 'r') as v:
 def args_dns():
    return '--dns 127.0.0.1 --dns 1.1.1.1'

-@pytest.fixture()
-def args_caps():
-    return '--cap-add=NET_ADMIN'
-
@pytest.fixture()
 def args_volumes():
    return '-v /dev/null:/etc/pihole/adlists.default'
@ -28,21 +24,21 @@ def args_env():
    return '-e ServerIP="127.0.0.1" -e ServerIPv6="::1"'

@pytest.fixture()
-def args(args_dns, args_caps, args_volumes, args_env):
-    return "{} {} {} {}".format(args_dns, args_caps, args_volumes, args_env)
+def args(args_dns, args_volumes, args_env):
+    return "{} {} {}".format(args_dns, args_volumes, args_env)

@pytest.fixture()
-def test_args(request):
-    ''' arguments provided by tests '''
+def test_args():
+    ''' test override fixture to provide arguments seperate from our core args '''
    return ''

-def DockerGeneric(request, args, test_args, image, cmd, entrypoint):
+def DockerGeneric(request, _test_args, _args, _image, _cmd, _entrypoint):
    assert 'docker' in check_output('id'), "Are you in the docker group?"
    # Always appended PYTEST arg to tell pihole we're testing
-    if 'pihole' in image:
-       args = '{} -e PYTEST=1'.format(args)
+    if 'pihole' in _image and 'PYTEST=1' not in _args:
+       _args = '{} -e PYTEST=1'.format(_args)
    docker_run = 'docker run -d -t {args} {test_args} {entry} {image} {cmd}'\
-        .format(args=args, test_args=test_args, entry=entrypoint, image=image, cmd=cmd)
+        .format(args=_args, test_args=_test_args, entry=_entrypoint, image=_image, cmd=_cmd)
    print docker_run
    docker_id = check_output(docker_run)

@ -67,6 +63,7 @@ def DockerGeneric(request, args, test_args, image, cmd, entrypoint):
        return out

    funcType = type(docker_container.run)
+    # override run function to use bash not sh
    docker_container.run = funcType(run_bash, docker_container, testinfra.backend.docker.DockerBackend)
    return docker_container

@ -77,11 +74,10 @@ def Docker(request, test_args, args, image, cmd, entrypoint):
    return DockerGeneric(request, test_args, args, image, cmd, entrypoint)

@pytest.fixture(scope='module')
-def DockerPersist(request, persist_args, persist_image, persist_cmd, Dig):
+def DockerPersist(request, persist_test_args, persist_args, persist_image, persist_cmd, persist_entrypoint, Dig):
    ''' Persistent Docker container for multiple tests, instead of stopping container after one test '''
    ''' Uses DUP'd module scoped fixtures because smaller scoped fixtures won't mix with module scope '''
-    default_args = '--dns 127.0.0.1 --dns 1.1.1.1 -v /dev/null:/etc/pihole/adlists.default -e PYTEST=1 --cap-add=NET_ADMIN'
-    persistent_container = DockerGeneric(request, default_args, persist_args, persist_image, persist_cmd, '')
+    persistent_container = DockerGeneric(request, persist_test_args, persist_args, persist_image, persist_cmd, persist_entrypoint)
    ''' attach a dig conatiner for lookups '''
    persistent_container.dig = Dig(persistent_container.id)
    return persistent_container
@ -95,58 +91,79 @@ def arch(request):
    return request.param

@pytest.fixture()
-def version(request):
+def version():
    return __version__

@pytest.fixture()
-def tag(request, version, arch):
+def tag(version, arch):
    return '{}_{}'.format(version, arch)

@pytest.fixture
-def webserver(request, tag):
+def webserver(tag):
    ''' TODO: this is obvious without alpine+nginx as the alternative, remove fixture, hard code lighttpd in tests? '''
    return 'lighttpd'

@pytest.fixture()
-def image(request, tag):
+def image(tag):
    image = 'pihole'
    return '{}:{}'.format(image, tag)

@pytest.fixture()
-def cmd(request):
+def cmd():
    return 'tail -f /dev/null'

-@pytest.fixture(scope='module', params=['amd64'])
-def persist_arch(request):
+@pytest.fixture(scope='module')
+def persist_arch():
    '''amd64 only, dnsmasq/pihole-FTL(?untested?) will not start under qemu-user-static :('''
-    return request.param
+    return 'amd64'

@pytest.fixture(scope='module')
-def persist_version(request):
+def persist_version():
    return __version__

@pytest.fixture(scope='module')
-def persist_args(request):
+def persist_args_dns():
+    return '--dns 127.0.0.1 --dns 1.1.1.1'
+
+@pytest.fixture(scope='module')
+def persist_args_volumes():
+    return '-v /dev/null:/etc/pihole/adlists.default'
+
+@pytest.fixture(scope='module')
+def persist_args_env():
    return '-e ServerIP="127.0.0.1" -e ServerIPv6="::1"'

@pytest.fixture(scope='module')
-def persist_tag(request, persist_version, persist_arch):
+def persist_args(persist_args_dns, persist_args_volumes, persist_args_env):
+    return "{} {} {}".format(args_dns, args_volumes, args_env)
+
+@pytest.fixture(scope='module')
+def persist_test_args():
+    ''' test override fixture to provide arguments seperate from our core args '''
+    return ''
+
+@pytest.fixture(scope='module')
+def persist_tag(persist_version, persist_arch):
    return '{}_{}'.format(persist_version, persist_arch)

@pytest.fixture(scope='module')
-def persist_webserver(request, persist_tag):
+def persist_webserver(persist_tag):
    ''' TODO: this is obvious without alpine+nginx as the alternative, remove fixture, hard code lighttpd in tests? '''
    return 'lighttpd'

@pytest.fixture(scope='module')
-def persist_image(request, persist_tag):
+def persist_image(persist_tag):
    image = 'pihole'
    return '{}:{}'.format(image, persist_tag)

@pytest.fixture(scope='module')
-def persist_cmd(request):
+def persist_cmd():
    return 'tail -f /dev/null'

+@pytest.fixture(scope='module')
+def persist_entrypoint():
+    return ''
+
@pytest.fixture
 def Slow():
    """
@ -168,7 +185,7 @@ def Slow():
    return slow

@pytest.fixture(scope='module')
-def Dig(request):
+def Dig():
    ''' separate container to link to pi-hole and perform lookups '''
    ''' a docker pull is faster than running an install of dnsutils '''
    def dig(docker_id):
--- a/test/test_pihole_scripts.py
+++ b/test/test_pihole_scripts.py
@ -1,7 +1,7 @@
 import pytest


-@pytest.fixture
+@pytest.fixture(scope='module')
 def start_cmd():
    ''' broken by default, required override '''
    return None
--- a/test/test_start.py
+++ b/test/test_start.py
@ -5,9 +5,8 @@ import time
        docker containers (esp. musl based OSs) stripped down nature '''

 # If the test runs /start.sh, do not let s6 run it too!  Kill entrypoint to avoid race condition/duplicated execution
-@pytest.mark.parametrize('entrypoint,cmd', [('--entrypoint=tail','-f /dev/null')])
-@pytest.mark.parametrize('args', [ '' ])
-def test_ServerIP_missing_triggers_start_error(Docker):
+@pytest.mark.parametrize('persist_entrypoint,persist_cmd,persist_args_env', [('--entrypoint=tail','-f /dev/null','')])
+def test_ServerIP_missing_is_not_required_anymore(RunningPiHole):
    ''' When args to docker are empty start.sh exits saying ServerIP is required '''
    start = Docker.run('/start.sh')
    error_msg = "ERROR: To function correctly you must pass an environment variables of 'ServerIP' into the docker container"