Merge pull request #1864 from pi-hole/development

Development

.github/workflows/build-and-publish.yml

@@ -16,11 +16,23 @@ env:
 
 jobs:
   build:
-    runs-on: ubuntu-latest
+    runs-on: ${{ matrix.runner }}
     strategy:
       fail-fast: false
       matrix:
-        platform: [linux/amd64, linux/386, linux/arm/v6, linux/arm/v7, linux/arm64, linux/riscv64]
+        include:
+          - platform: linux/amd64
+            runner: ubuntu-latest
+          - platform: linux/386
+            runner: ubuntu-latest
+          - platform: linux/arm/v6
+            runner: ubuntu-24.04-arm
+          - platform: linux/arm/v7
+            runner: ubuntu-24.04-arm
+          - platform: linux/arm64
+            runner: ubuntu-24.04-arm
+          - platform: linux/riscv64
+            runner: ubuntu-24.04-arm
           
     steps:
       - name: Prepare name for digest up/download

.github/workflows/build-and-test.yml

@@ -3,30 +3,49 @@ on:
   pull_request:
 
 jobs:
-  test:
+  build-and-test:
-    runs-on: ubuntu-latest
+    runs-on: ${{ matrix.runner }}
     strategy:
       fail-fast: false
       matrix:
-        # Official docker images for docker are only available for amd64 and arm64
+        include:
-        # TODO: Look at: https://github.com/docker-library/official-images#architectures-other-than-amd64
+          - platform: linux/amd64
-        # Is testing on all platforms really necessary?
+            runner: ubuntu-latest
-        # Disabled arm64 tests for the time being, something is wrong with the test config and the volumes are getting shared between the test containers on different architectures
+          - platform: linux/386
-        #platform: [linux/amd64, linux/arm64]
+            runner: ubuntu-latest
-        platform: [linux/amd64]
+          - platform: linux/arm/v6
+            runner: ubuntu-24.04-arm
+          - platform: linux/arm/v7
+            runner: ubuntu-24.04-arm
+          - platform: linux/arm64
+            runner: ubuntu-24.04-arm
+          - platform: linux/riscv64
+            runner: ubuntu-24.04-arm
+    env:
+      CI_ARCH: ${{ matrix.platform }}
     steps:
     - name: Checkout Repo
       uses: actions/checkout@v4
 
     - name: Set up QEMU
       uses: docker/setup-qemu-action@v3
+
+    - name: Set up Python
+      uses: actions/setup-python@v5.6.0
       with:
-        platforms: ${{ matrix.platform }}
+        python-version: "3.13"
-
+    
-    - name: Set up Docker Buildx
+    - name: Run black formatter
-      uses: docker/setup-buildx-action@v3
-
-    - name: Run Tests
       run: |
-        echo "Building image to test"
+        pip install black
-        PLATFORM=${{ matrix.platform }} ./build-and-test.sh
+        black --check --diff test/tests/
+
+    - name: Install wheel
+      run:  pip install wheel
+
+    - name: Install dependencies
+      run: pip install -r test/requirements.txt
+
+    - name: Test with tox
+      run: |
+        CIPLATFORM=${{ env.CI_ARCH }} tox -c test/tox.ini

README.md

