-
-
Notifications
You must be signed in to change notification settings - Fork 100
Hard-code IPv4 in the FPing probe #159
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
base: master
Are you sure you want to change the base?
Conversation
|
I am a bot, here are the test results for this PR:
|
|
This pull request has been automatically marked as stale because it has not had recent activity. This might be due to missing feedback from OP. It will be closed if no further activity occurs. Thank you for your contributions. |
|
Please merge this PR, or suggest why it is not merged |
it's not merged because you're the only user to ever request it, we have a backlog, we do this in our free time, and you aren't special. wait patiently. |
|
You will need to find a user with a fully proper setup (where ipv6 isn't disabled on the host/network) to test your changes. This change should not be necessary, generally speaking, for normal users. Once this is validated to NOT cause issues for other users, we can merge. |
|
I'd suggest that the fact that it caused issues on my setup where there was only partial IPv6 support is enough to warrant hardcoding the fix. Users who want to use ipv6 have their own probe for that purpose.
Git bot makes me post these comments. I was waiting patiently until it said my issue would be closed automatically and accused me of inactivity. |
got it, i've marked it wip, the bot will no longer bother you, but it won't get merged until it's tested by the intended audience. |
|
Might I suggest creating a new probe type called |
|
I like that quite a bit actually, good point! Very forwards-compatible. Will work on making an |
|
I am a bot, here are the test results for this PR:
|
|
I am a bot, here are the test results for this PR:
|
|
I am a bot, here are the test results for this PR:
|
|
I am a bot, here are the test results for this PR:
|
|
I am a bot, here are the test results for this PR:
|
|
I am a bot, here are the test results for this PR:
|
|
I am a bot, here are the test results for this PR:
|
|
I am a bot, here are the test results for this PR:
|
b987292 to
ed3dd6f
Compare
|
I am a bot, here are the test results for this PR:
|
|
I am a bot, here are the test results for this PR:
|
|
I am a bot, here are the test results for this PR:
|
|
I am a bot, here are the test results for this PR:
|
|
I am a bot, here are the test results for this PR:
|
|
I am a bot, here are the test results for this PR:
|
|
I am a bot, here are the test results for this PR:
|
|
I am a bot, here are the test results for this PR:
|
|
I am a bot, here are the test results for this PR:
|
|
I am a bot, here are the test results for this PR:
|
|
I am a bot, here are the test results for this PR:
|
|
I am a bot, here are the test results for this PR:
|
|
I am a bot, here are the test results for this PR:
|
|
I am a bot, here are the test results for this PR:
|
|
I've tested this in the requested/intended environment, and believe it is good to merge. |
|
I am a bot, here are the test results for this PR:
|
|
I am a bot, here are the test results for this PR:
|
drizuid
left a comment
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.
if we're going to hardcode protocol 4 on fping, should we not hardcode it for fping6 for consistency
with that said, according to the smokeping documentation, there is an fping6 binary
https://oss.oetiker.ch/smokeping/probe/FPing6.en.html
Maybe this is a bad assumption, but I assumed that fping6 forces IPv6, hence its name? |
|
That is my assumption as well, but that isn't the binary being used in the pr that I saw. I could be blind, I checked on phone |
|
I am a bot, here are the test results for this PR:
|
|
ahh just checked latest container and while fping6 is in the smokeping documentation, it doesnt exist in a normal install. if you add protocol = 6 on the fping6 section, ill approve this and merge it update: |
Solves an issue related to DNS lookups failing on an IPv4 workstation
|
Rebased and force pushed as the branch was quite out-of-sync. Thinking about this more, I propose three separate ping tools: FPing, FPing4, and FPing6. This gives users the opportunity to explicitly pick what they want to happen, which full backwards-compatibility with curent FPing (which selects "the best IP version"). |
|
I am a bot, here are the test results for this PR:
|
Solves an issue related to DNS lookups failing on an IPv4 workstation
Description:
Hard-code IPv4 in the FPing probe
Benefits of this PR and context:
How Has This Been Tested?
Simple testing with default setup otherwise.
Source / References:
N/A