diff options
| -rwxr-xr-x | openstack/common/config/generator.py | 7 | ||||
| -rw-r--r-- | openstack/common/log.py | 3 | ||||
| -rw-r--r-- | openstack/common/rootwrap/filters.py | 17 | ||||
| -rw-r--r-- | openstack/common/service.py | 70 | ||||
| -rw-r--r-- | test-requirements.txt | 2 | ||||
| -rw-r--r-- | tests/unit/test_rootwrap.py | 39 | ||||
| -rw-r--r-- | tests/unit/test_service.py | 27 | ||||
| -rw-r--r-- | tools/install_venv_common.py | 42 | ||||
| -rw-r--r-- | tools/patch_tox_venv.py | 2 | ||||
| -rw-r--r-- | tox.ini | 2 |
10 files changed, 153 insertions, 58 deletions
diff --git a/openstack/common/config/generator.py b/openstack/common/config/generator.py index 8ebfba1..0dd7c97 100755 --- a/openstack/common/config/generator.py +++ b/openstack/common/config/generator.py @@ -188,7 +188,12 @@ def _get_my_ip(): def _sanitize_default(s): """Set up a reasonably sensible default for pybasedir, my_ip and host.""" - if s.startswith(BASEDIR): + if s.startswith(sys.prefix): + # NOTE(jd) Don't use os.path.join, because it is likely to think the + # second part is an absolute pathname and therefore drop the first + # part. + s = os.path.normpath("/usr/" + s[len(sys.prefix):]) + elif s.startswith(BASEDIR): return s.replace(BASEDIR, '/usr/lib/python/site-packages') elif BASEDIR in s: return s.replace(BASEDIR, '') diff --git a/openstack/common/log.py b/openstack/common/log.py index 8097b23..0447a52 100644 --- a/openstack/common/log.py +++ b/openstack/common/log.py @@ -74,7 +74,8 @@ logging_cli_opts = [ cfg.StrOpt('log-format', default=None, metavar='FORMAT', - help='A logging.Formatter log message format string which may ' + help='DEPRECATED. ' + 'A logging.Formatter log message format string which may ' 'use any of the available logging.LogRecord attributes. ' 'This option is deprecated. Please use ' 'logging_context_format_string and ' diff --git a/openstack/common/rootwrap/filters.py b/openstack/common/rootwrap/filters.py index b40fdfd..660434a 100644 --- a/openstack/common/rootwrap/filters.py +++ b/openstack/common/rootwrap/filters.py @@ -47,7 +47,7 @@ class CommandFilter(object): def match(self, userargs): """Only check that the first argument (command) matches exec_path.""" - return os.path.basename(self.exec_path) == userargs[0] + return userargs and os.path.basename(self.exec_path) == userargs[0] def get_command(self, userargs, exec_dirs=[]): """Returns command to execute (with sudo -u if run_as != root).""" @@ -67,7 +67,7 @@ class RegExpFilter(CommandFilter): def match(self, userargs): # Early skip if command or number of args don't match - if (len(self.args) != len(userargs)): + if (not userargs or len(self.args) != len(userargs)): # DENY: argument numbers don't match return False # Compare each arg (anchoring pattern explicitly at end of string) @@ -101,6 +101,9 @@ class PathFilter(CommandFilter): """ def match(self, userargs): + if not userargs or len(userargs) < 2: + return False + command, arguments = userargs[0], userargs[1:] equal_args_num = len(self.args) == len(arguments) @@ -178,7 +181,7 @@ class KillFilter(CommandFilter): super(KillFilter, self).__init__("/bin/kill", *args) def match(self, userargs): - if userargs[0] != "kill": + if not userargs or userargs[0] != "kill": return False args = list(userargs) if len(args) == 3: @@ -229,13 +232,7 @@ class ReadFileFilter(CommandFilter): super(ReadFileFilter, self).__init__("/bin/cat", "root", *args) def match(self, userargs): - if userargs[0] != 'cat': - return False - if userargs[1] != self.file_path: - return False - if len(userargs) != 2: - return False - return True + return (userargs == ['cat', self.file_path]) class IpFilter(CommandFilter): diff --git a/openstack/common/service.py b/openstack/common/service.py index 55e23ed..36cf300 100644 --- a/openstack/common/service.py +++ b/openstack/common/service.py @@ -27,6 +27,7 @@ import sys import time import eventlet +from eventlet import event import logging as std_logging from oslo.config import cfg @@ -51,20 +52,9 @@ class Launcher(object): :returns: None """ - self._services = threadgroup.ThreadGroup() + self.services = Services() self.backdoor_port = eventlet_backdoor.initialize_if_enabled() - @staticmethod - def run_service(service): - """Start and wait for a service to finish. - - :param service: service to run and wait for. - :returns: None - - """ - service.start() - service.wait() - def launch_service(self, service): """Load and start the given service. @@ -73,7 +63,7 @@ class Launcher(object): """ service.backdoor_port = self.backdoor_port - self._services.add_thread(self.run_service, service) + self.services.add(service) def stop(self): """Stop all services which are currently running. @@ -81,7 +71,7 @@ class Launcher(object): :returns: None """ - self._services.stop() + self.services.stop() def wait(self): """Waits until all services have been stopped, and then returns. @@ -89,7 +79,7 @@ class Launcher(object): :returns: None """ - self._services.wait() + self.services.wait() class SignalExit(SystemExit): @@ -124,9 +114,9 @@ class ServiceLauncher(Launcher): except SystemExit as exc: status = exc.code finally: + self.stop() if rpc: rpc.cleanup() - self.stop() return status @@ -189,7 +179,8 @@ class ProcessLauncher(object): random.seed() launcher = Launcher() - launcher.run_service(service) + launcher.launch_service(service) + launcher.wait() def _start_child(self, wrap): if len(wrap.forktimes) > wrap.workers: @@ -313,15 +304,60 @@ class Service(object): def __init__(self, threads=1000): self.tg = threadgroup.ThreadGroup(threads) + # signal that the service is done shutting itself down: + self._done = event.Event() + def start(self): pass def stop(self): self.tg.stop() + self.tg.wait() + self._done.send() + + def wait(self): + self._done.wait() + + +class Services(object): + + def __init__(self): + self.services = [] + self.tg = threadgroup.ThreadGroup() + self.done = event.Event() + + def add(self, service): + self.services.append(service) + self.tg.add_thread(self.run_service, service, self.done) + + def stop(self): + # wait for graceful shutdown of services: + for service in self.services: + service.stop() + service.wait() + + # each service has performed cleanup, now signal that the run_service + # wrapper threads can now die: + self.done.send() + + # reap threads: + self.tg.stop() def wait(self): self.tg.wait() + @staticmethod + def run_service(service, done): + """Service start wrapper. + + :param service: service to run + :param done: event to wait on until a shutdown is triggered + :returns: None + + """ + service.start() + done.wait() + def launch(service, workers=None): if workers: diff --git a/test-requirements.txt b/test-requirements.txt index 7ffabfe..8fbc5ab 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -2,7 +2,7 @@ coverage discover fixtures>=0.3.12 flake8==2.0 -hacking>=0.5.3,<0.6 +hacking>=0.5.6,<0.6 mock mox==0.5.3 mysql-python diff --git a/tests/unit/test_rootwrap.py b/tests/unit/test_rootwrap.py index 6e1e6e6..a649660 100644 --- a/tests/unit/test_rootwrap.py +++ b/tests/unit/test_rootwrap.py @@ -40,6 +40,32 @@ class RootwrapTestCase(utils.BaseTestCase): filters.CommandFilter("/bin/cat", "root") # Keep this one last ] + def test_CommandFilter(self): + f = filters.CommandFilter("sleep", 'root', '10') + self.assertFalse(f.match(["sleep2"])) + + # verify that any arguments are accepted + self.assertTrue(f.match(["sleep"])) + self.assertTrue(f.match(["sleep", "anything"])) + self.assertTrue(f.match(["sleep", "10"])) + f = filters.CommandFilter("sleep", 'root') + self.assertTrue(f.match(["sleep", "10"])) + + def test_empty_commandfilter(self): + f = filters.CommandFilter("sleep", "root") + self.assertFalse(f.match([])) + self.assertFalse(f.match(None)) + + def test_empty_regexpfilter(self): + f = filters.RegExpFilter("sleep", "root", "sleep") + self.assertFalse(f.match([])) + self.assertFalse(f.match(None)) + + def test_empty_invalid_regexpfilter(self): + f = filters.RegExpFilter("sleep", "root") + self.assertFalse(f.match(["anything"])) + self.assertFalse(f.match([])) + def test_RegExpFilter_match(self): usercmd = ["ls", "/root"] filtermatch = wrapper.match_filter(self.filters, usercmd) @@ -195,6 +221,9 @@ class RootwrapTestCase(utils.BaseTestCase): # Providing something that is not a pid should be False usercmd = ['kill', 'notapid'] self.assertFalse(f.match(usercmd)) + # no arguments should also be fine + self.assertFalse(f.match([])) + self.assertFalse(f.match(None)) def test_KillFilter_deleted_exe(self): """Makes sure deleted exe's are killed correctly.""" @@ -289,6 +318,12 @@ class RootwrapTestCase(utils.BaseTestCase): self.assertRaises(wrapper.NoFilterMatched, wrapper.match_filter, filter_list, args) + def test_ReadFileFilter_empty_args(self): + goodfn = '/good/file.name' + f = filters.ReadFileFilter(goodfn) + self.assertFalse(f.match([])) + self.assertFalse(f.match(None)) + def test_exec_dirs_search(self): # This test supposes you have /bin/cat or /usr/bin/cat locally f = filters.CommandFilter("cat", "root") @@ -387,6 +422,10 @@ class PathFilterTestCase(utils.BaseTestCase): self.SYMLINK_OUTSIDE_DIR = os.path.join(tmpdir.path, gen_name()) os.symlink(os.path.join('/tmp', 'some_file'), self.SYMLINK_OUTSIDE_DIR) + def test_empty_args(self): + self.assertFalse(self.f.match([])) + self.assertFalse(self.f.match(None)) + def test_argument_pass_constraint(self): f = filters.PathFilter('/bin/chown', 'root', 'pass', 'pass') diff --git a/tests/unit/test_service.py b/tests/unit/test_service.py index 0f93830..20007de 100644 --- a/tests/unit/test_service.py +++ b/tests/unit/test_service.py @@ -31,6 +31,7 @@ import socket import time import traceback +from eventlet import event from oslo.config import cfg from openstack.common import eventlet_backdoor @@ -195,6 +196,20 @@ class ServiceLauncherTest(utils.BaseTestCase): self.assertEqual(os.WEXITSTATUS(status), 0) +class _Service(service.Service): + def __init__(self): + super(_Service, self).__init__() + self.init = event.Event() + self.cleaned_up = False + + def start(self): + self.init.send() + + def stop(self): + self.cleaned_up = True + super(_Service, self).stop() + + class LauncherTest(utils.BaseTestCase): def test_backdoor_port(self): @@ -252,3 +267,15 @@ class LauncherTest(utils.BaseTestCase): svc = service.Service() self.assertRaises(eventlet_backdoor.EventletBackdoorConfigValueError, service.launch, svc) + + def test_graceful_shutdown(self): + # test that services are given a chance to clean up: + svc = _Service() + + launcher = service.launch(svc) + # wait on 'init' so we know the service had time to start: + svc.init.wait() + + launcher.stop() + self.assertTrue(svc.cleaned_up) + self.assertTrue(svc._done.ready()) diff --git a/tools/install_venv_common.py b/tools/install_venv_common.py index 42a44e8..f428c1e 100644 --- a/tools/install_venv_common.py +++ b/tools/install_venv_common.py @@ -34,12 +34,13 @@ import sys class InstallVenv(object): - def __init__(self, root, venv, pip_requires, test_requires, py_version, + def __init__(self, root, venv, requirements, + test_requirements, py_version, project): self.root = root self.venv = venv - self.pip_requires = pip_requires - self.test_requires = test_requires + self.requirements = requirements + self.test_requirements = test_requirements self.py_version = py_version self.project = project @@ -75,11 +76,13 @@ class InstallVenv(object): def get_distro(self): if (os.path.exists('/etc/fedora-release') or os.path.exists('/etc/redhat-release')): - return Fedora(self.root, self.venv, self.pip_requires, - self.test_requires, self.py_version, self.project) + return Fedora( + self.root, self.venv, self.requirements, + self.test_requirements, self.py_version, self.project) else: - return Distro(self.root, self.venv, self.pip_requires, - self.test_requires, self.py_version, self.project) + return Distro( + self.root, self.venv, self.requirements, + self.test_requirements, self.py_version, self.project) def check_dependencies(self): self.get_distro().install_virtualenv() @@ -98,11 +101,6 @@ class InstallVenv(object): else: self.run_command(['virtualenv', '-q', self.venv]) print('done.') - print('Installing pip in venv...', end=' ') - if not self.run_command(['tools/with_venv.sh', 'easy_install', - 'pip>1.0']).strip(): - self.die("Failed to install pip.") - print('done.') else: print("venv already exists...") pass @@ -116,20 +114,12 @@ class InstallVenv(object): print('Installing dependencies with pip (this can take a while)...') # First things first, make sure our venv has the latest pip and - # distribute. - # NOTE: we keep pip at version 1.1 since the most recent version causes - # the .venv creation to fail. See: - # https://bugs.launchpad.net/nova/+bug/1047120 - self.pip_install('pip==1.1') - self.pip_install('distribute') - - # Install greenlet by hand - just listing it in the requires file does - # not - # get it installed in the right order - self.pip_install('greenlet') - - self.pip_install('-r', self.pip_requires) - self.pip_install('-r', self.test_requires) + # setuptools. + self.pip_install('pip>=1.3') + self.pip_install('setuptools') + + self.pip_install('-r', self.requirements) + self.pip_install('-r', self.test_requirements) def post_process(self): self.get_distro().post_process() diff --git a/tools/patch_tox_venv.py b/tools/patch_tox_venv.py index a3340f2..dc9ce83 100644 --- a/tools/patch_tox_venv.py +++ b/tools/patch_tox_venv.py @@ -17,7 +17,7 @@ import os import sys -import install_venv_common as install_venv +import install_venv_common as install_venv # noqa def first_file(file_list): @@ -13,7 +13,7 @@ commands = [flake8] show-source = True -ignore = H302,H304 +ignore = H302 exclude = .venv,.tox,dist,doc,*.egg,.update-venv [testenv:pep8] |