From 6179854ae77960d4ae883349626069023b407cb0 Mon Sep 17 00:00:00 2001 From: Ryan Rossiter Date: Thu, 25 Jun 2015 14:41:49 +0000 Subject: [PATCH] Add hacking check for eventlet.spawn() Change Id52c30bb5ded2184d772e6026b0f04f9a0efeb55 added a hacking check for greenthread.spawn(). Since eventlet.spawn() calls greenthread.spawn() under the covers, it should also be checked. Because there are still occurrences of eventlet.spawn(), these were also cleaned up in order to pass the added hacking check. Co-Authored-By: Qin Zhao Change-Id: Ia125b4ad5e84bf48091af5a7a483b89629f0ec31 Related-Bug: #1404268 Closes-Bug: #1468513 --- HACKING.rst | 2 +- nova/hacking/checks.py | 20 +++++++++++-------- nova/image/s3.py | 3 +-- nova/network/manager.py | 5 ++--- nova/network/model.py | 4 ++-- nova/tests/unit/test_hacking.py | 8 ++++++++ .../unit/virt/hyperv/test_eventhandler.py | 3 ++- nova/virt/hyperv/eventhandler.py | 9 +++++---- nova/virt/libvirt/host.py | 3 +-- nova/vnc/xvp_proxy.py | 7 ++++--- nova/wsgi.py | 3 ++- 11 files changed, 40 insertions(+), 27 deletions(-) diff --git a/HACKING.rst b/HACKING.rst index 8683b2ed92..6f41579637 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -50,7 +50,7 @@ Nova Specific Commandments assertEqual(A in B, False) or assertEqual(False, A in B) to the more specific assertIn/NotIn(A, B) - [N339] Check common raise_feature_not_supported() is used for v2.1 HTTPNotImplemented response. -- [N340] Check nova.utils.spawn() is used instead of greenthread.spawn() +- [N340] Check nova.utils.spawn() is used instead of greenthread.spawn() and eventlet.spawn() Creating Unit Tests ------------------- diff --git a/nova/hacking/checks.py b/nova/hacking/checks.py index bb9cff71ac..ddd9218c82 100644 --- a/nova/hacking/checks.py +++ b/nova/hacking/checks.py @@ -96,8 +96,8 @@ api_version_re = re.compile(r"@.*api_version") dict_constructor_with_list_copy_re = re.compile(r".*\bdict\((\[)?(\(|\[)") decorator_re = re.compile(r"@.*") http_not_implemented_re = re.compile(r"raise .*HTTPNotImplemented\(") -greenthread_spawn_re = re.compile(r".*greenthread.spawn\(.*\)") -greenthread_spawn_n_re = re.compile(r".*greenthread.spawn_n\(.*\)") +spawn_re = re.compile( + r".*(eventlet|greenthread)\.(?Pspawn(_n)?)\(.*\)") class BaseASTChecker(ast.NodeVisitor): @@ -523,16 +523,20 @@ def check_http_not_implemented(logical_line, physical_line, filename): def check_greenthread_spawns(logical_line, physical_line, filename): - """Check for use of greenthread.spawn() and greenthread.spawn_n() + """Check for use of greenthread.spawn(), greenthread.spawn_n(), + eventlet.spawn(), and eventlet.spawn_n() N340 """ msg = ("N340: Use nova.utils.%(spawn)s() rather than " - "greenthread.%(spawn)s()") - if re.match(greenthread_spawn_re, logical_line): - yield (0, msg % {'spawn': 'spawn'}) - if re.match(greenthread_spawn_n_re, logical_line): - yield (0, msg % {'spawn': 'spawn_n'}) + "greenthread.%(spawn)s() and eventlet.%(spawn)s()") + if "nova/utils.py" in filename or "nova/tests/" in filename: + return + + match = re.match(spawn_re, logical_line) + + if match: + yield (0, msg % {'spawn': match.group('spawn_part')}) def factory(register): diff --git a/nova/image/s3.py b/nova/image/s3.py index 6abe396e9f..5b6b526b41 100644 --- a/nova/image/s3.py +++ b/nova/image/s3.py @@ -24,7 +24,6 @@ import tarfile import tempfile import boto.s3.connection -import eventlet from lxml import etree from oslo_concurrency import processutils from oslo_config import cfg @@ -385,7 +384,7 @@ class S3ImageService(object): LOG.info(_LI("Image %s was deleted underneath us"), image_uuid) return - eventlet.spawn_n(delayed_create) + utils.spawn_n(delayed_create) return image diff --git a/nova/network/manager.py b/nova/network/manager.py index dc56ed7762..92b3cef850 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -32,7 +32,6 @@ import math import re import uuid -import eventlet import netaddr from oslo_config import cfg from oslo_log import log as logging @@ -184,7 +183,7 @@ class RPCAllocateFixedIP(object): network_p) if host != self.host: # need to call allocate_fixed_ip to correct network host - green_threads.append(eventlet.spawn( + green_threads.append(utils.spawn( self.network_rpcapi._rpc_allocate_fixed_ip, context, instance_id, network['id'], address, vpn, host)) @@ -1516,7 +1515,7 @@ class NetworkManager(manager.Manager): call_func(context, network) else: # i'm not the right host, run call on correct host - green_threads.append(eventlet.spawn( + green_threads.append(utils.spawn( self.network_rpcapi.rpc_setup_network_on_host, context, network.id, teardown, host)) diff --git a/nova/network/model.py b/nova/network/model.py index f6ecbacba1..910f5dc37d 100644 --- a/nova/network/model.py +++ b/nova/network/model.py @@ -15,13 +15,13 @@ import functools -import eventlet import netaddr from oslo_serialization import jsonutils import six from nova import exception from nova.i18n import _ +from nova import utils def ensure_string_keys(d): @@ -484,7 +484,7 @@ class NetworkInfoAsyncWrapper(NetworkInfo): """ def __init__(self, async_method, *args, **kwargs): - self._gt = eventlet.spawn(async_method, *args, **kwargs) + self._gt = utils.spawn(async_method, *args, **kwargs) methods = ['json', 'fixed_ips', 'floating_ips'] for method in methods: fn = getattr(self, method) diff --git a/nova/tests/unit/test_hacking.py b/nova/tests/unit/test_hacking.py index 47e38870ec..e4efdb044e 100644 --- a/nova/tests/unit/test_hacking.py +++ b/nova/tests/unit/test_hacking.py @@ -550,6 +550,14 @@ class HackingTestCase(test.NoDBTestCase): self._assert_has_errors(code, checks.check_greenthread_spawns, expected_errors=errors) + code = "eventlet.spawn(func, arg1, kwarg1=kwarg1)" + self._assert_has_errors(code, checks.check_greenthread_spawns, + expected_errors=errors) + + code = "eventlet.spawn_n(func, arg1, kwarg1=kwarg1)" + self._assert_has_errors(code, checks.check_greenthread_spawns, + expected_errors=errors) + code = "nova.utils.spawn(func, arg1, kwarg1=kwarg1)" self._assert_has_no_errors(code, checks.check_greenthread_spawns) diff --git a/nova/tests/unit/virt/hyperv/test_eventhandler.py b/nova/tests/unit/virt/hyperv/test_eventhandler.py index 5f599535af..55b983e21a 100644 --- a/nova/tests/unit/virt/hyperv/test_eventhandler.py +++ b/nova/tests/unit/virt/hyperv/test_eventhandler.py @@ -18,6 +18,7 @@ import mock from nova import exception from nova.tests.unit.virt.hyperv import test_base +from nova import utils from nova.virt.hyperv import constants from nova.virt.hyperv import eventhandler from nova.virt.hyperv import utilsfactory @@ -103,7 +104,7 @@ class EventHandlerTestCase(test_base.HyperVBaseTestCase): self._test_dispatch_event(missing_uuid=True) @mock.patch.object(eventhandler.InstanceEventHandler, '_get_virt_event') - @mock.patch.object(eventlet, 'spawn_n') + @mock.patch.object(utils, 'spawn_n') def test_emit_event(self, mock_spawn, mock_get_event): self._event_handler._emit_event(mock.sentinel.instance_name, mock.sentinel.instance_uuid, diff --git a/nova/virt/hyperv/eventhandler.py b/nova/virt/hyperv/eventhandler.py index b5247c2551..b953f269d5 100644 --- a/nova/virt/hyperv/eventhandler.py +++ b/nova/virt/hyperv/eventhandler.py @@ -25,6 +25,7 @@ from oslo_log import log as logging from nova import exception from nova.i18n import _LW +from nova import utils from nova.virt import event as virtevent from nova.virt.hyperv import constants from nova.virt.hyperv import utilsfactory @@ -71,7 +72,7 @@ class InstanceEventHandler(object): self._running_state_callback = running_state_callback def start_listener(self): - eventlet.spawn_n(self._poll_events) + utils.spawn_n(self._poll_events) def _poll_events(self): while True: @@ -101,11 +102,11 @@ class InstanceEventHandler(object): def _emit_event(self, instance_name, instance_uuid, instance_state): virt_event = self._get_virt_event(instance_uuid, instance_state) - eventlet.spawn_n(self._state_change_callback, virt_event) + utils.spawn_n(self._state_change_callback, virt_event) if instance_state == constants.HYPERV_VM_STATE_ENABLED: - eventlet.spawn_n(self._running_state_callback, - instance_name, instance_uuid) + utils.spawn_n(self._running_state_callback, + instance_name, instance_uuid) def _get_instance_uuid(self, instance_name): try: diff --git a/nova/virt/libvirt/host.py b/nova/virt/libvirt/host.py index 44ebc7e9fe..110f6bb827 100644 --- a/nova/virt/libvirt/host.py +++ b/nova/virt/libvirt/host.py @@ -33,7 +33,6 @@ import socket import sys import threading -import eventlet from eventlet import greenio from eventlet import greenthread from eventlet import patcher @@ -457,7 +456,7 @@ class Host(object): self._event_thread.start() LOG.debug("Starting green dispatch thread") - eventlet.spawn(self._dispatch_thread) + utils.spawn(self._dispatch_thread) def _get_new_connection(self): # call with _wrapped_conn_lock held diff --git a/nova/vnc/xvp_proxy.py b/nova/vnc/xvp_proxy.py index 726c3bcab4..1c868540ad 100644 --- a/nova/vnc/xvp_proxy.py +++ b/nova/vnc/xvp_proxy.py @@ -29,6 +29,7 @@ import webob from nova.consoleauth import rpcapi as consoleauth_rpcapi from nova import context from nova.i18n import _LI +from nova import utils from nova import version from nova import wsgi @@ -108,7 +109,7 @@ class XCPVNCProxy(object): def proxy_connection(self, req, connect_info, start_response): """Spawn bi-directional vnc proxy.""" sockets = {} - t0 = eventlet.spawn(self.handshake, req, connect_info, sockets) + t0 = utils.spawn(self.handshake, req, connect_info, sockets) t0.wait() if not sockets.get('client') or not sockets.get('server'): @@ -120,8 +121,8 @@ class XCPVNCProxy(object): client = sockets['client'] server = sockets['server'] - t1 = eventlet.spawn(self.one_way_proxy, client, server) - t2 = eventlet.spawn(self.one_way_proxy, server, client) + t1 = utils.spawn(self.one_way_proxy, client, server) + t2 = utils.spawn(self.one_way_proxy, server, client) t1.wait() t2.wait() diff --git a/nova/wsgi.py b/nova/wsgi.py index b3d3f33239..6d18215a9a 100644 --- a/nova/wsgi.py +++ b/nova/wsgi.py @@ -39,6 +39,7 @@ import webob.exc from nova import exception from nova.i18n import _, _LE, _LI +from nova import utils wsgi_opts = [ cfg.StrOpt('api_paste_config', @@ -236,7 +237,7 @@ class Server(service.ServiceBase): if self._max_url_len: wsgi_kwargs['url_length_limit'] = self._max_url_len - self._server = eventlet.spawn(**wsgi_kwargs) + self._server = utils.spawn(**wsgi_kwargs) def reset(self): """Reset server greenpool size to default.