S6 no longer in image

[PromoFaux]

Hey, i am not sure if this is exactly my issue, but i contributed to a separate docker image based on the official pihole:latest image and am now trying to update it to the latest changed. In the past the image used /etc/services.d/ to start other services separately from running the main pihole entrypoint without overwriting anything. This was a very good and working approach, but now i cannot use this anymore, because it seems like the alpine image does not contain any kind of init system.
I tried changing the entrypoint (which i really do not want to do, as i do not want to interferre with the entrypoint of the base pihole image) but that did not work either, i kept getting errors, that it couldnt start and stopped the container.

Therefore could you please add an init system OR add a mechanic do expand the startup of the image maybe through additional scripts in a specific directory.

Originally posted by @Joly0 in #1701

[PromoFaux]

@Joly0 - created a new issue because the discussion is not on topic to the issue you tacked onto .

This is not likely to happen, I'm afraid. V6 was a chance to redesign the image from the ground up, and as we no longer need to run multiple binaries, we no longer needed the s6 init system. This also is more in fitting with the "docker way" of doing things, which is one service per container.

Take a look at projects such as this one https://github.com/klutchell/balena-pihole, where additional services are added in a stack, rather than injecting them into a single monolithic image.

[Joly0]

Hey @PromoFaux thank your for creating a separate issue. I have looked through various github issues to see, how others are creating their adjusted image and most of them (if not all of them now) create a separate wrapper script, that overwrites the default start.sh script, executes the custom stuff and in the end usually executes the start.sh. Also balena-pihole uses docker-compose, which is not always a good solution in my opinion and is not always available as a choice.

While all in all this is a solution one could use and is working, i dislike this approach, because you have to keep track of the upstream (meaning the repo here pi-hole/docker-pi-hole) and check regularly if the entrypoint has changed, which is not super likely but can happen and would probably break the container one created.

I do understand the issue regarding one service per container and that this project no longer needs an init system, but it makes it harder to built on top of it and create separate images based on your great work here. Also, and this is only my personal opinion, an init system does not violate the "one service per container" rule, because it is not really a service itself, but part of another service. The init system is only needed to start and gracefully stop the actual services in a way, so no zombie processes are left behind. Even docker themselves recommend using a proper init system. But of course i understand, that an init system would require a slightly larger image from you and would require some more work to keep the init system itself up to date.

So while it is ok for me, that this image does no longer have an init system, it would be great to have some kind of extendability. I mentioned already, that the start.sh script could also be extended in a way to load scripts in a specific folder on startup, like linuxserver.io images do https://docs.linuxserver.io/general/container-customization/#custom-scripts atleast that way users could built upon the pi-hole image without interferring with the way it starts and manages itself.

[yubiuser]

We could use tini as init system (it adds almost nothing), using it as entrypoint and run start.sh as CMD

[PromoFaux]

0.1 MB in fact.

We did have tini, but it was removed because... well, it wasn't actually doing anything.

I wonder if people that wanted to use this container as a base could use the built-in docker support for tini...

https://docs.docker.com/reference/cli/docker/container/run/#init

[Joly0]

Hey, i have looked into it and from what i see, tini wont solve the problem as tini only manages one process and docker has no option to "extend" an existing cmd without overwriting it, which would bring us back to the root of my issue. Also afaik tini hasnt been updated for quite a while.

From googling about this i found dinit https://gitlab.com/tozd/dinit which is updated and should work for the usecase here, is small in size and also requires minimal changes to the base image, but would allow downstream developers to extend the baseimage without overwriting your entrypoint/cmd.

Dinit has a comparision with other init systems on their gitlab page with advantages and disadvantages.

Personally i would prefer s6-overlay over anything else, because it is well updated and widely used in the docker community, but it would require some deeper changes and is larger is size fron what i know than most other init systems.

Also if wanted, i could create pr's for any init system, so it wouldnt be all your job to handle this, we are still open source community here after all :)

[PromoFaux]

I'll preface this by saying that this is probably going to sound standoffish, but I truly don't mean it to, I'm just in a bad mood.

Personally i would prefer s6-overlay over anything else,

One of the beauties of opensource is that if you don't like something the way it is, you can fork and change to suit your needs.

I get what you're saying about PRing a change, and appreciate the offer, but if you get hit by a bus the maintenance lands on me/the team to keep something working that to us is unnessacery bloat in the container.

A decision was made with the v6 container not to include an init system - we simply don't need one. Adding one in increases complexity and maintenance burden on our part on the small chance that someone downstream wants to build "Pi-hole in docker, except it also has speedtest-cli running inside it" or "Pi-hole docker, but this time it also has Plex Media Server"

I'm being facetious with my examples, I know.

IMHO, unbound/stubby/cloudflared etc should be set up in a compose stack - in what situations would compose not be available as a choice?

[Joly0]

IMHO, unbound/stubby/cloudflared etc should be set up in a compose stack - in what situations would compose not be available as a choice?

Unraid.

Unraid does not ship with docker-compose, while still being used my hundreds of thousands if not millions of homelabbers. There does exist a plugin for unraid that adds docker-compose, but this is an unofficial third party plugin which does not work together with the app store that unraid also includes. Therefore "combined" images for multiple services is a pretty normal thing in unraid world or images that ship with s6-overlay (mostly by linuxserver.io) which one can extend easily.
The docker image i am trying to update alone has 50k+ pulls on dockerhub, so its not a small container used by just a small amount of people (ofc its still hilariously small compared to pihole image itself) who would really like to get the image updated to v6 without loosing the ability to have one container that can be installed and just works oob with unbound/stubby/cloudflared.

I get what you're saying about PRing a change, and appreciate the offer, but if you get hit by a bus the maintenance lands on me/the team to keep something working that to us is unnessacery bloat in the container.

I guess, thats the situation we are right now basically, init system is no longer included, i rely on it and i have to bother you guys now with this problem, because i would have to substantially change the way the container is build and have to take care constantly of any changes to the entrypoint/cmd (other than the change from debian to alpine as the base image, which is also a lot of work, but atleast its one-time).

I'll preface this by saying that this is probably going to sound standoffish, but I truly don't mean it to, I'm just in a bad mood.

All good, we all have such days :)

[Joly0]

Hey guys, just wanted to ask if anything is going to happen regarding my issue here? Because if not, then this issue can be closed and i will have to find a new different way to build my docker image

[yubiuser]

Currently, there is nothing happening in this regard. We are busy with all the fixes and support due to the V6 upgrade.
But to be honestly, I don't see much chance of adding back an init system, and no chance to go back to s6-overlay.

[Joly0]

Would you accept a PR with my approach like this:

#!/bin/bash

if [ "${PH_VERBOSE:-0}" -gt 0 ]; then
  set -x
fi

trap stop TERM INT QUIT HUP ERR

CAPSH_PID=""
TRAP_TRIGGERED=0

# New function to run custom initialization scripts
run_custom_init_scripts() {
  local custom_init_dir="/custom-cont-init.d"
  
  # Check if the directory exists
  if [ ! -d "$custom_init_dir" ]; then
    echo "  [i] Custom init directory $custom_init_dir does not exist, skipping"
    return 0
  fi
  
  echo "  [i] Checking for custom initialization scripts in $custom_init_dir"
  
  # Find all executable scripts or make them executable
  find "$custom_init_dir" -type f | sort | while read -r script; do
    # Check if file is already executable
    if [ ! -x "$script" ]; then
      echo "  [i] Making $script executable"
      chmod +x "$script" 2>/dev/null || {
        echo "  [!] Failed to make $script executable, skipping"
        continue
      }
    fi
    
    # Extract the filename
    filename=$(basename "$script")
    
    # Check if the filename starts with a number
    if [[ "$filename" =~ ^[0-9]+ ]]; then
      echo "  [i] Running custom init script: $script"
      # Run the script in background with output redirected to docker logs
      # Use 'exec' to run in a subshell and avoid blocking
      (exec "$script" 2>&1 | sed "s/^/[Custom Init: $filename] /") &
      
      # Store the PID in case we want to track it later
      local script_pid=$!
      echo "  [i] Started $script with PID $script_pid"
    else
      echo "  [i] Skipping $script, filename doesn't start with a number"
    fi
  done
  
  echo "  [i] Finished processing custom initialization scripts"
  return 0
}

start() {

  local v5_volume=0

  # The below functions are all contained in bash_functions.sh
  # shellcheck source=/dev/null
  . /usr/bin/bash_functions.sh

  # If the file /etc/pihole/setupVars.conf exists, but /etc/pihole/pihole.toml does not, then we are migrating v5->v6
  # FTL Will handle the migration of the config files
  if [[ -f /etc/pihole/setupVars.conf && ! -f /etc/pihole/pihole.toml ]]; then
    echo "  [i] v5 files detected that have not yet been migrated to v6"
    echo ""
    migrate_v5_configs
  fi

  # ===========================
  # Initial checks
  # ===========================

  # If PIHOLE_UID is set, modify the pihole user's id to match
  set_uid_gid

  # Configure FTL with any environment variables if needed
  echo "  [i] Starting FTL configuration"
  ftl_config

  # Install additional packages inside the container if requested
  install_additional_packages

  # Start crond for scheduled scripts (logrotate, pihole flush, gravity update etc)
  start_cron

  # Install the logrotate config file
  install_logrotate

  # Run custom initialization scripts (new addition)
  run_custom_init_scripts || echo "  [!] Error running custom init scripts, continuing with startup"

  #migrate Gravity Database if needed:
  migrate_gravity

  echo "  [i] pihole-FTL pre-start checks"
  # Remove possible leftovers from previous pihole-FTL processes
  rm -f /dev/shm/FTL-* 2>/dev/null
  rm -f /run/pihole/FTL.sock

  fix_capabilities
  sh /opt/pihole/pihole-FTL-prestart.sh

  echo "  [i] Starting pihole-FTL ($FTL_CMD) as ${DNSMASQ_USER}"
  echo ""

  capsh --user="${DNSMASQ_USER}" --keep=1 -- -c "/usr/bin/pihole-FTL $FTL_CMD >/dev/null" &
  # Notes on above:
  # - DNSMASQ_USER default of pihole is in Dockerfile & can be overwritten by runtime container env
  # - /var/log/pihole/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

  # We need the PID of the capsh process so that we can wait for it to finish
  CAPSH_PID=$!

  # Wait until the log file exists before continuing
  while [ ! -f /var/log/pihole/FTL.log ]; do
    sleep 0.5
  done

  #  Wait until the FTL log contains the "FTL started" message before continuing
  while ! grep -q '########## FTL started' /var/log/pihole/FTL.log; do
    sleep 0.5
  done
  
  pihole updatechecker
  local versionsOutput
  versionsOutput=$(pihole -v)
  echo "  [i] Version info:"
  printf "%b" "${versionsOutput}\\n" | sed 's/^/      /' 
  echo ""

  if [ "${TAIL_FTL_LOG:-1}" -eq 1 ]; then
    # Start tailing the FTL log from the most recent "FTL Started" message
    # Get the line number
    startFrom=$(grep -n '########## FTL started' /var/log/pihole/FTL.log | tail -1 | cut -d: -f1)
    # Start the tail from the line number and background it
    tail --follow=name -n +"${startFrom}" /var/log/pihole/FTL.log &
  else
    echo "  [i] FTL log output is disabled. Remove the Environment variable TAIL_FTL_LOG, or set it to 1 to enable FTL log output."
  fi

  # Wait for the capsh process (which spawned FTL) to finish
  wait $CAPSH_PID
  FTL_EXIT_CODE=$?
  
  
  # If we are here, then FTL has exited.
  # If the trap was triggered, then stop will have already been called
  if [ $TRAP_TRIGGERED -eq 0 ]; then
    # Pass the exit code through to the stop function
    stop $FTL_EXIT_CODE
  fi
}

