-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Added Alpine Linux support #4827
Conversation
Signed-off-by: Giulio Coa <giulioc008@gmail.com>
Signed-off-by: Giulio Coa <giulioc008@gmail.com>
Signed-off-by: Giulio Coa <giulioc008@gmail.com>
…port # Conflicts: # automated install/basic-install.sh
Thanks for the contribution!
This might make it easier to have an official alpine image, too.
This reminds me, I keep thinking about setting up dev container for vscode to make running tests locally easier. It should just be a case of having Python 3.8 installed, install requirements from
You can ignore this, and probably we can remove it, but that's for another PR
I can do the latter for now. Can you get me an output from |
Just a note, for tests to run here you'll need to add |
Thanks for the help
|
Done |
I did what you said, but I get this output:
|
Actually, thinking about it - I'm trying to remember if I even had to install python/the requirements locally... I vaguely recall just installing https://github.com/pi-hole/pi-hole/tree/master/test#readme For me, the test are running - but they are all failing, so that will need looking at once you have the tests running locally |
It is the only test that the code fails. |
It's the From your OP:
So obviously the prerequisites need to be installed in the image used to run the tests :) Adding |
Some tests do not pass because they need to be modified to add support to Alpine (use of OpenRC, etc.) |
@PromoFaux I don't understand why these 3 tests fail |
I also need to know how to mock commands like On Alpine, the paths for the scripts are:
You can get they from |
You don't need to mock |
It is what I said, but I don't know how to do it |
Signed-off-by: Giulio Coa <giulioc008@gmail.com>
So, now the only two things to do are:
@PromoFaux @dschaper is there any idea? |
I'll have a play about locally, but I don't really know anything about alpine - I also don't really know the test system all that well, so it's just a case of trial and error (or throwing mud at the wall to see what sticks) |
So, taking one of the failing tests, e.g
Do you actually have a working Pi-hole install on an Alpine system? I cannot even get the installer to run inside the container... |
Ah. The installer is downloading the standard Inside # 64bit
printf "%b %b Detected x86_64 processor\\n" "${OVER}" "${TICK}"
# set the binary to be used
detected_os=$(grep '^ID=' /etc/os-release | cut -d '=' -f2 | tr -d '"')
if [[ "${detected_os}" == "alpine" ]]; then
l_binary="pihole-FTL-musl-linux-x86_64"
else
l_binary="pihole-FTL-linux-x86_64"
fi That solves the |
And other processors ? |
Signed-off-by: Giulio Coa <giulioc008@gmail.com>
Good question, and to be honest - I think maybe we need to take a step back here and rethink things. Some unordered thoughts:
Ultimately, what I'm saying is "If we're going to do this, let's do it properly". That's not to say the effort you've put into looking at this is not appreciated - but I just think that maybe it needs to be approached from another angle |
Are you sure ? |
The thinking being that the most obvious reason to use alpine over other distros is having a smaller image size for containers (maybe on hardware, but I've never personally seen it) - so being POSIX compatible and removing the need for bash brings down that overall image size. Maybe not by much, but I've seen people lose their minds (hyperbole!) When the diff between one release of an image and another is 3-4MB I've not built any images with/without bash to compare though, maybe it's not all that much different. |
Not Alpine. It uses |
Let's try this in a clean and fresh PR. I won't accept this PR:
Please try to do these changes on your own fork and get things to a place where the PR can be accepted with only a review of code and functionality. We are volunteering our time and trying to review a moving target is asking too much of us. |
I set up my docker image based on alpine (with also unbound).. I know, there might be a lot of things that can be improved.. but it is a baseline. I welcome any help |
Thank you for your contribution to the Pi-hole Community!
Please read the comments below to help us consider your Pull Request.
We are all volunteers and completing the process outlined will help us review your commits quicker.
Please make sure you
I would like to add Alpine Linux support so that I can create lighter Pi-hole Docker Images.
I have done the changes to adapt the code to Alpine Linux.
The pre-requisites to run Pi-hole on Alpine are:
bash
installed (apk add bash
)busybox-initscripts
installed (apk add busybox-initscripts
)root
or havesudo
installed (apk add sudo; echo '%wheel ALL=(ALL) ALL' > /etc/sudoers.d/wheel
)I tried to tests my changes trough
tox
, but I failed...The only file I haven't changed completely is
advance/Scripts/setupLCD.sh
because it uses Adafruits.I don't know about project Adafruits and the links used in the code are no longer valid.
To completely support Alpine Linux, you must add it to
versions.pi-hole.net
anddev-supportedos.pi-hole.net
.Replace this with a detailed list of any necessary changes
By submitting this pull request, I confirm the following:
git rebase
)