diff --git a/doc/source/overview_ring.rst b/doc/source/overview_ring.rst index 1723220317..4fc327b6be 100644 --- a/doc/source/overview_ring.rst +++ b/doc/source/overview_ring.rst @@ -38,6 +38,8 @@ to be assigned to a device who's failure domain does not already have a replica for the part. Only a single replica of a part may be assigned to each device - you must have as many devices as replicas. +.. _ring_builder: + ------------ Ring Builder ------------ @@ -348,6 +350,10 @@ Ring Builder Analyzer --------------------- .. automodule:: swift.cli.ring_builder_analyzer +--------------- +Composite Rings +--------------- +.. automodule:: swift.common.ring.composite_builder ------- History diff --git a/doc/source/ring.rst b/doc/source/ring.rst index d8f5a611f4..cbb5e97d96 100644 --- a/doc/source/ring.rst +++ b/doc/source/ring.rst @@ -23,3 +23,11 @@ Ring Builder :members: :undoc-members: :show-inheritance: + +Composite Ring Builder +====================== + +.. automodule:: swift.common.ring.composite_builder + :members: + :undoc-members: + :show-inheritance: diff --git a/swift/cli/ringbuilder.py b/swift/cli/ringbuilder.py index bc7d1f9593..1e1f2d97a4 100644 --- a/swift/cli/ringbuilder.py +++ b/swift/cli/ringbuilder.py @@ -466,7 +466,12 @@ swift-ring-builder DEL - indicates that the device is marked for removal from ring and will be removed in next rebalance. """ - print('%s, build version %d' % (builder_file, builder.version)) + try: + builder_id = builder.id + except AttributeError: + builder_id = "(not assigned)" + print('%s, build version %d, id %s' % + (builder_file, builder.version, builder_id)) regions = 0 zones = 0 balance = 0 diff --git a/swift/common/ring/builder.py b/swift/common/ring/builder.py index 88932e490d..9a5d833556 100644 --- a/swift/common/ring/builder.py +++ b/swift/common/ring/builder.py @@ -19,6 +19,8 @@ import itertools import logging import math import random +import uuid + import six.moves.cPickle as pickle from copy import deepcopy from contextlib import contextmanager @@ -91,6 +93,7 @@ class RingBuilder(object): self.devs_changed = False self.version = 0 self.overload = 0.0 + self._id = None # _replica2part2dev maps from replica number to partition number to # device id. So, for a three replica, 2**23 ring, it's an array of @@ -127,6 +130,21 @@ class RingBuilder(object): # silence "no handler for X" error messages self.logger.addHandler(NullHandler()) + @property + def id(self): + if self._id is None: + # We don't automatically assign an id here because we want a caller + # to explicitly know when a builder needs an id to be assigned. In + # that case the caller must save the builder in order that a newly + # assigned id is persisted. + raise AttributeError( + 'id attribute has not been initialised by calling save()') + return self._id + + @property + def part_shift(self): + return 32 - self.part_power + def _set_part_moved(self, part): self._last_part_moves[part] = 0 byte, bit = divmod(part, 8) @@ -204,6 +222,7 @@ class RingBuilder(object): self._last_part_moves = builder._last_part_moves self._last_part_gather_start = builder._last_part_gather_start self._remove_devs = builder._remove_devs + self._id = getattr(builder, '_id', None) else: self.part_power = builder['part_power'] self.replicas = builder['replicas'] @@ -220,6 +239,7 @@ class RingBuilder(object): self._dispersion_graph = builder.get('_dispersion_graph', {}) self.dispersion = builder.get('dispersion') self._remove_devs = builder['_remove_devs'] + self._id = builder.get('id') self._ring = None # Old builders may not have a region defined for their devices, in @@ -254,7 +274,8 @@ class RingBuilder(object): '_last_part_gather_start': self._last_part_gather_start, '_dispersion_graph': self._dispersion_graph, 'dispersion': self.dispersion, - '_remove_devs': self._remove_devs} + '_remove_devs': self._remove_devs, + 'id': self._id} def change_min_part_hours(self, min_part_hours): """ @@ -315,12 +336,12 @@ class RingBuilder(object): # shift an unsigned int >I right to obtain the partition for the # int). if not self._replica2part2dev: - self._ring = RingData([], devs, 32 - self.part_power) + self._ring = RingData([], devs, self.part_shift) else: self._ring = \ RingData([array('H', p2d) for p2d in self._replica2part2dev], - devs, 32 - self.part_power) + devs, self.part_shift) return self._ring def add_dev(self, dev): @@ -369,6 +390,7 @@ class RingBuilder(object): self.devs.append(None) dev['weight'] = float(dev['weight']) dev['parts'] = 0 + dev.setdefault('meta', '') self.devs[dev['id']] = dev self.devs_changed = True self.version += 1 @@ -1650,6 +1672,10 @@ class RingBuilder(object): builder_dict = builder builder = RingBuilder(1, 1, 1) builder.copy_from(builder_dict) + + if not hasattr(builder, '_id'): + builder._id = None + for dev in builder.devs: # really old rings didn't have meta keys if dev and 'meta' not in dev: @@ -1668,8 +1694,21 @@ class RingBuilder(object): :param builder_file: path to builder file to save """ - with open(builder_file, 'wb') as f: - pickle.dump(self.to_dict(), f, protocol=2) + # We want to be sure the builder id's are persistent, so this is the + # only place where the id is assigned. Newly created instances of this + # class, or instances loaded from legacy builder files that have no + # persisted id, must be saved in order for an id to be assigned. + id_persisted = True + if self._id is None: + id_persisted = False + self._id = uuid.uuid4().hex + try: + with open(builder_file, 'wb') as f: + pickle.dump(self.to_dict(), f, protocol=2) + except Exception: + if not id_persisted: + self._id = None + raise def search_devs(self, search_values): """Search devices by parameters. diff --git a/swift/common/ring/composite_builder.py b/swift/common/ring/composite_builder.py new file mode 100644 index 0000000000..7bce4cb93b --- /dev/null +++ b/swift/common/ring/composite_builder.py @@ -0,0 +1,548 @@ +# Copyright (c) 2010-2017 OpenStack Foundation +# +# 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. +""" +A standard ring built using the :ref:`ring-builder ` will attempt +to randomly disperse replicas or erasure-coded fragments across failure +domains, but does not provide any guarantees such as placing at least one +replica of every partition into each region. Composite rings are intended to +provide operators with greater control over the dispersion of object replicas +or fragments across a cluster, in particular when there is a desire to +guarantee that some replicas or fragments are placed in certain failure +domains. + +A composite ring comprises two or more component rings that are combined to +form a single ring with a replica count equal to the sum of the component +rings. The component rings are built independently, using distinct devices in +distinct regions, which means that the dispersion of replicas between the +components can be guaranteed. The composite_builder utilities may +then be used to combine components into a composite ring. + +For example, consider a normal ring ``ring0`` with replica count of 4 and +devices in two regions ``r1`` and ``r2``. Despite the best efforts of the +ring-builder, it is possible for there to be three replicas of a particular +partition placed in one region and only one replica placed in the other region. +For example:: + + part_n -> r1z1h110/sdb r1z2h12/sdb r1z3h13/sdb r2z1h21/sdb + +Now consider two normal rings each with replica count of 2: ``ring1`` has +devices in only ``r1``; ``ring2`` has devices in only ``r2``. +When these rings are combined into a composite ring then every partition is +guaranteed to be mapped to two devices in each of ``r1`` and ``r2``, for +example:: + + part_n -> r1z1h10/sdb r1z2h20/sdb r2z1h21/sdb r2z2h22/sdb + |_____________________| |_____________________| + | | + ring1 ring2 + +The dispersion of partition replicas across failure domains within each of the +two component rings may change as they are modified and rebalanced, but the +dispersion of replicas between the two regions is guaranteed by the use of a +composite ring. + +For rings to be formed into a composite they must satisfy the following +requirements: + +* All component rings must have the same number of partitions +* All component rings must have an integer replica count +* Each region may only be used in one component ring +* Each device may only be used in one component ring + +Under the hood, the composite ring has a replica2part2dev_id table that is the +union of the tables from the component rings. Whenever the component rings are +rebalanced, the composite ring must be rebuilt. There is no dynamic rebuilding +of the composite ring. + +.. note:: + The order in which component rings are combined into a composite ring is + very significant because it determines the order in which the + Ring.get_part_nodes() method will provide primary nodes for the composite + ring and consequently the node indexes assigned to the primary nodes. For + an erasure-coded policy, inadvertent changes to the primary node indexes + could result in large amounts of data movement due to fragments being moved + to their new correct primary. + + The ``id`` of each component RingBuilder is therefore stored in metadata of + the composite and used to check for the component ordering when the same + composite ring is re-composed. RingBuilder id's are only assigned when a + RingBuilder instance is first saved. Older RingBuilders instances loaded + from file may not have an ``id`` assigned and will need to be saved before + they can be used as components of a composite ring. This can be achieved + by, for example:: + + swift-ring-builder rebalance --force + +""" + +import copy +import json +import os + +from swift.common.ring import RingBuilder +from swift.common.ring import RingData +from collections import defaultdict +from itertools import combinations + +MUST_MATCH_ATTRS = ( + 'part_power', +) + + +def pre_validate_all_builders(builders): + """ + Pre-validation for all component ring builders that are to be included in + the composite ring. Checks that all component rings are valid with respect + to each other. + + :param builders: a list of :class:`swift.common.ring.builder.RingBuilder` + instances + :raises ValueError: if the builders are invalid with respect to each other + """ + if len(builders) < 2: + raise ValueError('Two or more component builders are required.') + + # all ring builders should be consistent for each MUST_MATCH_ATTRS + for attr in MUST_MATCH_ATTRS: + attr_dict = defaultdict(list) + for i, builder in enumerate(builders): + value = getattr(builder, attr, None) + attr_dict[value].append(i) + if len(attr_dict) > 1: + variations = ['%s=%s found at indexes %s' % + (attr, val, indexes) + for val, indexes in attr_dict.items()] + raise ValueError( + 'All builders must have same value for %r.\n%s' + % (attr, '\n '.join(variations))) + + # all ring builders should have int replica count and not have dirty mods + errors = [] + for index, builder in enumerate(builders): + if int(builder.replicas) != builder.replicas: + errors.append( + 'Non integer replica count %s found at index %s' % + (builder.replicas, index)) + if builder.devs_changed: + errors.append( + 'Builder needs rebalance to apply changes at index %s' % + index) + if errors: + raise ValueError( + 'Problem with builders.\n%s' % ('\n '.join(errors))) + + # check regions + regions_info = {} + for builder in builders: + regions_info[builder] = set( + [dev['region'] for dev in builder._iter_devs()]) + for first_region_set, second_region_set in combinations( + regions_info.values(), 2): + inter = first_region_set & second_region_set + if inter: + raise ValueError('Same region found in different rings') + + # check device uniquness + check_for_dev_uniqueness(builders) + + +def check_for_dev_uniqueness(builders): + """ + Check that no device appears in more than one of the given list of + builders. + + :param builders: a list of :class:`swift.common.ring.builder.RingBuilder` + instances + :raises ValueError: if the same device is found in more than one builder + """ + builder2devs = [] + for i, builder in enumerate(builders): + dev_set = set() + for dev in builder._iter_devs(): + ip, port, device = (dev['ip'], dev['port'], dev['device']) + for j, (other_builder, devs) in enumerate(builder2devs): + if (ip, port, device) in devs: + raise ValueError( + 'Duplicate ip/port/device combination %s/%s/%s found ' + 'in builders at indexes %s and %s' % + (ip, port, device, j, i) + ) + dev_set.add((ip, port, device)) + builder2devs.append((builder, dev_set)) + + +def _make_composite_ring(builders): + """ + Given a list of component ring builders, return a composite RingData + instance. + + :param builders: a list of + :class:`swift.common.ring.builder.RingBuilder` instances + :return: a new RingData instance built from the component builders + :raises ValueError: if the builders are invalid with respect to each other + """ + composite_r2p2d = [] + composite_devs = [] + device_offset = 0 + for builder in builders: + # copy all devs list and replica2part2dev table to be able + # to modify the id for each dev + devs = copy.deepcopy(builder.devs) + r2p2d = copy.deepcopy(builder._replica2part2dev) + for part2dev in r2p2d: + for part, dev in enumerate(part2dev): + part2dev[part] += device_offset + for dev in [d for d in devs if d]: + # note that some devs may not be referenced in r2p2d but update + # their dev id nonetheless + dev['id'] += device_offset + composite_r2p2d.extend(r2p2d) + composite_devs.extend(devs) + device_offset += len(builder.devs) + + return RingData(composite_r2p2d, composite_devs, builders[0].part_shift) + + +def compose_rings(builders): + """ + Given a list of component ring builders, perform validation on the list of + builders and return a composite RingData instance. + + :param builders: a list of + :class:`swift.common.ring.builder.RingBuilder` instances + :return: a new RingData instance built from the component builders + :raises ValueError: if the builders are invalid with respect to each other + """ + pre_validate_all_builders(builders) + rd = _make_composite_ring(builders) + return rd + + +def _make_component_meta(builder): + """ + Return a dict of selected builder attributes to save in composite meta. The + dict has keys ``version``, ``replicas`` and ``id``. + :param builder: a :class:`swift.common.ring.builder.RingBuilder` + instance + :return: a dict of component metadata + """ + attrs = ['version', 'replicas', 'id'] + metadata = dict((attr, getattr(builder, attr)) for attr in attrs) + return metadata + + +def _make_composite_metadata(builders): + """ + Return a dict with key ``components`` that maps to a list of dicts, each + dict being of the form returned by :func:`_make_component_meta`. + + :param builders: a list of + :class:`swift.common.ring.builder.RingBuilder` instances + :return: a dict of composite metadata + """ + component_meta = [_make_component_meta(builder) for builder in builders] + return {'components': component_meta} + + +def check_same_builder(old_component, new_component): + """ + Check that the given new_component metadata describes the same builder as + the given old_component metadata. The new_component builder does not + necessarily need to be in the same state as when the old_component metadata + was created to satisfy this check e.g. it may have changed devs and been + rebalanced. + + :param old_component: a dict of metadata describing a component builder + :param new_component: a dict of metadata describing a component builder + :raises ValueError: if the new_component is not the same as that described + by the old_component + """ + for key in ['replicas', 'id']: + if old_component[key] != new_component[key]: + raise ValueError("Attribute mismatch for %s: %r != %r" % + (key, old_component[key], new_component[key])) + + +def is_builder_newer(old_component, new_component): + """ + Return True if the given builder has been modified with respect to its + state when the given component_meta was created. + + :param old_component: a dict of metadata describing a component ring + :param new_component: a dict of metadata describing a component ring + :return: True if the builder has been modified, False otherwise. + :raises ValueError: if the version of the new_component is older than the + version of the existing component. + """ + + if new_component['version'] < old_component['version']: + raise ValueError('Older builder version: %s < %s' % + (new_component['version'], old_component['version'])) + return old_component['version'] < new_component['version'] + + +def check_against_existing(old_composite_meta, new_composite_meta): + """ + Check that the given builders and their order are the same as that + used to build an existing composite ring. Return True if any of the given + builders has been modified with respect to its state when the given + component_meta was created. + + :param old_composite_meta: a dict of the form returned by + :func:`_make_composite_meta` + :param new_composite_meta: a dict of the form returned by + :func:`_make_composite_meta` + :return: True if any of the components has been modified, False otherwise. + :raises Value Error: if proposed new components do not match any existing + components. + """ + errors = [] + newer = False + old_components = old_composite_meta['components'] + new_components = new_composite_meta['components'] + for i, old_component in enumerate(old_components): + try: + new_component = new_components[i] + except IndexError: + errors.append("Missing builder at index %d" % i) + continue + try: + # check we have same component builder in this position vs existing + check_same_builder(old_component, new_component) + newer |= is_builder_newer(old_component, new_component) + except ValueError as err: + errors.append("Invalid builder change at index %d: %s" % (i, err)) + + for j, new_component in enumerate(new_components[i + 1:], start=i + 1): + errors.append("Unexpected extra builder at index %d: %r" % + (j, new_component)) + if errors: + raise ValueError('\n'.join(errors)) + return newer + + +def check_builder_ids(builders): + """ + Check that all builders in the given list have id's assigned and that no + id appears more than once in the list. + + :param builders: a list instances of + :class:`swift.common.ring.builder.RingBuilder` + :raises: ValueError if any builder id is missing or repeated + """ + id2index = defaultdict(list) + errors = [] + for i, builder in enumerate(builders): + try: + id2index[builder.id].append(str(i)) + except AttributeError as err: + errors.append("Problem with builder at index %d: %s" % (i, err)) + + for builder_id, index in id2index.items(): + if len(index) > 1: + errors.append("Builder id %r used at indexes %s" % + (builder_id, ', '.join(index))) + + if errors: + raise ValueError('\n'.join(errors)) + + +class CompositeRingBuilder(object): + """ + Provides facility to create, persist, load and update composite rings, for + example:: + + # create a CompositeRingBuilder instance with a list of + # component builder files + crb = CompositeRingBuilder(["region1.builder", "region2.builder"]) + + # call compose which will make a new RingData instance + ring_data = crb.compose() + + # save the composite ring file + ring_data.save("composite_ring.gz"") + + # save the composite metadata file + crb.save("composite_builder.composite") + + # load the persisted composite metadata file + crb = CompositeRingBuilder.load("composite_builder.composite") + + # compose (optionally update the paths to the component builder files) + crb.compose(["/path/to/region1.builder", "/path/to/region2.builder"]) + + Composite ring metadata is persisted to file in JSON format. The metadata + has the structure shown below (using example values):: + + { + "version": 4, + "components": [ + { + "version": 3, + "id": "8e56f3b692d43d9a666440a3d945a03a", + "replicas": 1 + }, + { + "version": 5, + "id": "96085923c2b644999dbfd74664f4301b", + "replicas": 1 + } + ] + "component_builder_files": { + "8e56f3b692d43d9a666440a3d945a03a": "/etc/swift/region1.builder", + "96085923c2b644999dbfd74664f4301b": "/etc/swift/region2.builder", + } + "serialization_version": 1, + "saved_path": "/etc/swift/multi-ring-1.composite", + } + + `version` is an integer representing the current version of the composite + ring, which increments each time the ring is successfully (re)composed. + + `components` is a list of dicts, each of which describes relevant + properties of a component ring + + `component_builder_files` is a dict that maps component ring builder ids to + the file from which that component ring builder was loaded. + + `serialization_version` is an integer constant. + + `saved_path` is the path to which the metadata was written. + + :params builder_files: a list of paths to builder files that will be used + as components of the composite ring. + """ + def __init__(self, builder_files=None): + self.version = 0 + self.components = [] + self.ring_data = None + self._builder_files = None + self._set_builder_files(builder_files or []) + + def _set_builder_files(self, builder_files): + self._builder_files = [os.path.abspath(bf) for bf in builder_files] + + @classmethod + def load(cls, path_to_file): + """ + Load composite ring metadata. + + :param path_to_file: Absolute path to a composite ring JSON file. + :return: an instance of :class:`CompositeRingBuilder` + :raises IOError: if there is a problem opening the file + :raises ValueError: if the file does not contain valid composite ring + metadata + """ + try: + with open(path_to_file, 'rb') as fp: + metadata = json.load(fp) + builder_files = [metadata['component_builder_files'][comp['id']] + for comp in metadata['components']] + + builder = CompositeRingBuilder(builder_files) + builder.components = metadata['components'] + builder.version = metadata['version'] + except (ValueError, TypeError, KeyError): + raise ValueError("File does not contain valid composite ring data") + return builder + + def to_dict(self): + """ + Transform the composite ring attributes to a dict. See + :class:`CompositeRingBuilder` for details of the persisted metadata + format. + + :return: a composite ring metadata dict + """ + id2builder_file = dict((component['id'], self._builder_files[i]) + for i, component in enumerate(self.components)) + return {'components': self.components, + 'component_builder_files': id2builder_file, + 'version': self.version} + + def save(self, path_to_file): + """ + Save composite ring metadata to given file. See + :class:`CompositeRingBuilder` for details of the persisted metadata + format. + + :param path_to_file: Absolute path to a composite ring file + :raises ValueError: if no composite ring has been built yet with this + instance + """ + if not self.components or not self._builder_files: + raise ValueError("No composed ring to save.") + # persist relative paths to builder files + with open(path_to_file, 'wb') as fp: + metadata = self.to_dict() + # future-proofing: + # - saving abs path to component builder files and this file should + # allow the relative paths to be derived if required when loading + # a set of {composite builder file, component builder files} that + # has been moved, so long as their relative locations are + # unchanged. + # - save a serialization format version number + metadata['saved_path'] = os.path.abspath(path_to_file) + metadata['serialization_version'] = 1 + json.dump(metadata, fp) + + def compose(self, builder_files=None, force=False): + """ + Builds a composite ring using component ring builders loaded from a + list of builder files. + + If a list of component ring builder files is given then that will be + used to load component ring builders. Otherwise, component ring + builders will be loaded using the list of builder files that was set + when the instance was constructed. + + In either case, if metadata for an existing composite ring has been + loaded then the component ring builders are verified for consistency + with the existing composition of builders, unless the optional + ``force`` flag if set True. + + + :param builder_files: Optional list of paths to ring builder + files that will be used to load the component ring builders. + Typically the list of component builder files will have been set + when the instance was constructed, for example when using the + load() class method. However, this parameter may be used if the + component builder file paths have moved, or, in conjunction with + the ``force`` parameter, if a new list of component builders is to + be used. + :param force: if True then do not verify given builders are consistent + with any existing composite ring. + :return: An instance of :class:`swift.common.ring.ring.RingData` + :raises: ValueError if the component ring builders are not suitable for + composing with each other, or are inconsistent with any existing + composite ring, or if there has been no change with respect to the + existing ring. + """ + builder_files = builder_files or self._builder_files + builders = [RingBuilder.load(f) for f in builder_files] + check_builder_ids(builders) + new_metadata = _make_composite_metadata(builders) + if self.components and self._builder_files and not force: + modified = check_against_existing(self.to_dict(), new_metadata) + if not modified: + raise ValueError( + "None of the component builders has been modified" + " since the existing composite ring was built.") + + self.ring_data = compose_rings(builders) + self.version += 1 + self.components = new_metadata['components'] + self._set_builder_files(builder_files) + return self.ring_data diff --git a/test/unit/cli/test_default_output.stub b/test/unit/cli/test_default_output.stub index 3b4786b479..e2daa77d38 100644 --- a/test/unit/cli/test_default_output.stub +++ b/test/unit/cli/test_default_output.stub @@ -1,4 +1,4 @@ -__RINGFILE__, build version 4 +__RINGFILE__, build version 4, id (not assigned) 64 partitions, 3.000000 replicas, 4 regions, 4 zones, 4 devices, 100.00 balance, 0.00 dispersion The minimum number of hours before a partition can be reassigned is 1 (0:00:00 remaining) The overload factor is 0.00% (0.000000) diff --git a/test/unit/cli/test_default_output_id_assigned.stub b/test/unit/cli/test_default_output_id_assigned.stub new file mode 100644 index 0000000000..9f60c03b50 --- /dev/null +++ b/test/unit/cli/test_default_output_id_assigned.stub @@ -0,0 +1,11 @@ +__RINGFILE__, build version 4, id __BUILDER_ID__ +64 partitions, 3.000000 replicas, 4 regions, 4 zones, 4 devices, 100.00 balance, 0.00 dispersion +The minimum number of hours before a partition can be reassigned is 1 (0:00:00 remaining) +The overload factor is 0.00% (0.000000) +Ring file __RINGFILE__.ring.gz not found, probably it hasn't been written yet +Devices: id region zone ip address:port replication ip:port name weight partitions balance flags meta + 0 0 0 127.0.0.1:6200 127.0.0.1:6200 sda1 100.00 0 -100.00 some meta data + 1 1 1 127.0.0.2:6201 127.0.0.2:6201 sda2 100.00 0 -100.00 + 2 2 2 127.0.0.3:6202 127.0.0.3:6202 sdc3 100.00 0 -100.00 + 3 3 3 127.0.0.4:6203 127.0.0.4:6203 sdd4 100.00 0 -100.00 + diff --git a/test/unit/cli/test_ipv6_output.stub b/test/unit/cli/test_ipv6_output.stub index 1691bb43af..423b8fae16 100644 --- a/test/unit/cli/test_ipv6_output.stub +++ b/test/unit/cli/test_ipv6_output.stub @@ -1,4 +1,4 @@ -__RINGFILE__, build version 4 +__RINGFILE__, build version 4, id __BUILDER_ID__ 256 partitions, 3.000000 replicas, 4 regions, 4 zones, 4 devices, 100.00 balance, 0.00 dispersion The minimum number of hours before a partition can be reassigned is 1 (0:00:00 remaining) The overload factor is 0.00% (0.000000) diff --git a/test/unit/cli/test_ringbuilder.py b/test/unit/cli/test_ringbuilder.py index cbd4be3449..568286c019 100644 --- a/test/unit/cli/test_ringbuilder.py +++ b/test/unit/cli/test_ringbuilder.py @@ -95,7 +95,8 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): except OSError: pass - def assertOutputStub(self, output, ext='stub'): + def assertOutputStub(self, output, ext='stub', + builder_id='(not assigned)'): """ assert that the given output string is equal to a in-tree stub file, if a test needs to check multiple outputs it can use custom ext's @@ -113,6 +114,7 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): else: self.fail('%r could not be read (%s)' % (filepath, e)) output = output.replace(self.tempfile, '__RINGFILE__') + stub = stub.replace('__BUILDER_ID__', builder_id) for i, (value, expected) in enumerate( itertools.izip_longest( output.splitlines(), stub.splitlines())): @@ -179,6 +181,7 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): 'device': 'sdd4' }) ring.save(self.tmpfile) + return ring def assertSystemExit(self, return_code, func, *argv): with self.assertRaises(SystemExit) as cm: @@ -1656,10 +1659,16 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) def test_default_output(self): - self.create_sample_ring() + with mock.patch('uuid.uuid4', return_value=mock.Mock(hex=None)): + self.create_sample_ring() out, err = self.run_srb('') self.assertOutputStub(out) + def test_default_output_id_assigned(self): + ring = self.create_sample_ring() + out, err = self.run_srb('') + self.assertOutputStub(out, builder_id=ring.id) + def test_ipv6_output(self): ring = RingBuilder(8, 3, 1) ring.add_dev({'weight': 100.0, @@ -1697,13 +1706,13 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): }) ring.save(self.tmpfile) out, err = self.run_srb('') - self.assertOutputStub(out) + self.assertOutputStub(out, builder_id=ring.id) def test_default_show_removed(self): mock_stdout = six.StringIO() mock_stderr = six.StringIO() - self.create_sample_ring() + ring = self.create_sample_ring() # Note: it also sets device's weight to zero. argv = ["", self.tmpfile, "remove", "--id", "1"] @@ -1719,7 +1728,7 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): with mock.patch("sys.stderr", mock_stderr): self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) - expected = "%s, build version 6\n" \ + expected = "%s, build version 6, id %s\n" \ "64 partitions, 3.000000 replicas, 4 regions, 4 zones, " \ "4 devices, 100.00 balance, 0.00 dispersion\n" \ "The minimum number of hours before a partition can be " \ @@ -1741,7 +1750,8 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): " 0 -100.00 \n" \ " 3 3 3 127.0.0.4:6203 " \ " 127.0.0.4:6203 sdd4 0.00" \ - " 0 0.00 \n" % (self.tmpfile, self.tmpfile) + " 0 0.00 \n" %\ + (self.tmpfile, ring.id, self.tmpfile) self.assertEqual(expected, mock_stdout.getvalue()) def test_default_ringfile_check(self): diff --git a/test/unit/common/ring/test_builder.py b/test/unit/common/ring/test_builder.py index 8fdc4c9629..7f5502b45e 100644 --- a/test/unit/common/ring/test_builder.py +++ b/test/unit/common/ring/test_builder.py @@ -2022,6 +2022,109 @@ class TestRingBuilder(unittest.TestCase): mock_fh.__enter__(), protocol=2) + def test_id(self): + rb = ring.RingBuilder(8, 3, 1) + # check id is assigned after save + builder_file = os.path.join(self.testdir, 'test_save.builder') + rb.save(builder_file) + assigned_id = rb.id + # check id doesn't change when builder is saved again + rb.save(builder_file) + self.assertEqual(assigned_id, rb.id) + # check same id after loading + loaded_rb = ring.RingBuilder.load(builder_file) + self.assertEqual(assigned_id, loaded_rb.id) + # check id doesn't change when loaded builder is saved + rb.save(builder_file) + self.assertEqual(assigned_id, rb.id) + # check same id after loading again + loaded_rb = ring.RingBuilder.load(builder_file) + self.assertEqual(assigned_id, loaded_rb.id) + # check id remains once assigned, even when save fails + with self.assertRaises(IOError): + rb.save(os.path.join( + self.testdir, 'non_existent_dir', 'test_save.file')) + self.assertEqual(assigned_id, rb.id) + + # sanity check that different builders get different id's + other_rb = ring.RingBuilder(8, 3, 1) + other_builder_file = os.path.join(self.testdir, 'test_save_2.builder') + other_rb.save(other_builder_file) + self.assertNotEqual(assigned_id, other_rb.id) + + def test_id_copy_from(self): + # copy_from preserves the same id + orig_rb = ring.RingBuilder(8, 3, 1) + copy_rb = ring.RingBuilder(8, 3, 1) + copy_rb.copy_from(orig_rb) + for rb in(orig_rb, copy_rb): + with self.assertRaises(AttributeError) as cm: + rb.id + self.assertIn('id attribute has not been initialised', + cm.exception.message) + + builder_file = os.path.join(self.testdir, 'test_save.builder') + orig_rb.save(builder_file) + copy_rb = ring.RingBuilder(8, 3, 1) + copy_rb.copy_from(orig_rb) + self.assertEqual(orig_rb.id, copy_rb.id) + + def test_id_legacy_builder_file(self): + builder_file = os.path.join(self.testdir, 'legacy.builder') + + def do_test(): + # load legacy file + loaded_rb = ring.RingBuilder.load(builder_file) + with self.assertRaises(AttributeError) as cm: + loaded_rb.id + self.assertIn('id attribute has not been initialised', + cm.exception.message) + + # check saving assigns an id, and that it is persisted + loaded_rb.save(builder_file) + assigned_id = loaded_rb.id + self.assertIsNotNone(assigned_id) + loaded_rb = ring.RingBuilder.load(builder_file) + self.assertEqual(assigned_id, loaded_rb.id) + + # older builders had no id so the pickled builder dict had no id key + rb = ring.RingBuilder(8, 3, 1) + orig_to_dict = rb.to_dict + + def mock_to_dict(): + result = orig_to_dict() + result.pop('id') + return result + + with mock.patch.object(rb, 'to_dict', mock_to_dict): + rb.save(builder_file) + do_test() + + # even older builders pickled the class instance, which would have had + # no _id attribute + rb = ring.RingBuilder(8, 3, 1) + del rb.logger # logger type cannot be pickled + del rb._id + builder_file = os.path.join(self.testdir, 'legacy.builder') + with open(builder_file, 'wb') as f: + pickle.dump(rb, f, protocol=2) + do_test() + + def test_id_not_initialised_errors(self): + rb = ring.RingBuilder(8, 3, 1) + # id is not set until builder has been saved + with self.assertRaises(AttributeError) as cm: + rb.id + self.assertIn('id attribute has not been initialised', + cm.exception.message) + # save must succeed for id to be assigned + with self.assertRaises(IOError): + rb.save(self.testdir + '/non-existent-dir/foo.builder') + with self.assertRaises(AttributeError) as cm: + rb.id + self.assertIn('id attribute has not been initialised', + cm.exception.message) + def test_search_devs(self): rb = ring.RingBuilder(8, 3, 1) devs = [{'id': 0, 'region': 0, 'zone': 0, 'weight': 1, diff --git a/test/unit/common/ring/test_composite_builder.py b/test/unit/common/ring/test_composite_builder.py new file mode 100644 index 0000000000..b6e8de8a16 --- /dev/null +++ b/test/unit/common/ring/test_composite_builder.py @@ -0,0 +1,725 @@ +# Copyright (c) 2010-2017 OpenStack Foundation +# +# 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 os +import random +import tempfile +import unittest +import shutil +import copy + +from swift.common.ring import RingBuilder, Ring +from swift.common.ring.composite_builder import ( + compose_rings, CompositeRingBuilder) + + +def make_device_iter(): + x = 0 + base_port = 6000 + while True: + yield {'region': 0, # Note that region may be replaced on the tests + 'zone': 0, + 'ip': '10.0.0.%s' % x, + 'replication_ip': '10.0.0.%s' % x, + 'port': base_port + x, + 'replication_port': base_port + x, + 'device': 'sda', + 'weight': 100.0, } + x += 1 + + +class BaseTestCompositeBuilder(unittest.TestCase): + def setUp(self): + self.tmpdir = tempfile.mkdtemp() + self.device_iter = make_device_iter() + self.output_ring = os.path.join(self.tmpdir, 'composite.ring.gz') + + def pop_region_device(self, region): + dev = next(self.device_iter) + dev.update({'region': region}) + return dev + + def tearDown(self): + try: + shutil.rmtree(self.tmpdir, True) + except OSError: + pass + + def save_builder_with_no_id(self, builder, fname): + orig_to_dict = builder.to_dict + + def fake_to_dict(): + res = orig_to_dict() + res.pop('id') + return res + + with mock.patch.object(builder, 'to_dict', fake_to_dict): + builder.save(fname) + + def save_builders(self, builders, missing_ids=None, prefix='builder'): + missing_ids = missing_ids or [] + builder_files = [] + for i, builder in enumerate(builders): + fname = os.path.join(self.tmpdir, '%s_%s.builder' % (prefix, i)) + if i in missing_ids: + self.save_builder_with_no_id(builder, fname) + else: + builder.save(fname) + builder_files.append(fname) + return builder_files + + def create_sample_ringbuilders(self, num_builders=2): + """ + Create sample rings with four devices + + :returns: a list of ring builder instances + """ + + builders = [] + for region in range(num_builders): + fname = os.path.join(self.tmpdir, 'builder_%s.builder' % region) + builder = RingBuilder(6, 3, 0) + for _ in range(5): + dev = self.pop_region_device(region) + builder.add_dev(dev) + # remove last dev to simulate a ring with some history + builder.remove_dev(dev['id']) + # add a dev that won't be assigned any parts + new_dev = self.pop_region_device(region) + new_dev['weight'] = 0 + builder.add_dev(new_dev) + builder.rebalance() + builder.save(fname) + self.assertTrue(os.path.exists(fname)) + builders.append(builder) + + return builders + + def add_dev_and_rebalance(self, builder, weight=None): + dev = next(builder._iter_devs()) + new_dev = self.pop_region_device(dev['region']) + if weight is not None: + new_dev['weight'] = weight + builder.add_dev(new_dev) + builder.rebalance() + + def assertDevices(self, composite_ring, builders): + """ + :param composite_ring: a Ring instance + :param builders: a list of RingBuilder instances for assertion + """ + # assert all component devices are in composite device table + builder_devs = [] + for builder in builders: + builder_devs.extend([ + (dev['ip'], dev['port'], dev['device']) + for dev in builder._iter_devs()]) + + got_devices = [ + (dev['ip'], dev['port'], dev['device']) + for dev in composite_ring.devs if dev] + self.assertEqual(sorted(builder_devs), sorted(got_devices), + "composite_ring mismatched with part of the rings") + + # assert composite device ids correctly index into the dev list + dev_ids = [] + for i, dev in enumerate(composite_ring.devs): + if dev: + self.assertEqual(i, dev['id']) + dev_ids.append(dev['id']) + self.assertEqual(len(builder_devs), len(dev_ids)) + + def uniqueness(dev): + return (dev['ip'], dev['port'], dev['device']) + + # assert part assignment is ordered by ring order + part_count = composite_ring.partition_count + for part in range(part_count): + primaries = [uniqueness(primary) for primary in + composite_ring.get_part_nodes(part)] + offset = 0 + for builder in builders: + sub_primaries = [uniqueness(primary) for primary in + builder.get_part_devices(part)] + self.assertEqual( + primaries[offset:offset + builder.replicas], + sub_primaries, + "composite ring is not ordered by ring order, %s, %s" + % (primaries, sub_primaries)) + offset += builder.replicas + + def check_composite_ring(self, ring_file, builders): + got_ring = Ring(ring_file) + self.assertEqual(got_ring.partition_count, builders[0].parts) + self.assertEqual(got_ring.replica_count, + sum(b.replicas for b in builders)) + self.assertEqual(got_ring._part_shift, builders[0].part_shift) + self.assertDevices(got_ring, builders) + + def check_composite_meta(self, cb_file, builder_files, version=1): + with open(cb_file) as fd: + actual = json.load(fd) + builders = [RingBuilder.load(fname) for fname in builder_files] + expected_metadata = { + 'saved_path': os.path.abspath(cb_file), + 'serialization_version': 1, + 'version': version, + 'components': [ + {'id': builder.id, + 'version': builder.version, + 'replicas': builder.replicas, + } + for builder in builders + ], + 'component_builder_files': + dict((builder.id, os.path.abspath(builder_files[i])) + for i, builder in enumerate(builders)) + } + self.assertEqual(expected_metadata, actual) + + +class TestCompositeBuilder(BaseTestCompositeBuilder): + def test_compose_rings(self): + def do_test(builder_count): + builders = self.create_sample_ringbuilders(builder_count) + rd = compose_rings(builders) + rd.save(self.output_ring) + self.check_composite_ring(self.output_ring, builders) + + do_test(2) + do_test(3) + do_test(4) + + def test_composite_same_region_in_the_different_rings_error(self): + builder_1 = self.create_sample_ringbuilders(1) + builder_2 = self.create_sample_ringbuilders(1) + builders = builder_1 + builder_2 + with self.assertRaises(ValueError) as cm: + compose_rings(builders) + self.assertIn('Same region found in different rings', + cm.exception.message) + + def test_composite_only_one_ring_in_the_args_error(self): + builders = self.create_sample_ringbuilders(1) + with self.assertRaises(ValueError) as cm: + compose_rings(builders) + self.assertIn( + 'Two or more component builders are required.', + cm.exception.message) + + def test_composite_same_device_in_the_different_rings_error(self): + builders = self.create_sample_ringbuilders(2) + same_device = copy.deepcopy(builders[0].devs[0]) + + # create one more ring which duplicates a device in the first ring + builder = RingBuilder(6, 3, 1) + _, fname = tempfile.mkstemp(dir=self.tmpdir) + # add info to feed to add_dev + same_device.update({'region': 2, 'weight': 100}) + builder.add_dev(same_device) + + # add rest of the devices, which are unique + for _ in range(3): + dev = self.pop_region_device(2) + builder.add_dev(dev) + builder.rebalance() + builder.save(fname) + # sanity + self.assertTrue(os.path.exists(fname)) + + builders.append(builder) + + with self.assertRaises(ValueError) as cm: + compose_rings(builders) + self.assertIn( + 'Duplicate ip/port/device combination %(ip)s/%(port)s/%(device)s ' + 'found in builders at indexes 0 and 2' % + same_device, cm.exception.message) + + def test_different_part_power_error(self): + # create a ring builder + # (default, part power is 6 with create_sample_ringbuilders) + builders = self.create_sample_ringbuilders(1) + + # prepare another ring which has different part power + incorrect_builder = RingBuilder(4, 3, 1) + _, fname = tempfile.mkstemp(dir=self.tmpdir) + for _ in range(4): + dev = self.pop_region_device(1) + incorrect_builder.add_dev(dev) + incorrect_builder.rebalance() + incorrect_builder.save(fname) + # sanity + self.assertTrue(os.path.exists(fname)) + + # sanity + correct_builder = builders[0] + self.assertNotEqual(correct_builder.part_shift, + incorrect_builder.part_shift) + self.assertNotEqual(correct_builder.part_power, + incorrect_builder.part_power) + + builders.append(incorrect_builder) + with self.assertRaises(ValueError) as cm: + compose_rings(builders) + self.assertIn("All builders must have same value for 'part_power'", + cm.exception.message) + + def test_compose_rings_float_replica_count_builder_error(self): + builders = self.create_sample_ringbuilders(1) + + # prepare another ring which has float replica count + incorrect_builder = RingBuilder(6, 1.5, 1) + _, fname = tempfile.mkstemp(dir=self.tmpdir) + for _ in range(4): + dev = self.pop_region_device(1) + incorrect_builder.add_dev(dev) + incorrect_builder.rebalance() + incorrect_builder.save(fname) + # sanity + self.assertTrue(os.path.exists(fname)) + self.assertEqual(1.5, incorrect_builder.replicas) + # the first replica has 2 ** 6 partitions + self.assertEqual( + 2 ** 6, len(incorrect_builder._replica2part2dev[0])) + # but the second replica has the half of the first partitions + self.assertEqual( + 2 ** 5, len(incorrect_builder._replica2part2dev[1])) + builders.append(incorrect_builder) + + with self.assertRaises(ValueError) as cm: + compose_rings(builders) + self.assertIn("Problem with builders", cm.exception.message) + self.assertIn("Non integer replica count", cm.exception.message) + + def test_compose_rings_rebalance_needed(self): + builders = self.create_sample_ringbuilders(2) + + # add a new device to builider 1 but no rebalance + dev = self.pop_region_device(1) + builders[1].add_dev(dev) + self.assertTrue(builders[1].devs_changed) # sanity check + with self.assertRaises(ValueError) as cm: + compose_rings(builders) + self.assertIn("Problem with builders", cm.exception.message) + self.assertIn("Builder needs rebalance", cm.exception.message) + # after rebalance, that works (sanity) + builders[1].rebalance() + compose_rings(builders) + + def test_different_replica_count_works(self): + # create a ring builder + # (default, part power is 6 with create_sample_ringbuilders) + builders = self.create_sample_ringbuilders(1) + + # prepare another ring which has different part power + builder = RingBuilder(6, 1, 1) + _, fname = tempfile.mkstemp(dir=self.tmpdir) + for _ in range(4): + dev = self.pop_region_device(1) + builder.add_dev(dev) + builder.rebalance() + builder.save(fname) + # sanity + self.assertTrue(os.path.exists(fname)) + builders.append(builder) + + rd = compose_rings(builders) + rd.save(self.output_ring) + got_ring = Ring(self.output_ring) + self.assertEqual(got_ring.partition_count, 2 ** 6) + self.assertEqual(got_ring.replica_count, 4) # 3 + 1 + self.assertEqual(got_ring._part_shift, 26) + self.assertDevices(got_ring, builders) + + def test_ring_swap(self): + # sanity + builders = sorted(self.create_sample_ringbuilders(2)) + rd = compose_rings(builders) + rd.save(self.output_ring) + got_ring = Ring(self.output_ring) + self.assertEqual(got_ring.partition_count, 2 ** 6) + self.assertEqual(got_ring.replica_count, 6) + self.assertEqual(got_ring._part_shift, 26) + self.assertDevices(got_ring, builders) + + # even if swapped, it works + reverse_builders = sorted(builders, reverse=True) + self.assertNotEqual(reverse_builders, builders) + rd = compose_rings(reverse_builders) + rd.save(self.output_ring) + got_ring = Ring(self.output_ring) + self.assertEqual(got_ring.partition_count, 2 ** 6) + self.assertEqual(got_ring.replica_count, 6) + self.assertEqual(got_ring._part_shift, 26) + self.assertDevices(got_ring, reverse_builders) + + # but if the composite rings are different order, the composite ring + # *will* be different. Note that the CompositeRingBuilder class will + # check builder order against the existing ring and fail if the order + # is different (actually checking the metadata). See also + # test_compose_different_builder_order + with self.assertRaises(AssertionError) as cm: + self.assertDevices(got_ring, builders) + + self.assertIn("composite ring is not ordered by ring order", + cm.exception.message) + + +class TestCompositeRingBuilder(BaseTestCompositeBuilder): + def test_compose_with_builder_files(self): + cb_file = os.path.join(self.tmpdir, 'test-composite-ring.json') + builders = self.create_sample_ringbuilders(2) + cb = CompositeRingBuilder(self.save_builders(builders)) + cb.compose().save(self.output_ring) + self.check_composite_ring(self.output_ring, builders) + cb.save(cb_file) + + for i, b in enumerate(builders): + self.add_dev_and_rebalance(b) + self.save_builders(builders) + cb = CompositeRingBuilder.load(cb_file) + cb.compose().save(self.output_ring) + self.check_composite_ring(self.output_ring, builders) + + def _make_composite_builder(self, builders): + # helper to compose a ring, save it and sanity check it + builder_files = self.save_builders(builders) + cb = CompositeRingBuilder(builder_files) + cb.compose().save(self.output_ring) + self.check_composite_ring(self.output_ring, builders) + return cb, builder_files + + def test_compose_ok(self): + cb_file = os.path.join(self.tmpdir, 'test-composite-ring.json') + builders = self.create_sample_ringbuilders(2) + # make first version of composite ring + cb, builder_files = self._make_composite_builder(builders) + # check composite builder persists ok + cb.save(cb_file) + self.assertTrue(os.path.exists(cb_file)) + self.check_composite_meta(cb_file, builder_files) + # and reloads ok + cb = CompositeRingBuilder.load(cb_file) + self.assertEqual(1, cb.version) + # composes after with no component builder changes will fail... + with self.assertRaises(ValueError) as cm: + cb.compose() + self.assertIn('None of the component builders has been modified', + cm.exception.message) + self.assertEqual(1, cb.version) + # ...unless we force it + cb.compose(force=True).save(self.output_ring) + self.check_composite_ring(self.output_ring, builders) + # check composite builder persists ok again + cb_file = os.path.join(self.tmpdir, 'test-composite-ring.json2') + cb.save(cb_file) + self.assertTrue(os.path.exists(cb_file)) + self.check_composite_meta(cb_file, builder_files, version=2) + + def test_compose_modified_component_builders(self): + # check it's ok to compose again with same but modified builders + cb_file = os.path.join(self.tmpdir, 'test-composite-ring.json') + builders = self.create_sample_ringbuilders(2) + cb, builder_files = self._make_composite_builder(builders) + ring = Ring(self.output_ring) + orig_devs = [dev for dev in ring.devs if dev] + self.assertEqual(10, len(orig_devs)) # sanity check + self.add_dev_and_rebalance(builders[1]) + builder_files = self.save_builders(builders) + cb.compose().save(self.output_ring) + self.check_composite_ring(self.output_ring, builders) + ring = Ring(self.output_ring) + modified_devs = [dev for dev in ring.devs if dev] + self.assertEqual(len(orig_devs) + 1, len(modified_devs)) + # check composite builder persists ok + cb.save(cb_file) + self.assertTrue(os.path.exists(cb_file)) + self.check_composite_meta(cb_file, builder_files, version=2) + # and reloads ok + cb = CompositeRingBuilder.load(cb_file) + # and composes ok after reload + cb.compose(force=True).save(self.output_ring) + self.check_composite_ring(self.output_ring, builders) + # check composite builder persists ok again + cb_file = os.path.join(self.tmpdir, 'test-composite-ring.json2') + cb.save(cb_file) + self.assertTrue(os.path.exists(cb_file)) + self.check_composite_meta(cb_file, builder_files, version=3) + + def test_compose_override_component_builders(self): + # check passing different builder files to the compose() method + # overrides loaded builder files + cb_file = os.path.join(self.tmpdir, 'test-composite-ring.json') + builders = self.create_sample_ringbuilders(2) + cb, builder_files = self._make_composite_builder(builders) + # modify builders and save in different files + self.add_dev_and_rebalance(builders[1]) + with self.assertRaises(ValueError): + cb.compose(builder_files) # sanity check - originals are unchanged + other_files = self.save_builders(builders, prefix='other') + cb.compose(other_files).save(self.output_ring) + self.check_composite_ring(self.output_ring, builders) + # check composite builder persists ok + cb.save(cb_file) + self.assertTrue(os.path.exists(cb_file)) + self.check_composite_meta(cb_file, other_files, version=2) + # and reloads ok + cb = CompositeRingBuilder.load(cb_file) + # and composes ok after reload + cb.compose(force=True).save(self.output_ring) + self.check_composite_ring(self.output_ring, builders) + # check composite builder persists ok again + cb_file = os.path.join(self.tmpdir, 'test-composite-ring.json2') + cb.save(cb_file) + self.assertTrue(os.path.exists(cb_file)) + self.check_composite_meta(cb_file, other_files, version=3) + + def test_abs_paths_persisted(self): + cwd = os.getcwd() + try: + os.chdir(self.tmpdir) + builders = self.create_sample_ringbuilders(2) + builder_files = self.save_builders(builders) + rel_builder_files = [os.path.basename(bf) for bf in builder_files] + cb = CompositeRingBuilder(rel_builder_files) + cb.compose().save(self.output_ring) + self.check_composite_ring(self.output_ring, builders) + cb_file = os.path.join(self.tmpdir, 'test-composite-ring.json') + rel_cb_file = os.path.basename(cb_file) + cb.save(rel_cb_file) + self.check_composite_meta(rel_cb_file, rel_builder_files) + finally: + os.chdir(cwd) + + def test_compose_insufficient_builders(self): + def do_test(builder_files): + cb = CompositeRingBuilder(builder_files) + with self.assertRaises(ValueError) as cm: + cb.compose() + self.assertIn('Two or more component builders are required', + cm.exception.message) + + cb = CompositeRingBuilder() + with self.assertRaises(ValueError) as cm: + cb.compose(builder_files) + self.assertIn('Two or more component builders are required', + cm.exception.message) + + builders = self.create_sample_ringbuilders(3) + builder_files = self.save_builders(builders) + do_test([]) + do_test(builder_files[:1]) + + def test_compose_missing_builder_id(self): + def check_missing_id(cb, builders): + # not ok to compose with builder_files that have no id assigned + orig_version = cb.version + no_id = random.randint(0, len(builders) - 1) + # rewrite the builder files so that one has missing id + self.save_builders(builders, missing_ids=[no_id]) + with self.assertRaises(ValueError) as cm: + cb.compose() + error_lines = cm.exception.message.split('\n') + self.assertIn("Problem with builder at index %s" % no_id, + error_lines[0]) + self.assertIn("id attribute has not been initialised", + error_lines[0]) + self.assertFalse(error_lines[1:]) + self.assertEqual(orig_version, cb.version) + # check with compose not previously called, cb has no existing metadata + builders = self.create_sample_ringbuilders(3) + builder_files = self.save_builders(builders) + cb = CompositeRingBuilder(builder_files) + check_missing_id(cb, builders) + # now save good copies of builders and compose so this cb has + # existing component metadata + builder_files = self.save_builders(builders) + cb = CompositeRingBuilder(builder_files) + cb.compose() # cb now has component metadata + check_missing_id(cb, builders) + + def test_compose_duplicate_builder_ids(self): + builders = self.create_sample_ringbuilders(3) + builders[2]._id = builders[0]._id + cb = CompositeRingBuilder(self.save_builders(builders)) + with self.assertRaises(ValueError) as cm: + cb.compose() + error_lines = cm.exception.message.split('\n') + self.assertIn("Builder id %r used at indexes 0, 2" % builders[0].id, + error_lines[0]) + self.assertFalse(error_lines[1:]) + self.assertEqual(0, cb.version) + + def test_compose_ring_unchanged_builders(self): + def do_test(cb, builder_files): + with self.assertRaises(ValueError) as cm: + cb.compose(builder_files) + error_lines = cm.exception.message.split('\n') + self.assertIn("None of the component builders has been modified", + error_lines[0]) + self.assertFalse(error_lines[1:]) + self.assertEqual(1, cb.version) + + builders = self.create_sample_ringbuilders(2) + cb, builder_files = self._make_composite_builder(builders) + # not ok to compose again with same *unchanged* builders + do_test(cb, builder_files) + # even if we rewrite the files + builder_files = self.save_builders(builders) + do_test(cb, builder_files) + # even if we rename the files + builder_files = self.save_builders(builders, prefix='other') + do_test(cb, builder_files) + + def test_compose_older_builder(self): + # make first version of composite ring + builders = self.create_sample_ringbuilders(2) + cb, builder_files = self._make_composite_builder(builders) + old_builders = [copy.deepcopy(b) for b in builders] + for i, b in enumerate(builders): + self.add_dev_and_rebalance(b) + self.assertLess(old_builders[i].version, b.version) + self.save_builders(builders) + cb.compose() # newer version + self.assertEqual(2, cb.version) # sanity check + # not ok to use old versions of same builders + self.save_builders([old_builders[0], builders[1]]) + with self.assertRaises(ValueError) as cm: + cb.compose() + error_lines = cm.exception.message.split('\n') + self.assertIn("Invalid builder change at index 0", error_lines[0]) + self.assertIn("Older builder version", error_lines[0]) + self.assertFalse(error_lines[1:]) + self.assertEqual(2, cb.version) + # not even if one component ring has changed + self.add_dev_and_rebalance(builders[1]) + self.save_builders([old_builders[0], builders[1]]) + with self.assertRaises(ValueError) as cm: + cb.compose() + error_lines = cm.exception.message.split('\n') + self.assertIn("Invalid builder change at index 0", error_lines[0]) + self.assertIn("Older builder version", error_lines[0]) + self.assertFalse(error_lines[1:]) + self.assertEqual(2, cb.version) + + def test_compose_different_number_builders(self): + # not ok to use a different number of component rings + builders = self.create_sample_ringbuilders(3) + cb, builder_files = self._make_composite_builder(builders[:2]) + + def do_test(bad_builders): + with self.assertRaises(ValueError) as cm: + cb.compose(self.save_builders(bad_builders)) + error_lines = cm.exception.message.split('\n') + self.assertFalse(error_lines[1:]) + self.assertEqual(1, cb.version) + return error_lines + + error_lines = do_test(builders[:1]) # too few + self.assertIn("Missing builder at index 1", error_lines[0]) + error_lines = do_test(builders) # too many + self.assertIn("Unexpected extra builder at index 2", error_lines[0]) + + def test_compose_different_builders(self): + # not ok to change component rings + builders = self.create_sample_ringbuilders(3) + cb, builder_files = self._make_composite_builder(builders[:2]) + # ensure builder[0] is newer version so that's not the problem + self.add_dev_and_rebalance(builders[0]) + with self.assertRaises(ValueError) as cm: + cb.compose(self.save_builders([builders[0], builders[2]])) + error_lines = cm.exception.message.split('\n') + self.assertIn("Invalid builder change at index 1", error_lines[0]) + self.assertIn("Attribute mismatch for id", error_lines[0]) + self.assertFalse(error_lines[1:]) + self.assertEqual(1, cb.version) + + def test_compose_different_builder_order(self): + # not ok to change order of component rings + builders = self.create_sample_ringbuilders(4) + cb, builder_files = self._make_composite_builder(builders) + builder_files.reverse() + with self.assertRaises(ValueError) as cm: + cb.compose(builder_files) + error_lines = cm.exception.message.split('\n') + for i, line in enumerate(error_lines): + self.assertIn("Invalid builder change at index %s" % i, line) + self.assertIn("Attribute mismatch for id", line) + self.assertEqual(1, cb.version) + + def test_compose_different_replica_count(self): + # not ok to change the number of replicas in a ring + builders = self.create_sample_ringbuilders(3) + cb, builder_files = self._make_composite_builder(builders) + builders[0].set_replicas(4) + self.save_builders(builders) + with self.assertRaises(ValueError) as cm: + cb.compose() + error_lines = cm.exception.message.split('\n') + for i, line in enumerate(error_lines): + self.assertIn("Invalid builder change at index 0", line) + self.assertIn("Attribute mismatch for replicas", line) + self.assertEqual(1, cb.version) + + def test_load_errors(self): + bad_file = os.path.join(self.tmpdir, 'bad_file.json') + with self.assertRaises(IOError): + CompositeRingBuilder.load(bad_file) + + def check_bad_content(content): + with open(bad_file, 'wb') as fp: + fp.write(content) + try: + with self.assertRaises(ValueError) as cm: + CompositeRingBuilder.load(bad_file) + self.assertIn( + "File does not contain valid composite ring data", + cm.exception.message) + except AssertionError as err: + raise AssertionError('With content %r: %s' % (content, err)) + + for content in ('', 'not json', json.dumps({}), json.dumps([])): + check_bad_content(content) + + good_content = { + 'components': [ + {'version': 1, 'id': 'uuid_x', 'replicas': 12}, + {'version': 2, 'id': 'uuid_y', 'replicas': 12} + ], + 'builder_files': {'uuid_x': '/path/to/file_x', + 'uuid_y': '/path/to/file_y'}, + 'version': 99} + for missing in good_content: + bad_content = dict(good_content) + bad_content.pop(missing) + check_bad_content(json.dumps(bad_content)) + + def test_save_errors(self): + cb_file = os.path.join(self.tmpdir, 'test-composite-ring.json') + + def do_test(cb): + with self.assertRaises(ValueError) as cm: + cb.save(cb_file) + self.assertIn("No composed ring to save", cm.exception.message) + + do_test(CompositeRingBuilder()) + do_test(CompositeRingBuilder([])) + do_test(CompositeRingBuilder(['file1', 'file2'])) + + +if __name__ == '__main__': + unittest.main()