Merge lp:~sseman/juju-chaos-monkey/command-validation into lp:juju-chaos-monkey

[Seman]

This branch adds command validation.

[lp:~sseman/juju-chaos-monkey/command-validation]

16. By Seman on 2015-05-16

Added command validation.

17. By Seman on 2015-05-18

Used exception rather than returning errors.

18. By Seman on 2015-05-19

Updated exception error messages.

[John George (jog)]

Thank you.

review: Approve

=== modified file 'chaos_monkey.py'
--- chaos_monkey.py	2015-05-13 17:01:50 +0000
+++ chaos_monkey.py	2015-05-19 00:17:54 +0000
@@ -57,6 +57,14 @@
             self.chaos.remove(group)
 
     @staticmethod
+    def get_all_groups():
+        return list(set(c.group for c in ChaosMonkey.get_all_chaos()[0]))
+
+    @staticmethod
+    def get_all_commands():
+        return [c.command_str for c in ChaosMonkey.get_all_chaos()[0]]
+
+    @staticmethod
     def get_groups(groups, chaos):
         ret_groups = []
         for group in groups:

=== modified file 'runner.py'
--- runner.py	2015-05-14 06:46:36 +0000
+++ runner.py	2015-05-19 00:17:54 +0000
@@ -81,10 +81,9 @@
         if enablement_timeout < 0:
             raise BadRequest("Invalid value for enablement timeout")
 
-        self.filter_commands(include_group=include_group,
-                             exclude_group=exclude_group,
-                             include_command=include_command,
-                             exclude_command=exclude_command)
+        self.filter_commands(
+            include_group=include_group, exclude_group=exclude_group,
+            include_command=include_command, exclude_command=exclude_command)
         expire_time = time() + run_timeout
         while time() < expire_time:
             if self.stop_chaos or self.dry_run:
@@ -106,23 +105,37 @@
 
     def filter_commands(self, include_group=None, exclude_group=None,
                         include_command=None, exclude_command=None):
+        all_groups = ChaosMonkey.get_all_groups()
+        all_commands = ChaosMonkey.get_all_commands()
+
         # If any groups and any commands are not included, assume the intent
         #  is to include all groups and all commands.
         if not include_group and not include_command:
             self.chaos_monkey.include_group('all')
         if include_group:
-            include_group = split_arg_string(include_group)
+            include_group = self._validate(include_group, all_groups)
             self.chaos_monkey.include_group(include_group)
         if exclude_group:
-            exclude_group = split_arg_string(exclude_group)
+            exclude_group = self._validate(exclude_group, all_groups)
             self.chaos_monkey.exclude_group(exclude_group)
         if include_command:
-            include_command = split_arg_string(include_command)
+            include_command = self._validate(
+                include_command, all_commands)
             self.chaos_monkey.include_command(include_command)
         if exclude_command:
-            exclude_command = split_arg_string(exclude_command)
+            exclude_command = self._validate(
+                exclude_command, all_commands)
             self.chaos_monkey.exclude_command(exclude_command)
 