@@ -144,7 +144,7 @@ If this variable is not detected and you have not already set one via `pihole se
 | `FTLCONF_[SETTING]` | unset | As per documentation | Customize pihole.toml with settings described in the [API Documentation](https://docs.pi-hole.net/api).<br><br>Replace `.` with `_`, e.g for `dns.dnssec=true` use `FTLCONF_dns_dnssec: 'true'`.<br/>Array type configs should be delimited with `;`.|
 | `PIHOLE_UID` | `1000` | Number | Overrides image's default pihole user id to match a host user id.<br/>**IMPORTANT**: id must not already be in use inside the container!|
 | `PIHOLE_GID` | `1000` | Number | Overrides image's default pihole group id to match a host group id.<br/>**IMPORTANT**: id must not already be in use inside the container!|
-| `WEBPASSWORD_FILE` | unset| `<Docker secret file>` | Set an Admin password using [Docker secrets](https://docs.docker.com/engine/swarm/secrets/). If `FTLCONF_webserver_api_password` is set, `WEBPASSWORD_FILE` is ignored. If `FTLCONF_webserver_api_password` is empty, and `WEBPASSWORD_FILE` is set to a valid readable file, then `FTLCONF_webserver_api_password` will be set to the contents of `WEBPASSWORD_FILE`. |
+| `WEBPASSWORD_FILE` | unset| `<Docker secret file>` | Set an Admin password using Docker secrets with [Swarm](https://docs.docker.com/engine/swarm/secrets/) or [Compose](https://docs.docker.com/compose/how-tos/use-secrets/). If `FTLCONF_webserver_api_password` is set, `WEBPASSWORD_FILE` is ignored. If `FTLCONF_webserver_api_password` is empty, and `WEBPASSWORD_FILE` is set to a valid readable file, then `FTLCONF_webserver_api_password` will be set to the contents of `WEBPASSWORD_FILE`. See [WEBPASSWORD_FILE Example](https://docs.pi-hole.net/docker/configuration/#webpassword_file-example) for additional information.|
 
 ### Advanced Variables
 
@@ -183,36 +183,8 @@ Here is a rundown of other arguments for your docker-compose / docker run.
 - Docker's default network mode `bridge` isolates the container from the host's network. This is a more secure setting, but requires setting the Pi-hole DNS option for _Interface listening behavior_ to "Listen on all interfaces, permit all origins".
 - If you're using a Red Hat based distribution with an SELinux Enforcing policy, add `:z` to line with volumes.
 
-### Installing on Ubuntu or Fedora
+> [!TIP]
-
+> All further tips and tricks can be found in the [Pi-hole documentation](https://docs.pi-hole.net/docker/tips-and-tricks/)
-Modern releases of Ubuntu (17.10+) and Fedora (33+) include [`systemd-resolved`](http://manpages.ubuntu.com/manpages/bionic/man8/systemd-resolved.service.8.html) which is configured by default to implement a caching DNS stub resolver. This will prevent pi-hole from listening on port 53.
-The stub resolver should be disabled with: `sudo sed -r -i.orig 's/#?DNSStubListener=yes/DNSStubListener=no/g' /etc/systemd/resolved.conf`.
-
-This will not change the nameserver settings, which point to the stub resolver thus preventing DNS resolution. Change the `/etc/resolv.conf` symlink to point to `/run/systemd/resolve/resolv.conf`, which is automatically updated to follow the system's [`netplan`](https://netplan.io/):
-`sudo sh -c 'rm /etc/resolv.conf && ln -s /run/systemd/resolve/resolv.conf /etc/resolv.conf'`.
-After making these changes, you should restart systemd-resolved using `systemctl restart systemd-resolved`.
-
-Once pi-hole is installed, you'll want to configure your clients to use it ([see here](https://discourse.pi-hole.net/t/how-do-i-configure-my-devices-to-use-pi-hole-as-their-dns-server/245)). If you used the symlink above, your docker host will either use whatever is served by DHCP, or whatever static setting you've configured. If you want to explicitly set your docker host's nameservers you can edit the netplan(s) found at `/etc/netplan`, then run `sudo netplan apply`.
-
-Example netplan:
-
-```yaml
-network:
-    ethernets:
-        ens160:
-            dhcp4: true
-            dhcp4-overrides:
-                use-dns: false
-            nameservers:
-                addresses: [127.0.0.1]
-    version: 2
-```
-
-Note that it is also possible to disable `systemd-resolved` entirely. However, this can cause problems with name resolution in vpns ([see bug report](https://bugs.launchpad.net/network-manager/+bug/1624317)).\
-It also disables the functionality of netplan since systemd-resolved is used as the default renderer ([see `man netplan`](http://manpages.ubuntu.com/manpages/bionic/man5/netplan.5.html#description)).\
-If you choose to disable the service, you will need to manually set the nameservers, for example by creating a new `/etc/resolv.conf`.
-
-Users of older Ubuntu releases (circa 17.04) will need to disable dnsmasq.
 
 ## Installing on Dokku
 

build-and-test.sh

@@ -1,24 +0,0 @@
-#!/usr/bin/env bash
-set -ex
-
-if [[ "$1" == "enter" ]]; then
-    enter="-it"
-    cmd="sh"
-fi
-
-GIT_BRANCH=$(git rev-parse --abbrev-ref HEAD | sed "s/\//-/g")
-GIT_TAG=$(git describe --tags --exact-match 2>/dev/null || true)
-GIT_TAG="${GIT_TAG:-$GIT_BRANCH}"
-PLATFORM="${PLATFORM:-linux/amd64}"
-
-# generate and build dockerfile
-docker buildx build --load --platform=${PLATFORM} --tag image_pipenv --file test/Dockerfile test/
-docker run --rm \
-    --volume /var/run/docker.sock:/var/run/docker.sock \
-    --volume "$(pwd):/$(pwd)" \
-    --workdir "$(pwd)" \
-    --env PIPENV_CACHE_DIR="$(pwd)/.pipenv" \
-    --env GIT_TAG="${GIT_TAG}" \
-    --env PY_COLORS=1 \
-    --env TARGETPLATFORM="${PLATFORM}" \
-    ${enter} image_pipenv ${cmd}

src/Dockerfile

@@ -1,17 +1,7 @@
 # syntax=docker/dockerfile:1
 ARG FTL_SOURCE=remote
 # Pull Stable images
-FROM alpine:3.22 AS base-stable
+FROM alpine:3.22 AS base
-FROM base-stable AS base-386
-FROM base-stable AS base-amd64
-FROM base-stable AS base-arm
-FROM base-stable AS base-arm64
-# Pull Edge images
-FROM alpine:edge AS base-edge
-FROM base-edge   AS base-riscv64
-# Use the base image for the current architecture
-FROM base-${TARGETARCH} AS base
-# https://docs.docker.com/engine/reference/builder/#automatic-platform-args-in-the-global-scope
 
 ARG TARGETPLATFORM
 ARG WEB_BRANCH="development"
@@ -40,7 +30,7 @@ RUN apk add --no-cache \
     git \
     # Install grep to avoid issues in pihole -w/b with the default busybox grep
     grep \
-    iproute2-ss \
+    iproute2 \
     jq \
     libcap \
     logrotate \

test/Dockerfile

@@ -1,22 +0,0 @@
-ARG alpine_version="3.22"
-ARG docker_version="28.2.2"
-
-FROM docker:${docker_version}-cli-alpine${alpine_version}
-
-COPY --chmod=0755 ./cmd.sh /usr/local/bin/cmd.sh
-COPY requirements.txt /root/
-WORKDIR /root
-
-RUN apk add --no-cache \
-    python3-dev \
-    py3-pip \
-    curl \
-    && pip3 install --break-system-packages --no-cache-dir -U pip  \
-    && pip3 install --break-system-packages --no-cache-dir -r requirements.txt \
-    # Replace hardcoded /bin/sh with /bin/bash in testinfra docker backend
-    # see https://github.com/pytest-dev/pytest-testinfra/issues/582 and similar issues
-    && pythonversion=$(python3 -c 'import sys; print(".".join(map(str, sys.version_info[:2])))') \
-    && sed -i 's|/bin/sh|/bin/bash|g' /usr/lib/python${pythonversion}/site-packages/testinfra/backend/docker.py
-
-SHELL ["/bin/sh", "-c"]
-CMD ["cmd.sh"]

test/TESTING.md

@@ -1,19 +1,12 @@
 # Prerequisites
 
-Make sure you have bash & docker installed.
+Make sure you have `docker`, `python` and `tox` installed.
-Python and some test hacks are crammed into the `Dockerfile_build` file for now.
-Revisions in the future may re-enable running python on your host (not just in docker).
 
 # Running tests locally
 
-`./build-and-test.sh`
+`tox -c test/tox.ini`
 
 Should result in:
 
-- An image named `pihole:[branch-name]` being built
+- An image named `pihole:CI_container` being built
 - Tests being ran to confirm the image doesn't have any regressions
-
-# Modify Pipfile
-
-You can enter into the test docker image using `./build-and-test.sh enter`.
-From there, you can `cd test` and execute any needed pipenv commands.

test/cmd.sh

@@ -1,11 +0,0 @@
-#!/usr/bin/env sh
-set -eux
-
-docker buildx build ./src --build-arg TARGETPLATFORM="${TARGETPLATFORM}" --tag pihole:${GIT_TAG} --no-cache
-docker images pihole:${GIT_TAG}
-
-# auto-format the pytest code
-python3 -m black ./test/tests/
-
-# TODO: Add junitxml output and have something consume it
-COLUMNS=120 py.test -vv -n auto ./test/tests/

test/requirements.txt

@@ -1,5 +1,6 @@
-pytest == 8.4.0
+pytest == 8.4.1
-pytest-xdist == 3.7.0
 pytest-testinfra == 10.2.2
-black == 25.1.0
 pytest-clarity == 1.0.1
+tox == 4.27.0
+# Not adding pytest-xdist as using pytest with n > 1 cores 
+# causes random issues with the emulated architectures

test/tests/conftest.py

@@ -1,131 +1,64 @@
-import os
 import pytest
 import subprocess
 import testinfra
-
+import testinfra.backend.docker
-local_host = testinfra.get_host("local://")
+import os
-check_output = local_host.check_output
-
-TAIL_DEV_NULL = "tail -f /dev/null"
 
 
-@pytest.fixture()
+# Monkeypatch sh to bash, if they ever support non hard code /bin/sh this can go away
-def run_and_stream_command_output():
+# https://github.com/pytest-dev/pytest-testinfra/blob/master/testinfra/backend/docker.py
-    def run_and_stream_command_output_inner(command, verbose=False):
+def run_bash(self, command, *args, **kwargs):
-        print("Running", command)
+    cmd = self.get_command(command, *args)
-        build_env = os.environ.copy()
+    if self.user is not None:
-        build_env["PIHOLE_DOCKER_TAG"] = version
+        out = self.run_local(
-        build_result = subprocess.Popen(
+            "docker exec -u %s %s /bin/bash -c %s", self.user, self.name, cmd
-            command.split(),
-            env=build_env,
-            stdout=subprocess.PIPE,
-            stderr=subprocess.STDOUT,
-            bufsize=1,
-            universal_newlines=True,
         )
-        if verbose:
+    else:
-            while build_result.poll() is None:
+        out = self.run_local("docker exec %s /bin/bash -c %s", self.name, cmd)
-                for line in build_result.stdout:
+    out.command = self.encode(cmd)
-                    print(line, end="")
+    return out
-        build_result.wait()
-        if build_result.returncode != 0:
-            print(f"       [i] Error running: {command}")
-            print(build_result.stderr)
-
-    return run_and_stream_command_output_inner
 
 
-@pytest.fixture()
+testinfra.backend.docker.DockerBackend.run = run_bash
-def args_env():
-    return '-e TZ="Europe/London" -e FTLCONF_dns_upstreams="8.8.8.8"'
 
 
-@pytest.fixture()
+# scope='session' uses the same container for all the tests;
-def args(args_env):
+# scope='function' uses a new container per test function.
-    return "{}".format(args_env)
+@pytest.fixture(scope="function")
+def docker(request):
+    # Get platform from environment variable, default to None if not set
+    platform = os.environ.get("CIPLATFORM")
 
+    # build the docker run command with args
+    cmd = ["docker", "run", "-d", "-t"]
 
-@pytest.fixture()
+    # Only add platform flag if CIPLATFORM is set
-def test_args():
+    if platform:
-    """test override fixture to provide arguments separate from our core args"""
+        cmd.extend(["--platform", platform])
-    return ""
 
+    # Get env vars from parameterization
+    env_vars = getattr(request, "param", [])
+    if isinstance(env_vars, str):
+        env_vars = [env_vars]
 
-def docker_generic(request, _test_args, _args, _image, _cmd, _entrypoint):
+    # add parameterized environment variables
-    # assert 'docker' in check_output('id'), "Are you in the docker group?"
+    for env_var in env_vars:
-    # Always appended PYTEST arg to tell pihole we're testing
+        cmd.extend(["-e", env_var])
-    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
-    )
-    # Print a human runable version of the container run command for faster debugging
-    print(docker_run.replace("-d -t", "--rm -it").replace(TAIL_DEV_NULL, "bash"))
-    docker_id = check_output(docker_run)
 
-    def teardown():
+    # ensure PYTEST=1 is set
-        check_output("docker logs {}".format(docker_id))
+    if not any("PYTEST=1" in arg for arg in cmd):
-        check_output("docker rm -f {}".format(docker_id))
+        cmd.extend(["-e", "PYTEST=1"])
 
-    request.addfinalizer(teardown)
+    # add default TZ if not already set
-    docker_container = testinfra.backend.get_backend(
+    if not any("TZ=" in arg for arg in cmd):
-        "docker://" + docker_id, sudo=False
+        cmd.extend(["-e", 'TZ="Europe/London"'])
-    )
-    docker_container.id = docker_id
 
-    return docker_container
+    # add the image name
+    cmd.append("pihole:CI_container")
 
-
+    # run a container
-@pytest.fixture
+    docker_id = subprocess.check_output(cmd).decode().strip()
-def docker(request, test_args, args, image, cmd, entrypoint):
+    # return a testinfra connection to the container
-    """One-off Docker container run"""
+    yield testinfra.get_host("docker://" + docker_id)
-    return docker_generic(request, test_args, args, image, cmd, entrypoint)
+    # at the end of the test suite, destroy the container
-
+    subprocess.check_call(["docker", "rm", "-f", docker_id])
-
-@pytest.fixture
-def entrypoint():
-    return ""
-
-
-@pytest.fixture()
-def version():
-    return os.environ.get("GIT_TAG", None)
-
-
-@pytest.fixture()
-def tag(version):
-    return "{}".format(version)
-
-
-@pytest.fixture()
-def image(tag):
-    image = "pihole"
-    return "{}:{}".format(image, tag)
-
-
-@pytest.fixture()
-def cmd():
-    return TAIL_DEV_NULL
-
-
-@pytest.fixture
-def slow():
-    """
-    Run a slow check, check if the state is correct for `timeout` seconds.
-    """
-    import time
-
-    def _slow(check, timeout=20):
-        timeout_at = time.time() + timeout
-        while True:
-            try:
-                assert check()
-            except AssertionError as e:
-                if time.time() < timeout_at:
-                    time.sleep(1)
-                else:
-                    raise e
-            else:
-                return
-
-    return _slow

test/tests/test_bash_functions.py

@@ -1,17 +1,33 @@
 import pytest
 
 
-@pytest.mark.parametrize("test_args", ['-e "FTLCONF_webserver_port=999"'])
+# Adding 5 seconds sleep to give the emulated architecture time to run
+@pytest.mark.parametrize("docker", ["FTLCONF_webserver_port=999"], indirect=True)
 def test_ftlconf_webserver_port(docker):
-    func = docker.run("pihole-FTL --config webserver.port")
+    func = docker.run("echo ${FTLCONF_webserver_port}")
+    assert "999" in func.stdout
+    func = docker.run(
+        """
+        sleep 5
+        pihole-FTL --config webserver.port
+        """
+    )
     assert "999" in func.stdout
 
 
+# Adding 5 seconds sleep to give the emulated architecture time to run
 @pytest.mark.parametrize(
-    "test_args", ['-e "FTLCONF_dns_upstreams=1.2.3.4;5.6.7.8#1234"']
+    "docker", ["FTLCONF_dns_upstreams=1.2.3.4;5.6.7.8#1234"], indirect=True
 )
 def test_ftlconf_dns_upstreams(docker):
-    func = docker.run("pihole-FTL --config dns.upstreams")
+    func = docker.run("echo ${FTLCONF_dns_upstreams}")
+    assert "1.2.3.4;5.6.7.8#1234" in func.stdout
+    func = docker.run(
+        """
+        sleep 5
+        pihole-FTL --config dns.upstreams
+        """
+    )
     assert "[ 1.2.3.4, 5.6.7.8#1234 ]" in func.stdout
 
 
@@ -24,7 +40,7 @@ def test_random_password_assigned_fresh_start(docker):
 
 
 @pytest.mark.parametrize(
-    "test_args", ['-e "FTLCONF_webserver_api_password=1234567890"']
+    "docker", ["FTLCONF_webserver_api_password=1234567890"], indirect=True
 )
 def test_password_set_by_envvar(docker):
     func = docker.run(CMD_SETUP_WEB_PASSWORD)

test/tests/test_general.py

@@ -1,15 +1,32 @@
 import pytest
+import os
 
 
-@pytest.mark.parametrize("test_args", ['-e "PIHOLE_UID=456"'])
+# Adding 5 seconds sleep to give the emulated architecture time to run
+@pytest.mark.parametrize("docker", ["PIHOLE_UID=456"], indirect=True)
 def test_pihole_uid_env_var(docker):
-    func = docker.run("id -u pihole")
+    func = docker.run("echo ${PIHOLE_UID}")
+    assert "456" in func.stdout
+    func = docker.run(
+        """
+        sleep 5
+        id -u pihole
+        """
+    )
     assert "456" in func.stdout
 
 
-@pytest.mark.parametrize("test_args", ['-e "PIHOLE_GID=456"'])
+# Adding 5 seconds sleep to give the emulated architecture time to run
+@pytest.mark.parametrize("docker", ["PIHOLE_GID=456"], indirect=True)
 def test_pihole_gid_env_var(docker):
-    func = docker.run("id -g pihole")
+    func = docker.run("echo ${PIHOLE_GID}")
+    assert "456" in func.stdout
+    func = docker.run(
+        """
+        sleep 5
+        id -g pihole
+        """
+    )
     assert "456" in func.stdout
 
 
@@ -19,6 +36,19 @@ def test_pihole_ftl_version(docker):
     assert "Version:" in func.stdout
 
 
+@pytest.mark.skipif(
+    not os.environ.get("CIPLATFORM"),
+    reason="CIPLATFORM environment variable not set, running locally",
+)
+def test_pihole_ftl_architecture(docker):
+    func = docker.run("pihole-FTL -vv")
+    assert func.rc == 0
+    assert "Architecture:" in func.stdout
+    # Get the expected architecture from CIPLATFORM environment variable
+    platform = os.environ.get("CIPLATFORM")
+    assert platform in func.stdout
+
+
 # Wait 5 seconds for startup, then kill the start.sh script
 # Finally, grep the FTL log to see if it has been shut down cleanly
 def test_pihole_ftl_clean_shutdown(docker):

test/tox.ini

@@ -0,0 +1,16 @@
+[tox]
+envlist = py3
+
+[testenv:py3]
+allowlist_externals = docker
+deps = -rrequirements.txt
+passenv = CIPLATFORM
+setenv =
+    COLUMNS=120
+    PY_COLORS=1 
+commands =  # Build the Docker image for testing depending on the architecture, fall back to 'local' if not set
+            # This allows us to run the tests on the host architecture if not on CI
+            docker buildx build --load --platform={env:CIPLATFORM:local} --progress plain -f ../src/Dockerfile -t pihole:CI_container ../src/
+            # run the tests
+            # # Not using > 1 cores as it causes random issues with the emulated architectures
+            pytest {posargs:-vv} ./tests/