From 6f27fca4a76aceaadab3776c87c48743671ce502 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Tue, 21 Nov 2017 17:05:43 -0800 Subject: [PATCH] Zuul: support plugin dependencies Change-Id: I81302e8988fe6498fea9f08ed66f5d0cc1fce161 --- .gitignore | 1 + .../library/devstack_local_conf.py | 215 +++++++++++++----- .../write-devstack-local-conf/library/test.py | 166 ++++++++++++++ .../write-devstack-local-conf/tasks/main.yaml | 1 + tests/test_write_devstack_local_conf_role.sh | 9 + 5 files changed, 339 insertions(+), 53 deletions(-) create mode 100644 roles/write-devstack-local-conf/library/test.py create mode 100755 tests/test_write_devstack_local_conf_role.sh diff --git a/.gitignore b/.gitignore index d2c127d099..8553b3f849 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ *.log *.log.[1-9] *.pem +*.pyc .localrc.auto .localrc.password .prereqs diff --git a/roles/write-devstack-local-conf/library/devstack_local_conf.py b/roles/write-devstack-local-conf/library/devstack_local_conf.py index 55ba4afb69..746f54f921 100644 --- a/roles/write-devstack-local-conf/library/devstack_local_conf.py +++ b/roles/write-devstack-local-conf/library/devstack_local_conf.py @@ -14,16 +14,69 @@ # See the License for the specific language governing permissions and # limitations under the License. +import os import re -class VarGraph(object): +class DependencyGraph(object): # This is based on the JobGraph from Zuul. + def __init__(self): + self._names = set() + self._dependencies = {} # dependent_name -> set(parent_names) + + def add(self, name, dependencies): + # Append the dependency information + self._dependencies.setdefault(name, set()) + try: + for dependency in dependencies: + # Make sure a circular dependency is never created + ancestors = self._getParentNamesRecursively( + dependency, soft=True) + ancestors.add(dependency) + if name in ancestors: + raise Exception("Dependency cycle detected in {}". + format(name)) + self._dependencies[name].add(dependency) + except Exception: + del self._dependencies[name] + raise + + def getDependenciesRecursively(self, parent): + dependencies = [] + + current_dependencies = self._dependencies[parent] + for current in current_dependencies: + if current not in dependencies: + dependencies.append(current) + for dep in self.getDependenciesRecursively(current): + if dep not in dependencies: + dependencies.append(dep) + return dependencies + + def _getParentNamesRecursively(self, dependent, soft=False): + all_parent_items = set() + items_to_iterate = set([dependent]) + while len(items_to_iterate) > 0: + current_item = items_to_iterate.pop() + current_parent_items = self._dependencies.get(current_item) + if current_parent_items is None: + if soft: + current_parent_items = set() + else: + raise Exception("Dependent item {} not found: ".format( + dependent)) + new_parent_items = current_parent_items - all_parent_items + items_to_iterate |= new_parent_items + all_parent_items |= new_parent_items + return all_parent_items + + +class VarGraph(DependencyGraph): def __init__(self, vars): + super(VarGraph, self).__init__() self.vars = {} self._varnames = set() - self._dependencies = {} # dependent_var_name -> set(parent_var_names) for k, v in vars.items(): self._varnames.add(k) for k, v in vars.items(): @@ -38,28 +91,21 @@ class VarGraph(object): raise Exception("Variable {} already added".format(key)) self.vars[key] = value # Append the dependency information - self._dependencies.setdefault(key, set()) + dependencies = set() + for dependency in self.getDependencies(value): + if dependency == key: + # A variable is allowed to reference itself; no + # dependency link needed in that case. + continue + if dependency not in self._varnames: + # It's not necessary to create a link for an + # external variable. + continue + dependencies.add(dependency) try: - for dependency in self.getDependencies(value): - if dependency == key: - # A variable is allowed to reference itself; no - # dependency link needed in that case. - continue - if dependency not in self._varnames: - # It's not necessary to create a link for an - # external variable. - continue - # Make sure a circular dependency is never created - ancestor_vars = self._getParentVarNamesRecursively( - dependency, soft=True) - ancestor_vars.add(dependency) - if any((key == anc_var) for anc_var in ancestor_vars): - raise Exception("Dependency cycle detected in var {}". - format(key)) - self._dependencies[key].add(dependency) + self.add(key, dependencies) except Exception: del self.vars[key] - del self._dependencies[key] raise def getVars(self): @@ -67,48 +113,105 @@ class VarGraph(object): keys = sorted(self.vars.keys()) seen = set() for key in keys: - dependencies = self.getDependentVarsRecursively(key) + dependencies = self.getDependenciesRecursively(key) for var in dependencies + [key]: if var not in seen: ret.append((var, self.vars[var])) seen.add(var) return ret - def getDependentVarsRecursively(self, parent_var): - dependent_vars = [] - current_dependent_vars = self._dependencies[parent_var] - for current_var in current_dependent_vars: - if current_var not in dependent_vars: - dependent_vars.append(current_var) - for dep in self.getDependentVarsRecursively(current_var): - if dep not in dependent_vars: - dependent_vars.append(dep) - return dependent_vars +class PluginGraph(DependencyGraph): + def __init__(self, base_dir, plugins): + super(PluginGraph, self).__init__() + # The dependency trees expressed by all the plugins we found + # (which may be more than those the job is using). + self._plugin_dependencies = {} + self.loadPluginNames(base_dir) - def _getParentVarNamesRecursively(self, dependent_var, soft=False): - all_parent_vars = set() - vars_to_iterate = set([dependent_var]) - while len(vars_to_iterate) > 0: - current_var = vars_to_iterate.pop() - current_parent_vars = self._dependencies.get(current_var) - if current_parent_vars is None: - if soft: - current_parent_vars = set() - else: - raise Exception("Dependent var {} not found: ".format( - dependent_var)) - new_parent_vars = current_parent_vars - all_parent_vars - vars_to_iterate |= new_parent_vars - all_parent_vars |= new_parent_vars - return all_parent_vars + self.plugins = {} + self._pluginnames = set() + for k, v in plugins.items(): + self._pluginnames.add(k) + for k, v in plugins.items(): + self._addPlugin(k, str(v)) + + def loadPluginNames(self, base_dir): + if base_dir is None: + return + git_roots = [] + for root, dirs, files in os.walk(base_dir): + if '.git' not in dirs: + continue + # Don't go deeper than git roots + dirs[:] = [] + git_roots.append(root) + for root in git_roots: + devstack = os.path.join(root, 'devstack') + if not (os.path.exists(devstack) and os.path.isdir(devstack)): + continue + settings = os.path.join(devstack, 'settings') + if not (os.path.exists(settings) and os.path.isfile(settings)): + continue + self.loadDevstackPluginInfo(settings) + + define_re = re.compile(r'^define_plugin\s+(\w+).*') + require_re = re.compile(r'^plugin_requires\s+(\w+)\s+(\w+).*') + def loadDevstackPluginInfo(self, fn): + name = None + reqs = set() + with open(fn) as f: + for line in f: + m = self.define_re.match(line) + if m: + name = m.group(1) + m = self.require_re.match(line) + if m: + if name == m.group(1): + reqs.add(m.group(2)) + if name and reqs: + self._plugin_dependencies[name] = reqs + + def getDependencies(self, value): + return self._plugin_dependencies.get(value, []) + + def _addPlugin(self, key, value): + if key in self.plugins: + raise Exception("Plugin {} already added".format(key)) + self.plugins[key] = value + # Append the dependency information + dependencies = set() + for dependency in self.getDependencies(key): + if dependency == key: + continue + dependencies.add(dependency) + try: + self.add(key, dependencies) + except Exception: + del self.plugins[key] + raise + + def getPlugins(self): + ret = [] + keys = sorted(self.plugins.keys()) + seen = set() + for key in keys: + dependencies = self.getDependenciesRecursively(key) + for plugin in dependencies + [key]: + if plugin not in seen: + ret.append((plugin, self.plugins[plugin])) + seen.add(plugin) + return ret class LocalConf(object): - def __init__(self, localrc, localconf, base_services, services, plugins): + def __init__(self, localrc, localconf, base_services, services, plugins, + base_dir): self.localrc = [] self.meta_sections = {} + self.plugin_deps = {} + self.base_dir = base_dir if plugins: self.handle_plugins(plugins) if services or base_services: @@ -119,7 +222,8 @@ class LocalConf(object): self.handle_localconf(localconf) def handle_plugins(self, plugins): - for k, v in plugins.items(): + pg = PluginGraph(self.base_dir, plugins) + for k, v in pg.getPlugins(): if v: self.localrc.append('enable_plugin {} {}'.format(k, v)) @@ -171,6 +275,7 @@ def main(): services=dict(type='dict'), localrc=dict(type='dict'), local_conf=dict(type='dict'), + base_dir=dict(type='path'), path=dict(type='str'), ) ) @@ -180,14 +285,18 @@ def main(): p.get('local_conf'), p.get('base_services'), p.get('services'), - p.get('plugins')) + p.get('plugins'), + p.get('base_dir')) lc.write(p['path']) module.exit_json() -from ansible.module_utils.basic import * # noqa -from ansible.module_utils.basic import AnsibleModule +try: + from ansible.module_utils.basic import * # noqa + from ansible.module_utils.basic import AnsibleModule +except ImportError: + pass if __name__ == '__main__': main() diff --git a/roles/write-devstack-local-conf/library/test.py b/roles/write-devstack-local-conf/library/test.py new file mode 100644 index 0000000000..843ca6e9fd --- /dev/null +++ b/roles/write-devstack-local-conf/library/test.py @@ -0,0 +1,166 @@ +# Copyright (C) 2017 Red Hat, Inc. +# +# 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 os +import shutil +import tempfile +import unittest + +from devstack_local_conf import LocalConf +from collections import OrderedDict + +class TestDevstackLocalConf(unittest.TestCase): + def setUp(self): + self.tmpdir = tempfile.mkdtemp() + + def tearDown(self): + shutil.rmtree(self.tmpdir) + + def test_plugins(self): + "Test that plugins without dependencies work" + localrc = {'test_localrc': '1'} + local_conf = {'install': + {'nova.conf': + {'main': + {'test_conf': '2'}}}} + services = {'cinder': True} + # We use ordereddict here to make sure the plugins are in the + # *wrong* order for testing. + plugins = OrderedDict([ + ('bar', 'git://git.openstack.org/openstack/bar-plugin'), + ('foo', 'git://git.openstack.org/openstack/foo-plugin'), + ('baz', 'git://git.openstack.org/openstack/baz-plugin'), + ]) + p = dict(localrc=localrc, + local_conf=local_conf, + base_services=[], + services=services, + plugins=plugins, + base_dir='./test', + path=os.path.join(self.tmpdir, 'test.local.conf')) + lc = LocalConf(p.get('localrc'), + p.get('local_conf'), + p.get('base_services'), + p.get('services'), + p.get('plugins'), + p.get('base_dir')) + lc.write(p['path']) + + plugins = [] + with open(p['path']) as f: + for line in f: + if line.startswith('enable_plugin'): + plugins.append(line.split()[1]) + self.assertEqual(['bar', 'baz', 'foo'], plugins) + + def test_plugin_deps(self): + "Test that plugins with dependencies work" + os.makedirs(os.path.join(self.tmpdir, 'foo-plugin', 'devstack')) + os.makedirs(os.path.join(self.tmpdir, 'foo-plugin', '.git')) + os.makedirs(os.path.join(self.tmpdir, 'bar-plugin', 'devstack')) + os.makedirs(os.path.join(self.tmpdir, 'bar-plugin', '.git')) + with open(os.path.join( + self.tmpdir, + 'foo-plugin', 'devstack', 'settings'), 'w') as f: + f.write('define_plugin foo\n') + with open(os.path.join( + self.tmpdir, + 'bar-plugin', 'devstack', 'settings'), 'w') as f: + f.write('define_plugin bar\n') + f.write('plugin_requires bar foo\n') + + localrc = {'test_localrc': '1'} + local_conf = {'install': + {'nova.conf': + {'main': + {'test_conf': '2'}}}} + services = {'cinder': True} + # We use ordereddict here to make sure the plugins are in the + # *wrong* order for testing. + plugins = OrderedDict([ + ('bar', 'git://git.openstack.org/openstack/bar-plugin'), + ('foo', 'git://git.openstack.org/openstack/foo-plugin'), + ]) + p = dict(localrc=localrc, + local_conf=local_conf, + base_services=[], + services=services, + plugins=plugins, + base_dir=self.tmpdir, + path=os.path.join(self.tmpdir, 'test.local.conf')) + lc = LocalConf(p.get('localrc'), + p.get('local_conf'), + p.get('base_services'), + p.get('services'), + p.get('plugins'), + p.get('base_dir')) + lc.write(p['path']) + + plugins = [] + with open(p['path']) as f: + for line in f: + if line.startswith('enable_plugin'): + plugins.append(line.split()[1]) + self.assertEqual(['foo', 'bar'], plugins) + + def test_plugin_circular_deps(self): + "Test that plugins with circular dependencies fail" + os.makedirs(os.path.join(self.tmpdir, 'foo-plugin', 'devstack')) + os.makedirs(os.path.join(self.tmpdir, 'foo-plugin', '.git')) + os.makedirs(os.path.join(self.tmpdir, 'bar-plugin', 'devstack')) + os.makedirs(os.path.join(self.tmpdir, 'bar-plugin', '.git')) + with open(os.path.join( + self.tmpdir, + 'foo-plugin', 'devstack', 'settings'), 'w') as f: + f.write('define_plugin foo\n') + f.write('plugin_requires foo bar\n') + with open(os.path.join( + self.tmpdir, + 'bar-plugin', 'devstack', 'settings'), 'w') as f: + f.write('define_plugin bar\n') + f.write('plugin_requires bar foo\n') + + localrc = {'test_localrc': '1'} + local_conf = {'install': + {'nova.conf': + {'main': + {'test_conf': '2'}}}} + services = {'cinder': True} + # We use ordereddict here to make sure the plugins are in the + # *wrong* order for testing. + plugins = OrderedDict([ + ('bar', 'git://git.openstack.org/openstack/bar-plugin'), + ('foo', 'git://git.openstack.org/openstack/foo-plugin'), + ]) + p = dict(localrc=localrc, + local_conf=local_conf, + base_services=[], + services=services, + plugins=plugins, + base_dir=self.tmpdir, + path=os.path.join(self.tmpdir, 'test.local.conf')) + with self.assertRaises(Exception): + lc = LocalConf(p.get('localrc'), + p.get('local_conf'), + p.get('base_services'), + p.get('services'), + p.get('plugins'), + p.get('base_dir')) + lc.write(p['path']) + + +if __name__ == '__main__': + unittest.main() diff --git a/roles/write-devstack-local-conf/tasks/main.yaml b/roles/write-devstack-local-conf/tasks/main.yaml index cc21426b89..2a9f8985fc 100644 --- a/roles/write-devstack-local-conf/tasks/main.yaml +++ b/roles/write-devstack-local-conf/tasks/main.yaml @@ -8,3 +8,4 @@ services: "{{ devstack_services|default(omit) }}" localrc: "{{ devstack_localrc|default(omit) }}" local_conf: "{{ devstack_local_conf|default(omit) }}" + base_dir: "{{ devstack_base_dir|default(omit) }}" diff --git a/tests/test_write_devstack_local_conf_role.sh b/tests/test_write_devstack_local_conf_role.sh new file mode 100755 index 0000000000..b2bc0a2c46 --- /dev/null +++ b/tests/test_write_devstack_local_conf_role.sh @@ -0,0 +1,9 @@ +#!/usr/bin/env bash + +TOP=$(cd $(dirname "$0")/.. && pwd) + +# Import common functions +source $TOP/functions +source $TOP/tests/unittest.sh + +python ./roles/write-devstack-local-conf/library/test.py