Skip to content

piholeDebug - Get default route robustly #6303

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 11, 2025
Merged

piholeDebug - Get default route robustly #6303

merged 1 commit into from
Jul 11, 2025

Conversation

rrobgill
Copy link
Contributor

@rrobgill rrobgill commented Jun 16, 2025 •
edited
Loading

What does this PR aim to accomplish?:

Get information about the default route in a more reliable way.

Problem with identifying this information noted on discourse and at bug pi-hole/FTL#2525

How does this PR accomplish the above?:

Determine address and interface of default route by preceding 'via' and 'dev' fields in ip's json output, instead of field position in the plain text which is not always reliable.

Logs if unable to determine default gateway

Link documentation PRs if any are needed to support this PR:

N/A

By submitting this pull request, I confirm the following:

  1. I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
  2. I have commented my proposed changes within the code and I have tested my changes.
  3. I am willing to help maintain this change if there are issues with it later.
  4. It is compatible with the EUPL 1.2 license
  5. I have squashed any insignificant commits. (git rebase)
  6. I have checked that another pull request for this purpose does not exist.
  7. I have considered, and confirmed that this submission will be valuable to others.
  8. I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  9. I give this submission freely, and claim no ownership to its content.
  • I have read the above and my PR is ready for review. Check this box to confirm

@rrobgill rrobgill requested a review from a team as a code owner June 16, 2025 23:15
yubiuser
yubiuser reviewed Jun 17, 2025
View reviewed changes
Copy link
Member

@yubiuser yubiuser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your PR.
Regex always give me a headache - whats about a more human-friendly approach using jp -j?

nanopi@nanopi:~$ ip -4 -j route show default
[{"dst":"default","gateway":"10.0.1.1","dev":"end0","protocol":"static","metric":100,"flags":[]}]

nanopi@nanopi:~$ ip -j -4 route show default | jq -r '.[0].dev'
end0
nanopi@nanopi:~$ ip -j -4 route show default | jq -r '.[0].gateway'
10.0.1.1

It's slower than the sed but I think this does not matter in the debug script

@rrobgill
Copy link
Contributor Author

rrobgill commented Jun 17, 2025 •
edited by yubiuser
Loading

Regex always give me a headache - whats about a more human-friendly approach using ip -j?

I must say I like the explicit "null" from jq if there is nothing there.

I'll update with jq instead of sed.

@rrobgill rrobgill marked this pull request as draft June 17, 2025 09:59
@rrobgill rrobgill force-pushed the defaultroute branch 2 times, most recently from 4b22b1d to eeca1cd Compare June 17, 2025 10:53
@rdwebdesign
Copy link
Member

The jq solution will fail inside our alpine docker containers.

The ip command inside the container is not the same from iproute2 package, it is a /bin/busybox version. This version doesn't have the -j option.

Alternatively, we could install iproute2 in the container and use the jq solution, but this is a discussion for docker-pi-hole repo.

@rrobgill rrobgill marked this pull request as ready for review June 18, 2025 20:18
@rrobgill
piholeDebug - Get default route robustly
19d5943 
    Determine address and interface of default route by preceeding
    'via' and 'dev' fields in json output instead of plain text
    field position.

    Log if unable to determine default gateway

Signed-off-by: Rob Gill <rrobgill@protonmail.com>
@rrobgill rrobgill mentioned this pull request Jun 24, 2025
Merged
4 tasks
@dschaper
Copy link
Member

The jq calls may fail in the docker alpine images but we don't really need to know any of the network related stuff from docker. That is all handled externally by the docker engine. There are some modifications of the debug process for docker and I think just using the output if it's there and if that fails because of alpine then echo something to the output that says "busybox found, docker managed networking". Could probably check if ip is a symlink to busybox to check if this is a busybox based OS.

@rrobgill
Copy link
Contributor Author

ip is now provided by iproute2 in development (pi-hole/docker-pi-hole#1854), and jq (full version, not busybox) was already in the Dockerfile.

@dschaper
Copy link
Member

Great!

@dschaper dschaper merged commit 1e88ce4 into pi-hole:development Jul 11, 2025
15 checks passed
@rrobgill rrobgill deleted the defaultroute branch July 11, 2025 23:55
@dschaper dschaper added this to the v6.1.3 milestone Jul 12, 2025
@dschaper dschaper mentioned this pull request Jul 14, 2025
Merged
@pralor-bot
Copy link

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/pi-hole-core-v6-1-3-released/81168/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yubiuser yubiuser yubiuser left review comments

@dschaper dschaper dschaper approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v6.1.3
Development

Successfully merging this pull request may close these issues.
5 participants
@rrobgill @rdwebdesign @dschaper @pralor-bot @yubiuser