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

Adding optional service_check_rw parameter to check for read-only filesystem #2086

Merged
merged 3 commits into from
Aug 24, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion disk/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ See [metadata.csv][4] for a list of metrics provided by this integration.
The Disk check does not include any events at this time.

### Service Checks
The Disk check does not include any service checks at this time.

**`disk.service_check_rw`**:
Returns `CRITICAL` if filesystem is in read-only mode.

## Troubleshooting
Need help? Contact [Datadog Support][5].
Expand Down
4 changes: 4 additions & 0 deletions disk/datadog_checks/disk/data/conf.yaml.default
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,7 @@ instances:
# tags:
# - optional_tag1
# - optional_tag2
#
# The (optional) service_check_rw parameter notifies when one of the partitions is in
# read-only mode
# service_check_rw: false
18 changes: 18 additions & 0 deletions disk/datadog_checks/disk/disk.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ def _load_conf(self, instance):
instance.get('all_partitions', False))
self._device_tag_re = instance.get('device_tag_re', {})
self._custom_tags = instance.get('tags', [])
self._service_check_rw = _is_affirmative(
instance.get('service_check_rw', False))

# Force exclusion of CDROM (iso9660) from disk check
self._excluded_filesystems.append('iso9660')
Expand Down Expand Up @@ -127,6 +129,22 @@ def collect_metrics_psutil(self):
self.gauge(metric_name, metric_value,
tags=tags, device_name=device_name)

# Add in a disk read write or read only check
if self._service_check_rw:
rwro = list(set(['rw', 'ro']) & set(part.opts.split(',')))
if len(rwro) == 1:
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we send a service check with AgentCheck.UNKNOWN status when neither rw nor ro is there ?

self.service_check(
'disk.read_write',
AgentCheck.OK if rwro[0] == 'rw' else AgentCheck.CRITICAL,
tags=tags+['device:%s' % (device_name)]
)
else:
self.service_check(
'disk.read_write', AgentCheck.UNKNOWN,
tags=tags+['device:%s' % (device_name)]
)


self.collect_latency_metrics()

def _exclude_disk_psutil(self, part):
Expand Down
11 changes: 11 additions & 0 deletions disk/service_checks.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
[
{
"agent_version": "5.22.3",
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be 6.5.0 since it will be released in the next version of the agent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry didn't know what that version was for

"integration":"disk",
"check": "disk.service_check_rw",
"statuses": ["ok", "critical", "unknown"],
"groups": ["device"],
"name": "Disk read_write",
"description": "Returns `CRITICAL` if filesystem is in read-only mode."
}
]
14 changes: 13 additions & 1 deletion disk/tests/test_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,11 @@


class MockPart(object):
def __init__(self, device=DEFAULT_DEVICE_NAME, fstype='ext4', mountpoint='/'):
def __init__(self, device=DEFAULT_DEVICE_NAME, fstype='ext4', mountpoint='/', opts='ro'):
self.device = device
self.fstype = fstype
self.mountpoint = mountpoint
self.opts = opts


class MockDiskMetrics(object):
Expand Down Expand Up @@ -121,6 +122,7 @@ def test_default_options():
assert check._all_partitions is False
assert check._excluded_disk_re == re.compile('^$')
assert check._device_tag_re == []
assert check._service_check_rw is False

def test_disk_check(aggregator):
"""
Expand Down Expand Up @@ -224,6 +226,16 @@ def test_use_mount(aggregator, psutil_mocks):

assert aggregator.metrics_asserted_pct == 100.0

def test_psutil_rw(aggregator, psutil_mocks):
"""
Check for 'ro' option in the mounts
"""
instances = [{'service_check_rw': 'yes'}]
c = Disk('disk', None, {}, instances)
c.check(instances[0])

aggregator.assert_service_check('disk.read_write', status=Disk.CRITICAL)

def mock_df_output(fname):
"""
Load fixtures from tests/fixtures/ folder and return a tuple matching the
Expand Down