Skip to content

Added functionality to expand startup of container with custom scripts #1773

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

Closed
wants to merge 3 commits into from

Conversation

Joly0
Copy link

@Joly0 Joly0 commented Mar 3, 2025

{Please select 'base: development' as target branch above! (you can delete this line)}

Description

Added functionality to the start.sh script so custom scripts can be mounted and started from the "/custom-cont-init.d" directory inside the docker container. All scripts in the directory are chmod +x´ed and then executed in sorted order, but in background and "only" with a 10 seconds sleep timer between each script execution. So if scripts are dependant on each other, one would need a mechanic to handle that.

As an example i created 3 test scripts:

  • 10-successful-script.sh
  • 20-failing-script.sh
  • 30-endless-script.sh

These are executed in order with a 10 second delay in between but if the first script would take longer to execute, the second script would start nontheless. So each script does neither interfere with other scripts from that folder, nor do they interfere with the way the pihole processes are started, as everything is run in background/parallel. Also output from those scripts should be captured correctly and forwarded so they can be viewed from the docker logs, though i am not sure if processes that are started in background from one of those scripts are handled in the same way. Thats something i cant really tell for sure.

Motivation and Context

This PR solves #1752
With this PR the pihole docker image can be used as a base image for other containers so other containers can have their own startup scripts without interfering with the way pihole starts.
Also this PR adds a mechanic for users to simply mount bash scripts into the container, that are executed at runtime for whatever reason, might solve #1701 (comment)

How Has This Been Tested?

I have created the above mentioned scripts with this code:
10-successful-script.sh:

#!/bin/bash

# Test script that will always succeed
echo "[SUCCESS] Custom init script 10-successful-script.sh is running"

# Create a test file to verify the script ran
touch /tmp/custom-init-success.txt
echo "Script executed at $(date)" > /tmp/custom-init-success.txt

# Simulate some work with output to Docker logs
for i in {1..5}; do
  echo "[SUCCESS] Working... step $i of 5"
  sleep 1
done

echo "[SUCCESS] Custom init script completed successfully"
echo "[SUCCESS] Also wrote to /tmp/custom-init-success.txt for verification"

# Exit with success
exit 0

20-failing-script.sh:

#!/bin/bash

# Test script that will fail
echo "[FAILURE TEST] Custom init script 20-failing-script.sh is running"

# Create a test file to verify the script ran but failed
touch /tmp/custom-init-failure.txt
echo "Script started at $(date)" > /tmp/custom-init-failure.txt

# Simulate some work before failing
echo "[FAILURE TEST] Doing some work before failure"
sleep 2

# Try to create a file in a location that doesn't exist (this will cause an error)
echo "[FAILURE TEST] About to fail..."
mkdir -p /nonexistent_directory_that_requires_root_permissions/test 2>&1

# This command will generate an error
echo "This should not appear in the success results" > /nonexistent_directory_that_requires_root_permissions/test/file.txt 2>&1

# Record failure in our log file and Docker logs
echo "[FAILURE TEST] Script failed with exit code $?" | tee -a /tmp/custom-init-failure.txt

# Exit with error
exit 1

30-endless-script.sh:

#!/bin/bash

# Configuration
UPDATE_INTERVAL=30  # Output status every 30 seconds

# Initialize with startup message
echo "[ENDLESS] Script started at $(date)"
echo "[ENDLESS] This script will run indefinitely and log status every $UPDATE_INTERVAL seconds"
echo "[ENDLESS] Monitor the output in docker logs"

# Counter for iterations
counter=0

# Create a function to handle cleanup when the script is terminated
cleanup() {
  echo "[ENDLESS] Script terminating at $(date) - received signal"
  exit 0
}

# Set up signal handling
trap cleanup SIGTERM SIGINT

# Endless loop
while true; do
  # Increment counter
  ((counter++))
  
  # Log current status to Docker logs
  echo "[ENDLESS] Still running - iteration $counter at $(date)"
  echo "[ENDLESS] Memory usage: $(free -m | grep Mem | awk '{print $3}')MB used"
  echo "[ENDLESS] System load: $(cat /proc/loadavg)"
  echo "-------------------------------------------"
  
  # Sleep for the specified interval
  sleep $UPDATE_INTERVAL
done

The first script just runs once and exits without an error, the second one exits with an error code of 1 and the third script keeps running endlessly and continuesly outputs to the stdout which is visible in the docker logs.

All these scripts can be placed in the current working directory and the docker container can be started with a command similar to this:
docker run -d --name pihole-test -v "$(pwd):/custom-cont-init.d" pihole/pihole:latest
Afterwards viewing the docker logs should show the output of the different scripts that have been mounted.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

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

@PromoFaux
Copy link
Member

Thanks for the submission. You've managed to include some additional unrelated commits from the master branch here, but not to worry - I'll review based on the code and if we decide to go this way we can cherrypick.

I'll be honest, I'm still in two minds about the inclusion of this. What concerns me the most is what becomes effectively the ability for arbitrary scripts to be injected into the container and run as root.

@Joly0
Copy link
Author

Joly0 commented Mar 3, 2025

Thanks for the submission. You've managed to include some additional unrelated commits from the master branch here, but not to worry - I'll review based on the code and if we decide to go this way we can cherrypick.

I'll be honest, I'm still in two minds about the inclusion of this. What concerns me the most is what becomes effectively the ability for arbitrary scripts to be injected into the container and run as root.

Ok, thank you for your feedback. I also noticed the unrelated commits, but i made my pushes against master and not against development, i think thats why there are additional commits in my PR.

Regarding your concern i think this is a non-issue. The user would have to mount the scripts folder on the host and someone would need to have access to that mounted folder on the host and the unlikely event, that the container is restarted in a near future. While i see your point, if someone has that level of access to place any files in the mounted folder and restart the container, then that person would most probably have access to the host system itself and could do whatever they want. If pihole itself had a security problem and someone gained access to the pihole container that wouldnt be an issue aswell, because pihole is running as a non root user, which means the user would be a non root user aswell, limiting access to the scripts folder.
I might be overlooking something and maybe you have something that i do not currently have in mind, but this is my opinion on this.

@PromoFaux
Copy link
Member

Closing out - will continue on the original issue thread, but short version: "Not at this time"

@PromoFaux PromoFaux closed this Mar 6, 2025
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.

3 participants