-
-
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
Prevent "pihole disable $timeout" from messing up future state changes #2887
Conversation
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.
pkill
is contained in procps
. This package is often not available in minimal environments. It might even be missing in the official Pi-hole docker container (@diginc would know more).
If we decide to use pkill
, we would - at least - have to add procps
to the installer dependencies (see automated install/basic-install.sh
).
procps is already part of the docker deps: https://github.com/pi-hole/docker-pi-hole/blob/4eb10fd63af493d81226209823aa3950c9b282d1/install.sh#L17 |
d01708e
to
6209d6d
Compare
Thank you for checking up on
|
Since the file will be in |
yes, but anyways, the process name of
I tried testing this branch but i have no idea how to install pihole from git. any hints? |
Good point! Renamed to pihole-reenable.sh |
though if you wanted to try this branch specifically, then do this after moving into the directory:
@DL6ER , do your change requests still stand? |
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.
@tlk Please could you resolve the merge conflicts on PR, thanks!
@PromoFaux Sure. Just checking - is pi-hole:development the correct target branch? |
Yep! All new code goes into |
@tlk it looks like this PR has been waiting for a rebase for almost 6 months. Do you have time for this rebase or would you rather someone else open a PR and drive this work? |
Signed-off-by: Thomas L. Kjeldsen <tlk@closureconsulting.com>
Signed-off-by: Thomas L. Kjeldsen <tlk@closureconsulting.com>
Signed-off-by: Thomas L. Kjeldsen <tlk@closureconsulting.com>
@HighMileage Thank you for the kind reminder! @PromoFaux The PR has now been rebased to |
@PromoFaux can you take a look at this PR 👀? It's been rebased and should be good to merge now |
This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there: https://discourse.pi-hole.net/t/pi-hole-5-1-released/35577/1 |
By submitting this pull request, I confirm the following:
please fill any appropriate checkboxes, e.g: [X]
and have included unit tests where possible.git rebase
)Please make sure you Sign Off all commits. Pi-hole enforces the DCO.
What does this PR aim to accomplish?:
Fixes #2879
Prevent "pihole disable $timeout" from messing up future state changes.
Example:
Without this fix the following command sequence produces a somewhat surprising result, namely that pihole ends up being enabled. This is because pihole was enabled by a "reenable" job 30s after the first "disable" command. The operator, however, did not expect this because the last command to pihole was to disable itself.
With this fix the above command sequence will leave pihole disabled.
Please note that this fix depends on the existence of thepkill
command.I can confirm that pkill exists on my device running Raspbian GNU/Linux 10 (buster).