From 0746aa76dd01065c22075657d2ab2d869c619f59 Mon Sep 17 00:00:00 2001 From: Paul Bailey Date: Tue, 24 Sep 2019 16:44:49 -0500 Subject: [PATCH 1/4] remove __new__ method since it was removed from parent class --- salt/utils/event.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/salt/utils/event.py b/salt/utils/event.py index ad2c93883e..aae97de6ea 100644 --- a/salt/utils/event.py +++ b/salt/utils/event.py @@ -1162,22 +1162,19 @@ class EventReturn(salt.utils.process.SignalHandlingMultiprocessingProcess): A dedicated process which listens to the master event bus and queues and forwards events to the specified returner. ''' - def __new__(cls, *args, **kwargs): - if sys.platform.startswith('win'): - # This is required for Windows. On Linux, when a process is - # forked, the module namespace is copied and the current process - # gets all of sys.modules from where the fork happens. This is not - # the case for Windows. - import salt.minion - instance = super(EventReturn, cls).__new__(cls, *args, **kwargs) - return instance - def __init__(self, opts, **kwargs): ''' Initialize the EventReturn system Return an EventReturn instance ''' + if sys.platform.startswith('win'): + # This is required for Windows. On Linux, when a process is + # forked, the module namespace is copied and the current process + # gets all of sys.modules from where the fork happens. This is not + # the case for Windows. + import salt.minion + super(EventReturn, self).__init__(**kwargs) self.opts = opts From 3f8a3824d79372519ea91980a77349371010de62 Mon Sep 17 00:00:00 2001 From: Paul Bailey Date: Tue, 24 Sep 2019 17:10:47 -0500 Subject: [PATCH 2/4] fix module import --- salt/utils/event.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/salt/utils/event.py b/salt/utils/event.py index aae97de6ea..0f7b9d1b87 100644 --- a/salt/utils/event.py +++ b/salt/utils/event.py @@ -1168,12 +1168,9 @@ class EventReturn(salt.utils.process.SignalHandlingMultiprocessingProcess): Return an EventReturn instance ''' - if sys.platform.startswith('win'): - # This is required for Windows. On Linux, when a process is - # forked, the module namespace is copied and the current process - # gets all of sys.modules from where the fork happens. This is not - # the case for Windows. - import salt.minion + # This is required because the process is forked and the module no + # longer exists in the global namespace. + import salt.minion super(EventReturn, self).__init__(**kwargs) From 54995181bb15f5ba52845985c1fd81a7f25cdb3c Mon Sep 17 00:00:00 2001 From: Paul Bailey Date: Tue, 24 Sep 2019 17:49:48 -0500 Subject: [PATCH 3/4] remove unnecessary import --- salt/utils/event.py | 1 - 1 file changed, 1 deletion(-) diff --git a/salt/utils/event.py b/salt/utils/event.py index 0f7b9d1b87..0b877980aa 100644 --- a/salt/utils/event.py +++ b/salt/utils/event.py @@ -59,7 +59,6 @@ import fnmatch import hashlib import logging import datetime -import sys try: from collections.abc import MutableMapping From bdf24f4064a63e60eae307dd7300451d6add0339 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Wed, 25 Sep 2019 00:26:35 +0100 Subject: [PATCH 4/4] Make sure we tests salt-master's `event_return` setting --- tests/integration/__init__.py | 6 + .../files/returners/noop_returner.py | 38 +++++++ tests/integration/master/__init__.py | 1 + tests/integration/master/test_event_return.py | 105 ++++++++++++++++++ .../integration/returners/test_noop_return.py | 32 ++++++ tests/unit/test_module_names.py | 2 + tests/unit/utils/test_event.py | 27 +++++ 7 files changed, 211 insertions(+) create mode 100644 tests/integration/files/returners/noop_returner.py create mode 100644 tests/integration/master/__init__.py create mode 100644 tests/integration/master/test_event_return.py create mode 100644 tests/integration/returners/test_noop_return.py diff --git a/tests/integration/__init__.py b/tests/integration/__init__.py index 4155310288..9183e5ba4b 100644 --- a/tests/integration/__init__.py +++ b/tests/integration/__init__.py @@ -768,6 +768,12 @@ class TestDaemon(object): } master_opts['ext_pillar'].append({'file_tree': file_tree}) + # Config settings to test `event_return` + if 'returner_dirs' not in master_opts: + master_opts['returner_dirs'] = [] + master_opts['returner_dirs'].append(os.path.join(RUNTIME_VARS.FILES, 'returners')) + master_opts['event_return'] = 'runtests_noop' + # Under windows we can't seem to properly create a virtualenv off of another # virtualenv, we can on linux but we will still point to the virtualenv binary # outside the virtualenv running the test suite, if that's the case. diff --git a/tests/integration/files/returners/noop_returner.py b/tests/integration/files/returners/noop_returner.py new file mode 100644 index 0000000000..2e33af3967 --- /dev/null +++ b/tests/integration/files/returners/noop_returner.py @@ -0,0 +1,38 @@ +# -*- coding: utf-8 -*- +''' +noop_returner +~~~~~~~~~~~~~ + +A returner that does nothing which is used to test the salt-master `event_return` functionality +''' + +# Import python libs +from __future__ import absolute_import, print_function, unicode_literals +import logging + +# Import Salt libs +import salt.utils.jid + + +log = logging.getLogger(__name__) + +__virtualname__ = 'runtests_noop' + + +def __virtual__(): + return True + + +def event_return(events): + log.debug('NOOP_RETURN.event_return - Events: %s', events) + + +def returner(ret): + log.debug('NOOP_RETURN.returner - Ret: %s', ret) + + +def prep_jid(nocache=False, passed_jid=None): # pylint: disable=unused-argument + ''' + Do any work necessary to prepare a JID, including sending a custom id + ''' + return passed_jid if passed_jid is not None else salt.utils.jid.gen_jid(__opts__) diff --git a/tests/integration/master/__init__.py b/tests/integration/master/__init__.py new file mode 100644 index 0000000000..40a96afc6f --- /dev/null +++ b/tests/integration/master/__init__.py @@ -0,0 +1 @@ +# -*- coding: utf-8 -*- diff --git a/tests/integration/master/test_event_return.py b/tests/integration/master/test_event_return.py new file mode 100644 index 0000000000..3076015bab --- /dev/null +++ b/tests/integration/master/test_event_return.py @@ -0,0 +1,105 @@ +# -*- coding: utf-8 -*- +''' +tests.integration.master.test_event_return +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + + This test module is meant to cover the issue being fixed by: + + https://github.com/saltstack/salt/pull/54731 +''' +# Import Python libs +from __future__ import absolute_import, print_function, unicode_literals +import os +import time +import logging +import shutil +import subprocess + +# Import Salt Testing libs +from tests.support.case import TestCase +from tests.support.paths import ScriptPathMixin +from tests.support.helpers import get_unused_localhost_port +from tests.support.mixins import AdaptedConfigurationTestCaseMixin + +# Import 3rd-party libs +from tests.support.processes import terminate_process + +# Import Salt libs +import salt.ext.six as six +from salt.utils.nb_popen import NonBlockingPopen + +log = logging.getLogger(__name__) + + +class TestEventReturn(AdaptedConfigurationTestCaseMixin, ScriptPathMixin, TestCase): + + @classmethod + def setUpClass(cls): + overrides = { + 'publish_port': get_unused_localhost_port(), + 'ret_port': get_unused_localhost_port(), + 'tcp_master_pub_port': get_unused_localhost_port(), + 'tcp_master_pull_port': get_unused_localhost_port(), + 'tcp_master_publish_pull': get_unused_localhost_port(), + 'tcp_master_workers': get_unused_localhost_port(), + 'runtests_conn_check_port': get_unused_localhost_port(), + 'runtests_log_port': get_unused_localhost_port() + } + overrides['pytest_engine_port'] = overrides['runtests_conn_check_port'] + temp_config = AdaptedConfigurationTestCaseMixin.get_temp_config('master', **overrides) + cls.root_dir = temp_config['root_dir'] + cls.config_dir = os.path.dirname(temp_config['conf_file']) + + @classmethod + def tearDownClass(cls): + shutil.rmtree(cls.root_dir) + cls.root_dir = cls.config_dir = None + + def test_master_startup(self): + proc = NonBlockingPopen( + [ + self.get_script_path('master'), + '-c', + self.config_dir, + '-l', + 'info' + ], + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT + ) + out = six.b('') + err = six.b('') + + # Testing this should never be longer than 1 minute + max_time = time.time() + 60 + try: + while True: + if time.time() > max_time: + assert False, 'Max timeout ocurred' + time.sleep(0.5) + _out = proc.recv() + _err = proc.recv_err() + if _out: + out += _out + if _err: + err += _err + + print(1, repr(out)) + + if six.b('DeprecationWarning: object() takes no parameters') in out: + self.fail('\'DeprecationWarning: object() takes no parameters\' was seen in output') + + if six.b('TypeError: object() takes no parameters') in out: + self.fail('\'TypeError: object() takes no parameters\' was seen in output') + + if six.b('Setting up the master communication server') in out: + # We got past the place we need, stop the process + break + + if out is None and err is None: + break + + if proc.poll() is not None: + break + finally: + terminate_process(proc.pid, kill_children=True) diff --git a/tests/integration/returners/test_noop_return.py b/tests/integration/returners/test_noop_return.py new file mode 100644 index 0000000000..148be19573 --- /dev/null +++ b/tests/integration/returners/test_noop_return.py @@ -0,0 +1,32 @@ +# -*- coding: utf-8 -*- +''' + tests.integration.returners.test_noop_return + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + + This test module is meant to cover the issue being fixed by: + + https://github.com/saltstack/salt/pull/54731 +''' +# Import Python libs +from __future__ import absolute_import, print_function, unicode_literals +import logging + +# Import Salt Testing libs +from tests.support.case import ModuleCase +from tests.support.unit import skipIf +from tests.support.helpers import TestsLoggingHandler + +# Import 3rd-party tests +import salt.ext.six as six + + +log = logging.getLogger(__name__) + + +@skipIf(six.PY3, 'Runtest Log Hander Disabled for PY3, #41836') +class TestEventReturn(ModuleCase): + + def test_noop_return(self): + with TestsLoggingHandler(format='%(message)s', level=logging.DEBUG) as handler: + self.run_function('test.ping') + assert any('NOOP_RETURN' in s for s in handler.messages) is True, 'NOOP_RETURN not found in log messages' diff --git a/tests/unit/test_module_names.py b/tests/unit/test_module_names.py index 33d38e37c5..cc53a88a60 100644 --- a/tests/unit/test_module_names.py +++ b/tests/unit/test_module_names.py @@ -132,6 +132,7 @@ class BadTestModuleNamesTestCase(TestCase): 'integration.loader.test_ext_grains', 'integration.loader.test_ext_modules', 'integration.logging.test_jid_logging', + 'integration.master.test_event_return', 'integration.minion.test_blackout', 'integration.minion.test_pillar', 'integration.minion.test_timeout', @@ -147,6 +148,7 @@ class BadTestModuleNamesTestCase(TestCase): 'integration.proxy.test_shell', 'integration.proxy.test_simple', 'integration.reactor.test_reactor', + 'integration.returners.test_noop_return', 'integration.runners.test_runner_returns', 'integration.scheduler.test_error', 'integration.scheduler.test_eval', diff --git a/tests/unit/utils/test_event.py b/tests/unit/utils/test_event.py index d93e0b94f8..a06982212b 100644 --- a/tests/unit/utils/test_event.py +++ b/tests/unit/utils/test_event.py @@ -12,6 +12,7 @@ from __future__ import absolute_import, unicode_literals, print_function import os import hashlib import time +import warnings from tornado.testing import AsyncTestCase import zmq import zmq.eventloop.ioloop @@ -25,6 +26,7 @@ from multiprocessing import Process from tests.support.unit import expectedFailure, skipIf, TestCase # Import salt libs +import salt.config import salt.utils.event import salt.utils.stringutils import tests.integration as integration @@ -32,6 +34,7 @@ from salt.utils.process import clean_proc # Import 3rd-+arty libs from salt.ext.six.moves import range # pylint: disable=import-error,redefined-builtin +from tests.support.processes import terminate_process SOCK_DIR = os.path.join(integration.TMP, 'test-socks') @@ -366,3 +369,27 @@ class TestAsyncEventPublisher(AsyncTestCase): self.assertEqual(self.tag, 'evt1') self.data.pop('_stamp') # drop the stamp self.assertEqual(self.data, {'data': 'foo1'}) + + +class TestEventReturn(TestCase): + + def test_event_return(self): + # Once salt is py3 only, the warnings part of this test no longer applies + evt = None + try: + with warnings.catch_warnings(record=True) as w: + # Cause all warnings to always be triggered. + warnings.simplefilter("always") + evt = None + try: + evt = salt.utils.event.EventReturn(salt.config.DEFAULT_MASTER_OPTS.copy()) + evt.start() + except TypeError as exc: + if 'object' in str(exc): + self.fail('\'{}\' TypeError should have not been raised'.format(exc)) + for warning in w: + if warning.category is DeprecationWarning: + assert 'object() takes no parameters' not in warning.message + finally: + if evt is not None: + terminate_process(evt.pid, kill_children=True)