diff --git a/monit_docker/adapters/rules.py b/monit_docker/adapters/rules.py index f8487dc..3257c6f 100644 --- a/monit_docker/adapters/rules.py +++ b/monit_docker/adapters/rules.py @@ -21,6 +21,14 @@ _NUMERIC_CONDITION_PARTS = (('value', 'value_unit'), ('pre_value', 'pre_value_unit')) +def _byte_value(literal): + """Read a sized literal as bytes; reject a unit the parser cannot resolve.""" + try: + return bitmath.parse_string(literal).bytes + except ValueError: + raise RuleSyntaxError('unsupported unit in expression value: %r' % literal) from None + + class RuleParser(object): def __init__(self, commands=None, conditions=None, dir_groups=None): self.dir_groups = directory_groups(dir_groups) @@ -44,10 +52,10 @@ def _load_conditions_conf_finalize(self, name, exprs): m = m.groupdict() if m.get('pre_value_unit'): - m['pre_value'] = bitmath.parse_string(m['pre_value']).bytes + m['pre_value'] = _byte_value(m['pre_value']) if m.get('value_unit'): - m['value'] = bitmath.parse_string(m['value']).bytes + m['value'] = _byte_value(m['value']) r.append({'real': expr, 'parsed': m}) @@ -89,10 +97,10 @@ def _parse_exprs(self, expr, datatypes): m = m.groupdict() if m.get('pre_value_unit'): - m['pre_value'] = bitmath.parse_string(m['pre_value']).bytes + m['pre_value'] = _byte_value(m['pre_value']) if m.get('value_unit'): - m['value'] = bitmath.parse_string(m['value']).bytes + m['value'] = _byte_value(m['value']) if m['cond_alias']: cond_alias = m['cond_alias'] diff --git a/monit_docker/core/rules.py b/monit_docker/core/rules.py index fb32cd1..26b18ec 100644 --- a/monit_docker/core/rules.py +++ b/monit_docker/core/rules.py @@ -69,7 +69,9 @@ def _matches_value(self, condition, actual): expected = type(actual)(condition.value) if pre_value is not None: pre_value = type(actual)(pre_value) - except TypeError: + except (TypeError, ValueError): + # A literal that does not fit the measured value's type is a + # value error, not an unexpected exception. raise MonitoringError(110, 'invalid value for expression if: %r' % condition.source) if None not in (condition.pre_operator, pre_value): diff --git a/tests/test_monit_docker.py b/tests/test_monit_docker.py index b42f741..efd95fa 100644 --- a/tests/test_monit_docker.py +++ b/tests/test_monit_docker.py @@ -182,6 +182,18 @@ def test_invalid_later_rule_prevents_any_earlier_action_or_connection(self): self.client.containers.list.assert_not_called() md.docker.from_env.assert_not_called() + def test_unsupported_rule_unit_is_a_rule_syntax_error(self): + obj = self.client.containers.list.return_value[0] + self.assertEqual(self.invoke('monit', '--cmd-if', 'mem_usage > 100 mb ? restart'), 140) + obj.restart.assert_not_called() + self.client.containers.list.assert_not_called() + md.docker.from_env.assert_not_called() + + def test_rule_value_that_does_not_fit_the_resource_is_a_value_error(self): + obj = self.client.containers.list.return_value[0] + self.assertEqual(self.invoke('monit', '--cmd-if', 'mem_usage > 1.5 ? restart'), 110) + obj.restart.assert_not_called() + def test_status_exit_closes_client_and_does_not_sample(self): obj = self.client.containers.list.return_value[0] obj.status = 'exited' diff --git a/tests/test_rule_literals.py b/tests/test_rule_literals.py new file mode 100644 index 0000000..47ea01b --- /dev/null +++ b/tests/test_rule_literals.py @@ -0,0 +1,54 @@ +"""Rule literals that cannot be compared must fail as documented errors. + +An unsupported size unit or a literal that does not fit the measured value's +type used to escape as an unhandled ValueError, so the historical runtime CLI +reported the reserved "unexpected exception" status instead of a rule or value +error. +""" +import unittest + +from monit_docker.adapters.rules import RuleParser +from monit_docker.core.rules import RuleEvaluator +from monit_docker.domain.errors import MonitoringError, RuleSyntaxError +from monit_docker.domain.models import ContainerSnapshot + + +class RuleLiteralTests(unittest.TestCase): + def test_unsupported_unit_is_a_rule_syntax_error(self): + for expression in ('mem_usage > 100 mb ? restart', + 'mem_limit > 100 KB ? restart', + '100 mb < mem_usage < 200 MiB ? restart'): + with self.subTest(expression=expression): + with self.assertRaises(RuleSyntaxError): + RuleParser().parse(expression) + + def test_unsupported_unit_in_a_configured_condition_is_a_rule_syntax_error(self): + with self.assertRaises(RuleSyntaxError): + RuleParser(conditions={'heavy': {'expr': ['mem_usage > 100 mb']}}) + + def test_value_that_does_not_fit_the_resource_is_a_value_error(self): + parser = RuleParser() + snapshot = ContainerSnapshot(mem_usage=1048576, mem_limit=8388608, + cpu_percent=12.5, mem_percent=25.0) + for expression in ('mem_usage > 1.5 ? restart', + '1.5 < mem_usage < 2048 ? restart', + 'cpu_percent > healthy ? restart'): + with self.subTest(expression=expression): + with self.assertRaises(MonitoringError) as error: + RuleEvaluator().matches(parser.parse(expression), snapshot) + self.assertEqual(error.exception.code, 110) + + def test_supported_units_and_integer_values_still_evaluate(self): + parser = RuleParser() + snapshot = ContainerSnapshot(mem_usage=209715200, mem_limit=8388608, + cpu_percent=12.5, mem_percent=25.0) + self.assertTrue(RuleEvaluator().matches( + parser.parse('mem_usage > 100 MiB ? restart'), snapshot)) + self.assertTrue(RuleEvaluator().matches( + parser.parse('100 MiB < mem_usage < 300 MiB ? restart'), snapshot)) + self.assertTrue(RuleEvaluator().matches( + parser.parse('cpu_percent > 1.5 ? restart'), snapshot)) + + +if __name__ == '__main__': + unittest.main()