diff --git a/HACKING.rst b/HACKING.rst index c9f9db450071..d2d9d51ab8e2 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -13,6 +13,12 @@ Nova Specific Commandments This enforces a guideline defined in ``nova.openstack.common.db.sqlalchemy.session`` - [N310] timeutils.utcnow() wrapper must be used instead of direct calls to datetime.datetime.utcnow() to make it easy to override its return value in tests +- [N311] importing code from other virt drivers forbidden + Code that needs to be shared between virt drivers should be moved + into a common module +- [N312] using config vars from other virt drivers forbidden + Config parameters that need to be shared between virt drivers + should be moved into a common module - [N123] vim configuration should not be kept in source files. Creating Unit Tests diff --git a/nova/hacking/checks.py b/nova/hacking/checks.py index c7230e1b5706..411fd5a88703 100644 --- a/nova/hacking/checks.py +++ b/nova/hacking/checks.py @@ -18,6 +18,9 @@ import re session_check = re.compile("\w*def [a-zA-Z0-9].*[(].*session.*[)]") cfg_re = re.compile(".*\scfg\.") vi_header_re = re.compile("^#\s+vim?:.+") +virt_file_re = re.compile("\./nova/(?:tests/)?virt/(\w+)/") +virt_import_re = re.compile("from nova\.(?:tests\.)?virt\.(\w+) import") +virt_config_re = re.compile("CONF\.import_opt\('.*?', 'nova\.virt\.(\w+)('|.)") def import_no_db_in_virt(logical_line, filename): @@ -49,6 +52,59 @@ def use_timeutils_utcnow(logical_line): yield (pos, msg % f) +def _get_virt_name(regex, data): + m = regex.match(data) + if m is None: + return None + driver = m.group(1) + # Ignore things we mis-detect as virt drivers in the regex + if driver in ["test_virt_drivers", "driver", + "disk", "api", "imagecache", "cpu"]: + return None + # TODO(berrange): remove once bugs 1261826 and 126182 are + # fixed, or baremetal driver is removed, which is first. + if driver == "baremetal": + return None + return driver + + +def import_no_virt_driver_import_deps(physical_line, filename): + """Check virt drivers' modules aren't imported by other drivers + + Modules under each virt driver's directory are + considered private to that virt driver. Other drivers + in Nova must not access those drivers. Any code that + is to be shared should be refactored into a common + module + + N311 + """ + thisdriver = _get_virt_name(virt_file_re, filename) + thatdriver = _get_virt_name(virt_import_re, physical_line) + if (thatdriver is not None and + thisdriver is not None and + thisdriver != thatdriver): + return (0, "N312: importing code from other virt drivers forbidden") + + +def import_no_virt_driver_config_deps(physical_line, filename): + """Check virt drivers' config vars aren't used by other drivers + + Modules under each virt driver's directory are + considered private to that virt driver. Other drivers + in Nova must not use their config vars. Any config vars + that are to be shared should be moved into a common module + + N312 + """ + thisdriver = _get_virt_name(virt_file_re, filename) + thatdriver = _get_virt_name(virt_config_re, physical_line) + if (thatdriver is not None and + thisdriver is not None and + thisdriver != thatdriver): + return (0, "N313: using config vars from other virt drivers forbidden") + + def capital_cfg_help(logical_line, tokens): msg = "N500: capitalize help string" @@ -78,5 +134,7 @@ def factory(register): register(import_no_db_in_virt) register(no_db_session_in_public_api) register(use_timeutils_utcnow) + register(import_no_virt_driver_import_deps) + register(import_no_virt_driver_config_deps) register(capital_cfg_help) register(no_vi_headers) diff --git a/nova/tests/test_hacking.py b/nova/tests/test_hacking.py new file mode 100644 index 000000000000..e175169c9996 --- /dev/null +++ b/nova/tests/test_hacking.py @@ -0,0 +1,38 @@ +# Copyright 2014 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. + +from nova.hacking import checks +from nova import test + + +class HackingTestCase(test.NoDBTestCase): + def test_virt_driver_imports(self): + self.assertTrue(checks.import_no_virt_driver_import_deps( + "from nova.virt.libvirt import utils as libvirt_utils", + "./nova/virt/xenapi/driver.py")) + + self.assertIsNone(checks.import_no_virt_driver_import_deps( + "from nova.virt.libvirt import utils as libvirt_utils", + "./nova/virt/libvirt/driver.py")) + + def test_virt_driver_config_vars(self): + self.assertTrue(checks.import_no_virt_driver_config_deps( + "CONF.import_opt('volume_drivers', " + "'nova.virt.libvirt.driver', group='libvirt')", + "./nova/virt/xenapi/driver.py")) + + self.assertIsNone(checks.import_no_virt_driver_config_deps( + "CONF.import_opt('volume_drivers', " + "'nova.virt.libvirt.driver', group='libvirt')", + "./nova/virt/libvirt/volume.py"))