Import package and dependencies in correct order
Package and its dependencies are being imported in random order which allows to import package and then get an error while importing its dependency. This patch uses OrderedDict ordered by topological sort instead of regular dict to fix that. Cyclic requirements are imported in random order. Tests to check order of import are also added. Change-Id: Ia6cb087679b3ae4613ac3d1d19279a9826e537ef Closes-bug: #1585420 Co-Authored-By: ksnihyr <ksnihyr@mirantis.com>
This commit is contained in:
Valerii Kovalchuk
parent
5ff2902456
commit
a4c351cb47
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
Loading…
Reference in New Issue