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

Fail2ban polling update and dropping of the FW checking #7936

Merged
merged 3 commits into from Dec 30, 2017

Conversation

Projects
None yet
3 participants
@VVelox
Contributor

VVelox commented Dec 21, 2017

Since fail2ban on FreeBSD has moved to using anchors, the previous
issue of it dreking all over its self is no longer a problem.

And AFAIK it has never been an issue on Linux.

This also makes this script more cross platform as well instead.

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926

Dropping the firewall stuff as since Fail2ban as updated to using anchors(while esoteric as drek, are uber awesome it turns out) in PF instead of tables it means it is a lot more reliable on those systems and I can't seem to find any references to issues with IPFW.

librenms/librenms-agent#155

That update is also needed for this. The polling is not really backwards compatible. The poller is also massively cleaned up and better documented.

Also RRDs are backwards compatible. Any one with the old totals RRD will now just have a RRD with a unused field in it.

Update polling and drop the firewalled bit.
Since fail2ban on FreeBSD has moved to using anchors, the previous
issue of it dreking all over its self is no longer a problem.

And AFAIK it has never been an issue on Linux.

This also makes this script more cross platform as well instead.
@VVelox

This comment has been minimized.

Contributor

VVelox commented Dec 21, 2017

hmm... it turns out this is not totally backwards compatible... it did not like it only updating one value in the RRD.

Removing that old RRD fixes it. The jail specific ones are fine though.

@VVelox

This comment has been minimized.

Contributor

VVelox commented Dec 21, 2017

Was just chatting with Murrant and am going to rename the graph stuff.

@VVelox VVelox changed the title from Update polling and drop the firewalled bit. to Fail2ban polling update and dropping of the FW checking Dec 21, 2017

$rrd_def = RrdDefinition::make()
->addDataset('banned', 'GAUGE', 0)
->addDataset('firewalled', 'GAUGE', 0);
->addDataset('banned', 'GAUGE', 0);

This comment has been minimized.

@laf

laf Dec 26, 2017

Member

Does this break existing graphs as it would expect 2 entries for people who ran the code before this change?

This comment has been minimized.

@VVelox

VVelox Dec 26, 2017

Contributor

Talked to Murrant about this and ended up changing the name of the RRD as otherwise people would need to remove the old ones.

This comment has been minimized.

@laf

laf Dec 26, 2017

Member

Why not just leave the old file in place and update the firewalled value with 0 and remove from the webui so you never see it. Changing the filename results in people losing the historic data. @murrant - what do you think?

This comment has been minimized.

@VVelox

VVelox Dec 27, 2017

Contributor

Was worried about bloat with RRDs with pointless data in them. But then again it is very minor.

Decided to rework this a bit.

Just shoving U into the old unused value. I've tested it and it works. :)

@laf laf added the Refactor label Dec 26, 2017

@scrutinizer-notifier

This comment has been minimized.

scrutinizer-notifier commented Dec 27, 2017

The inspection completed: No new issues

@VVelox

This comment has been minimized.

Contributor

VVelox commented Dec 27, 2017

Running it with the U now and it it has looked good for like the last hour or so of updates.

@laf laf merged commit 0e13e77 into librenms:master Dec 30, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details
@lock

This comment has been minimized.

lock bot commented May 16, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed.

@lock lock bot locked as resolved and limited conversation to collaborators May 16, 2018

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