+    @staticmethod
+    def _validate(sub_string, all_list):
+        sub_list = split_arg_string(sub_string)
+        for item in sub_list:
+            if item not in all_list:
+                raise BadRequest(
+                    'Invalid value given on command line: {}'.format(item))
+        return sub_list
+
     def sig_handler(self, sig_num, frame):
         """Set the stop_chaos flag, to request a safe exit."""
         logging.info('Caught signal {}: Waiting for graceful exit.\n'.format(
@@ -144,7 +157,7 @@
         'path',
         help='An existing directory, to be used as a workspace.')
     parser.add_argument(
-        '-pt', '--enablement-timeout', default=10, type=int,
+        '-et', '--enablement-timeout', default=10, type=int,
         help="Enablement timeout in seconds")
     parser.add_argument(
         '-tt', '--total-timeout', default=60, type=int,
@@ -174,10 +187,16 @@
     runner.acquire_lock()
     logging.info('Chaos monkey started in {}'.format(args.path))
     logging.debug('Dry run is set to {}'.format(args.dry_run))
-    runner.random_chaos(run_timeout=args.total_timeout,
-                        enablement_timeout=args.enablement_timeout,
-                        include_group=args.include_group,
-                        exclude_group=args.exclude_group,
-                        include_command=args.include_command,
-                        exclude_command=args.exclude_command)
-    runner.cleanup()
+    try:
+        runner.random_chaos(
+            run_timeout=args.total_timeout,
+            enablement_timeout=args.enablement_timeout,
+            include_group=args.include_group,
+            exclude_group=args.exclude_group,
+            include_command=args.include_command,
+            exclude_command=args.exclude_command)
+    except Exception as e:
+        logging.error('{} ({})'.format(e, type(e).__name__))
+        sys.exit(1)
+    finally:
+        runner.cleanup()

=== modified file 'tests/test_chaos_monkey.py'
--- tests/test_chaos_monkey.py	2015-05-15 00:05:59 +0000
+++ tests/test_chaos_monkey.py	2015-05-19 00:17:54 +0000
@@ -26,7 +26,7 @@
         cm = ChaosMonkey.factory()
         all_chaos, all_factory_obj = cm.get_all_chaos()
         self.assertItemsEqual(
-            self._get_all_command_str(all_chaos),  self._command_strings())
+            self._get_command_str(all_chaos),  self._get_all_command_strings())
 
     def test_run_random_chaos(self):
         cm = ChaosMonkey.factory()
@@ -249,7 +249,7 @@
 
     def test_include_group_and_exclude_commands(self):
         groups = ['net']
-        commands = ['deny-all', 'deny-ssh']
+        commands = ['deny-all', 'deny-incoming']
         cm = ChaosMonkey.factory()
         cm.include_group(groups)
         self.assertGreaterEqual(len(cm.chaos), 1)
@@ -299,8 +299,19 @@
         self.assertTrue(any(c.group == 'net' for c in group))
         self.assertTrue(any(c.group == 'kill' for c in group))
 
-    def _get_all_command_str(self, chaos):
+    def test_get_all_commands(self):
+        all_commands = ChaosMonkey.get_all_commands()
+        self.assertItemsEqual(all_commands, self._get_all_command_strings())
+
+    def test_get_all_groups(self):
+        all_groups = ChaosMonkey.get_all_groups()
+        self.assertItemsEqual(all_groups, self._get_all_groups())
+
+    def _get_command_str(self, chaos):
         return [c.command_str for c in chaos]
 
-    def _command_strings(self):
+    def _get_all_command_strings(self):
         return get_all_net_commands() + get_all_kill_commands()
+
+    def _get_all_groups(self):
+        return ['net', 'kill']

=== modified file 'tests/test_runner.py'
--- tests/test_runner.py	2015-05-14 06:46:36 +0000
+++ tests/test_runner.py	2015-05-19 00:17:54 +0000
@@ -370,7 +370,7 @@
 
     def test_filter_commands_include_group_and_exclude_commands(self):
         include_group = 'net'
-        exclude_command = 'deny-all,deny-ssh'
+        exclude_command = 'deny-all,deny-incoming'
         with temp_dir() as directory:
             runner = Runner(directory, ChaosMonkey.factory())
             runner.filter_commands(include_group=include_group,
@@ -380,7 +380,7 @@
                         for c in runner.chaos_monkey.chaos))
         self.assertTrue(all(c.command_str != 'deny-all'
                         for c in runner.chaos_monkey.chaos))
-        self.assertTrue(all(c.command_str != 'deny-ssh'
+        self.assertTrue(all(c.command_str != 'deny-incoming'
                         for c in runner.chaos_monkey.chaos))
 
     def test_filter_commands_exclude_group_and_exclude_commands(self):
@@ -422,7 +422,7 @@
         include_group = 'net,kill'
         exclude_group = 'kill'
         include_command = 'jujud'
-        exclude_command = 'deny-all,monogd'
+        exclude_command = 'deny-all,mongod'
         with temp_dir() as directory:
             runner = Runner(directory, ChaosMonkey.factory())
             runner.filter_commands(include_group=include_group,
@@ -454,11 +454,11 @@
                                             include_group='net,kill',
                                             exclude_group='kill',
                                             include_command='deny-all',
-                                            exclude_command='deny-ssh')
+                                            exclude_command='deny-incoming')
         expected = {'include_group': 'net,kill',
                     'exclude_group': 'kill',
                     'include_command': 'deny-all',
-                    'exclude_command': 'deny-ssh'}
+                    'exclude_command': 'deny-incoming'}
         call_params = f_mock.call_args_list[0][1:]
         for k, v in call_params[0].items():
             self.assertEqual(expected[k], v)
