Skip to content
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 check #1427

Merged
merged 1 commit into from
Jul 28, 2015
Merged

Fail2ban check #1427

merged 1 commit into from
Jul 28, 2015

Conversation

brettlangdon
Copy link
Member

Add a fail2ban check, which reports stats from fail2ban-client status <jail>

@brettlangdon
Copy link
Member Author

tests seemed to have failed for python 2.6.9, I am using subprocess.check_output which was added in 2.7, will update to remove dependency on check_output

@brettlangdon
Copy link
Member Author

ok... now looks like I have to replace assertDictEquals and assertDictContainsSubset

@brettlangdon
Copy link
Member Author

it helps when I run the tests locally on python 2.6 :)

@remh
Copy link

remh commented Mar 16, 2015

Thanks a lot Brett! We'll review this for 5.4.0 !

@remh remh added this to the 5.4.0 milestone Mar 16, 2015
@remh remh modified the milestones: To be determined, 5.4.0 May 29, 2015
@remh remh modified the milestones: 5.5.0, To be determined Jul 6, 2015
import mock

# project
from tests.checks.common import AgentCheckTest, get_check
Copy link
Contributor

Choose a reason for hiding this comment

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

AgentCheckTest is an unused import right now

@talwai
Copy link
Contributor

talwai commented Jul 28, 2015

Thanks for contributing @brettlangdon and sorry for the delay! Looks great overall, added a few comments on how you could modify your tests to use the standards we want to keep in the agent test suite going forward. I'd recommend looking at a test such as the go_expvar one in case the AgentCheckTest interface is unfamiliar to you. Let me know when you get a chance to look at this again

@brettlangdon
Copy link
Member Author

👍 awesome, thanks for the review, ill gladly update the tests.
On Jul 27, 2015 10:02 PM, "Aaditya Talwai" [email protected] wrote:

Thanks for contributing @brettlangdon https://github.com/brettlangdon
and sorry for the delay! Looks great overall, added a few comments on how
you could modify your tests to use the standards we want to keep in the
agent test suite going forward. I'd recommend looking at a test such as the
go_expvar
https://github.com/DataDog/dd-agent/blob/aeef08d3d098ec3d6292cd8be2b106cb7a9b45ab/tests/checks/mock/test_go_expvar.py
one in case the AgentCheckTest interface is unfamiliar to you. Let me
know when you get a chance to look at this again


Reply to this email directly or view it on GitHub
#1427 (comment).

@brettlangdon
Copy link
Member Author

@talwai I just updated the PR with updated test cases. Hopefully this is more in line with everything else. Let me know if I need to update anything else.

@talwai
Copy link
Contributor

talwai commented Jul 28, 2015

@brettlangdon yup, looks great! Merging this

talwai added a commit that referenced this pull request Jul 28, 2015
@talwai talwai merged commit 3482245 into DataDog:master Jul 28, 2015
@talwai
Copy link
Contributor

talwai commented Aug 27, 2015

@brettlangdon we actually had some issues with this check during integration testing for 5.5.0, particularly with needing to pass sudo passwords into the subprocess. We have to revert for now, unfortunately, so we can get the next release out the door, but we have it slated for 5.6.0!

Sorry about the confusion, I'll elaborate further on how we'd like to improve the check in the next few days.

@talwai talwai mentioned this pull request Aug 27, 2015
@brettlangdon
Copy link
Member Author

No problems, I understand, it is a little wonky with having to grant sudo access for datadog

@talwai talwai added this to the 5.6.0 milestone Aug 27, 2015
@talwai talwai removed this from the 5.5.0 milestone Aug 27, 2015
@talwai
Copy link
Contributor

talwai commented Aug 27, 2015

Thanks for understanding. Would you mind reopening a PR from your fork so it can undergo further review? Github won't let us re-open this one since we already merged it ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants