diff --git a/bin/swift-ring-builder-analyzer b/bin/swift-ring-builder-analyzer new file mode 100755 index 0000000000..18365777f3 --- /dev/null +++ b/bin/swift-ring-builder-analyzer @@ -0,0 +1,22 @@ +#!/usr/bin/python +# Copyright (c) 2015 Samuel Merritt +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import sys +from swift.cli.ring_builder_analyzer import main + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/doc/source/overview_ring.rst b/doc/source/overview_ring.rst index 118a437788..d1f43affa5 100644 --- a/doc/source/overview_ring.rst +++ b/doc/source/overview_ring.rst @@ -237,6 +237,12 @@ when the balance doesn't improve by at least 1% (indicating we probably can't get perfect balance due to wildly imbalanced zones or too many partitions recently moved). +--------------------- +Ring Builder Analyzer +--------------------- +.. automodule:: swift.cli.ring_builder_analyzer + + ------- History ------- diff --git a/setup.cfg b/setup.cfg index 4b648b1109..d983a11a41 100644 --- a/setup.cfg +++ b/setup.cfg @@ -60,6 +60,7 @@ scripts = bin/swift-recon bin/swift-recon-cron bin/swift-ring-builder + bin/swift-ring-builder-analyzer bin/swift-temp-url [entry_points] diff --git a/swift/cli/ring_builder_analyzer.py b/swift/cli/ring_builder_analyzer.py new file mode 100644 index 0000000000..26d964bb8b --- /dev/null +++ b/swift/cli/ring_builder_analyzer.py @@ -0,0 +1,325 @@ +#! /usr/bin/env python +# Copyright (c) 2015 Samuel Merritt +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +This is a tool for analyzing how well the ring builder performs its job +in a particular scenario. It is intended to help developers quantify any +improvements or regressions in the ring builder; it is probably not useful +to others. + +The ring builder analyzer takes a scenario file containing some initial +parameters for a ring builder plus a certain number of rounds. In each +round, some modifications are made to the builder, e.g. add a device, remove +a device, change a device's weight. Then, the builder is repeatedly +rebalanced until it settles down. Data about that round is printed, and the +next round begins. + +Scenarios are specified in JSON. Example scenario for a gradual device +addition:: + + { + "part_power": 12, + "replicas": 3, + "overload": 0.1, + "random_seed": 203488, + + "rounds": [ + [ + ["add", "r1z2-10.20.30.40:6000/sda", 8000], + ["add", "r1z2-10.20.30.40:6000/sdb", 8000], + ["add", "r1z2-10.20.30.40:6000/sdc", 8000], + ["add", "r1z2-10.20.30.40:6000/sdd", 8000], + + ["add", "r1z2-10.20.30.41:6000/sda", 8000], + ["add", "r1z2-10.20.30.41:6000/sdb", 8000], + ["add", "r1z2-10.20.30.41:6000/sdc", 8000], + ["add", "r1z2-10.20.30.41:6000/sdd", 8000], + + ["add", "r1z2-10.20.30.43:6000/sda", 8000], + ["add", "r1z2-10.20.30.43:6000/sdb", 8000], + ["add", "r1z2-10.20.30.43:6000/sdc", 8000], + ["add", "r1z2-10.20.30.43:6000/sdd", 8000], + + ["add", "r1z2-10.20.30.44:6000/sda", 8000], + ["add", "r1z2-10.20.30.44:6000/sdb", 8000], + ["add", "r1z2-10.20.30.44:6000/sdc", 8000] + ], [ + ["add", "r1z2-10.20.30.44:6000/sdd", 1000] + ], [ + ["set_weight", 15, 2000] + ], [ + ["remove", 3], + ["set_weight", 15, 3000] + ], [ + ["set_weight", 15, 4000] + ], [ + ["set_weight", 15, 5000] + ], [ + ["set_weight", 15, 6000] + ], [ + ["set_weight", 15, 7000] + ], [ + ["set_weight", 15, 8000] + ]] + } + +""" + +import argparse +import json +import sys + +from swift.common.ring import builder +from swift.common.ring.utils import parse_add_value + + +ARG_PARSER = argparse.ArgumentParser( + description='Put the ring builder through its paces') +ARG_PARSER.add_argument( + '--check', '-c', action='store_true', + help="Just check the scenario, don't execute it.") +ARG_PARSER.add_argument( + 'scenario_path', + help="Path to the scenario file") + + +def _parse_weight(round_index, command_index, weight_str): + try: + weight = float(weight_str) + except ValueError as err: + raise ValueError( + "Invalid weight %r (round %d, command %d): %s" + % (weight_str, round_index, command_index, err)) + if weight < 0: + raise ValueError( + "Negative weight (round %d, command %d)" + % (round_index, command_index)) + return weight + + +def _parse_add_command(round_index, command_index, command): + if len(command) != 3: + raise ValueError( + "Invalid add command (round %d, command %d): expected array of " + "length 3, but got %d" + % (round_index, command_index, len(command))) + + dev_str = command[1] + weight_str = command[2] + + try: + dev = parse_add_value(dev_str) + except ValueError as err: + raise ValueError( + "Invalid device specifier '%s' in add (round %d, command %d): %s" + % (dev_str, round_index, command_index, err)) + + dev['weight'] = _parse_weight(round_index, command_index, weight_str) + + if dev['region'] is None: + dev['region'] = 1 + + return ['add', dev] + + +def _parse_remove_command(round_index, command_index, command): + if len(command) != 2: + raise ValueError( + "Invalid remove command (round %d, command %d): expected array of " + "length 2, but got %d" + % (round_index, command_index, len(command))) + + dev_str = command[1] + + try: + dev_id = int(dev_str) + except ValueError as err: + raise ValueError( + "Invalid device ID '%s' in remove (round %d, command %d): %s" + % (dev_str, round_index, command_index, err)) + + return ['remove', dev_id] + + +def _parse_set_weight_command(round_index, command_index, command): + if len(command) != 3: + raise ValueError( + "Invalid remove command (round %d, command %d): expected array of " + "length 3, but got %d" + % (round_index, command_index, len(command))) + + dev_str = command[1] + weight_str = command[2] + + try: + dev_id = int(dev_str) + except ValueError as err: + raise ValueError( + "Invalid device ID '%s' in set_weight (round %d, command %d): %s" + % (dev_str, round_index, command_index, err)) + + weight = _parse_weight(round_index, command_index, weight_str) + return ['set_weight', dev_id, weight] + + +def parse_scenario(scenario_data): + """ + Takes a serialized scenario and turns it into a data structure suitable + for feeding to run_scenario(). + + :returns: scenario + :raises: ValueError on invalid scenario + """ + + parsed_scenario = {} + + try: + raw_scenario = json.loads(scenario_data) + except ValueError as err: + raise ValueError("Invalid JSON in scenario file: %s" % err) + + if not isinstance(raw_scenario, dict): + raise ValueError("Scenario must be a JSON object, not array or string") + + if 'part_power' not in raw_scenario: + raise ValueError("part_power missing") + try: + parsed_scenario['part_power'] = int(raw_scenario['part_power']) + except ValueError as err: + raise ValueError("part_power not an integer: %s" % err) + if not 1 <= parsed_scenario['part_power'] <= 32: + raise ValueError("part_power must be between 1 and 32, but was %d" + % raw_scenario['part_power']) + + if 'replicas' not in raw_scenario: + raise ValueError("replicas missing") + try: + parsed_scenario['replicas'] = float(raw_scenario['replicas']) + except ValueError as err: + raise ValueError("replicas not a float: %s" % err) + if parsed_scenario['replicas'] < 1: + raise ValueError("replicas must be at least 1, but is %f" + % parsed_scenario['replicas']) + + if 'overload' not in raw_scenario: + raise ValueError("overload missing") + try: + parsed_scenario['overload'] = float(raw_scenario['overload']) + except ValueError as err: + raise ValueError("overload not a float: %s" % err) + if parsed_scenario['overload'] < 0: + raise ValueError("overload must be non-negative, but is %f" + % parsed_scenario['overload']) + + if 'random_seed' not in raw_scenario: + raise ValueError("random_seed missing") + try: + parsed_scenario['random_seed'] = int(raw_scenario['random_seed']) + except ValueError as err: + raise ValueError("replicas not an integer: %s" % err) + + if 'rounds' not in raw_scenario: + raise ValueError("rounds missing") + if not isinstance(raw_scenario['rounds'], list): + raise ValueError("rounds must be an array") + + parser_for_command = {'add': _parse_add_command, + 'remove': _parse_remove_command, + 'set_weight': _parse_set_weight_command} + + parsed_scenario['rounds'] = [] + for round_index, raw_round in enumerate(raw_scenario['rounds']): + if not isinstance(raw_round, list): + raise ValueError("round %d not an array" % round_index) + + parsed_round = [] + for command_index, command in enumerate(raw_round): + if command[0] not in parser_for_command: + raise ValueError( + "Unknown command (round %d, command %d): " + "'%s' should be one of %s" % + (round_index, command_index, command[0], + parser_for_command.keys())) + parsed_round.append( + parser_for_command[command[0]]( + round_index, command_index, command)) + parsed_scenario['rounds'].append(parsed_round) + return parsed_scenario + + +def run_scenario(scenario): + """ + Takes a parsed scenario (like from parse_scenario()) and runs it. + """ + seed = scenario['random_seed'] + + rb = builder.RingBuilder(scenario['part_power'], scenario['replicas'], 1) + rb.set_overload(scenario['overload']) + for round_index, commands in enumerate(scenario['rounds']): + print "Round %d" % (round_index + 1) + + for command in commands: + if command[0] == 'add': + rb.add_dev(command[1]) + elif command[0] == 'remove': + rb.remove_dev(command[1]) + elif command[0] == 'set_weight': + rb.set_dev_weight(command[1], command[2]) + else: + raise ValueError("unknown command %r" % (command[0],)) + + rebalance_number = 1 + parts_moved, old_balance = rb.rebalance(seed=seed) + rb.pretend_min_part_hours_passed() + print "\tRebalance 1: moved %d parts, balance is %.6f" % ( + parts_moved, old_balance) + + while True: + rebalance_number += 1 + parts_moved, new_balance = rb.rebalance(seed=seed) + rb.pretend_min_part_hours_passed() + print "\tRebalance %d: moved %d parts, balance is %.6f" % ( + rebalance_number, parts_moved, new_balance) + if parts_moved == 0: + break + if abs(new_balance - old_balance) < 1 and not ( + old_balance == builder.MAX_BALANCE and + new_balance == builder.MAX_BALANCE): + break + old_balance = new_balance + + +def main(argv=None): + args = ARG_PARSER.parse_args(argv) + + try: + with open(args.scenario_path) as sfh: + scenario_data = sfh.read() + except OSError as err: + sys.stderr.write("Error opening scenario %s: %s\n" % + (args.scenario_path, err)) + return 1 + + try: + scenario = parse_scenario(scenario_data) + except ValueError as err: + sys.stderr.write("Invalid scenario %s: %s\n" % + (args.scenario_path, err)) + return 1 + + if not args.check: + run_scenario(scenario) + return 0 diff --git a/swift/cli/ringbuilder.py b/swift/cli/ringbuilder.py index a886ff28ba..5e9d5b2c00 100755 --- a/swift/cli/ringbuilder.py +++ b/swift/cli/ringbuilder.py @@ -34,7 +34,7 @@ from swift.common.ring.utils import validate_args, \ validate_and_normalize_ip, build_dev_from_opts, \ parse_builder_ring_filename_args, parse_search_value, \ parse_search_values_from_opts, parse_change_values_from_opts, \ - dispersion_report, validate_device_name + dispersion_report, parse_add_value from swift.common.utils import lock_parent_directory MAJOR_VERSION = 1 @@ -129,37 +129,6 @@ def _parse_list_parts_values(argvish): exit(EXIT_ERROR) -def _parse_address(rest): - if rest.startswith('['): - # remove first [] for ip - rest = rest.replace('[', '', 1).replace(']', '', 1) - - pos = 0 - while (pos < len(rest) and - not (rest[pos] == 'R' or rest[pos] == '/')): - pos += 1 - address = rest[:pos] - rest = rest[pos:] - - port_start = address.rfind(':') - if port_start == -1: - raise ValueError('Invalid port in add value') - - ip = address[:port_start] - try: - port = int(address[(port_start + 1):]) - except (TypeError, ValueError): - raise ValueError( - 'Invalid port %s in add value' % address[port_start:]) - - # if this is an ipv6 address then we want to convert it - # to all lowercase and use its fully expanded representation - # to make searches easier - ip = validate_and_normalize_ip(ip) - - return (ip, port, rest) - - def _parse_add_values(argvish): """ Parse devices to add as specified on the command line. @@ -183,62 +152,25 @@ def _parse_add_values(argvish): islice(args, 1, len(args), 2)) for devstr, weightstr in devs_and_weights: - region = 1 - rest = devstr - if devstr.startswith('r'): - i = 1 - while i < len(devstr) and devstr[i].isdigit(): - i += 1 - region = int(devstr[1:i]) - rest = devstr[i:] - else: + dev_dict = parse_add_value(devstr) + + if dev_dict['region'] is None: stderr.write('WARNING: No region specified for %s. ' 'Defaulting to region 1.\n' % devstr) + dev_dict['region'] = 1 - if not rest.startswith('z'): - raise ValueError('Invalid add value: %s' % devstr) - i = 1 - while i < len(rest) and rest[i].isdigit(): - i += 1 - zone = int(rest[1:i]) - rest = rest[i:] + if dev_dict['replication_ip'] is None: + dev_dict['replication_ip'] = dev_dict['ip'] - if not rest.startswith('-'): - raise ValueError('Invalid add value: %s' % devstr) - - ip, port, rest = _parse_address(rest[1:]) - - replication_ip = ip - replication_port = port - if rest.startswith('R'): - replication_ip, replication_port, rest = \ - _parse_address(rest[1:]) - if not rest.startswith('/'): - raise ValueError( - 'Invalid add value: %s' % devstr) - i = 1 - while i < len(rest) and rest[i] != '_': - i += 1 - device_name = rest[1:i] - if not validate_device_name(device_name): - raise ValueError('Invalid device name') - - rest = rest[i:] - - meta = '' - if rest.startswith('_'): - meta = rest[1:] + if dev_dict['replication_port'] is None: + dev_dict['replication_port'] = dev_dict['port'] weight = float(weightstr) - if weight < 0: raise ValueError('Invalid weight value: %s' % devstr) + dev_dict['weight'] = weight - parsed_devs.append({'region': region, 'zone': zone, 'ip': ip, - 'port': port, 'device': device_name, - 'replication_ip': replication_ip, - 'replication_port': replication_port, - 'weight': weight, 'meta': meta}) + parsed_devs.append(dev_dict) else: parsed_devs.append(build_dev_from_opts(opts)) diff --git a/swift/common/ring/utils.py b/swift/common/ring/utils.py index 7d7856ebfc..ad9ff3fddd 100644 --- a/swift/common/ring/utils.py +++ b/swift/common/ring/utils.py @@ -403,7 +403,7 @@ def parse_search_values_from_opts(opts): Convert optparse style options into a dictionary for searching. :param opts: optparse style options - :returns: a dictonary with search values to filter devices, + :returns: a dictionary with search values to filter devices, supported parameters are id, region, zone, ip, port, replication_ip, replication_port, device, weight, meta """ @@ -440,6 +440,100 @@ def parse_change_values_from_opts(opts): return change_values +def parse_add_value(add_value): + """ + Convert an add value, like 'r1z2-10.1.2.3:7878/sdf', to a dictionary. + + If the string does not start with 'r', then the value of 'region' in + the returned dictionary will be None. Callers should check for this and + set a reasonable default. This is done so callers can emit errors or + warnings if desired. + + Similarly, 'replication_ip' and 'replication_port' will be None if not + specified. + + :returns: dictionary with keys 'region', 'zone', 'ip', 'port', 'device', + 'replication_ip', 'replication_port', 'meta' + :raises: ValueError if add_value is malformed + """ + region = None + rest = add_value + if add_value.startswith('r'): + i = 1 + while i < len(add_value) and add_value[i].isdigit(): + i += 1 + region = int(add_value[1:i]) + rest = add_value[i:] + + if not rest.startswith('z'): + raise ValueError('Invalid add value: %s' % add_value) + i = 1 + while i < len(rest) and rest[i].isdigit(): + i += 1 + zone = int(rest[1:i]) + rest = rest[i:] + + if not rest.startswith('-'): + raise ValueError('Invalid add value: %s' % add_value) + + ip, port, rest = parse_address(rest[1:]) + + replication_ip = replication_port = None + if rest.startswith('R'): + replication_ip, replication_port, rest = \ + parse_address(rest[1:]) + if not rest.startswith('/'): + raise ValueError( + 'Invalid add value: %s' % add_value) + i = 1 + while i < len(rest) and rest[i] != '_': + i += 1 + device_name = rest[1:i] + if not validate_device_name(device_name): + raise ValueError('Invalid device name') + + rest = rest[i:] + + meta = '' + if rest.startswith('_'): + meta = rest[1:] + + return {'region': region, 'zone': zone, 'ip': ip, 'port': port, + 'device': device_name, 'replication_ip': replication_ip, + 'replication_port': replication_port, 'meta': meta} + + +def parse_address(rest): + if rest.startswith('['): + # remove first [] for ip + rest = rest.replace('[', '', 1).replace(']', '', 1) + + pos = 0 + while (pos < len(rest) and + not (rest[pos] == 'R' or rest[pos] == '/')): + pos += 1 + address = rest[:pos] + rest = rest[pos:] + + port_start = address.rfind(':') + if port_start == -1: + raise ValueError('Invalid port in add value') + + ip = address[:port_start] + try: + port = int(address[(port_start + 1):]) + except (TypeError, ValueError): + raise ValueError( + 'Invalid port %s in add value' % address[port_start:]) + + # if this is an ipv6 address then we want to convert it + # to all lowercase and use its fully expanded representation + # to make searches easier + ip = validate_and_normalize_ip(ip) + + return (ip, port, rest) + + def validate_args(argvish): """ Build OptionParse and validate it whether the format is new command-line diff --git a/test/unit/cli/test_ring_builder_analyzer.py b/test/unit/cli/test_ring_builder_analyzer.py new file mode 100644 index 0000000000..52ceb8e354 --- /dev/null +++ b/test/unit/cli/test_ring_builder_analyzer.py @@ -0,0 +1,227 @@ +#! /usr/bin/env python +# Copyright (c) 2015 Samuel Merritt +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import json +import mock +import unittest +from StringIO import StringIO + +from swift.cli.ring_builder_analyzer import parse_scenario, run_scenario + + +class TestRunScenario(unittest.TestCase): + def test_it_runs(self): + scenario = { + 'replicas': 3, 'part_power': 8, 'random_seed': 123, 'overload': 0, + 'rounds': [[['add', 'r1z2-3.4.5.6:7/sda8', 100], + ['add', 'z2-3.4.5.6:7/sda9', 200]], + [['set_weight', 0, 150]], + [['remove', 1]]]} + parsed = parse_scenario(json.dumps(scenario)) + + fake_stdout = StringIO() + with mock.patch('sys.stdout', fake_stdout): + run_scenario(parsed) + + # Just test that it produced some output as it ran; the fact that + # this doesn't crash and produces output that resembles something + # useful is good enough. + self.assertTrue('Rebalance' in fake_stdout.getvalue()) + + +class TestParseScenario(unittest.TestCase): + def test_good(self): + scenario = { + 'replicas': 3, 'part_power': 8, 'random_seed': 123, 'overload': 0, + 'rounds': [[['add', 'r1z2-3.4.5.6:7/sda8', 100], + ['add', 'z2-3.4.5.6:7/sda9', 200]], + [['set_weight', 0, 150]], + [['remove', 1]]]} + parsed = parse_scenario(json.dumps(scenario)) + + self.assertEqual(parsed['replicas'], 3) + self.assertEqual(parsed['part_power'], 8) + self.assertEqual(parsed['random_seed'], 123) + self.assertEqual(parsed['overload'], 0) + self.assertEqual(parsed['rounds'], [ + [['add', {'device': 'sda8', + 'ip': '3.4.5.6', + 'meta': '', + 'port': 7, + 'region': 1, + 'replication_ip': None, + 'replication_port': None, + 'weight': 100.0, + 'zone': 2}], + ['add', {'device': u'sda9', + 'ip': u'3.4.5.6', + 'meta': '', + 'port': 7, + 'region': 1, + 'replication_ip': None, + 'replication_port': None, + 'weight': 200.0, + 'zone': 2}]], + [['set_weight', 0, 150.0]], + [['remove', 1]]]) + + # The rest of this test class is just a catalog of the myriad ways that + # the input can be malformed. + def test_invalid_json(self): + self.assertRaises(ValueError, parse_scenario, "{") + + def test_json_not_object(self): + self.assertRaises(ValueError, parse_scenario, "[]") + self.assertRaises(ValueError, parse_scenario, "\"stuff\"") + + def test_bad_replicas(self): + working_scenario = { + 'replicas': 3, 'part_power': 8, 'random_seed': 123, 'overload': 0, + 'rounds': [[['add', 'r1z2-3.4.5.6:7/sda8', 100]]]} + + busted = dict(working_scenario) + del busted['replicas'] + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + busted = dict(working_scenario, replicas='blahblah') + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + busted = dict(working_scenario, replicas=-1) + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + def test_bad_part_power(self): + working_scenario = { + 'replicas': 3, 'part_power': 8, 'random_seed': 123, 'overload': 0, + 'rounds': [[['add', 'r1z2-3.4.5.6:7/sda8', 100]]]} + + busted = dict(working_scenario) + del busted['part_power'] + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + busted = dict(working_scenario, part_power='blahblah') + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + busted = dict(working_scenario, part_power=0) + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + busted = dict(working_scenario, part_power=33) + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + def test_bad_random_seed(self): + working_scenario = { + 'replicas': 3, 'part_power': 8, 'random_seed': 123, 'overload': 0, + 'rounds': [[['add', 'r1z2-3.4.5.6:7/sda8', 100]]]} + + busted = dict(working_scenario) + del busted['random_seed'] + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + busted = dict(working_scenario, random_seed='blahblah') + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + def test_bad_overload(self): + working_scenario = { + 'replicas': 3, 'part_power': 8, 'random_seed': 123, 'overload': 0, + 'rounds': [[['add', 'r1z2-3.4.5.6:7/sda8', 100]]]} + + busted = dict(working_scenario) + del busted['overload'] + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + busted = dict(working_scenario, overload='blahblah') + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + busted = dict(working_scenario, overload=-0.01) + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + def test_bad_rounds(self): + base = { + 'replicas': 3, 'part_power': 8, 'random_seed': 123, 'overload': 0} + + self.assertRaises(ValueError, parse_scenario, json.dumps(base)) + + busted = dict(base, rounds={}) + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + busted = dict(base, rounds=[{}]) + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + busted = dict(base, rounds=[[['bork']]]) + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + def test_bad_add(self): + base = { + 'replicas': 3, 'part_power': 8, 'random_seed': 123, 'overload': 0} + + # no dev + busted = dict(base, rounds=[[['add']]]) + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + # no weight + busted = dict(base, rounds=[[['add', 'r1z2-1.2.3.4:6000/d7']]]) + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + # too many fields + busted = dict(base, rounds=[[['add', 'r1z2-1.2.3.4:6000/d7', 1, 2]]]) + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + # can't parse + busted = dict(base, rounds=[[['add', 'not a good value', 100]]]) + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + # negative weight + busted = dict(base, rounds=[[['add', 'r1z2-1.2.3.4:6000/d7', -1]]]) + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + def test_bad_remove(self): + base = { + 'replicas': 3, 'part_power': 8, 'random_seed': 123, 'overload': 0} + + # no dev + busted = dict(base, rounds=[[['remove']]]) + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + # bad dev id + busted = dict(base, rounds=[[['remove', 'not an int']]]) + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + # too many fields + busted = dict(base, rounds=[[['remove', 1, 2]]]) + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + def test_bad_set_weight(self): + base = { + 'replicas': 3, 'part_power': 8, 'random_seed': 123, 'overload': 0} + + # no dev + busted = dict(base, rounds=[[['set_weight']]]) + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + # no weight + busted = dict(base, rounds=[[['set_weight', 0]]]) + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + # bad dev id + busted = dict(base, rounds=[[['set_weight', 'not an int', 90]]]) + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + # negative weight + busted = dict(base, rounds=[[['set_weight', 1, -1]]]) + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) + + # bogus weight + busted = dict(base, rounds=[[['set_weight', 1, 'bogus']]]) + self.assertRaises(ValueError, parse_scenario, json.dumps(busted)) diff --git a/test/unit/cli/test_ringbuilder.py b/test/unit/cli/test_ringbuilder.py index 246f282f38..f3df11dc1f 100644 --- a/test/unit/cli/test_ringbuilder.py +++ b/test/unit/cli/test_ringbuilder.py @@ -180,14 +180,6 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): err = e self.assertEquals(err.code, 2) - def test_parse_address_old_format(self): - # Test old format - argv = "127.0.0.1:6000R127.0.0.1:6000/sda1_some meta data" - ip, port, rest = ringbuilder._parse_address(argv) - self.assertEqual(ip, '127.0.0.1') - self.assertEqual(port, 6000) - self.assertEqual(rest, 'R127.0.0.1:6000/sda1_some meta data') - def test_parse_add_values_number_of_arguments(self): # Test Number of arguments abnormal argv = ["--region", "2", "test"] diff --git a/test/unit/common/ring/test_utils.py b/test/unit/common/ring/test_utils.py index 4d078e6c00..efd073fde5 100644 --- a/test/unit/common/ring/test_utils.py +++ b/test/unit/common/ring/test_utils.py @@ -26,7 +26,8 @@ from swift.common.ring.utils import (tiers_for_dev, build_tier_tree, parse_change_values_from_opts, validate_args, parse_args, parse_builder_ring_filename_args, - build_dev_from_opts, dispersion_report) + build_dev_from_opts, dispersion_report, + parse_address) class TestUtils(unittest.TestCase): @@ -694,6 +695,14 @@ class TestUtils(unittest.TestCase): self.assertEqual(report['worst_tier'], 'r1z0-127.0.0.1') self.assertEqual(report['max_dispersion'], 30.078125) + def test_parse_address_old_format(self): + # Test old format + argv = "127.0.0.1:6000R127.0.0.1:6000/sda1_some meta data" + ip, port, rest = parse_address(argv) + self.assertEqual(ip, '127.0.0.1') + self.assertEqual(port, 6000) + self.assertEqual(rest, 'R127.0.0.1:6000/sda1_some meta data') + if __name__ == '__main__': unittest.main()