stop() {
  local FTL_EXIT_CODE=$1

  # if we have nothing in FTL_EXIT_CODE, then have been called by the trap. Close FTL and wait for the CAPSH_PID to finish
  if [ -z "${FTL_EXIT_CODE}" ]; then
    TRAP_TRIGGERED=1
    echo ""
    echo "  [i] Container stop requested..."
    echo "  [i] pihole-FTL is running - Attempting to shut it down cleanly"
    echo ""
    killall --signal 15 pihole-FTL

    wait $CAPSH_PID
    FTL_EXIT_CODE=$?
  fi

  # Wait for a few seconds to allow the FTL log tail to catch up before exiting the container
  sleep 2

  # ensure the exit code is an integer, if not set it to 1
  if ! [[ "${FTL_EXIT_CODE}" =~ ^[0-9]+$ ]]; then
    FTL_EXIT_CODE=1
  fi

  echo ""
  echo "  [i] pihole-FTL exited with status $FTL_EXIT_CODE"
  echo ""
  echo "  [i] Container will now stop or restart depending on your restart policy"
  echo "      https://docs.docker.com/engine/containers/start-containers-automatically/#use-a-restart-policy"
  echo ""

  # If we are running pytest, keep the container alive for a little longer
  # to allow the tests to complete
  if [[ ${PYTEST} ]]; then
    sleep 10
  fi

  exit "${FTL_EXIT_CODE}"

}

start

This is a slightly changed start.sh script that adds the "run_custom_init_scripts" function. This function reads the directory "/custom-cont-init.d" for bash scripts, sorts them based on the name (i thought about a naming convention like "10-firstscript.sh; 20-secondscript.sh" and so on), "chmod +x" those scripts and executes them in the background. The output is properly redirected and outputted to the stdout so its visible in docker logs with additional formatting so its visible to the end user which lines in the log come from pihole itself and which ones come from one of the custom scripts. This approach does not interfere with the normal pihole startup at all. So basically mimics the approach linuxserver.io images handle custom scripts in their images

[PromoFaux]

Potentially, yes.

If you PR, please add the meat of it to bash_functions.sh, and just call it from the start() function which already sources that script.

If you could add some examples and use cases to the PR body - it would be appreciated, so that I know exactly what I'm looking at/getting into when testing

[devzwf]

I have updated the image
as usual i use the official pi-hole container as base then add

• unbound
• cloudflared

i will push my dev brand to my repo later tonight

thanks pihole dev for this awesome app
Note : oh and side note , i am anything but a dev :(

[PromoFaux]

I've given this a lot of thought over the past couple of days, and at this stage I'm not willing to add this level of functionality to the image. While it may make it slightly easier for users to inject their own scripts, it's not something that is required by the base image.

@devzwf appears to already have a POC image (I assume this was the intended target for this discussion ultimately) which has worked around this by making an adjustment to the entrypoint - this is perfectly fine, and also pretty easy to keep in sync with upstream changes as the entrypoing is very very very unlikely to change from start.sh - unless we end up including an init system, but that's doubtful at this stage.

I appreciate this may not be exactly what you're looking for - but thanks nonetheless for the conversation and the submission. It was fun to think about!

[devzwf]

Hey @PromoFaux
thanks for taking the time to review this and provide your thought.

I apprecite the discussion as well.
keep up the good work