Merge pull request #54731 from pizzapanther/not-so-__new__-and-shiny

Fix returners not loading properly

salt/utils/event.py

@@ -59,7 +59,6 @@
 import hashlib
 import logging
 import datetime
-import sys
 
 try:
     from collections.abc import MutableMapping
@@ -1162,22 +1161,16 @@ 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
         '''
+        # 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)
 
         self.opts = opts

tests/integration/__init__.py

@@ -768,6 +768,12 @@ def transplant_configs(cls, transport='zeromq'):
         }
         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.

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__)

tests/integration/master/__init__.py

@@ -0,0 +1 @@
+# -*- coding: utf-8 -*-

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)

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'

tests/unit/test_module_names.py

@@ -132,6 +132,7 @@ def test_module_name_source_match(self):
             '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 @@ def test_module_name_source_match(self):
             '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',

tests/unit/utils/test_event.py

@@ -12,6 +12,7 @@
 import os
 import hashlib
 import time
+import warnings
 from tornado.testing import AsyncTestCase
 import zmq
 import zmq.eventloop.ioloop
@@ -25,13 +26,15 @@
 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
 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 @@ def test_event_subscription(self):
         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)