diff --git a/changelog/68553.fixed.md b/changelog/68553.fixed.md new file mode 100644 index 000000000000..f337f18ccd06 --- /dev/null +++ b/changelog/68553.fixed.md @@ -0,0 +1 @@ +Fixed minion process name pollution when multiprocessing is disabled diff --git a/salt/minion.py b/salt/minion.py index a22717a9eea3..67e2e34620fb 100644 --- a/salt/minion.py +++ b/salt/minion.py @@ -2024,7 +2024,8 @@ def _thread_return(cls, minion_instance, opts, data): minion_instance.gen_modules() fn_ = os.path.join(minion_instance.proc_dir, data["jid"]) - salt.utils.process.appendproctitle(f"{cls.__name__}._thread_return") + if opts.get("multiprocessing", True): + salt.utils.process.appendproctitle(f"{cls.__name__}._thread_return") sdata = {"pid": os.getpid()} sdata.update(data) @@ -2229,7 +2230,8 @@ def _thread_multi_return(cls, minion_instance, opts, data): minion_instance.gen_modules() fn_ = os.path.join(minion_instance.proc_dir, data["jid"]) - salt.utils.process.appendproctitle(f"{cls.__name__}._thread_multi_return") + if opts.get("multiprocessing", True): + salt.utils.process.appendproctitle(f"{cls.__name__}._thread_multi_return") sdata = {"pid": os.getpid()} sdata.update(data) diff --git a/salt/utils/process.py b/salt/utils/process.py index 8704d9ab307e..0c09b33bca9f 100644 --- a/salt/utils/process.py +++ b/salt/utils/process.py @@ -611,7 +611,12 @@ def run(self, asynchronous=False): Load and start all available api modules """ log.debug("Process Manager starting!") - if multiprocessing.current_process().name != "MainProcess": + # Always append for minion process managers, even in MainProcess + # (e.g., when running with --disable-keepalive) + if ( + multiprocessing.current_process().name != "MainProcess" + or "Minion" in self.name + ): appendproctitle(self.name) # make sure to kill the subprocesses if the parent is killed diff --git a/tests/pytests/integration/minion/test_process_name.py b/tests/pytests/integration/minion/test_process_name.py new file mode 100644 index 000000000000..54c859217fbf --- /dev/null +++ b/tests/pytests/integration/minion/test_process_name.py @@ -0,0 +1,161 @@ +""" +Test process name behavior with multiprocessing enabled and disabled. +""" + + +def test_process_name_no_pollution_when_multiprocessing_disabled( + salt_master_factory, +): + """ + Test that process names don't accumulate _thread_return strings + when multiprocessing is disabled (issue #68553). + + When multiprocessing=False, jobs run in threads in the main process. + This test ensures that the process name doesn't keep appending + "Minion._thread_return" with each job execution. + """ + # Create a fresh master for this test + master = salt_master_factory.salt_master_daemon("test-process-name-master") + + # Create a fresh minion with multiprocessing=False + minion = master.salt_minion_daemon( + "test-process-name-minion-no-mp", + overrides={"multiprocessing": False}, + ) + + cli = master.salt_cli() + + with master.started(), minion.started(): + # Execute multiple jobs to ensure the process name doesn't accumulate + for i in range(3): + ret = cli.run("test.ping", minion_tgt=minion.id) + assert ret.returncode == 0 + assert ret.data is True + + # Get the minion's PID directly from the fixture + minion_pid = minion.pid + assert minion_pid, "Minion PID not available" + + # Get the process info using the actual PID + ret = cli.run("ps.proc_info", minion_pid, minion_tgt=minion.id) + assert ret.returncode == 0, f"Failed to get process info for PID {minion_pid}" + + # Get the process command line + proc_info = ret.data + cmdline = " ".join(proc_info.get("cmdline", [])) + + # The process name should not contain _thread_return at all + # when multiprocessing=False, as we skip the appendproctitle call + assert "_thread_return" not in cmdline, ( + f"Process cmdline should not contain '_thread_return' when " + f"multiprocessing=False, but got: {cmdline}" + ) + + # Verify the process is a salt-minion process + assert ( + "salt-minion" in cmdline + or "salt_minion" in cmdline + or "cli_salt_minion" in cmdline + ) + + +def test_process_name_normal_when_multiprocessing_enabled( + salt_master_factory, +): + """ + Test that process names work normally when multiprocessing is enabled. + + When multiprocessing=True (default), jobs run in separate processes, + so the main minion process name should remain clean. + """ + # Create a fresh master for this test + master = salt_master_factory.salt_master_daemon("test-process-name-master-mp") + + # Create a fresh minion with multiprocessing=True + minion = master.salt_minion_daemon( + "test-process-name-minion-with-mp", + overrides={"multiprocessing": True}, + ) + + cli = master.salt_cli() + + with master.started(), minion.started(): + # Execute a job + ret = cli.run("test.ping", minion_tgt=minion.id) + assert ret.returncode == 0 + assert ret.data is True + + # Get the minion's PID directly from the fixture + minion_pid = minion.pid + assert minion_pid, "Minion PID not available" + + # Get the process info using the actual PID + ret = cli.run("ps.proc_info", minion_pid, minion_tgt=minion.id) + assert ret.returncode == 0, f"Failed to get process info for PID {minion_pid}" + + # Get the process command line + proc_info = ret.data + cmdline = " ".join(proc_info.get("cmdline", [])) + + # Verify the process is a salt-minion process + assert ( + "salt-minion" in cmdline + or "salt_minion" in cmdline + or "cli_salt_minion" in cmdline + ) + + # The main minion process should NOT accumulate _thread_return when multiprocessing=True + # because jobs run in separate child processes + assert "_thread_return" not in cmdline, ( + f"Main process cmdline should not contain '_thread_return' when " + f"multiprocessing=True, but got: {cmdline}" + ) + + +def test_process_name_includes_minion_process_manager( + salt_master_factory, +): + """ + Test that the process name includes MinionProcessManager. + + This verifies that minion process managers append their name to the + process title even when running in MainProcess (e.g., with --disable-keepalive). + """ + # Create a fresh master for this test + master = salt_master_factory.salt_master_daemon("test-process-name-master-pm") + + # Create a fresh minion + minion = master.salt_minion_daemon( + "test-process-name-minion-pm", + ) + + cli = master.salt_cli() + + with master.started(), minion.started(): + # Execute a job to ensure minion is fully running + ret = cli.run("test.ping", minion_tgt=minion.id) + assert ret.returncode == 0 + assert ret.data is True + + # Get the minion's PID directly from the fixture + minion_pid = minion.pid + assert minion_pid, "Minion PID not available" + + # Get the process info using the actual PID + ret = cli.run("ps.proc_info", minion_pid, minion_tgt=minion.id) + assert ret.returncode == 0, f"Failed to get process info for PID {minion_pid}" + + # Get the process command line + proc_info = ret.data + cmdline = " ".join(proc_info.get("cmdline", [])) + + # The process title should include either MinionProcessManager or MultiMinionProcessManager + # This validates the fix for minion process managers to append their name + # even when running in MainProcess + has_minion_pm = ( + "MinionProcessManager" in cmdline or "MultiMinionProcessManager" in cmdline + ) + assert has_minion_pm, ( + f"Process cmdline should contain 'MinionProcessManager' or " + f"'MultiMinionProcessManager', but got: {cmdline}" + )