openstack/oslo.privsep
Code Issues Proposed changes

Add lock around channel creation

Browse Source 
When a neutron agent starts up, it processes different resources
in separate eventlet threads. These can race creating the channel
which results in redundant privsep-helper processes. This patch
fixes that by adding a lock around channel creation.

Change-Id: I5de22b72059133b05d64be47f4c1d3f566b46a6e
Closes-Bug: #1864664
This commit is contained in:
Darragh O'Reilly
2020-02-25 15:28:20 +00:00
parent 7b8c4d4597
commit bbe24aa6e0
2 changed files with 23 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
oslo_privsep/priv_context.py
View File

@@ -19,6 +19,7 @@ import logging
import multiprocessing import multiprocessing
import shlex import shlex
import sys import sys
import threading
from oslo_config import cfg from oslo_config import cfg
from oslo_config import types from oslo_config import types
@@ -144,6 +145,7 @@ class PrivContext(object):
# The client_mode should be set to False on Windows. # The client_mode should be set to False on Windows.
self.client_mode = sys.platform != 'win32' self.client_mode = sys.platform != 'win32'
self.channel = None self.channel = None
self.start_lock = threading.Lock()
cfg.CONF.register_opts(OPTS, group=cfg_section) cfg.CONF.register_opts(OPTS, group=cfg_section)
cfg.CONF.set_default('capabilities', group=cfg_section, cfg.CONF.set_default('capabilities', group=cfg_section,
@@ -247,6 +249,7 @@ class PrivContext(object):
return func(*args, **kwargs) return func(*args, **kwargs)
def start(self, method=Method.ROOTWRAP): def start(self, method=Method.ROOTWRAP):
with self.start_lock:
if self.channel is not None: if self.channel is not None:
LOG.warning('privsep daemon already running') LOG.warning('privsep daemon already running')
return return

10
oslo_privsep/tests/test_priv_context.py
View File

@@ -127,6 +127,16 @@ class PrivContextTest(testctx.TestContextTestCase):
self.assertEqual(testctx.context.helper_command('/sock')[:3], self.assertEqual(testctx.context.helper_command('/sock')[:3],
['sudo', 'rootwrap', 'privsep-helper']) ['sudo', 'rootwrap', 'privsep-helper'])
def test_start_acquires_lock(self):
context = priv_context.PrivContext('test', capabilities=[])
context.channel = "something not None"
context.start_lock = mock.Mock()
context.start_lock.__enter__ = mock.Mock()
context.start_lock.__exit__ = mock.Mock()
self.assertFalse(context.start_lock.__enter__.called)
context.start()
self.assertTrue(context.start_lock.__enter__.called)
@testtools.skipIf(platform.system() != 'Linux', @testtools.skipIf(platform.system() != 'Linux',
'works only on Linux platform.') 'works only on Linux platform.')