-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
basic-install with openSUSE support #5001
Conversation
Thank you, could you please fill out the template for this PR. The template will be the documentation we refer to if this PR is merged and having the information there is very helpful when we need to refer back to what this PR was meant to do. |
This PR adds support for openSUSE as discussed in #4947
It adds determination for zypper as package manager. It adds packages (naming) used in openSUSE to achieve proper installation of the pihole package. Is uses systemd-service (s soon as available)
Comments and messages with relation to openSUSE are added to the installation script. |
Template was filled, let me know if you need adidtional information. |
You can make your modifications locally, commit them locally and push to your fork on github. This PR will automatically update. |
You need to Edit the PR text. |
Edited the description and added double quotes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The whole section L361 - L405 has a lot of duplicated code. Maybe you could separate what they OS have in common and only apply a if [ OS = XX]
when really needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I refactored your code a bit to reduce doubling. What do you think?
# These variable names match the ones for apt-get. See above for an explanation of what they are for.
PKG_INSTALL=("${PKG_MANAGER}" install -y)
OS_CHECK_DEPS=(grep bind-utils)
LIGHTTPD_USER="lighttpd"
LIGHTTPD_GROUP="lighttpd"
LIGHTTPD_CFG="lighttpd.conf.fedora"
if [ "${PKG_MANAGER}" = "dnf" ] || [ "${PKG_MANAGER}" = "yum" ] then
# CentOS package manager returns 100 when there are packages to update so we need to || true to prevent the script from exiting.
PKG_COUNT="${PKG_MANAGER} check-update | egrep '(.i686|.x86|.noarch|.arm|.src)' | wc -l || true"
INSTALLER_DEPS=(git dialog iproute newt procps-ng which chkconfig ca-certificates)
PIHOLE_DEPS=(cronie curl findutils sudo unzip libidn2 psmisc libcap nmap-ncat jq)
PIHOLE_WEB_DEPS=(lighttpd lighttpd-fastcgi php-common php-cli php-pdo php-xml php-json php-intl)
# If the host OS is centos (or a derivative), epel is required for lighttpd
if ! grep -qiE 'fedora|fedberry' /etc/redhat-release; then
if rpm -qa | grep -qi 'epel'; then
printf " %b EPEL repository already installed\\n" "${TICK}"
else
local RH_RELEASE EPEL_PKG
# EPEL not already installed, add it based on the release version
RH_RELEASE=$(grep -oP '(?<= )[0-9]+(?=\.?)' /etc/redhat-release)
EPEL_PKG="https://dl.fedoraproject.org/pub/epel/epel-release-latest-${RH_RELEASE}.noarch.rpm"
printf " %b Enabling EPEL package repository (https://fedoraproject.org/wiki/EPEL)\\n" "${INFO}"
"${PKG_INSTALL[@]}" "${EPEL_PKG}"
printf " %b Installed %s\\n" "${TICK}" "${EPEL_PKG}"
fi
fi
else
# openSUSE packages
# openSUSE package manager returns number when there are packages to update so we need to || true to prevent the script from exiting.
PKG_COUNT="${PKG_MANAGER} lu | egrep '(.i686|.x86|.noarch|.arm|.src)' | wc -l || true"
INSTALLER_DEPS=(git dialog iproute newt procps which ca-certificates)
PIHOLE_DEPS=(cronie curl findutils sudo unzip libidn2 psmisc libcap-ng0 nmap ncat jq)
PIHOLE_WEB_DEPS=(lighttpd php8 php8-fastcgi php8-cli php8-pdo php8-intl php8-openssl php8-sqlite)
I setup a test environment for openSUSE Thumbleweed to be able to test your PR. The test will pull the latest image of opensuse/tumbleweed and run our test suite against your changes. As you can see, it fails at some tests: To continue, please provide the output of |
Also: please rebase on latest development and sovle the merge conflicts. |
Thank you, I wanted to have a look into this on the weekend, but dont mind you were faster :-) |
Yes, you need to rebase locally and force push afterwards. |
|
Tumbleweed, as a rolling distribution, has only a |
oops, why was that closed. |
Tests are currently failing at
However, we don't use pi-hole/automated install/basic-install.sh Line 2273 in 983d79b
This was changed in #4907 This makes me think, something might went wrong with your rebase. Did you pull the |
This is what your PR reverrts: pi-hole/automated install/basic-install.sh Line 2284 in b2ec27c
|
I Think the commit line from this file mentioned something of 'which' in it. |
No worries. We all started somewhere and made the same mistakes. We are here to help.
You would need to refresh (pull) the latest development changes from |
No...it says I'm on the latest stage: This branch is not behind the upstream pi-hole:development (3 commits ahead) |
Tested the latest version of installation script successfully up to the point where pihole-ftl.service should be invoked. |
No it's not. I'm working on the test suite, but it needs some more work |
Tests are passing now. https://github.com/pi-hole/pi-hole/actions/runs/3527352033/jobs/5938401382 However, I discovered a few places in |
good. However, I feel Tumbleweed would be the better test target, as it is the basis for MicroOS (small, transactional, self-maintaining...) and this is IMO the best solution for a local DNS server...
OK, let me know once done. I had just synced the devel branch and that was OK. Or create the PR directly against my fork :-) |
Tumbleweed is a rolling release and we probably will only support stable releases. |
Test on (It fails for tumbleweed because it's not an supported OS, all other tests pass) |
Tumbleweed passes now as well: https://github.com/pi-hole/pi-hole/actions/runs/3553020655/jobs/5977810768 |
Signed-off-by: coogor <[email protected]>
every time I sync with devel, this gets closed |
@@ -1385,6 +1395,11 @@ installConfigs() { | |||
if [[ -d '/run/systemd/system' ]]; then | |||
install -T -m 0644 "${PI_HOLE_LOCAL_REPO}/advanced/Templates/pihole-FTL.systemd" '/etc/systemd/system/pihole-FTL.service' | |||
|
|||
# Set net admin permissions so that FTL can serve DNS, DHCP and IMAP (for DHCPv6). If this does not work, run FTL as root user. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you re-add this intentionally? It was removed by #5043
No. Probably the mess with rebasing / merging due to new changes in development branch. Can you remove it again? Thx!
Viele Grüße
Axel
--
Diese Nachricht wurde von meinem Android-Tablet mit K-9 Mail gesendet.
Am 21. Dezember 2022 00:23:34 MEZ schrieb yubiuser ***@***.***>:
…
@yubiuser commented on this pull request.
> @@ -1385,6 +1395,11 @@ installConfigs() {
if [[ -d '/run/systemd/system' ]]; then
install -T -m 0644 "${PI_HOLE_LOCAL_REPO}/advanced/Templates/pihole-FTL.systemd" '/etc/systemd/system/pihole-FTL.service'
+ # Set net admin permissions so that FTL can serve DNS, DHCP and IMAP (for DHCPv6). If this does not work, run FTL as root user.
Did you re-add this intentionally? It was removed by #5043
--
Reply to this email directly or view it on GitHub:
#5001 (review)
You are receiving this because you modified the open/close state.
Message ID: ***@***.***>
|
I cherry-picked your commit (to preserve your authorship) and removed the additional lines. I also combined it with my PR to implement the necessary test suites. See #5083 |
Thanks! looks like go-live gets closer :-) |
can we close this pull request? |
replacing PR #5000 against master
Signed-off-by: coogor [email protected]
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
This PR adds support for openSUSE as discussed in #4947
It adds determination for zypper as package manager. It adds packages (naming) used in openSUSE to achieve proper installation of the pihole package. Is uses systemd-service (as soon as available)
Comments and messages with relation to openSUSE are added to the installation script.
Installer was tested on openSUSE Tumbleweed and MicroOS
By submitting this pull request, I confirm the following:
git rebase
)