pi-hole / docker-pi-hole

Pi-hole in a docker container
https://pi-hole.net
Other
8.36k stars 1.12k forks source link

Tidy Up Dependencies #1359

Open PromoFaux opened 1 year ago

PromoFaux commented 1 year ago

Currently, the dockerfile contains:

RUN apk add --no-cache git \
bash \
curl \
bind-tools \
nmap-ncat \
psmisc \
sudo \
unzip \
wget \
libidn \
nettle \
libcap \
openresolv \
iproute2-ss \
jq \
coreutils \
ncurses \
dialog \
procps \
dhcpcd \
openrc \
newt

We need to go through this list and determine what can be removed. It was mostly copied from the existing image

PromoFaux commented 1 year ago

Pass 1 of this:

https://github.com/pi-hole/docker-pi-hole/pull/1366

Gontier-Julien commented 1 year ago

@PromoFaux i think for the 2nd pass, we could drop : unzip & wget

For unzip, unless it is used somewhere that i am not aware of (you know the code more than me), it could be dropped.

For wget i think if it is used somewhere we could replace it by curl instead.

For the rest:

bash is currently still required for the bash_functions.sh and other stuff. (we/i could potentially see if it is possible to make it all shell compliant, it would just involve a lot of work and testing tho. Or we could look into it in a near future)

git unless it used else where after the git clones it can be removed before the container is shipped (or with a multistage build)

coreutils could be dropped or moved in the dev-tools since alpine use busybox and should have most of what is needed (https://wiki.alpinelinux.org/wiki/GNU_core_utilities)

PromoFaux commented 1 year ago

Great suggestions, some of them in line with thoughts I'm still trying to order in my head :)

wget / unzip

No problem dropping, unless they are needed by any of the core scripts

bash

In the debian based containers, we've been pretty lazy and just run the install script from the core repo as though it was bare metal. To be fair, it's worked OK doing it like that - but this is a good opportunity to a) bring down the size of the container, b) make it follow "the docker way" more closely (it's never going to be perfect!)

A lot of the scripts in the core repo (https://github.com/pi-hole/pi-hole) are not (yet) posix compliant, so bash is around for as long as we're leaning on the scripts from that repo that require it.

git

I've been thinking about this one a lot recently, and I have a multi stage build locally that's along these lines but it needs more thought. This links in with #1388.

In the existing containers we allow users to run pihole checkout blah blah, which makes testing hotfix branches easier, however the more I think about it the more I think we probably just don't need that functionality in the container. It is nice, but ultimately a lazy option. #1384 aims to make it simple to build the container against custom branches if so desired/required to test.

coreutils

Worth the experment!

rdwebdesign commented 1 year ago

git is used by core (pihole -up and pihole updatechecker) to verify if there is an update. pihole -up is not used on container, but we still check for updates.

PromoFaux commented 1 year ago

but we still check for updates.

Thinking on it, we just need to get the versions when the container is built, and the only update we should be checking for inside the container (if at all) is the container version. pihole -up has always been disabled inside the container.

PromoFaux commented 1 year ago

Worth the experment!

Need to tweak some of the install calls, that's fine - I just copy pasted them from the installer in the first place, fully expected things to go wrong

------
 > [ 7/11] RUN cd /etc/.pihole &&     install -Dm755 -d /opt/pihole &&     install -Dm755 -t /opt/pihole gravity.sh &&     install -Dm755 -t /opt/pihole ./advanced/Scripts/*.sh &&     install -Dm755 -t /opt/pihole ./automated install/uninstall.sh &&     install -Dm755 -t /opt/pihole ./advanced/Scripts/COL_TABLE &&     install -Dm755 -t /usr/local/bin pihole &&     install -Dm644 ./advanced/bash-completion/pihole /etc/bash_completion.d/pihole &&     install -T -m 0755 ./advanced/Templates/pihole-FTL-prestart.sh /opt/pihole/pihole-FTL-prestart.sh &&     install -T -m 0755 ./advanced/Templates/pihole-FTL-poststop.sh /opt/pihole/pihole-FTL-poststop.sh:
#12 0.434 install: unrecognized option: T
#12 0.437 BusyBox v1.36.1 (2023-06-02 00:42:02 UTC) multi-call binary.
#12 0.437 
#12 0.437 Usage: install [-cdDsp] [-o USER] [-g GRP] [-m MODE] [-t DIR] [SOURCE]... DEST
#12 0.437 
#12 0.437 Copy files and set attributes
#12 0.437 
#12 0.437       -c      Just copy (default)
#12 0.437       -d      Create directories
#12 0.437       -D      Create leading target directories
#12 0.437       -s      Strip symbol table
#12 0.437       -p      Preserve date
#12 0.437       -o USER Set ownership
#12 0.437       -g GRP  Set group ownership
#12 0.437       -m MODE Set permissions
#12 0.437       -t DIR  Install to DIR
------

Easily fixed:

-    install -T -m 0755 ./advanced/Templates/pihole-FTL-prestart.sh /opt/pihole/pihole-FTL-prestart.sh && \
-    install -T -m 0755 ./advanced/Templates/pihole-FTL-poststop.sh /opt/pihole/pihole-FTL-poststop.sh
+    install -Dm755 ./advanced/Templates/pihole-FTL-prestart.sh /opt/pihole/pihole-FTL-prestart.sh && \
+    install -Dm755 ./advanced/Templates/pihole-FTL-poststop.sh /opt/pihole/pihole-FTL-poststop.sh

I'll run it locally for a bit and see if anything else goes bang (not massively worried about focussing here just yet though, total space saving by removing wget , unzip, and coreutils (I'm not ready for git yet) is a whopping 2MB 😅

On a more extreme branch I'm down to 99.5MB, but I've not tested anything in that yet. More of a "because I can" thing

Gontier-Julien commented 1 year ago

Yup agreed with what y'all said, about git (i've came across it when i was experimenting to reduce the size further, going to make the pull request after that)

PromoFaux commented 1 year ago

Removing ncurses broke the pihole -v output. I will add it back in for the time being until we decide the direction we will be going with the version checker/multistage build etc

pihole:/# pihole -v
/opt/pihole/COL_TABLE: line 2: tput: command not found
  Pi-hole version is development-v6 eda83a4 (Latest: v5.17.1)
  AdminLTE version is development-v6 395ceef (Latest: v5.20.1)
  FTL version is development-v6 vDev-5a6d86a (Latest: v5.23)
pihole:/# apk add ncurses
fetch https://dl-cdn.alpinelinux.org/alpine/v3.18/main/x86_64/APKINDEX.tar.gz
fetch https://dl-cdn.alpinelinux.org/alpine/v3.18/community/x86_64/APKINDEX.tar.gz
(1/1) Installing ncurses (6.4_p20230506-r0)
Executing busybox-1.36.1-r0.trigger
OK: 40 MiB in 66 packages
pihole:/# pihole -v
  Pi-hole version is development-v6 eda83a4 (Latest: v5.17.1)
  AdminLTE version is development-v6 395ceef (Latest: v5.20.1)
  FTL version is development-v6 vDev-5a6d86a (Latest: v5.23)
rdwebdesign commented 5 months ago

wget / unzip

No problem dropping, unless they are needed by any of the core scripts

I think unzip is not necessary as dependency.

It is used to decompress teleporter files, but FTL now has an embedded zip: https://github.com/pi-hole/FTL/tree/development-v6/src/zip.

PromoFaux commented 5 months ago

I think unzip is not necessary as dependency.

Cool, feel free to try it and PR if it works :)