tweak behaviour and add tests for kapacitor

salt/modules/kapacitor.py

@@ -48,8 +48,10 @@ def get_task(name):
     url = 'http://{0}:{1}/task?name={2}'.format(host, port, name)
     response = salt.utils.http.query(url)
     data = json.loads(response['body'])
+
     if 'Error' in data and data['Error'].startswith('unknown task'):
         return None
+
     return data
 
 
@@ -91,7 +93,7 @@ def define_task(name, tick_script, task_type='stream', database=None,
     if database and retention_policy:
         cmd += ' -dbrp {0}.{1}'.format(database, retention_policy)
 
-    return __salt__['cmd.run_all'](cmd)
+    return __salt__['cmd.retcode'](cmd) == 0
 
 
 def delete_task(name):
@@ -107,8 +109,8 @@ def delete_task(name):
 
         salt '*' kapacitor.delete_task cpu
     '''
-    cmd = 'kapacitor delete -name {0}'.format(name)
-    return __salt__['cmd.run_all'](cmd)
+    cmd = 'kapacitor delete tasks {0}'.format(name)
+    return __salt__['cmd.retcode'](cmd) == 0
 
 
 def enable_task(name):
@@ -125,7 +127,7 @@ def enable_task(name):
         salt '*' kapacitor.enable_task cpu
     '''
     cmd = 'kapacitor enable {0}'.format(name)
-    return __salt__['cmd.run_all'](cmd)
+    return __salt__['cmd.retcode'](cmd) == 0
 
 
 def disable_task(name):
@@ -142,4 +144,4 @@ def disable_task(name):
         salt '*' kapacitor.disable_task cpu
     '''
     cmd = 'kapacitor disable {0}'.format(name)
-    return __salt__['cmd.run_all'](cmd)
+    return __salt__['cmd.retcode'](cmd) == 0

salt/states/kapacitor.py

@@ -15,9 +15,13 @@ Kapacitor state module.
 .. versionadded:: 2016.3.0
 '''
 
+from __future__ import absolute_import
+
 import difflib
 import logging
 
+import salt.utils
+
 LOG = logging.getLogger(__name__)
 
 
@@ -71,7 +75,7 @@ def task_present(name,
     else:
         script_path = tick_script
 
-    with open(script_path, 'r') as file:
+    with salt.utils.fopen(script_path, 'r') as file:
         new_script = file.read()
 
     if old_script == new_script:
@@ -83,9 +87,10 @@ def task_present(name,
             result = __salt__['kapacitor.define_task'](name, script_path,
                 task_type=task_type, database=database,
                 retention_policy=retention_policy)
-            if result['retcode'] != 0:
+            if not result:
                 ret['result'] = False
-                ret['comment'] = result['stderr'] + result['stdout']
+                comments.append('Could not define task')
+                ret['comment'] = '\n'.join(comments)
                 return ret
         ret['changes']['diff'] = '\n'.join(difflib.unified_diff(
             old_script.splitlines(),
@@ -101,7 +106,12 @@ def task_present(name,
                 ret['result'] = None
                 comments.append('Task would have been enabled')
             else:
-                __salt__['kapacitor.enable_task'](name)
+                result = __salt__['kapacitor.enable_task'](name)
+                if not result:
+                    ret['result'] = False
+                    comments.append('Could not enable task')
+                    ret['comment'] = '\n'.join(comments)
+                    return ret
                 comments.append('Task was enabled')
             ret['changes']['enabled'] = True
     else:
@@ -112,7 +122,12 @@ def task_present(name,
                 ret['result'] = None
                 comments.append('Task would have been disabled')
             else:
-                __salt__['kapacitor.disable_task'](name)
+                result = __salt__['kapacitor.disable_task'](name)
+                if not result:
+                    ret['result'] = False
+                    comments.append('Could not disable task')
+                    ret['comment'] = '\n'.join(comments)
+                    return ret
                 comments.append('Task was disabled')
             ret['changes']['enabled'] = False
 

tests/unit/modules/kapacitor_test.py

@@ -0,0 +1,54 @@
+# -*- coding: utf-8 -*-
+
+# Import Python libs
+from __future__ import absolute_import
+
+# Import Salt libs
+from salt.modules import kapacitor
+
+# Import Salt testing libs
+from salttesting import TestCase
+from salttesting.mock import Mock, patch
+
+kapacitor.__salt__ = {
+    'config.option': Mock(side_effect=lambda key, default: default)
+}
+kapacitor.__env__ = 'test'
+
+
+class KapacitorTestCase(TestCase):
+    def test_get_task_success(self):
+        with patch('salt.utils.http.query', return_value={'body': '{"foo":"bar"}'}) as http_mock:
+            task = kapacitor.get_task('taskname')
+        http_mock.assert_called_once_with('http://localhost:9092/task?name=taskname')
+        assert {'foo': 'bar'} == task
+
+    def test_get_task_not_found(self):
+        with patch('salt.utils.http.query', return_value={'body': '{"Error":"unknown task taskname"}'}) as http_mock:
+            task = kapacitor.get_task('taskname')
+        http_mock.assert_called_once_with('http://localhost:9092/task?name=taskname')
+        assert None == task
+
+    def test_define_task(self):
+        cmd_mock = Mock(return_value=True)
+        with patch.dict(kapacitor.__salt__, {'cmd.retcode': cmd_mock}):
+            kapacitor.define_task('taskname', '/tmp/script.tick')
+        cmd_mock.assert_called_once_with('kapacitor define -name taskname -tick /tmp/script.tick -type stream')
+
+    def test_enable_task(self):
+        cmd_mock = Mock(return_value=True)
+        with patch.dict(kapacitor.__salt__, {'cmd.retcode': cmd_mock}):
+            kapacitor.enable_task('taskname')
+        cmd_mock.assert_called_once_with('kapacitor enable taskname')
+
+    def test_disable_task(self):
+        cmd_mock = Mock(return_value=True)
+        with patch.dict(kapacitor.__salt__, {'cmd.retcode': cmd_mock}):
+            kapacitor.disable_task('taskname')
+        cmd_mock.assert_called_once_with('kapacitor disable taskname')
+
+    def test_delete_task(self):
+        cmd_mock = Mock(return_value=True)
+        with patch.dict(kapacitor.__salt__, {'cmd.retcode': cmd_mock}):
+            kapacitor.delete_task('taskname')
+        cmd_mock.assert_called_once_with('kapacitor delete tasks taskname')

tests/unit/states/kapacitor_test.py

@@ -0,0 +1,71 @@
+# -*- coding: utf-8 -*-
+
+# Import Python libs
+from __future__ import absolute_import
+
+# Import Salt testing libs
+from salttesting import TestCase
+from salttesting.mock import Mock, patch, mock_open
+
+# Import Salt libs
+from salt.states import kapacitor
+
+kapacitor.__opts__ = {'test': False}
+kapacitor.__salt__ = {}
+kapacitor.__env__ = 'test'
+
+
+def _present(name='testname',
+             tick_script='/tmp/script.tick',
+             task_type='stream',
+             database='testdb',
+             retention_policy='default',
+             enable=True,
+             task=None,
+             define_result=True,
+             enable_result=True,
+             script='test'):
+    get_mock = Mock(return_value=task)
+    define_mock = Mock(return_value=define_result)
+    enable_mock = Mock(return_value=enable_result)
+    with patch.dict(kapacitor.__salt__, {
+        'kapacitor.get_task': get_mock,
+        'kapacitor.define_task': define_mock,
+        'kapacitor.enable_task': enable_mock,
+    }):
+        with patch('salt.utils.fopen', mock_open(read_data=script)) as open_mock:
+            retval = kapacitor.task_present(name, tick_script, task_type=task_type,
+                database=database, retention_policy=retention_policy, enable=enable)
+    return retval, get_mock, define_mock, enable_mock
+
+
+class KapacitorTestCase(TestCase):
+    def test_task_present_new_task(self):
+        ret, get_mock, define_mock, enable_mock = _present()
+        get_mock.assert_called_once_with('testname')
+        define_mock.assert_called_once_with('testname', '/tmp/script.tick',
+            database='testdb', retention_policy='default', task_type='stream')
+        enable_mock.assert_called_once_with('testname')
+        self.assertIn('diff', ret['changes'])
+        self.assertIn('enabled', ret['changes'])
+        self.assertEqual(True, ret['changes']['enabled'])
+
+    def test_task_present_existing_task(self):
+        old_task = {'TICKscript': 'old_task', 'Enabled': True}
+        ret, get_mock, define_mock, enable_mock = _present(task=old_task)
+        get_mock.assert_called_once_with('testname')
+        define_mock.assert_called_once_with('testname', '/tmp/script.tick',
+            database='testdb', retention_policy='default', task_type='stream')
+        self.assertEqual(False, enable_mock.called)
+        self.assertIn('diff', ret['changes'])
+        self.assertNotIn('enabled', ret['changes'])
+
+    def test_task_present_not_enabled(self):
+        old_task = {'TICKscript': 'test', 'Enabled': False}
+        ret, get_mock, define_mock, enable_mock = _present(task=old_task)
+        get_mock.assert_called_once_with('testname')
+        self.assertEqual(False, define_mock.called)
+        enable_mock.assert_called_once_with('testname')
+        self.assertNotIn('diff', ret['changes'])
+        self.assertIn('enabled', ret['changes'])
+        self.assertEqual(True, ret['changes']['enabled'])