Add hacking test to block cross-virt driver code usage
The baremetal driver has been accessing some libvirt driver private classes and config variables. Any code that is intended to be shared across virt drivers should be in a common package instead. Add a hacking file to enforce this for imports and config options so that no further problems like this are made in the future. NB, this skips verification of the baremetal driver until bugs 1261826 and 1261827 are fixed, or the baremetal driver is removed, whichever comes first. Change-Id: Ifbfe597795795a847830f9bd937dc459cd37d118 Closes-Bug: #1261842
This commit is contained in:
parent
b7e9257292
commit
848ea5ff01
@ -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
|
||||
|
@ -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)
|
||||
|
38
nova/tests/test_hacking.py
Normal file
38
nova/tests/test_hacking.py
Normal file
@ -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"))
|
Loading…
Reference in New Issue
Block a user