diff --git a/muranoclient/common/utils.py b/muranoclient/common/utils.py index 02dc6347..b68ad948 100644 --- a/muranoclient/common/utils.py +++ b/muranoclient/common/utils.py @@ -388,20 +388,92 @@ class Package(FileWrapperMixin): self._logo = None return self._logo - def requirements(self, base_url, path=None, dep_dict=None): - """Get dict of all requirements + def _get_package_order(self, packages_graph): + """Sorts packages according to dependencies between them - Recursively scan Require section of manifests of all the - dependencies. Returns a dict with FQPNs as keys and respective - Package objects as values + Murano allows cyclic dependencies. It is impossible + to do topological sort for graph with cycles, so at first + graph condensation should be built. + For condensation building Kosaraju's algorithm is used. + Packages in strongly connected components can be situated + in random order to each other. """ - if not dep_dict: - dep_dict = {} - dep_dict[self.manifest['FullName']] = self - if 'Require' in self.manifest: - for dep_name, ver in six.iteritems(self.manifest['Require']): - if dep_name in dep_dict: - continue + def topological_sort(graph, start_node): + order = [] + not_seen = set(graph) + + def dfs(node): + not_seen.discard(node) + for dep_node in graph[node]: + if dep_node in not_seen: + dfs(dep_node) + order.append(node) + + dfs(start_node) + return order + + def transpose_graph(graph): + transposed = collections.defaultdict(list) + for node, deps in six.viewitems(graph): + for dep in deps: + transposed[dep].append(node) + return transposed + + order = topological_sort(packages_graph, self.manifest['FullName']) + order.reverse() + transposed = transpose_graph(packages_graph) + + def top_sort_by_components(graph, component_order): + result = [] + seen = set() + + def dfs(node): + seen.add(node) + result.append(node) + for dep_node in graph[node]: + if dep_node not in seen: + dfs(dep_node) + for item in component_order: + if item not in seen: + dfs(item) + return reversed(result) + return top_sort_by_components(transposed, order) + + def requirements(self, base_url, path=None, dep_dict=None): + """Scans Require section of manifests of all the dependencies. + + Returns a dict with FQPNs as keys and respective Package objects + as values, ordered by topological sort. + + :param base_url: url of packages location + :param path: local path of packages location + :param dep_dict: unused. Left for backward compatibility + """ + + unordered_requirements = {} + requirements_graph = collections.defaultdict(list) + dep_queue = collections.deque([(self.manifest['FullName'], self)]) + while dep_queue: + dep_name, dep_file = dep_queue.popleft() + unordered_requirements[dep_name] = dep_file + direct_deps = Package._get_direct_deps(dep_file, base_url, path) + for name, file in direct_deps: + if name not in unordered_requirements: + dep_queue.append((name, file)) + requirements_graph[dep_name] = [dep[0] for dep in direct_deps] + + ordered_reqs_names = self._get_package_order(requirements_graph) + ordered_reqs_dict = collections.OrderedDict() + for name in ordered_reqs_names: + ordered_reqs_dict[name] = unordered_requirements[name] + + return ordered_reqs_dict + + @staticmethod + def _get_direct_deps(package, base_url, path): + result = [] + if 'Require' in package.manifest: + for dep_name, ver in six.iteritems(package.manifest['Require']): try: req_file = Package.from_location( dep_name, @@ -413,14 +485,10 @@ class Package(FileWrapperMixin): LOG.error("Error {0} occurred while parsing package {1}, " "required by {2} package".format( e, dep_name, - self.manifest['FullName'])) + package.manifest['FullName'])) continue - dep_dict.update(req_file.requirements( - base_url=base_url, - path=path, - dep_dict=dep_dict, - )) - return dep_dict + result.append((req_file.manifest['FullName'], req_file)) + return result def save_image_local(image_spec, base_url, dst): diff --git a/muranoclient/tests/unit/test_utils.py b/muranoclient/tests/unit/test_utils.py index cff0f65a..5ce2e57c 100644 --- a/muranoclient/tests/unit/test_utils.py +++ b/muranoclient/tests/unit/test_utils.py @@ -177,8 +177,99 @@ class PackageTest(testtools.TestCase): {'main_app': app, 'dep_app': mock.ANY, 'dep_of_dep': mock.ANY}, reqs) - @requests_mock.mock() - def test_cyclic_requirements(self, m): + @mock.patch('muranoclient.common.utils.Package.from_file') + def test_requirements_order(self, from_file): + """Test that dependencies are parsed in correct order.""" + + pkg5 = make_pkg({'FullName': 'd4', }) + pkg4 = make_pkg({'FullName': 'd3', 'Require': {'d4': None}, }) + pkg3 = make_pkg({'FullName': 'd2', 'Require': {'d3': None}, }) + pkg2 = make_pkg({'FullName': 'd1', 'Require': {'d3': None}, }) + pkg1 = make_pkg({'FullName': 'M', 'Require': {'d1': None, + 'd2': None, + 'd4': None}, }) + + def side_effect(name): + if 'M' in name: + return utils.Package(utils.File(pkg1)) + if 'd1' in name: + return utils.Package(utils.File(pkg2)) + if 'd2' in name: + return utils.Package(utils.File(pkg3)) + if 'd3' in name: + return utils.Package(utils.File(pkg4)) + if 'd4' in name: + return utils.Package(utils.File(pkg5)) + + from_file.side_effect = side_effect + app = from_file('M') + reqs = app.requirements(base_url=self.base_url) + + def key_position(key): + keys = list(six.iterkeys(reqs)) + return keys.index(key) + + self.assertTrue( + key_position('d4') < key_position('d3') and + key_position('d4') < key_position('M') and + key_position('d3') < key_position('d1') and + key_position('d3') < key_position('d2') < key_position('M') + ) + + @mock.patch('muranoclient.common.utils.Package.from_file') + def test_requirements_order2(self, from_file): + """Test that dependencies are parsed in correct order.""" + + pkg5 = make_pkg({'FullName': 'd4', 'Require': {'d6': None}, }) + pkg4 = make_pkg({'FullName': 'd3', 'Require': {'d4': None}, }) + pkg3 = make_pkg({'FullName': 'd1', 'Require': {'d3': None, + 'd7': None}, }) + pkg2 = make_pkg({'FullName': 'd2', 'Require': {'d3': None}, }) + pkg6 = make_pkg({'FullName': 'd6', }) + pkg7 = make_pkg({'FullName': 'd7', 'Require': {'d8': None}, }) + pkg8 = make_pkg({'FullName': 'd8', }) + + pkg1 = make_pkg({'FullName': 'M', 'Require': {'d1': None, + 'd2': None, + 'd4': None, }, }) + + def side_effect(name): + if 'M' in name: + return utils.Package(utils.File(pkg1)) + if 'd1' in name: + return utils.Package(utils.File(pkg2)) + if 'd2' in name: + return utils.Package(utils.File(pkg3)) + if 'd3' in name: + return utils.Package(utils.File(pkg4)) + if 'd4' in name: + return utils.Package(utils.File(pkg5)) + if 'd6' in name: + return utils.Package(utils.File(pkg6)) + if 'd7' in name: + return utils.Package(utils.File(pkg7)) + if 'd8' in name: + return utils.Package(utils.File(pkg8)) + + from_file.side_effect = side_effect + app = from_file('M') + reqs = app.requirements(base_url=self.base_url) + + def key_position(key): + keys = list(six.iterkeys(reqs)) + return keys.index(key) + + self.assertTrue( + key_position('d6') < key_position('d4') < + key_position('d3') < key_position('d1') and + key_position('d3') < key_position('d2') and + key_position('d1') < key_position('M') and + key_position('d2') < key_position('M') and + key_position('d8') < key_position('d7') < key_position('d1') + ) + + @mock.patch('muranoclient.common.utils.Package.from_file') + def test_cyclic_requirements(self, from_file): """Test that a cyclic dependency would be handled correctly.""" pkg3 = make_pkg({'FullName': 'dep_of_dep', 'Require': { 'main_app': None, 'dep_app': None}, }) @@ -187,16 +278,107 @@ class PackageTest(testtools.TestCase): pkg1 = make_pkg({'FullName': 'main_app', 'Require': { 'dep_app': None, 'dep_of_dep': None}, }) - m.get(self.base_url + '/apps/main_app.zip', body=pkg1) - m.get(self.base_url + '/apps/dep_app.zip', body=pkg2) - m.get(self.base_url + '/apps/dep_of_dep.zip', body=pkg3) - app = utils.Package.fromFile(pkg1) + def side_effect(name): + if 'main_app' in name: + return utils.Package(utils.File(pkg1)) + if 'dep_app' in name: + return utils.Package(utils.File(pkg2)) + if 'dep_of_dep' in name: + return utils.Package(utils.File(pkg3)) + + from_file.side_effect = side_effect + app = from_file('main_app') reqs = app.requirements(base_url=self.base_url) self.assertEqual( {'main_app': app, 'dep_app': mock.ANY, 'dep_of_dep': mock.ANY}, reqs) + @mock.patch('muranoclient.common.utils.Package.from_file') + def test_order_with_cyclic_requirements2(self, from_file): + """Test that dependencies are parsed in correct order.""" + + pkg6 = make_pkg({'FullName': 'd5', 'Require': {'d6': None}, }) + pkg7 = make_pkg({'FullName': 'd6', }) + pkg5 = make_pkg({'FullName': 'd4', 'Require': {'d3': None, + 'd5': None}}) + pkg4 = make_pkg({'FullName': 'd3', 'Require': {'d4': None}, }) + pkg3 = make_pkg({'FullName': 'd2', 'Require': {'d1': None, + 'd5': None, + 'd6': None}, }) + pkg2 = make_pkg({'FullName': 'd1', 'Require': {'d2': None}, }) + pkg1 = make_pkg({'FullName': 'M', 'Require': {'d1': None, + 'd3': None}, }) + + def side_effect(name): + if 'M' in name: + return utils.Package(utils.File(pkg1)) + if 'd1' in name: + return utils.Package(utils.File(pkg2)) + if 'd2' in name: + return utils.Package(utils.File(pkg3)) + if 'd3' in name: + return utils.Package(utils.File(pkg4)) + if 'd4' in name: + return utils.Package(utils.File(pkg5)) + if 'd5' in name: + return utils.Package(utils.File(pkg6)) + if 'd6' in name: + return utils.Package(utils.File(pkg7)) + + from_file.side_effect = side_effect + app = from_file('M') + reqs = app.requirements(base_url=self.base_url) + + def key_position(key): + keys = list(six.iterkeys(reqs)) + return keys.index(key) + + self.assertTrue( + key_position('d5') < key_position('d4') and + key_position('d5') < key_position('d2') and + key_position('d5') < key_position('d3') < key_position('M') and + key_position('d5') < key_position('d1') < key_position('M') + ) + + @mock.patch('muranoclient.common.utils.Package.from_file') + def test_order_with_cyclic_requirements3(self, from_file): + """Test that dependencies are parsed in correct order.""" + + pkg5 = make_pkg({'FullName': 'd4', }) + pkg4 = make_pkg({'FullName': 'd3', 'Require': {'M': None}, }) + pkg3 = make_pkg({'FullName': 'd2', 'Require': {'d3': None, + 'd4': None}, }) + pkg2 = make_pkg({'FullName': 'd1', 'Require': {'d2': None}, }) + pkg1 = make_pkg({'FullName': 'M', 'Require': {'d1': None}, }) + + def side_effect(name): + if 'M' in name: + return utils.Package(utils.File(pkg1)) + if 'd1' in name: + return utils.Package(utils.File(pkg2)) + if 'd2' in name: + return utils.Package(utils.File(pkg3)) + if 'd3' in name: + return utils.Package(utils.File(pkg4)) + if 'd4' in name: + return utils.Package(utils.File(pkg5)) + + from_file.side_effect = side_effect + app = from_file('M') + reqs = app.requirements(base_url=self.base_url) + + def key_position(key): + keys = list(six.iterkeys(reqs)) + return keys.index(key) + + self.assertTrue( + key_position('d4') < key_position('M') and + key_position('d4') < key_position('d1') and + key_position('d4') < key_position('d2') and + key_position('d4') < key_position('d3') + ) + def test_images(self): pkg = make_pkg({}) app = utils.Package.fromFile(pkg) diff --git a/muranoclient/v1/shell.py b/muranoclient/v1/shell.py index ca37d698..55d99761 100644 --- a/muranoclient/v1/shell.py +++ b/muranoclient/v1/shell.py @@ -12,6 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. +import collections import functools import json import os @@ -651,7 +652,7 @@ def do_package_import(mc, args): if args.categories: data["categories"] = args.categories - total_reqs = {} + total_reqs = collections.OrderedDict() main_packages_names = [] for filename in args.filename: if os.path.isfile(filename): @@ -755,7 +756,7 @@ def do_bundle_import(mc, args): file names, relative to location of the bundle file. Requirements are first searched in the same directory. """ - total_reqs = {} + total_reqs = collections.OrderedDict() for filename in args.filename: local_path = None if os.path.isfile(filename): @@ -875,7 +876,7 @@ def do_bundle_save(mc, args): else: dst = os.getcwd() - total_reqs = {} + total_reqs = collections.OrderedDict() if os.path.isfile(bundle): _file = bundle @@ -941,7 +942,7 @@ def do_package_save(mc, args): "ignoring version.") version = '' - total_reqs = {} + total_reqs = collections.OrderedDict() for package in args.package: _file = utils.to_url( package, diff --git a/releasenotes/notes/requirements-order-19ecc40ca6d34739.yaml b/releasenotes/notes/requirements-order-19ecc40ca6d34739.yaml new file mode 100644 index 00000000..dd6ff446 --- /dev/null +++ b/releasenotes/notes/requirements-order-19ecc40ca6d34739.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - Fixed order of packages import - main package can not be imported before + all its dependencies are imported. Cyclic requirements are imported in + random order.