@@ -469,6 +469,79 @@
         arg = split_arg_string('net')
         self.assertItemsEqual(arg, ['net'])
 
+    def test_validate_group(self):
+        groups = "net"
+        all_groups = ChaosMonkey.get_all_groups()
+        groups = Runner._validate(groups, all_groups)
+        self.assertItemsEqual(groups, ['net'])
+
+    def test_validate_groups(self):
+        groups = "net,kill"
+        all_groups = ChaosMonkey.get_all_groups()
+        groups = Runner._validate(groups, all_groups)
+        self.assertItemsEqual(groups, ['net', 'kill'])
+
+    def test_validate_incorrect_group(self):
+        groups = "net,killl"
+        all_groups = ChaosMonkey.get_all_groups()
+        with self.assertRaisesRegexp(
+                BadRequest,  "Invalid value given on command line: killl"):
+            Runner._validate(groups, all_groups)
+
+    def test_validate_command(self):
+        commands = "deny-all"
+        all_commands = ChaosMonkey.get_all_commands()
+        commands = Runner._validate(commands, all_commands)
+        self.assertItemsEqual(commands, ['deny-all'])
+
+    def test_validate_commands(self):
+        commands = "deny-all,jujud,deny-api-server"
+        all_commands = ChaosMonkey.get_all_commands()
+        commands = Runner._validate(commands, all_commands)
+        self.assertItemsEqual(
+            commands, ['deny-all', 'jujud', 'deny-api-server'])
+
+    def test_validate_incorrect_command(self):
+        commands = "deny-all,monogd,deny-api-server"
+        all_commands = ChaosMonkey.get_all_commands()
+        with self.assertRaisesRegexp(
+                BadRequest, "Invalid value given on command line: monogd"):
+            Runner._validate(commands, all_commands)
+
+    def test_filter_commands_include_incorrect_group(self):
+        include_group = 'net,killl'
+        with temp_dir() as directory:
+            runner = Runner(directory, ChaosMonkey.factory())
+            with self.assertRaisesRegexp(
+                    BadRequest, "Invalid value given on command line: killl"):
+                runner.filter_commands(include_group=include_group)
+
+    def test_filter_commands_exclude_incorrect_group(self):
+        exclude_group = 'net,killl'
+        with temp_dir() as directory:
+            runner = Runner(directory, ChaosMonkey.factory())
+            with self.assertRaisesRegexp(
+                    BadRequest, "Invalid value given on command line: killl"):
+                runner.filter_commands(exclude_group=exclude_group)
+
+    def test_filter_command_include_incorrect_command(self):
+        include_command = 'deny-all,deny-net'
+        with temp_dir() as directory:
+            runner = Runner(directory, ChaosMonkey.factory())
+            with self.assertRaisesRegexp(
+                    BadRequest,
+                    "Invalid value given on command line: deny-net"):
+                runner.filter_commands(include_command=include_command)
+
+    def test_filter_command_exclude_incorrect_command(self):
+        exclude_command = 'deny-all,deny-net,jujud'
+        with temp_dir() as directory:
+            runner = Runner(directory, ChaosMonkey.factory())
+            with self.assertRaisesRegexp(
+                    BadRequest,
+                    "Invalid value given on command line: deny-net"):
+                runner.filter_commands(exclude_command=exclude_command)
+
 
 def add_fake_group(chaos_monkey):
     chaos = Chaos(None, None, 'fake_group', 'fake_command_str', 'description')

=== modified file 'utility.py'
--- utility.py	2015-05-13 16:49:07 +0000
+++ utility.py	2015-05-19 00:17:54 +0000
@@ -50,6 +50,8 @@
 
 
 def split_arg_string(arg_string):
+    if not arg_string:
+        return []
     return arg_string.split(',') if ',' in arg_string else [arg_string]