Skip to content

Fix in ping telegraf plugin, the "count" to be a "arguments".#4359

Merged
fichtner merged 4 commits intoopnsense:masterfrom
alexwbaule:fix/using-binarty-ping-telegraf
Dec 1, 2024
Merged

Fix in ping telegraf plugin, the "count" to be a "arguments".#4359
fichtner merged 4 commits intoopnsense:masterfrom
alexwbaule:fix/using-binarty-ping-telegraf

Conversation

@alexwbaule
Copy link
Copy Markdown

its the correct way to configure ping to use the "binary" file. (the "count" is a parameter passed to the ping binary).

Alex W Baulé added 2 commits November 15, 2024 16:46
its the correct way to configure ping to use the "binary" file.
(the "count" is a parameter passed to the ping binary).

Signed-off-by: Alex W Baulé <alexwbaule@gmail.com>
Signed-off-by: Alex W Baulé <alexwbaule@gmail.com>
@alexwbaule
Copy link
Copy Markdown
Author

Hi @fichtner ,

that's the correct way to configure the ping using the binary file.

fix the revert made in #4349 because the #4316 and #4317

@alexwbaule alexwbaule changed the title Fix the "count" to be a "arguments". Fix in ping telegraf plugint, the "count" to be a "arguments". Nov 15, 2024
@alexwbaule alexwbaule mentioned this pull request Nov 15, 2024
3 tasks
@alexwbaule alexwbaule changed the title Fix in ping telegraf plugint, the "count" to be a "arguments". Fix in ping telegraf plugin, the "count" to be a "arguments". Nov 15, 2024
@fichtner
Copy link
Copy Markdown
Member

can we set binary for ipv6 case too for consistency? arguments look correct, ping6 binary appears to be deprecated. maybe it's worth asking for wider testing in the other thread first.

@fichtner fichtner self-assigned this Nov 15, 2024
@alexwbaule
Copy link
Copy Markdown
Author

@fichtner , yeap.. we can...

I'm set the ping because is the same binary, and if you call the "--help" on each binary, the response is the same, you must pass the "-6" as argument too, to ping6 a ipv6 address or host.

i will revert to "ping6", but keeping the "-6" as argument, like is recommended by the --help from ping/ping6.

Signed-off-by: Alex W Baulé <alexwbaule@gmail.com>
@fichtner
Copy link
Copy Markdown
Member

Meh sorry typo, I meant the binary for IPv4. “ping -6” was better. “ping” these days is ambiguous on host names so “ping -4” makes sense. “ping6” already implies “-6” … hope I remember this correctly.

@alexwbaule
Copy link
Copy Markdown
Author

@fichtner, im think too, that "ping6" implies "-6", but testing here i got some strange behavior, like ping ipv4 on "ping6" and vice-versa. Thats why i force the argument "-6" and "-4"...

Im tested here, with "run as root" unchecked, and works fine.

@alexwbaule
Copy link
Copy Markdown
Author

@fichtner it will be merged ?

@fichtner
Copy link
Copy Markdown
Member

I'd rather like to get rid of binary = "ping6" here for symmetry

@alexwbaule
Copy link
Copy Markdown
Author

Ok, when I get at home i will update it.

@fichtner
Copy link
Copy Markdown
Member

@alexwbaule ping :)

@alexwbaule
Copy link
Copy Markdown
Author

Sorry @fichtner , I forgot ! To many things to do... Promess, today !

@alexwbaule
Copy link
Copy Markdown
Author

@fichtner , done ! Sorry about the delay.

@fichtner
Copy link
Copy Markdown
Member

fichtner commented Dec 1, 2024

No worries, merged, thanks!

@fichtner fichtner merged commit 60b0bfe into opnsense:master Dec 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants